refactor(account): Friendlier email verification rate limit

ChangeLog.rst

@@ -13,7 +13,7 @@ Fixes
   verification emails, as the previous emails that were already sent still
   contain valid links. This is different from email verification by code.  Here,
   the session contains a specific code, meaning, silently skipping new
-  verification emails is not an option, and we must hard fail (429) instead. The
+  verification emails is not an option, and we must block the login instead. The
   latter was missing, fixed.
 
 

allauth/account/forms.py

@@ -176,6 +176,10 @@ def clean(self):
         adapter = get_adapter(self.request)
         user = adapter.authenticate(self.request, **credentials)
         if user:
+            login = Login(user=user, email=credentials.get("email"))
+            if flows.login.is_login_rate_limited(context.request, login):
+                raise adapter.validation_error("too_many_login_attempts")
+            self._login = login
             self.user = user
         else:
             auth_method = app_settings.AUTHENTICATION_METHOD
@@ -190,11 +194,8 @@ def clean(self):
 
     def login(self, request, redirect_url=None):
         credentials = self.user_credentials()
-        login = Login(
-            user=self.user,
-            redirect_url=redirect_url,
-            email=credentials.get("email"),
-        )
+        login = self._login
+        login.redirect_url = redirect_url
         ret = flows.login.perform_password_login(request, credentials, login)
         remember = app_settings.SESSION_REMEMBER
         if remember is None:

allauth/account/internal/flows/email_verification.py

@@ -7,7 +7,7 @@
 from allauth.account import app_settings, signals
 from allauth.account.adapter import get_adapter
 from allauth.account.internal.flows.manage_email import emit_email_changed
-from allauth.account.models import EmailAddress
+from allauth.account.models import EmailAddress, Login
 from allauth.core import ratelimit
 from allauth.core.exceptions import ImmediateHttpResponse
 from allauth.core.internal.httpkit import get_frontend_url
@@ -135,7 +135,15 @@ def login_on_verification(request, verification) -> Optional[HttpResponse]:
     return stage.exit()
 
 
-def handle_verification_email_rate_limit(request, email_address: EmailAddress) -> bool:
+def consume_email_verification_rate_limit(
+    request: HttpRequest, email: str, dry_run: bool = False
+) -> bool:
+    return ratelimit.consume(
+        request, action="confirm_email", key=email.lower(), dry_run=dry_run
+    )
+
+
+def handle_verification_email_rate_limit(request, email: str) -> bool:
     """
     For email verification by link, it is not an issue if the user runs into rate
     limits. The reason is that the link is session independent. Therefore, if the
@@ -146,11 +154,7 @@ def handle_verification_email_rate_limit(request, email_address: EmailAddress) -
     verification emails is not an option, and we must hard fail (429) instead. The
     latter was missing, fixed.
     """
-    rl_ok = ratelimit.consume(
-        request,
-        action="confirm_email",
-        key=email_address.email.lower(),
-    )
+    rl_ok = consume_email_verification_rate_limit(request, email)
     if not rl_ok and app_settings.EMAIL_VERIFICATION_BY_CODE_ENABLED:
         raise ImmediateHttpResponse(ratelimit.respond_429(request))
     return rl_ok
@@ -194,7 +198,7 @@ def send_verification_email(request, user, signup=False, email=None) -> bool:
         if email_address is not None:
             if not email_address.verified:
                 send_email = handle_verification_email_rate_limit(
-                    request, email_address
+                    request, email_address.email
                 )
                 if send_email:
                     send_email = adapter.should_send_confirmation_mail(
@@ -221,3 +225,26 @@ def send_verification_email(request, user, signup=False, email=None) -> bool:
                 {"email": email, "login": not signup, "signup": signup},
             )
     return sent
+
+
+def is_verification_rate_limited(request: HttpRequest, login: Login) -> bool:
+    """
+    Returns whether or not the email verification is *hard* rate limited.
+    Hard, meaning, it would be blocking login (verification by code, not link).
+    """
+    if (
+        (not login.email)
+        or (not app_settings.EMAIL_VERIFICATION_BY_CODE_ENABLED)
+        or login.email_verification != app_settings.EmailVerificationMethod.MANDATORY
+    ):
+        return False
+    try:
+        email_address = EmailAddress.objects.get_for_user(login.user, login.email)
+        if not email_address.verified:
+            if not consume_email_verification_rate_limit(
+                request, login.email, dry_run=True
+            ):
+                return True
+    except EmailAddress.DoesNotExist:
+        pass
+    return False

allauth/account/internal/flows/login.py

@@ -105,3 +105,13 @@ def resume_login(request: HttpRequest, login: Login) -> HttpResponse:
                 raise e
         response = e.response
     return response
