Merge pull request #13 from maykinmedia/feature/hijack-integration

Add support for django-hijack

.github/workflows/ci.yml

@@ -16,7 +16,7 @@ jobs:
     strategy:
       matrix:
         python: ['3.10', '3.11', '3.12']
-        django: ['3.2', 4.2']
+        django: ['3.2', '4.2']
         exclude:
           - python: '3.12'
             django: '3.2'

README.rst

@@ -23,6 +23,7 @@ Features
 * Multi-factor authentication is enforced for admin users, but...
 * Allows marking certain authentication backends (like Single-Sign-On solutions) as
   exempt from this rule
+* Works with django-hijack out of the box
 * Does not get in the way of using django-two-factor-auth for your public UI
 * Commitment to support (at least) maintained Django LTS versions
 

docs/index.rst

@@ -20,6 +20,7 @@ Features
 * Multi-factor authentication is enforced for admin users, but...
 * Allows marking certain authentication backends (like Single-Sign-On solutions) as
   exempt from this rule
+* Works with django-hijack out of the box
 * Does not get in the way of using django-two-factor-auth for your public UI
 * Commitment to support (at least) maintained Django LTS versions
 

docs/reference.rst

@@ -65,3 +65,20 @@ Test helpers
 .. automodule:: maykin_2fa.test
    :members:
 
+
+Django-hijack integration
+=========================
+
+Maykin 2FA works out of the box with django-hijack. It subscribes to the
+``hijack_started`` and ``hijack_ended`` signals to install a temporary hijack TOTP
+device.
+
+If you don't specify the setting ``HIJACK_PERMISSION_CHECK`` yet, you should update
+this to:
+
+.. code-block:: python
+
+    HIJACK_PERMISSION_CHECK = "maykin_2fa.hijack.superusers_only_and_is_verified"
+
+Alternatively, if you have a custom check, make sure to also check
+``hijacker.is_verified()``.

maykin_2fa/apps.py

@@ -1,4 +1,4 @@
-from django.apps import AppConfig
+from django.apps import AppConfig, apps
 
 
 class Maykin2FaConfig(AppConfig):
@@ -7,3 +7,7 @@ class Maykin2FaConfig(AppConfig):
     def ready(self):
         from . import checks  # noqa
         from . import signals  # noqa
+
+        # enable django-hijack integration if it's installed
+        if apps.is_installed("hijack"):
+            from . import hijack_signals  # noqa

maykin_2fa/hijack.py

@@ -0,0 +1,11 @@
+from hijack.permissions import superusers_only  # the library default
+
+from .typing import VerifiableUser
+
+
+def superusers_only_and_is_verified(
+    *, hijacker: VerifiableUser, hijacked: VerifiableUser
+):
+    return hijacker.is_verified() and superusers_only(
+        hijacker=hijacker, hijacked=hijacked
+    )

maykin_2fa/hijack_signals.py

@@ -0,0 +1,93 @@
+"""
+Support for django-hijack in the django admin interface when MFA is enforced.
+
+Originally authored by @Bartvaderkin as part of
+https://github.com/open-formulieren/open-forms.
+"""
+
+from typing import Any
+
+from django.core.exceptions import SuspiciousOperation
+from django.dispatch import receiver
+from django.http import HttpRequest
+
+from django_otp import login as otp_login
+from django_otp.plugins.otp_totp.models import TOTPDevice
+from hijack.signals import hijack_ended, hijack_started
+from two_factor.utils import default_device
+
+from .typing import VerifiableUser
+
+HIJACK_DEVICE_NAME = "hijack_device"
+
+
+@receiver(hijack_started, dispatch_uid="maykin_2fa.hijack_started.manage_totp_device")
+def handle_hijack_start(
+    sender: None,
+    hijacker: VerifiableUser,
+    hijacked: VerifiableUser,
+    request: HttpRequest,
+    **kwargs: Any,
+):
+    """
+    Create a hijack device if needed.
+
+    If a staff user gets hijacked, a multi-factor device needs to be set to be able
+    to use the admin interface as that user. Set a temporary hijack device to achieve
+    this.
+    """
+    # Crash if the hijacker was not MFA verified. This should prevent the hijack too
+    # because the hijack views are wrapped in atomic transactions. An explicit
+    # request.session.save() *may* still make the hijack go through since the signal
+    # only fires *after* the hijack is performed.
+    if not hijacker.is_verified():
+        raise SuspiciousOperation(
+            f"User {hijacker.get_username()} hijacked user "
+            f"{hijacked.get_username()} without being two-factor verified."
+        )
+
+    # XXX possibly we can create our own plugin/device type for this?
+    hijack_device, _ = TOTPDevice.objects.get_or_create(
+        user=hijacked,
+        name=HIJACK_DEVICE_NAME,
+    )
+    otp_login(request, hijack_device)
+
+
+@receiver(hijack_ended, dispatch_uid="maykin_2fa.hijack_ended.manage_totp_device")
+def handle_hijack_end(
+    sender: None,
+    hijacker: VerifiableUser,
+    hijacked: VerifiableUser,
+    request: HttpRequest,
+    **kwargs: Any,
+):
+    """
+    1. Remove any dummy OTP devices for the hijacked user.
+    2. Restore the original OTP device for the hijacker.
+
+    Determining the 'original' OTP device for the hijacker is not trivial - we can not
+    simply store a reference to ``hijacker.otp_device`` in the session data in the
+    :func:`maykin_2fa.hijack_signals.handle_hijack_start` handler because releasing the
+    user calls django's :func:`django.contrib.auth.login`, which flushes the
+    session. Flushing the session causes us to lose that information *before* the
+    ``hijack_ended`` signal fires.
+
+    Instead, we grab the default device from the hijacker and restore that - at the
+    time of writing it does not seem to be possible to use multiple devices (see also
+    https://github.com/maykinmedia/maykin-2fa/issues/11) - so that the hijacker is
+    verified again.
+
+    * django-hijack validates that a release can only be done after an acquire
+    * therefore, enforcing a verified user during hijack implies that only verified
+      users can release
+    """
+    TOTPDevice.objects.filter(user=hijacked, name=HIJACK_DEVICE_NAME).delete()
+
+    # restore 'original' device. See the docstring for why this is not guaranteed to be
+    # the original device.
+    original_device = default_device(hijacker)
+    if original_device is not None:
+        otp_login(request, original_device)
+    else:
+        hijacker.otp_device = None

maykin_2fa/typing.py

@@ -0,0 +1,19 @@
+from typing import Protocol
+
+from django_otp.models import Device
+
+
+class VerifiableUser(Protocol):
+    """
+    The `is_verified` callback gets added through the OTP middleware.
+    """
+
+    is_active: bool
+    is_anonymous: bool
+    is_authenticated: bool
+
+    otp_device: Device | None
+
+    def get_username(self) -> str: ...
+
+    def is_verified(self) -> bool: ...

tests/conftest.py

@@ -1,7 +1,18 @@
+from django.test import Client
+from django.urls import reverse
+
 import pytest
 from django_otp.plugins.otp_static.models import StaticToken
 from django_otp.util import random_hex
 
+from maykin_2fa.test import get_valid_totp_token
+
+
+def _create_totp_device(user):
+    # See https://github.com/jazzband/django-two-factor-auth/blob/
+    # 3c4888c79e37dc4c137bbccafb5680c1e4b74eaa/tests/test_views_login.py#L225
+    return user.totpdevice_set.create(name="default", key=random_hex())
+
 
 @pytest.fixture
 def user(db: None, django_user_model):
@@ -32,9 +43,7 @@ def totp_device(user):
     """
     Set up a TOTP generator device for the user fixture.
     """
-    # See https://github.com/jazzband/django-two-factor-auth/blob/
-    # 3c4888c79e37dc4c137bbccafb5680c1e4b74eaa/tests/test_views_login.py#L225
-    return user.totpdevice_set.create(name="default", key=random_hex())
+    return _create_totp_device(user)
 
 
 @pytest.fixture
@@ -48,3 +57,41 @@ def recovery_codes(user) -> list[str]:
     ]
     StaticToken.objects.bulk_create(tokens)
     return [token.token for token in tokens]
