fix(mfa): Prevent reuse of TOTP codes

Author: pennersr

Diff for: ChangeLog.rst

@@ -10,6 +10,14 @@ Note worthy changes
   originated, will be emailed.
 
 
+Security notice
+---------------
+
+- MFA: It was possible to reuse a valid TOTP code within its time window. This
+  has now been addressed. As a result, a user can now only login once per 30
+  seconds (``MFA_TOTP_PERIOD``).
+
+
 Backwards incompatible changes
 ------------------------------
 

Diff for: allauth/mfa/tests/test_views.py

@@ -2,6 +2,8 @@
 
 import django
 from django.conf import settings
+from django.core.cache import cache
+from django.test import Client
 from django.urls import reverse
 
 import pytest
@@ -301,3 +303,44 @@ def test_cannot_deactivate_totp(auth_client, user_with_totp, user_password):
         assert resp.context["form"].errors == {
             "__all__": [get_adapter().error_messages["cannot_delete_authenticator"]],
         }
+
+
+def test_totp_code_reuse(
+    user_with_totp, user_password, totp_validation_bypass, enable_cache
+):
+    for code, time_lapse, expect_success in [
+        # First use of code, SUCCESS
+        ("123", False, True),
+        # Second use, no time elapsed: FAIL
+        ("123", False, False),
+        # Different code, no time elapsed: SUCCESS
+        ("456", False, True),
+        # Again, previous code, no time elapsed: FAIL
+        ("123", False, False),
+        # Previous code, but time elapsed: SUCCESS
+        ("123", True, True),
+    ]:
+        if time_lapse:
+            cache.clear()
+        client = Client()
+        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")
+        # Note that this bypass only bypasses the actual code check, not the
+        # re-use check we're testing here.
+        with totp_validation_bypass():
+            resp = client.post(
+                reverse("mfa_authenticate"),
+                {"code": code},
+            )
+        if expect_success:
+            assert resp.status_code == 302
+            assert resp["location"] == settings.LOGIN_REDIRECT_URL
+        else:
+            assert resp.status_code == 200
+            assert resp.context["form"].errors == {
+                "code": [get_adapter().error_messages["incorrect_code"]]
+            }

Diff for: allauth/mfa/totp.py

@@ -7,6 +7,7 @@
 from io import BytesIO
 from urllib.parse import quote
 
+from django.core.cache import cache
 from django.utils.http import urlencode
 
 import qrcode
@@ -104,5 +105,20 @@ def deactivate(self):
         self.instance.delete()
 
     def validate_code(self, code):
+        if self._is_code_used(code):
+            return False
+
         secret = decrypt(self.instance.data["secret"])
-        return validate_totp_code(secret, code)
+        valid = validate_totp_code(secret, code)
+        if valid:
+            self._mark_code_used(code)
+        return valid
+
+    def _get_used_cache_key(self, code):
+        return f"allauth.mfa.totp.used?user={self.instance.user_id}&code={code}"
+
+    def _is_code_used(self, code):
+        return cache.get(self._get_used_cache_key(code)) == "y"
+
+    def _mark_code_used(self, code):
+        cache.set(self._get_used_cache_key(code), "y", timeout=app_settings.TOTP_PERIOD)