+
+
+def is_login_rate_limited(request, login: Login) -> bool:
+    from allauth.account.internal.flows.email_verification import (
+        is_verification_rate_limited,
+    )
+
+    if is_verification_rate_limited(request, login):
+        return True
+    return False

allauth/account/models.py

@@ -240,6 +240,7 @@ class Login:
     email: Optional[str]
     state: Dict
     initiated_at: float
+    redirect_url: Optional[str]
 
     def __init__(
         self,

allauth/account/tests/test_email_verification_by_code.py

@@ -176,4 +176,7 @@ def test_email_verification_rate_limits(
             assert resp.status_code == 302
             assert resp["location"] == reverse("account_email_verification_sent")
         else:
-            assert resp.status_code == 429
+            assert resp.status_code == 200
+            assert resp.context["form"].errors == {
+                "__all__": ["Too many failed login attempts. Try again later."]
+            }

allauth/core/ratelimit.py

@@ -91,7 +91,7 @@ def clear(request, *, action, key=None, user=None):
         cache.delete(cache_key)
 
 
-def consume(request, *, action, key=None, user=None):
+def consume(request, *, action, key=None, user=None, dry_run: bool = False):
     from allauth.account import app_settings
 
     if not request or request.method == "GET":
@@ -103,19 +103,21 @@ def consume(request, *, action, key=None, user=None):
 
     allowed = True
     for rate in rates:
-        if not _consume_rate(request, action=action, rate=rate, key=key, user=user):
+        if not _consume_rate(
+            request, action=action, rate=rate, key=key, user=user, dry_run=dry_run
+        ):
             allowed = False
     return allowed
 
 
-def _consume_rate(request, *, action, rate, key=None, user=None):
+def _consume_rate(request, *, action, rate, key=None, user=None, dry_run: bool = False):
     cache_key = _cache_key(request, action=action, rate=rate, key=key, user=user)
     history = cache.get(cache_key, [])
     now = time.time()
     while history and history[-1] <= now - rate.duration:
         history.pop()
     allowed = len(history) < rate.amount
-    if allowed:
+    if allowed and not dry_run:
         history.insert(0, now)
         cache.set(cache_key, history, rate.duration)
     return allowed

allauth/headless/account/inputs.py

@@ -13,7 +13,11 @@
     UserTokenForm,
 )
 from allauth.account.internal import flows
-from allauth.account.models import EmailAddress, get_emailconfirmation_model
+from allauth.account.models import (
+    EmailAddress,
+    Login,
+    get_emailconfirmation_model,
+)
 from allauth.core import context
 from allauth.headless.adapter import get_adapter
 from allauth.headless.internal.restkit import inputs
@@ -76,10 +80,14 @@ def clean(self):
             else:
                 credentials["username"] = username
                 auth_method = account_app_settings.AuthenticationMethod.USERNAME
-            self.user = get_account_adapter().authenticate(
-                context.request, **credentials
-            )
-            if not self.user:
+            user = get_account_adapter().authenticate(context.request, **credentials)
+            if user:
+                self.login = Login(user=user, email=credentials.get("email"))
+                if flows.login.is_login_rate_limited(context.request, self.login):
+                    raise get_account_adapter().validation_error(
+                        "too_many_login_attempts"
+                    )
+            else:
                 error_code = "%s_password_mismatch" % auth_method.value
                 self.add_error(
                     "password", get_account_adapter().validation_error(error_code)

allauth/headless/account/tests/test_email_verification_by_code.py

@@ -33,4 +33,13 @@ def test_email_verification_rate_limits(
             ][0]
             assert flow["id"] == Flow.VERIFY_EMAIL
         else:
-            assert resp.status_code == 429
+            assert resp.status_code == 400
+            assert resp.json() == {
+                "status": 400,
+                "errors": [
+                    {
+                        "message": "Too many failed login attempts. Try again later.",
+                        "code": "too_many_login_attempts",
+                    }
+                ],
+            }

allauth/headless/account/views.py

@@ -3,7 +3,7 @@
 from allauth.account.adapter import get_adapter as get_account_adapter
 from allauth.account.internal import flows
 from allauth.account.internal.flows import password_change, password_reset
-from allauth.account.models import EmailAddress, Login
+from allauth.account.models import EmailAddress
 from allauth.account.stages import EmailVerificationStage, LoginStageController
 from allauth.account.utils import send_email_confirmation
 from allauth.core import ratelimit
@@ -84,10 +84,7 @@ def post(self, request, *args, **kwargs):
         if request.user.is_authenticated:
             return ConflictResponse(request)
         credentials = self.input.cleaned_data
-        user = get_account_adapter().authenticate(self.request, **credentials)
-        if user:
-            login = Login(user=user, email=credentials.get("email"))
-            flows.login.perform_password_login(request, credentials, login)
+        flows.login.perform_password_login(request, credentials, self.input.login)
         return AuthenticationResponse(self.request)