+
+
+@pytest.fixture
+def mfa_admin_user(admin_user):
+    _create_totp_device(admin_user)
+    return admin_user
+
+
+@pytest.fixture
+def mfa_verified_client(mfa_admin_user):
+    """
+    Log in the MFA admin user and verify with token generator.
+    """
+    # use a separate client instance instead of the default fixture so that both
+    # can be used in the same test
+    client = Client()
+
+    admin_login_url = reverse("admin:login")
+    # login superuser who can hijack
+    client.post(
+        admin_login_url,
+        data={
+            "admin_login_view-current_step": "auth",
+            "auth-username": "admin",
+            "auth-password": "password",
+        },
+    )
+    client.post(
+        admin_login_url,
+        data={
+            "admin_login_view-current_step": "token",
+            "token-otp_token": get_valid_totp_token(mfa_admin_user),
+        },
+    )
+    # sanity check that we are actually fully verified
+    admin_index = client.get(reverse("admin:index"))
+    assert admin_index.status_code == 200
+    return client

tests/hijack_urls.py

@@ -0,0 +1,8 @@
+from django.urls import include, path
+
+from testapp.urls import urlpatterns as base_urlpatterns
+
+urlpatterns = [
+    *base_urlpatterns,
+    path("hijack/", include("hijack.urls")),
+]