Repository navigation
feat(users): login lockout across workers, source IPs and restarts (#530) - #555
Merged
Merged
Conversation
The users login lockout needs a counter that every worker sees. The host already builds one for RateLimitMiddleware (Redis when SM_REDIS_URL is set), so expose that instance as Services.rate_limit_store instead of opening a second client, and give WindowStore a non-counting peek() so a caller can refuse before doing work and count only what failed. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
failed_login_count, last_failed_login_at and locked_until, plus the migration. The two counter fields are excluded from audit diffs so failed attempts don't flood the log; locked_until stays audited. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
…ore (#530) users.auth_local.lockout: refused_for / record_failure / record_success. The account tier is one atomic UPDATE ... RETURNING on users_user with a window reset, locking through the ORM at login_lockout_account_failures (new setting, default 10). The source tier counts per IP+username, and unknown usernames against the account threshold, on the host store. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
#530) /auth/login and /auth/token now share users.auth_local.lockout: refuse when locked or throttled (429 + Retry-After), count every wrong password in both tiers, clear the account count on success. /auth/token keeps its 401 for bad credentials. The stock fastapi-users router at /auth-inner is no longer mounted: its login bypassed every throttle and nothing called its logout. The in-process LoginRateLimiter is gone; ThroughputLimiter is unchanged. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
PATCH /api/users/admin/{id}/unlock clears the lock and the counters
through the ORM, so the unlock is audited. UserListItem carries
locked_until (only while the lock holds); the users list shows a
"locked" badge and the edit page a "Locked until" row with Unlock.
Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
Users module: lockout section, columns, settings, unlock endpoint, token and logout rows. Request guards: the shared store and peek(). CHANGELOG [Unreleased] entries for the behaviour changes and the admin unlock. The plan is aligned with what was built and ruff-formatted. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
Enumeration: an unknown username now goes through the account rules on the store - a sliding counter matching the row's restart rule and a separate lock key held for the cooldown - so it trips on the same attempt, with the same status, detail and Retry-After, for as long as a real account. Before, it used a fixed window and unlocked after the window (300s) instead of the cooldown (900s). Races: attempts now take a slot before the password is checked (a conditional atomic UPDATE on the row, a hit on the store) and a correct password gives it back, so a parallel burst gets at most the threshold of password checks. The failure that takes the last slot locks and is answered 429. A success clears the count but never locked_until, and the lock write only touches columns no counting UPDATE can reach while the row has no free slot. Unverified-email 400s give their slot back. WindowStore gains reset() and hit(..., sliding=True) for this. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
…inside authenticate (#530) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
…ers, window-store helpers - lockout reuses auth_local.rate_limit.client_ip (renamed from _client_ip) - _client/_login/user_row/ALICE in _users_app_builders, one alice fixture in conftest - InProcessWindowStore._live_entry/_retry_s; RedisWindowStore replies via _parse Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
…ed prop defaults - lockout._admit_unknown builds its one RateSpec inline; no store alias - useUserActions initialLockedUntil and AccountCard lockedUntil are required Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
- lockout: usernames over 254 chars are hashed in store keys, so a large body cannot park a large Redis key (reviewer fix, now tested). - lockout: when the conditional slot UPDATE misses because the account was locked meanwhile, Retry-After reports the remaining lock time, matching the unknown-username lock; enumeration parity test checks mid-lock. - manager: a successful password reset clears locked_until and the failure counters through the ORM (audited), so mailbox control unlocks. - token_api: the bearer login looks the email up case-insensitively, like the session login; dummy-hash timing equaliser kept and tested. - docs/CHANGELOG: lockout trade-offs (targeted lockout, dead requests still count, in-process store eviction without Redis); reset unlocks. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Deploying simple-module-python with
|
| Latest commit: |
20b89de
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ea3ed533.simple-module-python.pages.dev |
| Branch Preview URL: | https://feat-login-lockout-530.simple-module-python.pages.dev |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #530
Summary
Login lockout now holds across web workers, source IPs and restarts, and covers both password routes.
users_usercolumns:failed_login_count,last_failed_login_atandlocked_until(migrationc8e4f1a2b6d9).UPDATE … RETURNINGbefore the password is checked. A parallel burst therefore gets at most the threshold of password checks.Retry-After(the time remaining), even with the correct password.app.state.sm.rate_limit_store: Redis whenSM_REDIS_URLis set, otherwise in-process.login:ip:<ip>:<user>limits a single source.login:acct:andlogin:acct-lock:with the same sliding window, threshold and cooldown as real accounts. A 429 and itsRetry-Aftertherefore don't reveal whether an account exists./auth/loginand/auth/tokenshare one lockout helper./auth/tokenwas previously unthrottled, and its email lookup is now case-insensitive. The unguarded stock/auth-innerrouter is removed.PATCH /api/users/admin/{id}/unlock(audited). The users list shows a "locked" badge, and the edit page shows "Locked until" with an Unlock button.login_lockout_account_failures(default 10, minimum 1). The window (300 s) and cooldown (900 s) reuse the existing settings.Design:
docs/superpowers/specs/2026-10-19-login-lockout-design.md. Plan:docs/superpowers/plans/2026-10-19-login-lockout.md. The trade-offs are documented indocs/modules/users.md:Verification
Retry-Afterat every step, including mid-lock.make migrations-roundtrip-pgpasses on a throwaway Postgres database.make lint,make test-jsand the full Python suite (4679 passed) pass./ship:Retry-Afterparity under a race, password reset unlocking, case-insensitive token lookup, and hashing usernames over 254 characters in store keys.https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4