✨ Fixes #3 -- make 2FA check conditional on authentication backend

docs/index.rst

@@ -11,6 +11,8 @@ Welcome to maykin-2fa's documentation!
 
 <One liner describing the project>
 
+Built on top of https://django-two-factor-auth.readthedocs.io/en/stable/index.html
+
 Features
 ========
 

docs/quickstart.rst

@@ -9,10 +9,89 @@ Install from PyPI with pip:
 
 .. code-block:: bash
 
-    pip install maykin_2fa
+    pip install maykin-2fa
 
 
+Settings
+========
+
+As this library bundles django-two-factor-auth, the installation instructions of it are
+relevant. For typical Maykin usage, you'll want to:
+
+**Add the required ``INSTALLED_APPS`` entries**
+
+.. code-block:: python
+
+    INSTALLED_APPS = [
+        ...,
+        # Required by django-two-factor-auth and its dependencies
+        "django_otp",
+        "django_otp.plugins.otp_static",
+        "django_otp.plugins.otp_totp",
+        "two_factor",
+        "two_factor.plugins.webauthn",
+        "maykin_2fa",
+        ...,
+    ]
+
+**Set up our middleware override**
+
+We ship a modified ``django-otp`` middleware - so instead of ``django_otp.middleware.OTPMiddleware``,
+you must set ``maykin_2fa.middleware.OTPMiddleware``, after the authentication middleware:
+
+.. code-block:: python
+
+    MIDDLEWARE = [
+        ...,
+        "django.contrib.auth.middleware.AuthenticationMiddleware",
+        "maykin_2fa.middleware.OTPMiddleware",
+        ...,
+    ]
+
+**Disable admin patching**
+
+.. code-block:: python
+
+    TWO_FACTOR_PATCH_ADMIN = False
+
+The default is ``True``. Patching the admin uses some project-global URLs, which we
+will manage ourselves instead.
+
+.. todo:: CHECK IF THIS STILL NEEDED!
+
+**Congigure WebAuthn relying party**
+
+This is required when using the WebAuthn plugin so you can use hardware tokens.
+
+.. code-block:: python
+
+    TWO_FACTOR_WEBAUTHN_RP_NAME = "TODO - figure out what the meaning is"
+
+.. todo:: look into details of this, but the setting is required.
+
+**Configure allow list to skip 2FA-enforcement**
+
+By default, this package ensures the admin enforces 2FA. However, when logging it
+through OpenID Connect or other Single-Sign-On solutions, this can lead to double 2FA
+flows. Since these alternative login flows typically come with a custom Django
+authentication backend, you can add them to an allowlist to bypass the application MFA.
+
+.. code-block::
+
+    AUTHENTICATION_BACKENDS = [
+        "django.contrib.auth.backends.ModelBackend",
+        "mozilla_django_oidc_db.backends.OIDCAuthenticationBackend",
+    ]
+
+    MAYKIN_2FA_ALLOW_MFA_BYPASS_BACKENDS = [
+        "mozilla_django_oidc_db.backends.OIDCAuthenticationBackend",
+    ]
+
+.. todo:: add system check to check that each backend is in the ``AUTHENTICATION_BACKENDS`` setting.
+
 Usage
 =====
 
-<document how to use the app here>
+Should be plug and play - there is no additional frontend stuff.
+
+.. todo:: Complete if relevant.

maykin_2fa/middleware.py

