Give each auth route its own rate-limit bucket, and release 0.17.2

CHANGELOG.md

@@ -1,5 +1,13 @@
 # Changelog
 
+## 0.17.2
+
+### Fixed
+
+- **The Redis challenge store raised on a non-positive TTL** where the in-memory one wrote an entry that was simply already expired. With `PASSKEY_CHALLENGE_TTL` set to `0`, beginning a passkey sign-in answered `500` on Redis, on a route that needs no account. Both backends now agree: nothing is stored and the next `pop()` returns `None`.
+- **Numeric settings reject values that cannot work.** Durations, expiries, attempt counts and rate-limit caps are validated at construction, so a mistake fails at startup instead of somewhere unrelated later: `MAX_LOGIN_ATTEMPTS=0` locked every account on its first failed login, and `PASSKEY_CHALLENGE_TTL=0` raised from Redis mid-request. The three settings that document a meaningful zero keep it: `JWT_LEEWAY_SECONDS`, `PASSWORD_MAX_LENGTH` and `REAUTH_MAX_AGE_SECONDS`. **If your configuration holds one of these impossible values, the application now refuses to start and says which setting.**
+- **Auth rate limits shared one bucket per IP on Redis.** Each route gets its own limiter with its own cap, but the key was the client IP alone, and every Redis-backed limiter writes into the same keyspace, so all six auth routes counted into a single sorted set. The smallest cap won for all of them: with `register` at 3, any three auth requests, a token refresh or a failed login included, returned `429` on the next registration attempt for the rest of the window. It also broke the separation `reauth` was given in 0.17.0, where failed password checks were meant not to touch the sign-in budget. The key now carries the route name. Only Redis was affected; the in-memory backend keeps its counts per limiter instance, which is why the test suite never saw it, and why the new regression test runs against both backends. **Note:** the key format changed, so existing counters reset once on deploy.
+
 ## 0.17.1
 
 ### Changed

docs/configuration.md

