Repository: pennersr/django-allauth
Pull request #3543 (2023-11-30 13:30:45)

fix(mfa): Use "one-time-code" autocomplete hint
This small change also makes it a little easier to detect the field in custom form templates (e.g. with `django-widget-tweaks`).

Reference: https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/autocomplete#sect16


coveralls: 
[![Coverage Status](https://coveralls.io/builds/64273421/badge)](https://coveralls.io/builds/64273421)

coverage: 95.745%. remained the same
when pulling **ec6ffdaed3ec1d30c338f062d50f6f706bb90006 on julianwachholz:patch-1**
into **04cf83dc2a8be02e844410c0a35dd0042f396364 on pennersr:main**.

pennersr: There is one more case here:

```
class ActivateTOTPForm(forms.Form):
    code = forms.CharField(label=_("Authenticator code"))
```

Can you give that the same treatment?

Thanks.

Patch:
diff --git a/allauth/account/authentication.py b/allauth/account/authentication.py
new file mode 100644
index 00000000..d330551c
--- /dev/null
+++ b/allauth/account/authentication.py
@@ -0,0 +1,40 @@
+import time
+
+
+AUTHENTICATION_METHODS_SESSION_KEY = "account_authentication_methods"
+
+
+def record_authentication(request, method, **extra_data):
+    """Here we keep a log of all authentication methods used within the current
+    session.  Important to note is that having entries here does not imply that
+    a user is fully signed in. For example, consider a case where a user
+    authenticates using a password, but fails to complete the 2FA challenge.
+    Or, a user successfully signs in into an inactive account or one that still
+    needs verification. In such cases, ``request.user`` is still anonymous, yet,
+    we do have an entry here.
+
+    Example data::
+
+        {'method': 'password',
+         'at': 1701423602.7184925,
+         'username': 'john.doe'}
+
+        {'method': 'socialaccount',
+         'at': 1701423567.6368647,
+         'provider': 'amazon',
+         'uid': 'amzn1.account.K2LI23KL2LK2'}
+
+        {'method': 'mfa',
+         'at': 1701423602.6392953,
+         'id': 1,
+         'type': 'totp'}
+
+    """
+    methods = request.session.get(AUTHENTICATION_METHODS_SESSION_KEY, [])
+    data = {
+        "method": method,
+        "at": time.time(),
+        **extra_data,
+    }
+    methods.append(data)
+    request.session[AUTHENTICATION_METHODS_SESSION_KEY] = methods
diff --git a/allauth/account/forms.py b/allauth/account/forms.py
index dcc83ac6..5854a8d0 100644
--- a/allauth/account/forms.py
+++ b/allauth/account/forms.py
@@ -9,6 +9,8 @@ from django.urls import NoReverseMatch, reverse
 from django.utils.safestring import mark_safe
 from django.utils.translation import gettext, gettext_lazy as _, pgettext
 
+from allauth.account.authentication import record_authentication
+
 from ..utils import (
     build_absolute_uri,
     get_username_max_length,
@@ -200,13 +202,19 @@ class LoginForm(forms.Form):
         return self.cleaned_data
 
     def login(self, request, redirect_url=None):
-        email = self.user_credentials().get("email")
+        credentials = self.user_credentials()
+        extra_data = {
+            field: credentials.get(field)
+            for field in ["email", "username"]
+            if field in credentials
+        }
+        record_authentication(request, method="password", **extra_data)
         ret = perform_login(
             request,
             self.user,
             email_verification=app_settings.EMAIL_VERIFICATION,
             redirect_url=redirect_url,
-            email=email,
+            email=credentials.get("email"),
         )
         remember = app_settings.SESSION_REMEMBER
         if remember is None:
diff --git a/allauth/account/reauthentication.py b/allauth/account/reauthentication.py
index f2df4e20..d5b167cb 100644
--- a/allauth/account/reauthentication.py
+++ b/allauth/account/reauthentication.py
@@ -43,6 +43,8 @@ def resume_request(request):
 
 
 def record_authentication(request, user):
+    # TODO: This is a different/independent mechanism from
+    # ``authentication.record_authentication()``.  We need to unify this.
     request.session[AUTHENTICATED_AT_SESSION_KEY] = time.time()
 
 
diff --git a/allauth/mfa/forms.py b/allauth/mfa/forms.py
index 96424924..02f40004 100644
--- a/allauth/mfa/forms.py
+++ b/allauth/mfa/forms.py
@@ -8,13 +8,14 @@ from allauth.core import context, ratelimit
 from allauth.mfa import totp
 from allauth.mfa.adapter import get_adapter
 from allauth.mfa.models import Authenticator
+from allauth.mfa.utils import post_authentication
 
 
 class AuthenticateForm(forms.Form):
     code = forms.CharField(
         label=_("Code"),
         widget=forms.TextInput(
-            attrs={"placeholder": _("Code"), "autocomplete": "off"},
+            attrs={"placeholder": _("Code"), "autocomplete": "one-time-code"},
         ),
     )
 
@@ -44,11 +45,16 @@ class AuthenticateForm(forms.Form):
         raise forms.ValidationError(get_adapter().error_messages["incorrect_code"])
 
     def save(self):
-        self.authenticator.record_usage()
+        post_authentication(context.request, self.authenticator)
 
 
 class ActivateTOTPForm(forms.Form):
-    code = forms.CharField(label=_("Authenticator code"))
+    code = forms.CharField(
+        label=_("Authenticator code"),
+        widget=forms.TextInput(
+            attrs={"placeholder": _("Code"), "autocomplete": "one-time-code"},
+        ),
+    )
 
     def __init__(self, *args, **kwargs):
         self.user = kwargs.pop("user")
diff --git a/allauth/mfa/utils.py b/allauth/mfa/utils.py
index 8c53a5b2..0ad82231 100644
--- a/allauth/mfa/utils.py
+++ b/allauth/mfa/utils.py
@@ -1,3 +1,4 @@
+from allauth.account.authentication import record_authentication
 from allauth.mfa.adapter import get_adapter
 from allauth.mfa.models import Authenticator
 
@@ -17,3 +18,12 @@ def is_mfa_enabled(user, types=None):
     if types is not None:
         qs = qs.filter(type__in=types)
     return qs.exists()
+
+
+def post_authentication(request, authenticator):
+    authenticator.record_usage()
+    extra_data = {
+        "id": authenticator.pk,
+        "type": authenticator.type,
+    }
+    record_authentication(request, "mfa", **extra_data)
diff --git a/allauth/socialaccount/helpers.py b/allauth/socialaccount/helpers.py
index 2a06a17a..e104eebb 100644
--- a/allauth/socialaccount/helpers.py
+++ b/allauth/socialaccount/helpers.py
@@ -6,6 +6,7 @@ from django.urls import reverse
 
 from allauth.account import app_settings as account_settings
 from allauth.account.adapter import get_adapter as get_account_adapter
+from allauth.account.authentication import record_authentication
 from allauth.account.reauthentication import reauthenticate_then_callback
 from allauth.account.utils import (
     assess_unique_email,
@@ -195,6 +196,14 @@ def _add_social_account(request, sociallogin):
 def complete_social_login(request, sociallogin):
     assert not sociallogin.is_existing
     sociallogin.lookup()
+    record_authentication(
+        request,
+        "socialaccount",
+        **{
+            "provider": sociallogin.account.provider,
+            "uid": sociallogin.account.uid,
+        }
+    )
     try:
         get_adapter().pre_social_login(request, sociallogin)
         signals.pre_social_login.send(
diff --git a/allauth/socialaccount/providers/gitlab/views.py b/allauth/socialaccount/providers/gitlab/views.py
index addcf932..62a7d1a7 100644
--- a/allauth/socialaccount/providers/gitlab/views.py
+++ b/allauth/socialaccount/providers/gitlab/views.py
@@ -1,7 +1,9 @@
 # -*- coding: utf-8 -*-
 import requests
 
+from allauth.core import context
 from allauth.socialaccount import app_settings
+from allauth.socialaccount.adapter import get_adapter
 from allauth.socialaccount.providers.gitlab.provider import GitLabProvider
 from allauth.socialaccount.providers.oauth2.client import OAuth2Error
 from allauth.socialaccount.providers.oauth2.views import (
@@ -46,12 +48,25 @@ class GitLabOAuth2Adapter(OAuth2Adapter):
     provider_default_url = "https://gitlab.com"
     provider_api_version = "v4"
 
-    settings = app_settings.PROVIDERS.get(provider_id, {})
-    provider_base_url = settings.get("GITLAB_URL", provider_default_url)
+    def _build_url(self, path):
+        settings = app_settings.PROVIDERS.get(self.provider_id, {})
+        gitlab_url = settings.get("GITLAB_URL", self.provider_default_url)
+        # Prefer app based setting.
+        app = get_adapter().get_app(context.request, provider=self.provider_id)
+        gitlab_url = app.settings.get("gitlab_url", gitlab_url)
+        return f"{gitlab_url}{path}"
 
-    access_token_url = "{0}/oauth/token".format(provider_base_url)
-    authorize_url = "{0}/oauth/authorize".format(provider_base_url)
-    profile_url = "{0}/api/{1}/user".format(provider_base_url, provider_api_version)
+    @property
+    def access_token_url(self):
+        return self._build_url("/oauth/token")
+
+    @property
+    def authorize_url(self):
+        return self._build_url("/oauth/authorize")
+
+    @property
+    def profile_url(self):
+        return self._build_url(f"/api/{self.provider_api_version}/user")
 
     def complete_login(self, request, app, token, response):
         response = requests.get(self.profile_url, params={"access_token": token.token})
diff --git a/allauth/urls.py b/allauth/urls.py
index d6a5291e..5543ef0b 100644
--- a/allauth/urls.py
+++ b/allauth/urls.py
@@ -24,10 +24,7 @@ provider_classes = [cls for cls in provider_classes if cls.id != "openid_connect
     cls for cls in provider_classes if cls.id == "openid_connect"
 ]
 for provider_class in provider_classes:
-    try:
-        prov_mod = import_module(provider_class.get_package() + ".urls")
-    except ImportError:
-        continue
+    prov_mod = import_module(provider_class.get_package() + ".urls")
     prov_urlpatterns = getattr(prov_mod, "urlpatterns", None)
     if prov_urlpatterns:
         provider_urlpatterns += prov_urlpatterns
diff --git a/docs/socialaccount/providers/gitlab.rst b/docs/socialaccount/providers/gitlab.rst
index 6cde3155..c704c8f2 100644
--- a/docs/socialaccount/providers/gitlab.rst
+++ b/docs/socialaccount/providers/gitlab.rst
@@ -22,8 +22,16 @@ Example:
 .. code-block:: python
 
     SOCIALACCOUNT_PROVIDERS = {
-        'gitlab': {
-            'GITLAB_URL': 'https://your.gitlab.server.tld',
-            'SCOPE': ['api'],
+        "gitlab": {
+            "SCOPE": ["api"],
+            "APPS": [
+                {
+                    "client_id": "<insert-id>",
+                    "secret": "<insert-secret>",
+                    "settings": {
+                        "gitlab_url": "https://your.gitlab.server.tld",
+                    }
+                }
+            ]
         },
     }


Test patch:
diff --git a/allauth/account/tests/test_login.py b/allauth/account/tests/test_login.py
index c3cfe23c..0a7ddf0b 100644
--- a/allauth/account/tests/test_login.py
+++ b/allauth/account/tests/test_login.py
@@ -1,5 +1,5 @@
 import json
-from unittest.mock import patch
+from unittest.mock import ANY, patch
 
 import django
 from django.conf import settings
@@ -9,6 +9,7 @@ from django.test.utils import override_settings
 from django.urls import NoReverseMatch, reverse
 
 from allauth.account import app_settings
+from allauth.account.authentication import AUTHENTICATION_METHODS_SESSION_KEY
 from allauth.account.forms import LoginForm
 from allauth.account.models import EmailAddress
 from allauth.tests import TestCase
@@ -46,6 +47,16 @@ class LoginTests(TestCase):
         self.assertRedirects(
             resp, settings.LOGIN_REDIRECT_URL, fetch_redirect_response=False
         )
+        self.assertEqual(
+            self.client.session[AUTHENTICATION_METHODS_SESSION_KEY],
+            [
+                {
+                    "at": ANY,
+                    "username": "@raymond.penners",
+                    "method": "password",
+                }
+            ],
+        )
 
     def _create_user(self, username="john", password="doe", **kwargs):
         user = get_user_model().objects.create(
diff --git a/allauth/mfa/tests/test_views.py b/allauth/mfa/tests/test_views.py
index 8638b33f..2d8c38a7 100644
--- a/allauth/mfa/tests/test_views.py
+++ b/allauth/mfa/tests/test_views.py
@@ -1,4 +1,4 @@
-from unittest.mock import patch
+from unittest.mock import ANY, patch
 
 import django
 from django.conf import settings
@@ -7,6 +7,7 @@ from django.urls import reverse
 import pytest
 from pytest_django.asserts import assertFormError
 
+from allauth.account.authentication import AUTHENTICATION_METHODS_SESSION_KEY
 from allauth.account.models import EmailAddress
 from allauth.mfa import app_settings
 from allauth.mfa.adapter import get_adapter
@@ -130,6 +131,10 @@ def test_totp_login(client, user_with_totp, user_password, totp_validation_bypas
         )
     assert resp.status_code == 302
     assert resp["location"] == settings.LOGIN_REDIRECT_URL
+    assert client.session[AUTHENTICATION_METHODS_SESSION_KEY] == [
+        {"method": "password", "at": ANY, "username": user_with_totp.username},
+        {"method": "mfa", "at": ANY, "id": ANY, "type": Authenticator.Type.TOTP},
+    ]
 
 
 def test_download_recovery_codes(auth_client, user_with_recovery_codes, user_password):
@@ -169,6 +174,41 @@ def test_generate_recovery_codes(auth_client, user_with_recovery_codes, user_pas
     assert not rc.validate_code(prev_code)
 
 
+def test_recovery_codes_login(
+    client, user_with_totp, user_with_recovery_codes, user_password
+):
+    resp = client.post(
+        reverse("account_login"),
+        {"login": user_with_totp.username, "password": user_password},
+    )
+    assert resp.status_code == 302
+    assert resp["location"] == reverse("mfa_authenticate")
+    resp = client.get(reverse("mfa_authenticate"))
+    assert resp.context["request"].user.is_anonymous
+    resp = client.post(reverse("mfa_authenticate"), {"code": "123"})
+    assert resp.context["form"].errors == {
+        "code": [get_adapter().error_messages["incorrect_code"]]
+    }
+    rc = Authenticator.objects.get(
+        user=user_with_recovery_codes, type=Authenticator.Type.RECOVERY_CODES
+    )
+    resp = client.post(
+        reverse("mfa_authenticate"),
+        {"code": rc.wrap().get_unused_codes()[0]},
+    )
+    assert resp.status_code == 302
+    assert resp["location"] == settings.LOGIN_REDIRECT_URL
+    assert client.session[AUTHENTICATION_METHODS_SESSION_KEY] == [
+        {"method": "password", "at": ANY, "username": user_with_totp.username},
+        {
+            "method": "mfa",
+            "at": ANY,
+            "id": ANY,
+            "type": Authenticator.Type.RECOVERY_CODES,
+        },
+    ]
+
+
 def test_add_email_not_allowed(auth_client, user_with_totp):
     resp = auth_client.post(
         reverse("account_email"),
diff --git a/allauth/socialaccount/tests/__init__.py b/allauth/socialaccount/tests/__init__.py
index 8777b128..f0fb1a57 100644
--- a/allauth/socialaccount/tests/__init__.py
+++ b/allauth/socialaccount/tests/__init__.py
@@ -4,7 +4,7 @@ import json
 import random
 import requests
 import warnings
-from unittest.mock import Mock, patch
+from unittest.mock import ANY, Mock, patch
 from urllib.parse import parse_qs, urlparse
 
 from django.conf import settings
@@ -15,6 +15,7 @@ from django.urls import reverse
 from django.utils.http import urlencode
 
 import allauth.app_settings
+from allauth.account.authentication import AUTHENTICATION_METHODS_SESSION_KEY
 from allauth.account.models import EmailAddress
 from allauth.account.utils import user_email, user_username
 from allauth.socialaccount import app_settings
@@ -81,6 +82,17 @@ class OAuthTestsMixin(object):
         provider_account.get_profile_url()
         provider_account.get_brand()
         provider_account.to_str()
+        self.assertEqual(
+            self.client.session[AUTHENTICATION_METHODS_SESSION_KEY],
+            [
+                {
+                    "at": ANY,
+                    "provider": self.provider_id,
+                    "method": "socialaccount",
+                    "uid": account.uid,
+                }
+            ],
+        )
         return account
 
     @override_settings(
@@ -211,6 +223,17 @@ class OAuth2TestsMixin(object):
             resp_mock,
         )
         self.assertRedirects(resp, reverse("socialaccount_signup"))
+        self.assertEqual(
+            self.client.session[AUTHENTICATION_METHODS_SESSION_KEY],
+            [
+                {
+                    "at": ANY,
+                    "provider": self.provider_id,
+                    "method": "socialaccount",
+                    "uid": ANY,
+                }
+            ],
+        )
 
     @override_settings(SOCIALACCOUNT_AUTO_SIGNUP=False)
     def test_login_with_pkce_disabled(self):