@@ -0,0 +1,60 @@
+import functools
+from typing import TypeAlias
+
+from django.conf import settings
+from django.contrib.auth.models import AbstractBaseUser, AnonymousUser
+from django.http import HttpRequest
+
+from django_otp.middleware import (
+    OTPMiddleware as _OTPMiddleware,
+    is_verified as otp_is_verified,
+)
+
+# probably a TypeVar(bound=...) makes more sense but we don't allow using generics here,
+# so let's cover the most ground using a simple union.
+AnyUser: TypeAlias = AbstractBaseUser | AnonymousUser
+
+
+def is_verified(user: AbstractBaseUser) -> bool:
+    """
+    Modified version of :func:`django_otp.middleware.is_verified` to add bypass logic.
+
+    This function may not be called with an :class:`AnonymousUser` instance!
+
+    If the user backend that the user authenticated with is on the allow list, we do
+    not perform the real OTP device check from the library, but just mark the user as
+    verified.
+    """
+    # check our allow list for authentication backends
+    backends_to_skip_verification_for = getattr(
+        settings, "MAYKIN_2FA_ALLOW_MFA_BYPASS_BACKENDS", []
+    )
+    if (
+        backends_to_skip_verification_for
+        and user.is_authenticated
+        and user.backend in backends_to_skip_verification_for  # type: ignore
+    ):
+        return True
+    # fall back to default library behaviour
+    return otp_is_verified(user)
+
+
+class OTPMiddleware(_OTPMiddleware):
+    """
+    Substitute our own :func:`is_verified` check instead of the upstream one.
+
+    This middleware *replaces* :class:`django_otp.middleware.OTPMiddleware` to add
+    allow-list behaviour for certain authentication backends. This marks the user
+    authentication as being verified even though the 2FA requirements in the project
+    itself have been bypassen.
+
+    This setup allows us to enforce 2FA when logging in with username + password, but
+    be less strict when signing it via OIDC/other SSO solutions that already enforce
+    MFA at another level outside of our scope.
+    """
+
+    def _verify_user(self, request: HttpRequest, user: AnyUser):
+        # call the super but replace the `is_verified` callable
+        user = super()._verify_user(request, user)
+        user.is_verified = functools.partial(is_verified, user)  # type: ignore
+        return user

pyproject.toml

@@ -27,7 +27,10 @@ classifiers = [
 ]
 requires-python = ">=3.10"
 dependencies = [
-    "django>=4.2"
+    "django>=4.2",
+    # TODO: check if we want phonenumberslite or phonenumbers - we typically only
+    # use OTP and/or hardware tokens so this may not be relevant at all.
+    "django-two-factor-auth[phonenumberslite,webauthn]",
 ]
 
 [project.urls]
@@ -71,6 +74,9 @@ sections=["FUTURE", "STDLIB", "DJANGO", "THIRDPARTY", "FIRSTPARTY", "LOCALFOLDER
 [tool.pytest.ini_options]
 testpaths = ["tests"]
 DJANGO_SETTINGS_MODULE = "testapp.settings"
+markers = [
+    "user_request_auth_backend: specify backend that a user has authenticated with",
+]
 
 [tool.bumpversion]
 current_version = "0.1.0"

testapp/backends.py

@@ -0,0 +1,9 @@
+from django.contrib.auth.backends import ModelBackend
+
+
+class No2FAModelBackend(ModelBackend):
+    """
+    Custom backend which is allow-listed to bypass 2FA via the settings.
+    """
+
+    pass

testapp/settings.py

@@ -23,6 +23,17 @@
     "django.contrib.sessions",
     "django.contrib.messages",
     "django.contrib.admin",
+    # Required by django-two-factor-auth and its dependencies
+    "django_otp",
+    "django_otp.plugins.otp_static",
+    "django_otp.plugins.otp_totp",
+    # 'django_otp.plugins.otp_email',  # <- if you want email capability.
+    "two_factor",
+    "two_factor.plugins.webauthn",
+    # 'two_factor.plugins.phonenumber',  # <- if you want phone number capability.
+    # 'two_factor.plugins.email',  # <- if you want email capability.
+    # 'two_factor.plugins.yubikey',  # <- for yubikey capability.
+    # Own package
     "maykin_2fa",
     # Test application for (unit) tests and demo.
     "testapp",
@@ -34,6 +45,7 @@
     "django.middleware.common.CommonMiddleware",
     "django.middleware.csrf.CsrfViewMiddleware",
     "django.contrib.auth.middleware.AuthenticationMiddleware",
+    "maykin_2fa.middleware.OTPMiddleware",
     "django.contrib.messages.middleware.MessageMiddleware",
     "django.middleware.clickjacking.XFrameOptionsMiddleware",
 ]
@@ -57,3 +69,17 @@
 STATIC_URL = "/static/"
 
 ROOT_URLCONF = "testapp.urls"