@@ -124,6 +124,11 @@ If you want to be defensively explicit that no file is ever read, pass `FullAuth
 
 ## Reference
 
+Durations, expiries, attempt counts and rate-limit caps must be at least 1, and a
+value that cannot work is rejected when the config is constructed rather than
+failing somewhere unrelated later. `JWT_LEEWAY_SECONDS`, `PASSWORD_MAX_LENGTH`
+and `REAUTH_MAX_AGE_SECONDS` accept `0`, which means something for each of them.
+
 ### Core
 
 | Option | Type | Default | Description |

fastapi_fullauth/__init__.py

@@ -1,4 +1,4 @@
-__version__ = "0.17.1"
+__version__ = "0.17.2"
 
 from fastapi_fullauth.config import AuthRateLimits, FullAuthConfig
 from fastapi_fullauth.fullauth import FullAuth

fastapi_fullauth/config.py

@@ -40,6 +40,14 @@ class AuthRateLimits:
     # bucket, so failed attempts cannot exhaust an IP's sign-in budget.
     reauth: int = 5
 
+    def __post_init__(self) -> None:
+        for name, value in vars(self).items():
+            if value < 1:
+                raise ValueError(
+                    f"AUTH_RATE_LIMITS.{name} must be at least 1; "
+                    f"{value} would reject every request to that route."
+                )
+
 
 class FullAuthConfig(BaseSettings):
     model_config = SettingsConfigDict(
@@ -61,19 +69,20 @@ class FullAuthConfig(BaseSettings):
     SECRET_KEY: str | None = None
     ALGORITHM: Literal["HS256", "HS384", "HS512"] = "HS256"
 
-    ACCESS_TOKEN_EXPIRE_MINUTES: int = 30
-    REFRESH_TOKEN_EXPIRE_DAYS: int = 30
+    ACCESS_TOKEN_EXPIRE_MINUTES: int = Field(default=30, ge=1)
+    REFRESH_TOKEN_EXPIRE_DAYS: int = Field(default=30, ge=1)
     REFRESH_TOKEN_ROTATION: bool = True
-    JWT_LEEWAY_SECONDS: int = 30
+    JWT_LEEWAY_SECONDS: int = Field(default=30, ge=0)
 
-    PASSWORD_RESET_EXPIRE_MINUTES: int = 15
-    EMAIL_VERIFY_EXPIRE_MINUTES: int = 1440
+    PASSWORD_RESET_EXPIRE_MINUTES: int = Field(default=15, ge=1)
+    EMAIL_VERIFY_EXPIRE_MINUTES: int = Field(default=1440, ge=1)
 
     PASSWORD_HASH_ALGORITHM: Literal["argon2id", "bcrypt"] = "argon2id"
-    PASSWORD_MIN_LENGTH: int = 8
+    PASSWORD_MIN_LENGTH: int = Field(default=8, ge=1)
     # Upper bound so an attacker can't spend the server's CPU hashing a huge
     # body. bcrypt caps at 72 bytes anyway; argon2 will hash whatever it is given.
-    PASSWORD_MAX_LENGTH: int = 4096
+    # 0 disables the cap, hence ge=0 rather than ge=1.
+    PASSWORD_MAX_LENGTH: int = Field(default=4096, ge=0)
 
     LOGIN_FIELD: str = "email"
     # When True, login runs a dummy password verify on unknown-user / no-password
@@ -86,12 +95,12 @@ class FullAuthConfig(BaseSettings):
     # destructive action (deleting the account, setting a first password).
     # Supplying the current password proves it too. 0 accepts only the password,
     # which locks passwordless accounts out of those actions.
-    REAUTH_MAX_AGE_SECONDS: int = 300
+    REAUTH_MAX_AGE_SECONDS: int = Field(default=300, ge=0)
 
     LOCKOUT_ENABLED: bool = True
     LOCKOUT_BACKEND: Literal["memory", "redis"] = "memory"
-    MAX_LOGIN_ATTEMPTS: int = 5
-    LOCKOUT_DURATION_MINUTES: int = 15
+    MAX_LOGIN_ATTEMPTS: int = Field(default=5, ge=1)
+    LOCKOUT_DURATION_MINUTES: int = Field(default=15, ge=1)
 
     RATE_LIMIT_BACKEND: Literal["memory", "redis"] = "memory"
     TRUSTED_PROXY_HEADERS: Annotated[list[str], NoDecode] = []
@@ -104,14 +113,14 @@ class FullAuthConfig(BaseSettings):
 
     AUTH_RATE_LIMIT_ENABLED: bool = True
     AUTH_RATE_LIMITS: AuthRateLimits = Field(default_factory=AuthRateLimits)
-    AUTH_RATE_LIMIT_WINDOW_SECONDS: int = 60
+    AUTH_RATE_LIMIT_WINDOW_SECONDS: int = Field(default=60, ge=1)
 
     REDIS_URL: str | None = None
 
     BLACKLIST_ENABLED: bool = True
     BLACKLIST_BACKEND: Literal["memory", "redis"] = "memory"
 
-    OAUTH_STATE_EXPIRE_SECONDS: int = 300
+    OAUTH_STATE_EXPIRE_SECONDS: int = Field(default=300, ge=1)
     OAUTH_AUTO_LINK_BY_EMAIL: bool = True
     OAUTH_PKCE_ENABLED: bool = True
 
@@ -124,7 +133,7 @@ class FullAuthConfig(BaseSettings):
     PASSKEY_RP_NAME: str | None = None
     PASSKEY_ORIGINS: Annotated[list[str], NoDecode] = []
     PASSKEY_CHALLENGE_BACKEND: Literal["memory", "redis"] = "memory"
-    PASSKEY_CHALLENGE_TTL: int = 60
+    PASSKEY_CHALLENGE_TTL: int = Field(default=60, ge=1)
     PASSKEY_REQUIRE_USER_VERIFICATION: bool = True
 
     API_PREFIX: str = "/api/v1"

fastapi_fullauth/protection/challenges.py

@@ -82,6 +82,13 @@ def __init__(self, redis_url: str) -> None:
         self._prefix = "fullauth:challenge:"
 
     async def store(self, key: str, challenge: str, ttl: int = 60) -> None:
+        # Redis rejects a non-positive expiry outright, where the in-memory store
+        # just writes an entry that is already expired. Skip the write instead,
+        # so both backends answer None to the next pop() rather than one of them
+        # raising into a 500 on a route that needs no account.
+        if ttl <= 0:
+            logger.warning("Challenge not stored; ttl=%s is not positive", ttl)
+            return
         await self._redis.set(f"{self._prefix}{key}", challenge, ex=ttl)
 
     async def pop(self, key: str) -> str | None:

fastapi_fullauth/protection/ratelimit.py

@@ -257,10 +257,24 @@ async def aclose(self) -> None:
 
     async def check(self, route_name: str, client_ip: str) -> None:
         limiter = self._limiters.get(route_name)
-        if limiter and not await limiter.is_allowed(client_ip):
+        if limiter is None:
+            return
+
+        # The route belongs in the key, not just in the limiter lookup. Every
+        # Redis-backed limiter shares one keyspace, so keying on the IP alone
+        # put all six routes in a single bucket and the smallest cap, register
+        # at 3, became the effective limit for all of them: three refreshes
+        # locked out registration for a minute. The in-memory backend keeps its
+        # counts per instance, which is why only Redis showed it. Namespacing
+        # here rather than inside a backend keeps custom backends working
+        # unchanged, and keeps these buckets clear of any limiter the
+        # application passes to RateLimitMiddleware.
+        key = f"{route_name}:{client_ip}"
+
+        if not await limiter.is_allowed(key):
             from fastapi import HTTPException
 
-            reset_in = await limiter.reset_time(client_ip)
+            reset_in = await limiter.reset_time(key)
             logger.warning("Auth rate limit exceeded: route=%s, ip=%s", route_name, client_ip)
             raise HTTPException(
                 status_code=429,

tests/test_config.py

@@ -7,6 +7,7 @@
 import pytest
 from fastapi import FastAPI, Request
 from httpx import ASGITransport, AsyncClient
+from pydantic import ValidationError
 from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine
 from sqlmodel import SQLModel
 
@@ -15,6 +16,8 @@
 from fastapi_fullauth.types import CreateUserSchema, UserSchema
 from tests.conftest import make_test_adapter
 
+SECRET = "test-secret-key-that-is-long-enough-32b"
+
 
 async def _make_db():
     engine = create_async_engine("sqlite+aiosqlite://", echo=False)
@@ -974,3 +977,48 @@ async def test_cookie_backend_with_csrf_no_warning(config, adapter):
     with warnings.catch_warnings():
         warnings.simplefilter("error")
         fa.init_app(app)
+
+
+# ── Numeric settings reject values that cannot work ──────────────────
+
+
+@pytest.mark.parametrize(
+    "setting",
+    [
+        "ACCESS_TOKEN_EXPIRE_MINUTES",
+        "REFRESH_TOKEN_EXPIRE_DAYS",
+        "PASSWORD_RESET_EXPIRE_MINUTES",
+        "EMAIL_VERIFY_EXPIRE_MINUTES",
+        "MAX_LOGIN_ATTEMPTS",
+        "LOCKOUT_DURATION_MINUTES",
+        "AUTH_RATE_LIMIT_WINDOW_SECONDS",
+        "OAUTH_STATE_EXPIRE_SECONDS",
+        "PASSKEY_CHALLENGE_TTL",
+    ],
+)
+def test_non_positive_durations_and_counts_are_rejected(setting):
+    """Zero or negative here is never meaningful, and the failure it causes is
+    remote from the cause: PASSKEY_CHALLENGE_TTL=0 raised from Redis on a public
+    route, and MAX_LOGIN_ATTEMPTS=0 locked every account on its first failure.
+    Fail at construction instead."""
+    with pytest.raises(ValidationError):
+        FullAuthConfig(SECRET_KEY=SECRET, **{setting: 0})
+
+
+def test_the_meaningful_zeros_are_still_accepted():
+    """Three settings document zero as a real choice, so they must keep it."""
+    config = FullAuthConfig(
+        SECRET_KEY=SECRET,
+        JWT_LEEWAY_SECONDS=0,  # clock-sensitive tests rely on this
+        PASSWORD_MAX_LENGTH=0,  # disables the cap
+        REAUTH_MAX_AGE_SECONDS=0,  # accept only the current password
+    )
+    assert config.JWT_LEEWAY_SECONDS == 0
+    assert config.PASSWORD_MAX_LENGTH == 0
+    assert config.REAUTH_MAX_AGE_SECONDS == 0
+
+
+def test_a_rate_limit_cap_below_one_is_rejected():
+    """A cap of 0 rejects every request to that route, which nobody means."""
+    with pytest.raises(ValueError, match="at least 1"):
+        AuthRateLimits(login=0)

tests/test_passkey.py

@@ -278,3 +278,25 @@ async def test_challenge_store_evicts_challenges_that_were_never_used():
         await store.store("fresh", "c", ttl=60)
 
     assert list(store._store) == ["fresh"]
+
+
+@pytest.mark.asyncio
+async def test_challenge_stores_agree_on_a_non_positive_ttl():
+    """Redis rejects a non-positive expiry where the in-memory store writes an
+    entry that is already expired. Production picks the Redis backend whenever
+    REDIS_URL is set, so a divergence here is a 500 on a route that needs no
+    account, reachable by setting PASSKEY_CHALLENGE_TTL badly."""
+    import fakeredis.aioredis
+
+    from fastapi_fullauth.protection.challenges import RedisChallengeStore
+
+    memory = InMemoryChallengeStore()
+    redis = RedisChallengeStore.__new__(RedisChallengeStore)
+    redis._redis = fakeredis.aioredis.FakeRedis(decode_responses=True)
+    redis._prefix = "fullauth:challenge:"
+
+    for ttl in (0, -5):
+        await memory.store("k", "challenge", ttl=ttl)
+        await redis.store("k", "challenge", ttl=ttl)
+        assert await memory.pop("k") is None
+        assert await redis.pop("k") is None

tests/test_security.py

@@ -790,3 +790,59 @@ async def test_in_memory_sweep_never_drops_live_entries():
         await blacklist.add("trigger", ttl_seconds=10)
         assert await blacklist.is_blacklisted("long-lived")
         assert "short" not in blacklist._blacklisted
+
+
+# ── Auth rate limits are per route ───────────────────────────────────
+
+
+def _auth_limiter(backend: str):
+    """An AuthRateLimiter on the given backend, with Redis faked in place.
+
+    The Redis limiters are built by the real code path, then pointed at one
+    shared fake server, which is what makes a cross-route collision visible.
+    """
+    from fastapi_fullauth import FullAuthConfig
+    from fastapi_fullauth.protection.ratelimit import AuthRateLimiter
+
+    kwargs = {"SECRET_KEY": "test-secret-key-that-is-long-enough-32b", "BACKEND": backend}
+    if backend == "redis":
+        kwargs["REDIS_URL"] = "redis://rate-limit-test:6379/0"
+    limiter = AuthRateLimiter(FullAuthConfig(**kwargs))
+
+    if backend == "redis":
+        import fakeredis.aioredis
+
+        server = fakeredis.aioredis.FakeServer()
+        for each in limiter._limiters.values():
+            each._redis = fakeredis.aioredis.FakeRedis(server=server, decode_responses=True)
+    return limiter
+
+
+@pytest.mark.parametrize("backend", ["memory", "redis"])
+@pytest.mark.asyncio
+async def test_auth_routes_do_not_share_a_rate_limit_bucket(backend):
+    """Every Redis limiter shares one keyspace, so the key has to carry the
+    route. Keyed on the IP alone, all six routes counted into one bucket and the
+    smallest cap won: three refreshes locked out registration. The in-memory
+    backend counts per instance and never showed it, hence both backends here."""
+    from fastapi import HTTPException
+
+    limiter = _auth_limiter(backend)
+    ip = "203.0.113.9"
+
+    # refresh allows 30; spend enough to exhaust register's cap of 3.
+    for _ in range(5):
+        await limiter.check("refresh", ip)
+
+    # register has not been called at all, so it must still have its full cap.
+    for _ in range(3):
+        await limiter.check("register", ip)
+
+    with pytest.raises(HTTPException) as exc:
+        await limiter.check("register", ip)
+    assert exc.value.status_code == 429
+
+    # And exhausting register must leave the documented separate bucket alone.
+    await limiter.check("reauth", ip)
+
+    await limiter.aclose()