diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e9b553fc..0cc8b5a5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,28 @@ All notable changes to this project are documented in this file. The format is b ## [Unreleased] ### Changed +- **Behaviour change (#530):** password-login lockout now holds across workers, + source IPs and restarts. `users.login_lockout_account_failures` (new, default + 10) failures for one account within `login_rate_limit_window_seconds`, from + any network, lock it for `login_rate_limit_cooldown_seconds`: every password + login for it answers 429 with `Retry-After`, the correct password included; + the failure that trips the lock is itself answered 429. Attempts are counted + before the password is checked (a correct password gives the slot back), so + a parallel burst gets no more tries than the threshold. + `login_rate_limit_failures` (5) attempts from one IP for one username per + window are admitted. Unknown usernames go through the same rules on the rate + limit store, so a 429 does not reveal whether an account exists. + Counters live on `users_user` (a migration adds `failed_login_count`, + `last_failed_login_at` and `locked_until`) and on the host rate-limit store + (Redis when `SM_REDIS_URL` is set). The per-worker `LoginRateLimiter` is gone. + A successful password reset lifts the lock and clears the failure count + (audited, like an admin unlock). +- **Behaviour change (#530):** `POST /api/users/auth/token` now has the same + lockout and throttle: 429 while locked; bad credentials are still 401. Its + email lookup is now case-insensitive, matching the session login. +- **Behaviour change (#530):** the stock fastapi-users auth router is no longer + mounted, so `/api/users/auth-inner/login` and `/api/users/auth-inner/logout` + are gone. Use `/api/users/auth/login` and `/api/users/auth/logout`. - **Behaviour change (#543):** with `allow_signup` off, an OAuth sign-in no longer creates an account. It answers 403 `OAUTH_SIGNUP_DISABLED` unless the identity is already linked or its email matches an existing account, which @@ -54,6 +76,12 @@ All notable changes to this project are documented in this file. The format is b previously served, and anonymous, in production. ### Added +- `PATCH /api/users/admin/{id}/unlock` lifts a login lockout early (audited). + `UserListItem` carries `locked_until`; the users list shows a "locked" badge + and the edit page a "Locked until" row with an Unlock action (#530). +- `app.state.sm.rate_limit_store` exposes the host's rate-limit store to + modules. `WindowStore` gains `peek()` (read without counting), `reset()` and + `hit(..., sliding=True)` (#530). - Durable event delivery (#459). `bus.subscribe_durable(EventType, handler, name=...)` and `await bus.publish_durable(event, db)` in `simple_module_core.events`; handlers are called as diff --git a/docs/framework/request-guards.md b/docs/framework/request-guards.md index ed5a23836..4307e6dbe 100644 --- a/docs/framework/request-guards.md +++ b/docs/framework/request-guards.md @@ -123,6 +123,18 @@ def register_public_routes(self, registry) -> None: recovery is automatic. The script also re-arms a counter that lost its TTL, so no key can block an IP permanently. The limiter never takes requests down. +### Sharing the store + +`create_app` builds the store once and exposes it as `app.state.sm.rate_limit_store` +(a `WindowStore`), so a module that throttles its own events counts in the same +place, across workers, without opening a second Redis client. Besides `hit`, +which counts, a store has `peek(key, spec)`, which reports the current window +without counting (`allowed` answers "would one more hit be within budget"), +`reset(key)`, which forgets a window, and `hit(..., sliding=True)`, which +restarts the window on every hit. The users login lockout +(`users/auth_local/lockout.py`) uses all three. Keys are +namespaced by the caller (`login:…`); Redis keys get the `sm:rl:` prefix. + ### Response shape `429` with a `Retry-After` header (seconds until the window resets). API callers @@ -130,10 +142,10 @@ get `{"detail": "...", "retry_after": N}`; browsers get the Inertia error page. ### Not covered -`users/auth_local/rate_limit.py` (`LoginRateLimiter`, `ThroughputLimiter`) stays -as it is: they are synchronous, per-key lockout and per-endpoint budget -primitives used from FastAPI dependencies, a different shape from this -middleware's async per-IP window. +`users/auth_local/rate_limit.py` (`ThroughputLimiter`) stays as it is: a +synchronous per-endpoint budget used from FastAPI dependencies, a different +shape from this middleware's async per-IP window. Password-login lockout uses +the shared store instead (see [Sharing the store](#sharing-the-store)). ## Where it lives diff --git a/docs/modules/users.md b/docs/modules/users.md index cef566d1b..6ffe74841 100644 --- a/docs/modules/users.md +++ b/docs/modules/users.md @@ -19,14 +19,15 @@ The module is built on [`fastapi-users`](https://fastapi-users.github.io/) for p - A signed session cookie (`sm_auth` by default) — the auth module's `AuthMiddleware` calls `UsersAuthProvider.resolve_user`, which reads it on every request and populates `request.state.user`. Bearer tokens (`Authorization: Bearer `) resolve against `users_access_token`. - OAuth / OIDC sign-in (Google, GitHub, Microsoft/Entra ID, and any generic OIDC provider) — see [OAuth providers](#oauth-providers). - An invite flow — admins generate an invite link; the recipient sets their password via `POST /api/users/auth/accept-invite`. -- `LoginRateLimiter` — N failures within a window triggers a cooldown per `email::ip` key. +- Login lockout — see [Login lockout](#login-lockout). Shared across workers, source IPs and restarts. - `ThroughputLimiter` — per-IP rate limit across register / forgot-password / accept-invite / verify-token endpoints. ### Auth endpoints | Method + path | Body | Notes | |---|---|---| -| `POST /api/users/auth/login` | `OAuth2PasswordRequestForm` | sets `sm_auth` cookie + `session["user_id"]`; rate-limited per email | +| `POST /api/users/auth/login` | `OAuth2PasswordRequestForm` | sets `sm_auth` cookie + `session["user_id"]`; `400 LOGIN_BAD_CREDENTIALS`; `429` + `Retry-After` while locked — see [Login lockout](#login-lockout) | +| `POST /api/users/auth/logout` | — | clears the `sm_auth` cookie and the session | | `POST /api/users/auth/register` | `UserCreate` | gated by `users.allow_signup`; rate-limited | | `POST /api/users/auth/forgot-password` | `PasswordReset` | rate-limited | | `POST /api/users/auth/reset-password` | `{token, password}` | | @@ -34,7 +35,7 @@ The module is built on [`fastapi-users`](https://fastapi-users.github.io/) for p | `POST /api/users/auth/verify` | `VerifyRequest` | | | `POST /api/users/auth/accept-invite` | `AcceptInviteRequest` | sets password + signs the user in | | `POST /api/users/auth/demo/{role}` | — | one-click sign-in as a shared demo account (`role` ∈ `admin`, `user`); `404` unless that account is configured; rate-limited per role — see [Demo mode](#demo-mode-hosting-a-showcase-instance) | -| `POST /api/users/auth/token` | `TokenRequest` (email + password) | bearer login for mobile / API clients → `{access_token, refresh_token, token_type, expires_in}`; `401` for external/SSO users | +| `POST /api/users/auth/token` | `TokenRequest` (email + password) | bearer login for mobile / API clients → `{access_token, refresh_token, token_type, expires_in}`; `401` for bad credentials and external/SSO users; `429` + `Retry-After` while locked — see [Login lockout](#login-lockout) | | `POST /api/users/auth/token/refresh` | `RefreshRequest` (refresh_token) | rotates a refresh token into a new pair (old one revoked) | | `DELETE /api/users/auth/token` | `RefreshRequest` (refresh_token) | revokes a refresh token (idempotent) | | `GET /api/users/auth/{provider}/login` | — | OAuth: redirect to the IdP (`provider` ∈ configured set) | @@ -58,6 +59,7 @@ The module is built on [`fastapi-users`](https://fastapi-users.github.io/) for p | `DELETE /api/users/admin/{user_id}` | → `204` (hard delete; an admin cannot delete their own account → `400`) | | `PATCH /api/users/admin/{user_id}/disable` | → `UserListItem` | | `PATCH /api/users/admin/{user_id}/enable` | → `UserListItem` | +| `PATCH /api/users/admin/{user_id}/unlock` | → `UserListItem` — lifts a [login lockout](#login-lockout) and clears the failure count (audited) | | `PUT /api/users/admin/{user_id}/roles` | `RoleAssignment` → `UserListItem` | | `PATCH /api/users/admin/{user_id}/verify` | → `UserListItem` (mark verified; idempotent) | | `POST /api/users/admin/{user_id}/reset-password-link` | → `PasswordResetLink` (`409` for external/SSO users) | @@ -142,6 +144,9 @@ from users.contracts.events import ( | `tenant_id` | `str \| None` | indexed; only set when multi-tenant | | `disabled_at` | `datetime \| None` | | | `last_login_at` | `datetime \| None` | indexed | +| `failed_login_count` | `int` | failed password logins in the current window; `server_default 0`; not audited | +| `last_failed_login_at` | `datetime \| None` | when the window's last failure happened; not audited | +| `locked_until` | `datetime \| None` | set while a [login lockout](#login-lockout) holds; audited | | `roles` | relationship → `Role` via `UserRole` | eagerly loaded by `AuthMiddleware` | `Role` (table `users_role`) — `id`, `name` (unique, indexed), `description`. @@ -190,9 +195,10 @@ Everything else is DB-backed (initial values are pydantic defaults; edit under U | `mailer` | `"console"` (or `"smtp"`) | | `base_url` | `"http://localhost:8000"` | | `smtp_host` / `smtp_port` / `smtp_username` / `smtp_password` / `smtp_from` / `smtp_tls` | SMTP config when `mailer="smtp"` | -| `login_rate_limit_failures` | `5` | -| `login_rate_limit_window_seconds` | `300` | -| `login_rate_limit_cooldown_seconds` | `900` | +| `login_rate_limit_failures` | `5` — failures from one IP for one username before that source is throttled for the window | +| `login_rate_limit_window_seconds` | `300` — the window for both lockout tiers | +| `login_rate_limit_cooldown_seconds` | `900` — how long an account lock lasts | +| `login_lockout_account_failures` | `10` (≥ 1) — failures for one account, from anywhere, before it locks | | `auth_rate_limit_attempts` | `10` | | `auth_rate_limit_window_seconds` | `300` | | `bootstrap_email`, `bootstrap_password`, `bootstrap_user_email`, `bootstrap_user_password` | `""` — see [Bootstrap](#bootstrap-the-first-admin) | @@ -358,6 +364,31 @@ every settings load. Do not run it against a database you care about. Blanking (`smpy users create-admin`, or the `SM_USERS_BOOTSTRAP_*` vars), because a read-only demo session cannot complete the wizard. +## Login lockout + +Both password logins — `POST /api/users/auth/login` and `POST /api/users/auth/token` — go through `users.auth_local.lockout` (GH #530). Each attempt **takes a slot before its password is checked** and a correct password gives it back, so checking for a free slot and taking it are one step: a parallel burst cannot get more password tries than the threshold. An unverified-email `400` gives its slot back too — it is not a failure. + +- **Account lock (database).** A conditional, atomic `UPDATE … RETURNING` on `users_user.failed_login_count` takes the slot. The count restarts at 1 when the previous attempt is older than `login_rate_limit_window_seconds`. Every worker shares that count. The attempt that takes the last of `login_lockout_account_failures` (default 10) slots and fails sets `locked_until` to now + `login_rate_limit_cooldown_seconds` through the ORM, so the audit log records the lock, and is itself answered `429`. While it holds, every password login for the account answers `429` — the correct password included. A success clears the count but never `locked_until`. +- **Source throttle (host rate-limit store).** `login:ip::` admits `login_rate_limit_failures` (default 5) attempts per window; a correct password clears it. +- **Unknown usernames.** A username with **no** account goes through the same account rules on the store — `login:acct:` on a sliding window that matches the row's restart rule, `login:acct-lock:` for the cooldown — so it trips on the same attempt, answers with the same status, detail and `Retry-After`, and stays locked for as long. The unknown path still spends a password hash, as fastapi-users already does. + +The store is the host's `app.state.sm.rate_limit_store` — Redis when `SM_REDIS_URL` is set, per-worker counters otherwise (see [Request guards](../framework/request-guards.md#storage)). Without Redis and with more than one worker, an unknown username's counters are per worker (and do not survive a restart) while an account's are not, so that deployment shape can still tell the two apart; run Redis with several workers. + +The refusal is always `429 {"detail": "Too many attempts — try again later"}` with a `Retry-After` header. The login page shows its "too many attempts" message for it. + +Admins see a **locked** badge on the users list and a "Locked until" row with an **Unlock** button on the edit page, backed by `PATCH /api/users/admin/{user_id}/unlock`. Unlocking is an ORM write and is audited; the failure counters are excluded from audit diffs so failed attempts do not flood the log. + +The account default is 10 rather than 5 because anyone who knows an address could otherwise lock its owner out from any network; the per-source throttle still stops a single source after 5. OAuth, Keycloak and invite-link sign-ins involve no password and are not counted. The stock fastapi-users router (`/api/users/auth-inner/*`) is not mounted. + +A successful password reset (`POST /api/users/auth/reset-password`, from the emailed link or an admin's reset link) also lifts the lock and clears the failure count: proving control of the mailbox is the standard way out of a lockout. Like an admin unlock it is an ORM write, so the audit log records it. The per-source throttle keys are left to expire, since they are keyed by an address the reset request does not know. + +Trade-offs to know about: + +- **Anyone can lock a known address.** The lock holds against the right password, so whoever knows an account's email can keep its owner out of password login for the cooldown, from any network. This is mitigated by the threshold of 10, by the per-source throttle (one source gets 5), by admin unlock, and by password reset now lifting the lock; OAuth/Keycloak sign-in is unaffected. +- **A dead request still counts.** An attempt takes its slot before the password is checked; if the request dies (client disconnect, worker crash) after taking it and before settling, the slot stays counted until the window passes. If it held the last slot, the account answers `429` until `login_rate_limit_window_seconds` after that attempt — no `locked_until` is written, since only a settled failure writes it. +- **Without Redis, entries can be evicted.** The in-process store caps its key count and evicts under that pressure, so a flood of distinct usernames can drop an unknown username's lock or a source's throttle early. Account locks live on the row and are unaffected. Set `SM_REDIS_URL` in production. +- Login values longer than an address can be (254 characters) are hashed before they become store keys, so a large request body cannot park a large key in Redis. + ## Mailer backends `mailer/` ships two implementations of the `Mailer` protocol: @@ -447,5 +478,5 @@ Components (under `pages/Users/components/`): ## Notes - `cookie_secure` is automatically flipped to `False` in dev (`SM_ENVIRONMENT=development`) so login works over plain HTTP. Don't override it in non-dev environments. -- `LoginRateLimiter` keys login attempts by `lowercased-email::client-ip`. Counters live in-process (no Redis), so a multi-worker deployment has independent counters per worker — swap for a Redis-backed store when running more than one worker. +- `ThroughputLimiter` (register, forgot-password, accept-invite, verify-token, `/me/password`) counts in-process, so with N workers its budget is up to N times the configured one. Password-login lockout does not have that limit — see [Login lockout](#login-lockout). - The functional `lower(email)` index makes case-insensitive lookups fast on Postgres; on SQLite the lookup falls back to `LOWER(email) = ?` which still uses the regular index. diff --git a/docs/superpowers/plans/2026-10-19-login-lockout.md b/docs/superpowers/plans/2026-10-19-login-lockout.md new file mode 100644 index 000000000..388da3377 --- /dev/null +++ b/docs/superpowers/plans/2026-10-19-login-lockout.md @@ -0,0 +1,693 @@ +# Login Lockout Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make password-login lockout hold across workers, source IPs and restarts, and cover every password login route (GH #530). + +**Architecture:** Tier 1 is an account lock stored on `users_user` (`failed_login_count`, `last_failed_login_at`, `locked_until`), bumped by one atomic `UPDATE … RETURNING` and locked through the ORM so the audit log sees it. Tier 2 is a per-IP-per-username throttle (plus an unknown-username counter) on the host's existing rate-limit `WindowStore`, which the framework now exposes as `app.state.sm.rate_limit_store`. One helper, `users/auth_local/lockout.py`, fronts `/api/users/auth/login` and `/api/users/auth/token`; the stock `/auth-inner` router and the in-process `LoginRateLimiter` go away. + +**Tech Stack:** FastAPI, SQLModel/SQLAlchemy 2 async, Alembic, Redis (optional, via the host store), React + Inertia + vitest. + +**Spec:** `docs/superpowers/specs/2026-10-19-login-lockout-design.md` + +## Global Constraints + +- New setting: `login_lockout_account_failures: int = Field(10, ge=1)` on `UsersSettings`. +- Existing settings keep their meaning: `login_rate_limit_failures` (5) is the tier-2 threshold, `login_rate_limit_window_seconds` (300) is the window for both tiers, `login_rate_limit_cooldown_seconds` (900) is the account lock length. +- Locked/throttled responses: HTTP 429, detail `"Too many attempts — try again later"`, `Retry-After` header (seconds, ≥ 1). +- `/auth/token` keeps its 401 `"Invalid credentials"`; `/auth/login` keeps its 400 `LOGIN_BAD_CREDENTIALS`; `LOGIN_USER_NOT_VERIFIED` (400) never counts as a failure. +- Store keys: `login:ip::` and `login:acct:` (username lower-cased and stripped). No new Redis client. +- Migration: one revision in `host/migrations/versions/`, `down_revision = "b5018b617142"` (the head of the chain holding the users revisions), no `branch_labels`. +- `failed_login_count` and `last_failed_login_at` are excluded from audit diffs (`User.__audit_exclude_fields__`). +- Admin unlock: `PATCH /api/users/admin/{id}/unlock`, ORM write, guarded by `users.manage` like its siblings. +- 300-line cap on `.py`/`.ts`/`.tsx`; SQLModel only; framework never imports modules (SM009); every `.tsx` string through `t(keys…)`; test basenames globally unique (prefix `users_`). +- Commit messages end with `Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4`. + +## Review Focus + +1. Case/whitespace variants of the username (`Alice@Example.com `) must hit the same counters and the same lock as `alice@example.com`, on both routes. → Task 3 test `test_username_variants_share_one_counter`. +2. Failure bookkeeping must survive the error response: the endpoint raises 400/401, the request session rolls back, and the count must still be there on the next request. → Task 4 test `test_failures_persist_across_requests` (the acceptance test also covers it). +3. A Redis outage must not turn login into a 500 nor drop the throttle: `peek` must fall back to per-worker counters exactly like `hit`. → Task 1 test `test_peek_falls_back_when_redis_down`. +4. A successful login of a different user must not reset someone else's counter, and a correct password during a lock must not reset the count. → Task 4 test `test_correct_password_while_locked_does_not_unlock`. +5. A lock that has expired must not show as locked in the admin UI, and must not refuse logins. → Task 5 test `test_expired_lock_is_not_reported` and Task 3 test `test_expired_lock_allows_login`. + +--- + +### Task 1: Framework exposes the rate-limit store, with a non-counting `peek` + +**Files:** +- Modify: `framework/core/simple_module_core/rate_limit.py` (protocol + `InProcessWindowStore.peek`) +- Modify: `framework/hosting/simple_module_hosting/_rate_limit.py` (`RedisWindowStore.peek`, shared `_eval`) +- Modify: `framework/core/simple_module_core/services.py` (`rate_limit_store` field) +- Modify: `framework/hosting/simple_module_hosting/_phase_helpers.py` (`install_middleware(..., rate_limit_store=)`) +- Modify: `framework/hosting/simple_module_hosting/app_builder.py` (build the store once, pass to both) +- Test: `framework/hosting/tests/test_rate_limit_store_shared.py` + +**Interfaces:** +- Produces: `WindowStore.peek(key: str, spec: RateSpec) -> WindowResult` — reports the current window **without counting**; `allowed` means "one more hit would be within budget" (`count < spec.limit`); `retry_after` is seconds left in the window (0 when no window is live). `Services.rate_limit_store: WindowStore` (defaults to a fresh `InProcessWindowStore`). The same store instance backs `RateLimitMiddleware`. + +- [ ] **Step 1: Write the failing tests** + +```python +"""The host's rate-limit store is shared with modules (GH #530).""" + +from __future__ import annotations + +from simple_module_core.rate_limit import InProcessWindowStore, RateSpec +from simple_module_core.services import Services +from simple_module_hosting._rate_limit import RateLimitMiddleware, RedisWindowStore + + +class _Clock: + def __init__(self) -> None: + self.now = 1_000.0 + + def __call__(self) -> float: + return self.now + + +async def test_peek_does_not_count() -> None: + store = InProcessWindowStore(clock=_Clock()) + spec = RateSpec(2, 60) + assert (await store.peek("k", spec)).count == 0 + await store.hit("k", spec) + await store.hit("k", spec) + first = await store.peek("k", spec) + second = await store.peek("k", spec) + assert (first.count, first.allowed) == (2, False) + assert second.count == 2 + + +async def test_peek_window_expires() -> None: + clock = _Clock() + store = InProcessWindowStore(clock=clock) + spec = RateSpec(1, 60) + await store.hit("k", spec) + assert (await store.peek("k", spec)).retry_after == 60 + clock.now += 61 + result = await store.peek("k", spec) + assert (result.count, result.allowed, result.retry_after) == (0, True, 0) + + +class _FakeRedis: + def __init__(self) -> None: + self.counts: dict[str, int] = {} + + async def eval(self, script: str, numkeys: int, key: str, *args): + if args: # hit + self.counts[key] = self.counts.get(key, 0) + 1 + return [self.counts[key], args[0]] + count = self.counts.get(key) + return [0, -2] if count is None else [count, 30_000] + + +class _DeadRedis: + async def eval(self, *a, **k): + raise ConnectionError("redis down") + + +async def test_redis_peek_reads_without_counting() -> None: + redis = _FakeRedis() + store = RedisWindowStore(redis) + spec = RateSpec(1, 60) + assert (await store.peek("k", spec)).allowed is True + await store.hit("k", spec) + result = await store.peek("k", spec) + assert (result.count, result.allowed, result.retry_after) == (1, False, 30) + assert redis.counts == {"sm:rl:k": 1} + + +async def test_peek_falls_back_when_redis_down() -> None: + store = RedisWindowStore(_DeadRedis()) + spec = RateSpec(1, 60) + await store.hit("k", spec) # counted by the per-worker fallback + assert (await store.peek("k", spec)).allowed is False + + +def test_services_default_store() -> None: + field = Services.__dataclass_fields__["rate_limit_store"] + assert isinstance(field.default_factory(), InProcessWindowStore) + + +async def test_create_app_shares_one_store(app) -> None: + store = app.state.sm.rate_limit_store + node = app.middleware_stack + while node is not None and not isinstance(node, RateLimitMiddleware): + node = getattr(node, "app", None) + assert node is not None and node.store is store +``` + +- [ ] **Step 2: Run to verify it fails** + +Run: `uv run pytest framework/hosting/tests/test_rate_limit_store_shared.py -q` +Expected: FAIL — `AttributeError: 'InProcessWindowStore' object has no attribute 'peek'`, `KeyError: 'rate_limit_store'`. + +- [ ] **Step 3: Implement** + +`simple_module_core/rate_limit.py`: + +```python +class WindowStore(Protocol): + async def hit(self, key: str, spec: RateSpec) -> WindowResult: + """Count one request against *key* and report whether it is within budget.""" + ... + + async def peek(self, key: str, spec: RateSpec) -> WindowResult: + """Report *key*'s current window without counting. + + ``allowed`` answers "would one more hit be within budget", so a caller + can refuse before doing work and count only what failed. + """ + ... + + +class InProcessWindowStore: + async def peek(self, key: str, spec: RateSpec) -> WindowResult: + now = self._clock() + entry = self._windows.get(key) + if entry is None or entry[0] <= now * 1000: + return WindowResult(allowed=spec.limit > 0, retry_after=0, count=0) + end_ms, count = entry + retry = max(1, math.ceil(end_ms / 1000 - now)) + return WindowResult(allowed=count < spec.limit, retry_after=retry, count=count) +``` + +`simple_module_hosting/_rate_limit.py` — factor the Redis call and its fail-open handling into `_eval`, add the read-only script: + +```python +_PEEK_LUA = """ +local c = redis.call('GET', KEYS[1]) +if not c then return {0, 0} end +return {tonumber(c), redis.call('PTTL', KEYS[1])} +""" + + async def _eval(self, script: str, key: str, *args: Any) -> Any: + """Run *script* on *key*; ``None`` means "use the per-worker fallback".""" + if self._clock() < self._down_until: + return None + try: + return await self._client.eval(script, 1, _KEY_PREFIX + key, *args) + except Exception as exc: + now = self._clock() + self._down_until = now + _BACKOFF + if now - self._last_warn >= _WARN_EVERY: + self._last_warn = now + logger.warning( + "Rate limiter Redis unavailable (%s); using per-worker counters", exc + ) + return None + + async def hit(self, key: str, spec: RateSpec) -> WindowResult: + reply = await self._eval(_LUA, key, spec.period * 1000) + if reply is None: + return await self._fallback.hit(key, spec) + count, pttl = int(reply[0]), int(reply[1]) + retry = max(1, -(-pttl // 1000)) if pttl > 0 else spec.period + return WindowResult(allowed=count <= spec.limit, retry_after=retry, count=count) + + async def peek(self, key: str, spec: RateSpec) -> WindowResult: + reply = await self._eval(_PEEK_LUA, key) + if reply is None: + return await self._fallback.peek(key, spec) + count, pttl = int(reply[0]), int(reply[1]) + retry = max(1, -(-pttl // 1000)) if pttl > 0 else 0 + return WindowResult(allowed=count < spec.limit, retry_after=retry, count=count) +``` + +`services.py` — new field after `view_prefixes`: + +```python + #: The store behind ``RateLimitMiddleware`` (Redis when ``SM_REDIS_URL`` is + #: set), shared so a module throttling its own events — the users login + #: lockout — counts in the same place across workers instead of opening a + #: second Redis client. Defaulted for apps built outside ``create_app``. + rate_limit_store: WindowStore = field(default_factory=InProcessWindowStore) +``` + +with `from simple_module_core.rate_limit import InProcessWindowStore, WindowStore` at runtime import level. + +`_phase_helpers.install_middleware` gains a keyword `rate_limit_store: WindowStore` and passes `store=rate_limit_store`; `app_builder.create_app` builds `rate_limit_store = build_store(settings.redis_url)` just before Phase 8, passes it to `install_middleware` and to `Services(rate_limit_store=rate_limit_store)`. + +- [ ] **Step 4: Run to verify it passes** + +Run: `uv run pytest framework/hosting/tests/test_rate_limit_store_shared.py framework/hosting/tests/test_request_guard_rate_limit.py -q` +Expected: PASS. + +- [ ] **Step 5: Commit** — `feat(hosting): share the rate-limit store on app.state.sm (#530)` + +--- + +### Task 2: Lockout columns on `users_user` + migration + audit exclusions + +**Files:** +- Modify: `modules/users/users/models/user.py` +- Create: `host/migrations/versions/c8e4f1a2b6d9_users_login_lockout.py` +- Test: `modules/users/tests/test_users_lockout_columns.py` + +**Interfaces:** +- Produces: `User.failed_login_count: int` (server default `0`), `User.last_failed_login_at: datetime | None`, `User.locked_until: datetime | None` (all `AwareDateTime`); `User.__audit_exclude_fields__ = {"failed_login_count", "last_failed_login_at"}`. + +- [ ] **Step 1: Write the failing test** + +```python +"""Lockout columns on users_user (GH #530).""" + +from __future__ import annotations + +from datetime import UTC, datetime + +from simple_module_db.audit import snapshot_changes +from sqlalchemy import select +from users.models import User + + +async def test_new_user_starts_unlocked(users_db) -> None: + user = (await users_db.execute(select(User).limit(1))).scalar_one() + assert user.failed_login_count == 0 + assert user.last_failed_login_at is None + assert user.locked_until is None + + +async def test_counter_fields_are_not_audited(users_db) -> None: + user = (await users_db.execute(select(User).limit(1))).scalar_one() + user.failed_login_count = 3 + user.last_failed_login_at = datetime.now(UTC) + assert snapshot_changes(users_db.sync_session, None, None) == [] + user.locked_until = datetime.now(UTC) + [record] = snapshot_changes(users_db.sync_session, None, None) + assert [c["field"] for c in record.changes] == ["locked_until"] +``` + +(Adjust the change-dict key to whatever `snapshot_changes` emits — read `framework/db/simple_module_db/audit.py` first.) + +- [ ] **Step 2: Run** `uv run pytest modules/users/tests/test_users_lockout_columns.py -q` — Expected: FAIL (`AttributeError: failed_login_count`). + +- [ ] **Step 3: Implement** — on `User`: + +```python + # Login lockout (GH #530). The counter is bumped by one atomic UPDATE per + # failed password login, restarting once ``last_failed_login_at`` is older + # than the window; reaching the threshold stamps ``locked_until`` through + # the ORM so the lock is audited. See ``users.auth_local.lockout``. + failed_login_count: int = Field(default=0, sa_column_kwargs={"server_default": text("0")}) + last_failed_login_at: datetime | None = Field(default=None, sa_type=AwareDateTime()) + locked_until: datetime | None = Field(default=None, sa_type=AwareDateTime()) + + # Every wrong password writes these two; auditing them would bury the + # lock/unlock rows that matter under one row per failed attempt. + __audit_exclude_fields__: ClassVar[set[str]] = {"failed_login_count", "last_failed_login_at"} +``` + +Migration: + +```python +"""users: login lockout columns (GH #530) + +Revision ID: c8e4f1a2b6d9 +Revises: b5018b617142 +""" + +revision: str = "c8e4f1a2b6d9" +down_revision: str | None = "b5018b617142" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.add_column( + "users_user", + sa.Column("failed_login_count", sa.Integer(), server_default=sa.text("0"), nullable=False), + ) + op.add_column( + "users_user", sa.Column("last_failed_login_at", sa.DateTime(timezone=True), nullable=True) + ) + op.add_column( + "users_user", sa.Column("locked_until", sa.DateTime(timezone=True), nullable=True) + ) + + +def downgrade() -> None: + op.drop_column("users_user", "locked_until") + op.drop_column("users_user", "last_failed_login_at") + op.drop_column("users_user", "failed_login_count") +``` + +- [ ] **Step 4: Verify** — the test passes; on a scratch SQLite file: `SM_DATABASE_URL=sqlite+aiosqlite:////tmp/claude-1001/lockout/m.db uv run --project host alembic -c host/alembic.ini upgrade heads` then `… check` (only the known `users_access_token.expires_at` drift may remain); `make migrations-roundtrip-pg` with `SM_MIGRATIONS_PG_URL` on a disposable DB if Postgres is reachable. + +- [ ] **Step 5: Commit** — `feat(users): lockout columns on users_user (#530)` + +--- + +### Task 3: Lockout service `users/auth_local/lockout.py` + +**Files:** +- Create: `modules/users/users/auth_local/lockout.py` +- Modify: `modules/users/users/settings.py` (`login_lockout_account_failures`) +- Test: `modules/users/tests/test_users_lockout_service.py` + +**Interfaces:** +- Consumes: `app.state.sm.rate_limit_store` (`hit`/`peek`, Task 1); `User` columns (Task 2). +- Produces: + - `LOCKED_DETAIL = "Too many attempts — try again later"` + - `LoginAttempt(username, ip, store, settings, now)` (frozen dataclass); `LoginAttempt.build(username, ip, store, settings, now)` lower-cases and strips the username + - `login_attempt(request: Request, username: str) -> LoginAttempt` + - `async refused_for(session, attempt) -> int | None` — seconds to wait, `None` to proceed + - `async record_failure(session, attempt) -> None` — commits the session + - `async record_success(session, user_id) -> None` — no commit (rides the request's unit of work) + - `too_many_attempts(retry_after: int) -> HTTPException` + - module-level `_utcnow()` — the clock; tests monkeypatch it or build `LoginAttempt` with an explicit `now`. + +- [ ] **Step 1: Write the failing tests** (`users_db` + a `_settings(**overrides)` helper building `UsersSettings`, an `InProcessWindowStore`, `LoginAttempt` built directly with explicit `now`): + +```python +async def test_account_locks_at_threshold(users_db, alice) -> None: + attempt = _attempt("alice@example.com", now=T0, account=3) + for _ in range(3): + await record_failure(users_db, attempt) + await users_db.refresh(alice) + assert alice.locked_until == T0 + timedelta(seconds=900) + assert alice.failed_login_count == 0 + assert await refused_for(users_db, attempt) == 900 + + +async def test_expired_window_restarts_count(users_db, alice) -> None: + await record_failure(users_db, _attempt("alice@example.com", now=T0)) + await record_failure(users_db, _attempt("alice@example.com", now=T0 + timedelta(seconds=301))) + await users_db.refresh(alice) + assert alice.failed_login_count == 1 + + +async def test_expired_lock_allows_login( + users_db, alice +) -> None: ... # lock at T0, refused_for at T0+901s is None + + +async def test_success_resets_count( + users_db, alice +) -> None: ... # two failures, record_success, count 0 and last_failed_login_at None + + +async def test_ip_throttle_after_tier2_limit(users_db, alice) -> None: + store = InProcessWindowStore() + for _ in range(5): + await record_failure(users_db, _attempt("alice@example.com", ip="1.1.1.1", store=store)) + assert await refused_for(users_db, _attempt("alice@example.com", ip="1.1.1.1", store=store)) + assert ( + await refused_for(users_db, _attempt("alice@example.com", ip="2.2.2.2", store=store)) + is None + ) + + +async def test_unknown_username_counts_against_account_threshold( + users_db, +) -> None: ... # 10 failures for ghost@example.com from 10 IPs → refused_for is not None + + +async def test_username_variants_share_one_counter( + users_db, alice +) -> None: ... # failures as " Alice@Example.COM " and "alice@example.com" add up on one row +``` + +- [ ] **Step 2: Run** — Expected: FAIL (`ModuleNotFoundError: users.auth_local.lockout`). + +- [ ] **Step 3: Implement** — settings: + +```python + # Rate limit (login). ``login_rate_limit_failures`` throttles one source + # (IP + username) on the shared rate-limit store; the account threshold + # locks the account itself, whichever networks the failures come from. + # 10 rather than 5: anyone who knows an address could otherwise lock its + # owner out. ``…_cooldown_seconds`` is how long that lock lasts. + login_rate_limit_failures: int = 5 + login_rate_limit_window_seconds: int = 300 + login_rate_limit_cooldown_seconds: int = 900 + login_lockout_account_failures: int = Field(10, ge=1) +``` + +`lockout.py` core: + +```python +def _by_username(username: str) -> Any: + return func.lower(User.email) == username + + +async def refused_for(session: AsyncSession, attempt: LoginAttempt) -> int | None: + waits: list[int] = [] + stmt = select(User.locked_until).where(_by_username(attempt.username)) + for locked_until in (await session.execute(stmt)).scalars(): + if locked_until is not None and locked_until > attempt.now: + waits.append(math.ceil((locked_until - attempt.now).total_seconds())) + for key, spec in attempt.store_checks(): + result = await attempt.store.peek(key, spec) + if not result.allowed: + waits.append(result.retry_after) + return max(waits) if waits else None + + +async def record_failure(session: AsyncSession, attempt: LoginAttempt) -> None: + s = attempt.settings + await attempt.store.hit(attempt.ip_key, attempt.ip_spec) + window_start = attempt.now - timedelta(seconds=s.login_rate_limit_window_seconds) + stale = User.last_failed_login_at.is_(None) | (User.last_failed_login_at < window_start) + stmt = ( + update(User) + .where(_by_username(attempt.username)) + .values( + failed_login_count=case((stale, 1), else_=User.failed_login_count + 1), + last_failed_login_at=attempt.now, + updated_at=User.updated_at, # a failed sign-in is not an edit of the account + ) + .returning(User.id, User.failed_login_count) + .execution_options(synchronize_session=False) + ) + rows = (await session.execute(stmt)).all() + if not rows: + await attempt.store.hit(attempt.account_key, attempt.account_spec) + for user_id, count in rows: + if count >= s.login_lockout_account_failures: + user = await session.get(User, user_id, populate_existing=True) + user.locked_until = attempt.now + timedelta(seconds=s.login_rate_limit_cooldown_seconds) + user.failed_login_count = 0 + await session.commit() + + +async def record_success(session: AsyncSession, user_id: uuid.UUID) -> None: + await session.execute( + update(User) + .where(User.id == user_id) + .where((User.failed_login_count != 0) | User.last_failed_login_at.is_not(None)) + .values(failed_login_count=0, last_failed_login_at=None, updated_at=User.updated_at) + .execution_options(synchronize_session=False) + ) + + +def too_many_attempts(retry_after: int) -> HTTPException: + return HTTPException( + status_code=status.HTTP_429_TOO_MANY_REQUESTS, + detail=LOCKED_DETAIL, + headers={"Retry-After": str(max(1, retry_after))}, + ) +``` + +- [ ] **Step 4: Run** `uv run pytest modules/users/tests/test_users_lockout_service.py -q` — Expected: PASS. + +- [ ] **Step 5: Commit** — `feat(users): login lockout service backed by the DB and the shared store (#530)` + +--- + +### Task 4: Wire both login routes; drop `/auth-inner` and `LoginRateLimiter` + +**Files:** +- Modify: `modules/users/users/auth_local/api.py` (login uses lockout; remove `/auth-inner` mount and `get_rate_limiter`) +- Modify: `modules/users/users/auth_local/token_api.py` (token login uses lockout) +- Modify: `modules/users/users/auth_local/rate_limit.py` (delete `LoginRateLimiter`, fix module docstring) +- Modify: `modules/users/users/startup.py`, `modules/users/users/state.py` (drop `rate_limiter`) +- Modify: `modules/users/tests/test_rate_limit.py` (drop `LoginRateLimiter` tests), `modules/users/tests/_users_app_builders.py` (`database_url`/`overrides` params, `redis_url=None`) +- Modify: comments naming `LoginRateLimiter` in `self_account.py`, `tests/test_profile_password.py`, `modules/site_lock/site_lock/rate_limit.py` +- Test: `modules/users/tests/test_users_login_lockout.py` + +**Interfaces:** +- Consumes: Task 3's `login_attempt`, `refused_for`, `record_failure`, `record_success`, `too_many_attempts`. +- Produces: `_build_users_app(monkeypatch, *, allow_signup, seed_admin=True, database_url="sqlite+aiosqlite:///:memory:", overrides=None)`. + +- [ ] **Step 1: Write the failing tests** + +```python +async def test_acceptance_three_ips_two_apps_survives_restart(monkeypatch, tmp_path) -> None: + url = f"sqlite+aiosqlite:///{tmp_path / 'lockout.db'}" + overrides = {"login_lockout_account_failures": ("5", "int")} + app_a, ctx_a = await _build_users_app(monkeypatch, allow_signup=False, database_url=url, overrides=overrides) + app_b, ctx_b = await _build_users_app(monkeypatch, allow_signup=False, database_url=url, overrides=overrides) + await _make_standard_user(app_a, "alice@example.com") + plan = [(app_a, "1.1.1.1"), (app_b, "2.2.2.2"), (app_a, "3.3.3.3"), (app_b, "1.1.1.1"), (app_a, "2.2.2.2")] + for app, ip in plan: + r = await _login(app, ip, "alice@example.com", "wrong") + assert r.status_code == 400 + r = await _login(app_b, "9.9.9.9", "alice@example.com", "UserPass1!") + assert r.status_code == 429 and int(r.headers["Retry-After"]) > 0 + # fresh app, same DB file + app_c, ctx_c = await _build_users_app(monkeypatch, allow_signup=False, database_url=url, overrides=overrides) + assert (await _login(app_c, "8.8.8.8", "alice@example.com", "UserPass1!")).status_code == 429 + r = await _token(app_c, "8.8.8.8", "alice@example.com", "UserPass1!") + assert r.status_code == 429 + + +async def test_unknown_and_locked_look_the_same(...) # same status + detail +async def test_token_bad_credentials_still_401(...) # 401 "Invalid credentials" until throttled, then 429 +async def test_failures_persist_across_requests(...) # failed_login_count in DB after 2 bad logins +async def test_correct_password_while_locked_does_not_unlock(...) +async def test_success_resets_count(...) # 2 bad, 1 good → count 0 +async def test_auth_inner_login_is_gone(anon_client) # POST /api/users/auth-inner/login → 404/405 +async def test_tier2_throttles_ip_not_account(...) # 5 bad from IP A → 6th from A 429, IP B → 400 +``` + +- [ ] **Step 2: Run** — Expected: FAIL (counts don't persist across apps; `/auth/token` never 429s; `/auth-inner/login` still answers). + +- [ ] **Step 3: Implement** — `login()`: + +```python + db: AsyncSession = Depends(get_db), +): + attempt = login_attempt(request, credentials.username) + if (wait := await refused_for(db, attempt)) is not None: + raise too_many_attempts(wait) + ... authenticate ... + if user is None or not user.is_active: + await record_failure(db, attempt) + raise HTTPException(status_code=400, detail="LOGIN_BAD_CREDENTIALS") + ... verification check (unchanged, not counted) ... + await record_success(db, user.id) +``` + +`token_login()` — same shape around its existing checks: refuse first; `record_failure` before each 401; `record_success` once the password verified and the verification check passed. + +Remove the `auth_inner` include (the wrappers own `/auth/login` and `/auth/logout`; grep showed no callers of `/auth-inner` in code, tests, docs or JS). Delete `LoginRateLimiter`, `get_rate_limiter`, `UsersState.rate_limiter` and its construction in `_build_limiters`. + +- [ ] **Step 4: Run** `uv run pytest modules/users/tests -q -n 8` — Expected: PASS. + +- [ ] **Step 5: Commit** — `feat(users): lock accounts across workers and IPs on both login routes (#530)` + +--- + +### Task 5: Admin unlock + locked state in the UI + +**Files:** +- Modify: `modules/users/users/contracts/schemas.py` (`UserListItem.locked_until`) +- Modify: `modules/users/users/admin/queries.py` (`to_list_item` reports only a live lock) +- Modify: `modules/users/users/admin/service.py` (`unlock`) +- Modify: `modules/users/users/admin/api.py` (`PATCH /{user_id}/unlock`) +- Modify: `modules/users/users/admin/components/user-list-item.ts`, `UserRow.tsx`, `UserCards.tsx`, `StatusPill.tsx` (`LockedBadge`) +- Modify: `modules/users/users/pages/Users/Edit.tsx`, `components/AccountCard.tsx`, `components/useUserActions.ts` +- Modify: `modules/users/users/locales/en.json`; regenerate `packages/i18n/src/*.generated*.ts` with `make gen-i18n` +- Test: `modules/users/tests/test_users_admin_unlock.py`, `modules/users/tests-js/AccountCardLocked.test.tsx` + +**Interfaces:** +- Consumes: Task 2 columns; Task 3 `record_failure` (to lock in tests). +- Produces: `UserService.unlock(user_id) -> User`; `UserListItem.locked_until: datetime | None` (None when not locked *now*); TS `UserListItem.locked_until?: string | null`; `useUserActions(...).lockedUntil`, `.unlock()`, `.savingUnlock`; `AccountCard` props `lockedUntil: string | null`, `savingUnlock: boolean`, `onUnlock: () => void`; ``. + +New i18n keys (`users` namespace): `metadata_card.locked_until` "Locked until", `account_status.unlock_button` "Unlock", `edit.toast_unlocked` "Account unlocked", `edit.toast_unlock_failed` "Could not unlock the account", `user_row.locked_badge` "locked". + +- [ ] **Step 1: Write the failing tests** + +```python +async def test_unlock_clears_lock_and_is_audited(users_app, admin_client) -> None: + # lock alice via record_failure ×10, then: + r = await admin_client.patch(f"/api/users/admin/{alice_id}/unlock") + assert r.status_code == 200 and r.json()["locked_until"] is None + # DB: locked_until/failed_login_count/last_failed_login_at cleared + # audit_log: an "updated" entry with locked_until for the lock and one for the unlock; + # no entry whose only changes are failed_login_count / last_failed_login_at + + +async def test_list_reports_locked_until(...) # GET /api/users/admin → locked_until set +async def test_expired_lock_is_not_reported(...) # locked_until in the past → None +async def test_unlock_requires_manage(user_client) # 403 +``` + +```tsx +// tests-js/AccountCardLocked.test.tsx +test('shows Locked until and calls onUnlock', () => { + const onUnlock = vi.fn(); + render(); + expect(screen.getByText('Locked until')).toBeInTheDocument(); + fireEvent.click(screen.getByRole('button', { name: 'Unlock' })); + expect(onUnlock).toHaveBeenCalled(); +}); +test('no lock row when unlocked', () => { ... lockedUntil={null} → queryByText('Locked until') is null }); +``` + +- [ ] **Step 2: Run** — Expected: FAIL (404 on `/unlock`; no `locked_until` key; no "Locked until"). + +- [ ] **Step 3: Implement** + +```python + async def unlock(self, user_id: uuid.UUID) -> User: + user = await self._require_user(user_id) + user.locked_until = None + user.failed_login_count = 0 + user.last_failed_login_at = None + await self._db.flush() + return user +``` + +```python +@admin_router.patch("/{user_id}/unlock", response_model=UserListItem) +async def admin_unlock_user(user_id: uuid.UUID, service: UserService = Depends(get_user_service)): + """Lift a login lockout early. An ORM write, so the audit log records it.""" + try: + user = await service.unlock(user_id) + except UserNotFoundError: + raise HTTPException(status_code=404, detail="User not found") from None + return service.to_list_item(user) +``` + +`to_list_item`: `locked_until=_live_lock(user.locked_until)` where `_live_lock` returns the value only when it is after `datetime.now(UTC)`. + +`AccountCard`: when `lockedUntil` is set, a `Row` labelled `t(keys.users.metadata_card.locked_until)` showing `formatDateTime`-style local time plus an outline `Button` `t(keys.users.account_status.unlock_button)` calling `onUnlock`. `useUserActions` gains `lockedUntil` state (initialised from the prop) and `unlock()` (PATCH `/api/users/admin/${userId}/unlock`, toast, set `null`). `UserRow`/`UserCards` render `` next to the status pill when `user.locked_until` is set. + +- [ ] **Step 4: Run** `uv run pytest modules/users/tests/test_users_admin_unlock.py -q`, `npx vitest run modules/users/tests-js/AccountCardLocked.test.tsx`, `make gen-i18n`, `make ci-check-untranslated` — Expected: PASS. + +- [ ] **Step 5: Commit** — `feat(users): admin unlock and locked state in the users UI (#530)` + +--- + +### Task 6: Docs + CHANGELOG + +**Files:** +- Modify: `docs/modules/users.md` (lockout section, settings table, admin endpoint, remove the in-process-counter note) +- Modify: `docs/framework/request-guards.md` (§ Not covered → the store is shared as `app.state.sm.rate_limit_store`; `peek`) +- Modify: `CHANGELOG.md` `[Unreleased]` + +- [ ] **Step 1: Write** — `CHANGELOG.md`: + +```markdown +### Changed +- **Behaviour change (#530):** password-login lockout now holds across workers, + source IPs and restarts. Ten failures for one account (from anywhere) within + `login_rate_limit_window_seconds` lock it for `login_rate_limit_cooldown_seconds` + — the right password included — answering 429 with `Retry-After`. Five + failures from one IP for one username throttle that source for the window. + New setting `users.login_lockout_account_failures` (default 10). Counters live + on `users_user` and on the host rate-limit store (Redis when `SM_REDIS_URL` + is set); a migration adds three columns. +- **Behaviour change (#530):** `POST /api/users/auth/token` is now covered by the + same lockout and throttle (429 when locked; bad credentials still 401). +- **Behaviour change (#530):** the stock fastapi-users router at + `/api/users/auth-inner/*` is no longer mounted; use `/api/users/auth/login` + and `/api/users/auth/logout`. + +### Added +- `PATCH /api/users/admin/{id}/unlock` lifts a lockout (audited); the user list + shows a "locked" badge and the edit page a "Locked until" row with Unlock. +- `app.state.sm.rate_limit_store` and `WindowStore.peek()`. +``` + +- [ ] **Step 2: Verify** — `uv run ruff format --check docs/` and `make lint`. + +- [ ] **Step 3: Commit** — `docs(users): document login lockout (#530)` + +--- + +## Final verification + +`rmdir /tmp/.git 2>/dev/null`; `uv run pytest -q -n 8`, `make test-js`, `make lint`, `make ci-check-untranslated`, `make doctor` — each redirected to `/tmp/claude-1001/lockout/.log` with the real exit code recorded. diff --git a/docs/superpowers/specs/2026-10-19-login-lockout-design.md b/docs/superpowers/specs/2026-10-19-login-lockout-design.md new file mode 100644 index 000000000..1e322cc81 --- /dev/null +++ b/docs/superpowers/specs/2026-10-19-login-lockout-design.md @@ -0,0 +1,141 @@ +# Login lockout across workers and source IPs (GH #530) + +Approved 2026-10-19. + +## Problem + +`modules/users/users/auth_local/rate_limit.py` keeps failure counters and locks in +process-local `TTLCache`s. The key is `username::ip`, and only +`POST /api/users/auth/login` consults them. The consequences: + +- With N workers, the effective threshold is N× the configured one. +- An attacker who rotates source IPs never trips the lock for an account. +- A restart clears every lock. +- `POST /api/users/auth/token` has no lockout and no throttle. +- The stock fastapi-users route `/api/users/auth-inner/login` has no throttle. + +## Design + +### Tier 1 — account lock (database) + +New nullable columns on `users_user`, added by one Alembic migration in +`host/migrations/versions/` that chains off the current users head: + +| column | type | default | +|---|---|---| +| `failed_login_count` | int, not null | `0` (server default) | +| `last_failed_login_at` | `AwareDateTime`, nullable | — | +| `locked_until` | `AwareDateTime`, nullable | — | + +On a failed password login for an existing user, a single +`UPDATE users_user … RETURNING failed_login_count` does the bookkeeping: + +- It increments the count. +- It resets the count to 1 when `last_failed_login_at` is older than + `login_rate_limit_window_seconds`. +- It stamps `last_failed_login_at`. + +The update is atomic across workers. When the returned count reaches +`login_lockout_account_failures`, the user row is loaded through the ORM and +updated: + +- `locked_until` = now + `login_rate_limit_cooldown_seconds`; +- `failed_login_count` = 0. + +Going through the ORM means the automatic audit capture records the lock. + +A successful login resets `failed_login_count` to 0 and clears +`last_failed_login_at`. + +While `locked_until > now`, every password login for that account gets a 429, +the correct password included. The response carries `Retry-After` and the +existing detail, "Too many attempts — try again later". + +### Tier 2 — per-IP + account throttle (host rate-limit store) + +The existing "5 failures from one IP within the window" check moves onto the +host's rate-limit `WindowStore`. That store is Redis-backed when `SM_REDIS_URL` +is set and in-process otherwise. The framework exposes the store it already +builds on `app.state.sm` (`rate_limit_store`); no new Redis client is created. + +The keys are: + +- `login:ip::` for failures from one IP. +- `login:acct:` for a username with **no** account. Unknown usernames + are counted against the account threshold here, so a 429 does not reveal + whether an account exists. + +A key that is over its limit refuses further attempts until its window ends. + +### Login paths + +Both login routes use one helper, `auth_local/lockout.py`: + +- **`/api/users/auth/login`** (session login). +- **`/api/users/auth/token`** — gains both tiers. It returns 429 when locked, and + its existing 401 for bad credentials is unchanged. + +The steps, in order: + +1. Refuse if locked: tier 1 by `locked_until`, tier 2 by store keys. +2. Authenticate. +3. On failure, record it in both tiers. +4. On success, reset tier 1. + +Two other changes to the login surface: + +- The stock `/api/users/auth-inner/login` is no longer mounted. Its logout + stays, unless nothing uses it either. The wrappers already shadow it, and its + callers are grepped first. +- The in-process `LoginRateLimiter` is removed once nothing uses it. + `ThroughputLimiter` (register, forgot-password and the other routes) is + unchanged. + +### Settings (`UsersSettings`) + +- New: `login_lockout_account_failures: int = Field(10, ge=1)`. +- Existing settings keep their meaning: + - `login_rate_limit_failures` (5) — tier-2 threshold. + - `login_rate_limit_window_seconds` (300) — window for both tiers. + - `login_rate_limit_cooldown_seconds` (900) — account lock length. + +The account default is 10 rather than the issue's 5. With 5, anyone who knows +an email could lock its owner out from any network. Tier 2 still stops a single +source after 5. + +### Admin + +- `PATCH /api/users/admin/{id}/unlock` clears `locked_until`, + `failed_login_count` and `last_failed_login_at` through the ORM, so the + unlock is audited. +- `UserListItem` gains `locked_until`. +- The user edit page shows "Locked until