Merge branch '65.8.x'

Author: pennersr

ChangeLog.rst

@@ -9,6 +9,23 @@ Note worthy changes
   Ninja.
 
 
+65.8.1 (2025-05-21)
+*******************
+
+Fixes
+-----
+
+- Fixed a compatibility issue with the newly released fido2 2.0.0 package.
+
+
+Security notice
+---------------
+
+- After a successful login, the rate limits for that login were cleared,
+  allowing a succesful login on a specific IP address to be used as a means to
+  clear the login failed rate limit for that IP address. Fixed.
+
+
 65.8.0 (2025-05-08)
 *******************
 

allauth/account/adapter.py

@@ -33,7 +33,8 @@
 
 from allauth import app_settings as allauth_app_settings
 from allauth.account import app_settings, signals
-from allauth.core import context, ratelimit
+from allauth.core import context
+from allauth.core.internal import ratelimit
 from allauth.core.internal.adapter import BaseAdapter
 from allauth.core.internal.cryptokit import generate_user_code
 from allauth.core.internal.httpkit import (
@@ -675,15 +676,30 @@ def _get_login_attempts_cache_key(self, request, **credentials):
 
     def _delete_login_attempts_cached_email(self, request, **credentials):
         cache_key = self._get_login_attempts_cache_key(request, **credentials)
-        ratelimit.clear(request, action="login_failed", key=cache_key)
+        # Here, we wipe the login failed rate limit, completely. This is safe,
+        # as we only do this on a succesful password reset, which is rate limited
+        # on itself (e.g. sending of email etc.).
+        ratelimit.clear(
+            request,
+            config=app_settings.RATE_LIMITS,
+            action="login_failed",
+            key=cache_key,
+        )
+
+    def _rollback_login_failed_rl_usage(self) -> None:
+        usage = getattr(self, "_login_failed_rl_usage", None)
+        if usage:
+            usage.rollback()
 
     def pre_authenticate(self, request, **credentials):
         cache_key = self._get_login_attempts_cache_key(request, **credentials)
-        if not ratelimit.consume(
+        self._login_failed_rl_usage = ratelimit.consume(
             request,
+            config=app_settings.RATE_LIMITS,
             action="login_failed",
             key=cache_key,
-        ):
+        )
+        if not self._login_failed_rl_usage:
             raise self.validation_error("too_many_login_attempts")
 
     def authenticate(self, request, **credentials):
@@ -696,7 +712,12 @@ def authenticate(self, request, **credentials):
         alt_user = AuthenticationBackend.unstash_authenticated_user()
         user = user or alt_user
         if user:
-            self._delete_login_attempts_cached_email(request, **credentials)
+            # On a succesful login, we cannot just wipe the login failed rate
+            # limit. That consists of 2 parts, a per IP limit, and, a per
+            # key(email) limit. Wiping it completely would allow an attacker to
+            # insert periodic successful logins during a brute force
+            # process. So instead, we are rolling back our consumption.
+            self._rollback_login_failed_rl_usage()
         else:
             self.authentication_failed(request, **credentials)
         return user
@@ -731,7 +752,7 @@ def is_ajax(self, request):
             ]
         )
 
-    def get_client_ip(self, request):
+    def get_client_ip(self, request) -> str:
         x_forwarded_for = request.META.get("HTTP_X_FORWARDED_FOR")
         if x_forwarded_for:
             ip = x_forwarded_for.split(",")[0]

allauth/core/internal/ratelimit.py

