[#2816] Split user/password auth from token verification

Previously, the UserModelEmailBackend was overloaded to
support both username/password authentication, as well as
TOTP validation. It did this, in part, by verifying the
2FA flag on the site configuration.

This was both confusing (an authentication backend should
do one thing only, multiple logics means multiple backends),
as well as mixing concerns (the _view_ should decide which
arguments to pass to to authentication backend based on the
site configuration, authentication backends should only
authenticate).

This commit separates both concerns into independent backends,
and adds some tests to ensure that they are properly invoked.

Author: swrichards

src/open_inwoner/accounts/backends.py

@@ -2,7 +2,7 @@
 
 from django.conf import settings
 from django.contrib.auth import get_user_model
-from django.contrib.auth.backends import ModelBackend
+from django.contrib.auth.backends import BaseBackend, ModelBackend
 from django.contrib.auth.hashers import check_password
 from django.urls import reverse, reverse_lazy
 
@@ -21,14 +21,14 @@
 class UserModelEmailBackend(ModelBackend):
     """
     Authentication backend for login with email address.
+
+    Based on the default ModelBackend, but with support for case insensitive
+    email lookups, scoped to the correct login type.
     """
 
-    def authenticate(
-        self, request, username=None, password=None, user=None, token=None, **kwargs
-    ):
-        config = SiteConfiguration.get_solo()
+    def authenticate(self, request, username=None, password=None):
         User = get_user_model()
-        if username and password and not config.login_2fa_sms:
+        if username and password:
             try:
                 user = User.objects.get(
                     email__iexact=username,
@@ -51,8 +51,15 @@ def authenticate(
                 User().set_password(password)
                 return None
 
+
+class Verify2FATokenBackend(BaseBackend):
+    """
+    Verify a TOTP token for a user.
+    """
+
+    def authenticate(self, request, *, user=None, token=None):
         # 2FA with sms verification
-        if config.login_2fa_sms and user and token:
+        if user and token:
             accepted, drift = accept_totp(
                 key=user.seed,
                 response=token,

src/open_inwoner/accounts/tests/test_auth.py

@@ -1,15 +1,17 @@
-from unittest.mock import patch
+from unittest.mock import ANY, patch
 from urllib.parse import urlencode
 
 from django.contrib.auth.signals import user_logged_in
 from django.contrib.sites.models import Site
 from django.core import mail
+from django.core.signing import TimestampSigner
 from django.test import RequestFactory, TestCase, override_settings
 from django.urls import reverse, reverse_lazy
 from django.utils.translation import gettext as _
 
 import requests_mock
 from django_webtest import WebTest
+from freezegun import freeze_time
 from furl import furl
 from pyquery import PyQuery as PQ
 
@@ -1715,6 +1717,38 @@ def test_login(self):
         # Verify that the user has been authenticated
         self.assertIn("_auth_user_id", self.app.session)
 
+    @patch("open_inwoner.accounts.gateways.gateway.send")
+    @freeze_time("2023-05-22 12:05:01")
+    def test_regular_login_with_valid_credentials_triggers_the_2fa_flow(
+        self, mock_gateway_send
+    ):
+        config = SiteConfiguration.get_solo()
+        config.login_2fa_sms = True
+        config.save()
+
+        response = self.app.get(reverse("login"))
+        mock_gateway_send.assert_not_called()
+
+        login_form = response.forms["login-form"]
+        login_form["username"] = self.user.email
+        login_form["password"] = "test"
+        login_form_response = login_form.submit()
+        login_form_response.follow()
+
+        mock_gateway_send.assert_called_once_with(to=self.user.phonenumber, token=ANY)
+
+        params = {
+            "next": "",
+            "user": TimestampSigner().sign(self.user.pk),
+        }
+        verify_token_url = furl(reverse("verify_token")).add(params).url
+        self.assertRedirects(login_form_response, verify_token_url)
+        self.assertNotIn(
+            "_auth_user_id",
+            self.app.session,
+            msg="Valid credentials alone should not authenticate a user, second factor required",
+        )
+
     def test_login_page_shows_correct_digid_login_url(self):
         config = OpenIDConnectDigiDConfig.get_solo()
 

src/open_inwoner/accounts/tests/test_backends.py

@@ -1,9 +1,15 @@
+import datetime
 from unittest.mock import patch
 
+from django.conf import settings
 from django.contrib import auth
 from django.test import RequestFactory, TestCase, override_settings
 from django.urls import reverse
 
+from freezegun import freeze_time
+from oath import totp
+
+from open_inwoner.accounts.choices import LoginTypeChoices
 from open_inwoner.accounts.tests.factories import UserFactory
 
 
@@ -69,3 +75,108 @@ def test_admin_oidc_use_correct_backend(
         result = auth.authenticate(request)
 
         self.assertEqual(result, self.user)
+
+
+@override_settings(
+    AUTHENTICATION_BACKENDS=[
+        "open_inwoner.accounts.backends.UserModelEmailBackend",
+    ]
+)
+class UserModelEmailBackendTestCase(TestCase):
+    @classmethod
+    def setUpTestData(cls):
+        super().setUpTestData()
+
+        cls.password = "keepitsecert"
+        cls.user = UserFactory(
+            login_type=LoginTypeChoices.default, password=cls.password
+        )
+        for login_type in LoginTypeChoices:
+            UserFactory(login_type=login_type)
+
+    def test_duplicate_emails_on_case_results_in_no_match(self):
+        request = RequestFactory().post(reverse("login"))
+
+        UserFactory(email=self.user.email.upper(), login_type=self.user.login_type)
+        result = auth.authenticate(
+            request, username=self.user.email, password=self.password
+        )
+        self.assertEqual(result, None)
+
+    def test_correct_username_password_return_user(self):
+        request = RequestFactory().post(reverse("login"))
+
+        result = auth.authenticate(
+            request, username=self.user.email, password=self.password
+        )
+        self.assertEqual(result, self.user)
+
+    def test_incorrect_username_password_return_none(self):
+        request = RequestFactory().post(reverse("login"))
+
+        for username, password in (
+            (self.user.email, "incorrect"),
+            ("incorrect", self.password),
+        ):
+            result = auth.authenticate(request, username=username, password=password)
+            self.assertEqual(result, None)
+
+    def test_missing_username_and_or_password_returns_none(self):
+        for username in (self.user.email, "", None):
+            for password in (self.password, "", None):
+                if username and password:
+                    # This is the successful case, exclude it, but ensure we
+                    # also have permutations with one valid/one invalid value.
+                    continue
+
+                with self.subTest(f"{username=} {password=}"):
+                    request = RequestFactory().post(reverse("login"))
+                    result = auth.authenticate(
+                        request, username=username, password=password
+                    )
+                    self.assertEqual(result, None)
+
+
+@override_settings(
+    AUTHENTICATION_BACKENDS=[
+        "open_inwoner.accounts.backends.Verify2FATokenBackend",
+    ]
+)
+class Verify2FATokenBackendTestCase(TestCase):
+    @classmethod
+    def setUpTestData(cls):
+        super().setUpTestData()
+
+        cls.user = UserFactory(login_type=LoginTypeChoices.default)
+        cls.expires_in = getattr(settings, "ACCOUNTS_USER_TOKEN_EXPIRE_TIME", 300)
+        cls.make_token = lambda: totp(cls.user.seed, period=cls.expires_in)
+
+    @freeze_time("2023-05-22 12:05:01")
+    def test_valid_token_and_user_returns_user(self):
+        request = RequestFactory().get(reverse("verify_token"))
+
+        result = auth.authenticate(request, user=self.user, token=self.make_token())
+        self.assertEqual(result, self.user)
+
+    def test_expired_token_and_valid_user_returns_none(self):
+        request = RequestFactory().get(reverse("verify_token"))
+
+        with freeze_time("2023-05-22 12:05:01") as ft:
+            token = self.make_token()
+
+            ft.tick(delta=datetime.timedelta(seconds=self.expires_in * 2))
+            result = auth.authenticate(request, user=self.user, token=token)
+            self.assertEqual(result, None)
+
+    def test_missing_user_and_or_token_returns_none(self):
+        for user in (self.user.email, "", None):
+            for token in (self.make_token(), "", None):
+                if user and token:
+                    # This is the successful case, exclude it, but ensure we
+                    # also have permutations with one valid/one invalid value.
+                    continue
+
+                with self.subTest(f"{user=} {token=}"):
+                    request = RequestFactory().get(reverse("verify_token"))
+                    result = auth.authenticate(request, user=user, token=token)
+                    self.assertEqual(result, None)

src/open_inwoner/conf/base.py

@@ -496,6 +496,7 @@
 AUTHENTICATION_BACKENDS = [
     "open_inwoner.accounts.backends.CustomAxesBackend",
     "open_inwoner.accounts.backends.UserModelEmailBackend",
+    "open_inwoner.accounts.backends.Verify2FATokenBackend",
     "digid_eherkenning.backends.DigiDBackend",
     "eherkenning.backends.eHerkenningBackend",
     "digid_eherkenning_oidc_generics.backends.OIDCAuthenticationDigiDBackend",

src/open_inwoner/conf/ci.py

@@ -32,6 +32,7 @@
 AUTHENTICATION_BACKENDS = [
     "open_inwoner.accounts.backends.CustomAxesBackend",
     "open_inwoner.accounts.backends.UserModelEmailBackend",
+    "open_inwoner.accounts.backends.Verify2FATokenBackend",
     "django.contrib.auth.backends.ModelBackend",
     # mock login like dev.py
     "digid_eherkenning.mock.backends.DigiDBackend",