Skip to content

Commit

Permalink
⚗️ [#2] Enforce MFA in the admin
Browse files Browse the repository at this point in the history
  • Loading branch information
sergei-maertens committed Jan 29, 2024
1 parent e8c4024 commit e30e18a
Show file tree
Hide file tree
Showing 9 changed files with 155 additions and 6 deletions.
30 changes: 25 additions & 5 deletions docs/quickstart.rst
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,36 @@ you must set ``maykin_2fa.middleware.OTPMiddleware``, after the authentication m
...,
]
**Disable admin patching**
**``urls.py``**

The admin must be monkeypatched so that third party packages registering to the default
admin are also properly 2FA-protected. Unfortunately monkeypatching seems to be the only
viable approach.

This should not be a problem in tests etc. since we can mark authentication backends as
no-2FA-required, such as the django webtest backend.

In your root ``urls.py``, ensure the admin is patched and include the URLs to the
custom views:

.. code-block:: python
TWO_FACTOR_PATCH_ADMIN = False
from maykin_2fa import monkeypatch_admin
monkeypatch_admin()
The default is ``True``. Patching the admin uses some project-global URLs, which we
will manage ourselves instead.
urlpatterns = [
# should come BEFORE the default admin URLs to override behaviour.
path("admin/", include("maykin_2fa.urls")),
path("admin/", admin.site.urls),
]
Additionally, the default login patching can/should be disabled as it's no longer
relevant by the above monkeypatching.

.. todo:: CHECK IF THIS STILL NEEDED!
.. code-block:: python
TWO_FACTOR_PATCH_ADMIN = False
**Congigure WebAuthn relying party**

Expand Down
22 changes: 22 additions & 0 deletions maykin_2fa/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
def monkeypatch_admin():
"""
Monkeypatch the admin to enforce 2FA.
2FA is enforced but can be bypassed by putting the relevant authentication
backend(s) on the allow list. In development, you might want to set all backends
as bypassed:
.. code-block::
MAYKIN_2FA_ALLOW_MFA_BYPASS_BACKENDS = AUTHENTICATION_BACKENDS
Upstream documentation: https://django-two-factor-auth.readthedocs.io/en/stable/implementing.html#admin-site
.. todo:: provide different login URLs machinery? Override the login URL to not use
the global settings?
"""
from django.contrib import admin

from .admin import MFARequired

admin.site.__class__ = MFARequired
13 changes: 13 additions & 0 deletions maykin_2fa/admin.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
from django.core.exceptions import ImproperlyConfigured

from two_factor.admin import AdminSiteOTPRequired


class MFARequired(AdminSiteOTPRequired):
def login(self, request, extra_context=None):
"""
Disabled to enforce usage of the custom login views.
"""
raise ImproperlyConfigured(
"Ensure the maykin_2fa urls are included *before* the default admin.site.urls."
)
3 changes: 3 additions & 0 deletions maykin_2fa/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,6 @@

class Maykin2FaConfig(AppConfig):
name = "maykin_2fa"

def ready(self):
from . import signals # noqa
3 changes: 3 additions & 0 deletions maykin_2fa/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from typing import TypeAlias

from django.conf import settings
from django.contrib.auth import BACKEND_SESSION_KEY
from django.contrib.auth.models import AbstractBaseUser, AnonymousUser
from django.http import HttpRequest

Expand Down Expand Up @@ -56,5 +57,7 @@ class OTPMiddleware(_OTPMiddleware):
def _verify_user(self, request: HttpRequest, user: AnyUser):
# call the super but replace the `is_verified` callable
user = super()._verify_user(request, user)
# this is *not* persisted on the user object after authenticate
user.backend = request.session[BACKEND_SESSION_KEY]
user.is_verified = functools.partial(is_verified, user) # type: ignore
return user
19 changes: 19 additions & 0 deletions maykin_2fa/signals.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import functools

from django.contrib.auth.signals import user_logged_in
from django.dispatch import receiver

from .middleware import is_verified


@receiver(user_logged_in, dispatch_uid="maykin_2fa.add_is_verified_method")
def add_is_verified_method(sender, request, user, **kwargs):
"""
Add the method ``is_verified`` to the user instance.
Normally this is added through middleware, but because django's ``login`` function
assigns `request.user = instance`` directly from the DB, this method is lost.
"""
user.is_verified = functools.partial(is_verified, user)
if not hasattr(user, "otp_device"):
user.otp_device = None
21 changes: 21 additions & 0 deletions maykin_2fa/urls.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
from django.urls import path

from .views import (
AdminLoginView,
AdminSetupView,
BackupTokensView,
QRGeneratorView,
SetupCompleteView,
)

app_name = "maykin_2fa"

# See two_factor/urls.py for a reference of the (core) urls

urlpatterns = [
path("login/", AdminLoginView.as_view(), name="login"),
path("mfa/setup/", AdminSetupView.as_view(), name="setup"),
path("mfa/qrcode/", QRGeneratorView.as_view(), name="qr"),
path("mfa/setup/complete/", SetupCompleteView.as_view(), name="setup_complete"),
path("mfa/backup/tokens/", BackupTokensView.as_view(), name="backup_tokens"),
]
43 changes: 43 additions & 0 deletions maykin_2fa/views.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
from django.shortcuts import resolve_url

from two_factor.views import (
BackupTokensView as _BackupTokensView,
LoginView as _LoginView,
QRGeneratorView as _QRGeneratorView,
SetupCompleteView as _SetupCompleteView,
SetupView as _SetupView,
)


class AdminLoginView(_LoginView):
template_name = "two_factor/core/login.html"
redirect_authenticated_user = True

def get_redirect_url(self):
# after succesful authentication, check if the user needs to set up 2FA. If MFA
# was configured already, login flow takes care of the OTP step.
user = self.request.user

if user.is_authenticated and not user.is_verified():
return resolve_url("maykin_2fa:setup")
return super().get_redirect_url()


class AdminSetupView(_SetupView):
# TODO: update to our own templates/URLs
success_url = "maykin_2fa:setup_complete"
qrcode_url = "maykin_2fa:qr"
template_name = "two_factor/core/setup.html"


class BackupTokensView(_BackupTokensView):
success_url = "maykin_2fa:backup_tokens"
template_name = "two_factor/core/backup_tokens.html"


class SetupCompleteView(_SetupCompleteView):
template_name = "two_factor/core/setup_complete.html"


class QRGeneratorView(_QRGeneratorView):
pass
7 changes: 6 additions & 1 deletion testapp/urls.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
from django.contrib import admin
from django.urls import path
from django.urls import include, path

from maykin_2fa import monkeypatch_admin

monkeypatch_admin()

urlpatterns = [
path("admin/", include("maykin_2fa.urls")),
path("admin/", admin.site.urls),
]

0 comments on commit e30e18a

Please sign in to comment.