@@ -0,0 +1,192 @@
+"""
+Rate limiting in this implementation relies on a cache and uses non-atomic
+operations, making it vulnerable to race conditions. As a result, users may
+occasionally bypass the intended rate limit due to concurrent access. However,
+such race conditions are rare in practice. For example, if the limit is set to
+10 requests per minute and a large number of parallel processes attempt to test
+that limit, you may occasionally observe slight overruns—such as 11 or 12
+requests slipping through. Nevertheless, exceeding the limit by a large margin
+is highly unlikely due to the low probability of many processes entering the
+critical non-atomic code section simultaneously.
+"""
+
+import hashlib
+import time
+from collections import namedtuple
+from dataclasses import dataclass
+from typing import Dict, List, Optional, Tuple, Union
+
+from django.core.cache import cache
+from django.core.exceptions import ImproperlyConfigured
+from django.http import HttpRequest, HttpResponse
+from django.shortcuts import render
+
+from allauth.core.exceptions import RateLimited
+
+
+Rate = namedtuple("Rate", "amount duration per")
+
+
+@dataclass
+class SingleRateLimitUsage:
+    cache_key: str
+    cache_duration: Union[float, int]
+    timestamp: float
+
+    def rollback(self) -> None:
+        history = cache.get(self.cache_key, [])
+        history = [ts for ts in history if ts != self.timestamp]
+        cache.set(self.cache_key, history, self.cache_duration)
+
+
+@dataclass
+class RateLimitUsage:
+    usage: List[SingleRateLimitUsage]
+
+    def rollback(self) -> None:
+        for usage in self.usage:
+            usage.rollback()
+
+
+def parse_duration(duration) -> Union[int, float]:
+    if len(duration) == 0:
+        raise ValueError(duration)
+    unit = duration[-1]
+    value = duration[0:-1]
+    unit_map = {"s": 1, "m": 60, "h": 3600, "d": 86400}
+    if unit not in unit_map:
+        raise ValueError("Invalid duration unit: %s" % unit)
+    if len(value) == 0:
+        value = 1
+    else:
+        value = float(value)
+    return value * unit_map[unit]
+
+
+def parse_rate(rate: str) -> Rate:
+    parts = rate.split("/")
+    if len(parts) == 2:
+        amount, duration = parts
+        per = "ip"
+    elif len(parts) == 3:
+        amount, duration, per = parts
+    else:
+        raise ValueError(rate)
+    amount_v = int(amount)
+    duration_v = parse_duration(duration)
+    return Rate(amount_v, duration_v, per)
+
+
+def parse_rates(rates: Optional[str]) -> List[Rate]:
+    ret = []
+    if rates:
+        rates = rates.strip()
+        if rates:
+            parts = rates.split(",")
+            for part in parts:
+                ret.append(parse_rate(part.strip()))
+    return ret
+
+
+def get_cache_key(request, *, action: str, rate: Rate, key=None, user=None):
+    from allauth.account.adapter import get_adapter
+
+    source: Tuple[str, ...]
+    if rate.per == "ip":
+        source = ("ip", get_adapter().get_client_ip(request))
+    elif rate.per == "user":
+        if user is None:
+            if not request.user.is_authenticated:
+                raise ImproperlyConfigured(
+                    "ratelimit configured per user but used anonymously"
+                )
+            user = request.user
+        source = ("user", str(user.pk))
+    elif rate.per == "key":
+        if key is None:
+            raise ImproperlyConfigured(
+                "ratelimit configured per key but no key specified"
+            )
+        key_hash = hashlib.sha256(key.encode("utf8")).hexdigest()
+        source = (key_hash,)
+    else:
+        raise ValueError(rate.per)
+    keys = ["allauth", "rl", action, *source]
+    return ":".join(keys)
+
+
+def _consume_single_rate(
+    request,
+    *,
+    action: str,
+    rate: Rate,
+    key=None,
+    user=None,
+    dry_run: bool = False,
+    raise_exception: bool = False
+) -> Optional[SingleRateLimitUsage]:
+    cache_key = get_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:
+        usage = SingleRateLimitUsage(
+            cache_key=cache_key, timestamp=now, cache_duration=rate.duration
+        )
+    else:
+        usage = None
+    if allowed and not dry_run:
+        history.insert(0, now)
+        cache.set(cache_key, history, rate.duration)
+    if not allowed and raise_exception:
+        raise RateLimited
+    return usage
+
+
+def consume(
+    request: HttpRequest,
+    *,
+    action: str,
+    config: Dict[str, str],
+    key=None,
+    user=None,
+    dry_run: bool = False,
+    raise_exception: bool = False
+) -> Optional[RateLimitUsage]:
+    usage = RateLimitUsage(usage=[])
+    if request.method == "GET":
+        return usage
+    rates = parse_rates(config.get(action))
+    if not rates:
+        return usage
+    allowed = True
+    for rate in rates:
+        single_usage = _consume_single_rate(
+            request,
+            action=action,
+            rate=rate,
+            key=key,
+            user=user,
+            dry_run=dry_run,
+            raise_exception=raise_exception,
+        )
+        if not single_usage:
+            allowed = False
+            break
+        usage.usage.append(single_usage)
+    return usage if allowed else None
+
+
+def handler429(request) -> HttpResponse:
+    from allauth.account import app_settings
+
+    return render(request, "429." + app_settings.TEMPLATE_EXTENSION, status=429)
+
+
+def clear(request, *, config: dict, action: str, key=None, user=None):
+    rates = parse_rates(config.get(action))
+    for rate in rates:
+        cache_key = get_cache_key(request, action=action, rate=rate, key=key, user=user)
+        cache.delete(cache_key)

allauth/core/tests/test_ratelimit.py renamed to allauth/core/internal/tests/test_ratelimit.py

@@ -1,6 +1,23 @@
 import pytest
 
-from allauth.core import ratelimit
+from allauth.core.internal import ratelimit
+
+
+def test_rollback_consume(rf, enable_cache):
+    def consume():
+        request = rf.post("/")
+        config = {"foo": "2/m/ip"}
+        return ratelimit.consume(request, config=config, action="foo")
+
+    usage1 = consume()
+    assert len(usage1.usage) > 0
+    usage2 = consume()
+    assert len(usage2.usage) > 0
+    no_usage = consume()
+    assert no_usage is None
+    usage1.rollback()
+    assert consume()
+    assert not consume()
 
 
 @pytest.mark.parametrize(
@@ -15,7 +32,7 @@
     ],
 )
 def test_parse(rate, values):
-    rates = ratelimit._parse_rates(rate)
+    rates = ratelimit.parse_rates(rate)
     assert len(rates) == len(values)
     for i, rate in enumerate(rates):
         assert rate.amount == values[i][0]