+
+AUTHENTICATION_BACKENDS = [
+    "django.contrib.auth.backends.ModelBackend",
+    "testapp.backends.No2FAModelBackend",
+]
+
+# we take care of this, conditionally
+# TODO: maybe tapping into some login signal could be sufficient to mark the user
+# as verified
+TWO_FACTOR_PATCH_ADMIN = False
+TWO_FACTOR_WEBAUTHN_RP_NAME = "TestApp"
+
+# Custom settings
+MAYKIN_2FA_ALLOW_MFA_BYPASS_BACKENDS = ["testapp.backends.No2FAModelBackend"]

tests/conftest.py

@@ -1,6 +1,22 @@
-# import pytest
+import pytest
 
 
-# @pytest.fixture
-# def some_fixture(request):
-#     return "foo"
+@pytest.fixture
+def user(db: None, django_user_model):
+    """
+    Inspired on pytest-django's admin_user fixture.
+    """
+    UserModel = django_user_model
+    username = "johny"
+    try:
+        # The default behavior of `get_by_natural_key()` is to look up by `username_field`.
+        # However the user model is free to override it with any sort of custom behavior.
+        # The Django authentication backend already assumes the lookup is by username,
+        # so we can assume so as well.
+        user = UserModel._default_manager.get_by_natural_key(username)
+    except UserModel.DoesNotExist:
+        user_data = {"email": "[email protected]"}
+        user_data["password"] = "password"
+        user_data["username"] = username
+        user = UserModel._default_manager.create_user(**user_data)
+    return user

tests/test_middleware.py

@@ -0,0 +1,65 @@
+from django.http import HttpRequest, HttpResponse
+from django.utils.functional import SimpleLazyObject
+
+import pytest
+
+from maykin_2fa.middleware import OTPMiddleware
+
+
+@pytest.fixture()
+def user_request(request, client, rf, user) -> HttpRequest:
+    marker = request.node.get_closest_marker("user_request_auth_backend")
+    backend = None if not marker else marker.args[0]
+    # sets the backend and session
+    client.force_login(user, backend=backend)
+
+    # create a standalone request instance, by copying over the session and mimicking
+    # the `django.contrib.auth.middleware.AuthenticationMiddleware` middleware
+    request = rf.get("/irrelevant")
+    request.session = client.session
+    request.user = SimpleLazyObject(lambda: user)
+    return request
+
+
+@pytest.mark.user_request_auth_backend("django.contrib.auth.backends.ModelBackend")
+def test_authenticated_but_not_verified(user_request, settings):
+    """
+    Test that a user who logs in is not verified by default.
+    """
+    settings.MAYKIN_2FA_ALLOW_MFA_BYPASS_BACKENDS = []
+    middleware = OTPMiddleware(lambda req: HttpResponse())
+
+    middleware(user_request)
+
+    # standard Django behaviour
+    user = user_request.user
+    assert user.is_authenticated
+    assert user.backend == "django.contrib.auth.backends.ModelBackend"
+    # OTP middleware adds the `is_verified` callable
+    assert hasattr(user, "is_verified")
+    assert callable(user.is_verified)
+    # we didn't go through any 2FA flows, so the user is *NOT* verified
+    assert user.is_verified() is False
+
+
+@pytest.mark.user_request_auth_backend("testapp.backends.No2FAModelBackend")
+def test_authenticated_and_2fa_verification_bypassed(user_request, settings):
+    """
+    Test that a user is "2FA-verified" when authenticated through a backend on the allow list.
+    """
+    settings.MAYKIN_2FA_ALLOW_MFA_BYPASS_BACKENDS = [
+        "testapp.backends.No2FAModelBackend"
+    ]
+    middleware = OTPMiddleware(lambda req: HttpResponse())
+
+    middleware(user_request)
+
+    # standard Django behaviour
+    user = user_request.user
+    assert user.is_authenticated
+    assert user.backend == "testapp.backends.No2FAModelBackend"
+    # OTP middleware adds the `is_verified` callable
+    assert hasattr(user, "is_verified")
+    assert callable(user.is_verified)
+    # didn't go through any 2FA flows, but the backend is on the allow list
+    assert user.is_verified()