Repository navigation
security(auth): MFA challenge codes can be guessed without limit — no attempt counter, no rate limit #689
Copy link
Copy link
Closed
Labels
area:backendGo, /internal, /pkgGo, /internal, /pkgarea:securityCybersecurity and threat intelligenceCybersecurity and threat intelligencepriority:P1HighHighpriority:P1-highBlocks a milestoneBlocks a milestonestatus:in-reviewPR openPR opentier:0-trustTrust: security, isolation, evidence integrityTrust: security, isolation, evidence integritytype:securitySecurity defect or hardeningSecurity defect or hardening
Milestone
Description
Problem
The second factor can be guessed without limit. After someone passes the password check, nothing stops them from submitting every TOTP code until one matches, which defeats MFA for anyone who holds a leaked or phished password.
Found by reading the code (2026-09-15), not exploited
- No throttle on the route:
POST /api/v1/auth/mfa/challenge(backend/cmd/server/main.go) is behindMFATokenMiddlewareonly. It has no rate limiter, unlike/auth/login. - No attempt counter:
MFAHandler.Challenge(backend/internal/handler/auth/mfa_handler.go) andChallengeMFAUseCase.Execute(backend/internal/application/auth/mfa_usecase.go) verify the code, then fall back to backup codes;- no failed attempt is counted;
- the challenge token is not invalidated after failures.
- Long window: the challenge token lives
MFAChallengeTTL = 5 * time.Minuteand can be reused for every attempt in that window. A new one is one more login away. - The arithmetic: a 6-digit TOTP with its tolerance window leaves a few valid codes out of 10^6. Unthrottled requests over 5 minutes, repeated across fresh challenge tokens, make a guess realistic.
- Audit only: each failure is written to the audit log, and nothing acts on it.
Also noted, lower severity: POST /auth/logout revokes the refresh token and clears cookies, but the access token stays valid until its 15-minute TTL, although a JTI blacklist exists (jtiBlacklistChecker).
Acceptance criteria
- Per-token limit: after 5 failed codes on one challenge token, that token is dead (JTI blacklisted), and further attempts with it answer 401.
- Per-account limit: failed MFA attempts for one user are counted across challenge tokens and IPs. After a threshold in a window, challenges for that user are refused for a bounded time, and the event is audited.
- Per-IP limit: the route has its own IP rate limit, separate from the login bucket (see the related throttle issue).
- Backup codes: backup-code attempts count towards the same limits.
- Logout: the access token's JTI is blacklisted, so the access token stops working immediately.
- Tests:
TestMFAChallenge_TokenDiesAfterFiveFailuresTestMFAChallenge_UserBackoffAcrossTokensTestMFAChallenge_BackupCodesCountedTestLogout_AccessTokenRejectedAfterLogout
Definition of Done
-
go build ./... && go vet ./... && go test ./... -racegreen, with output pasted here. - Manual check against a local instance, with an enrolled test account, for criteria 1, 2 and 5.
- Progress comment in the CLAUDE.md format; PR with
Closesthis issue.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
area:backendGo, /internal, /pkgGo, /internal, /pkgarea:securityCybersecurity and threat intelligenceCybersecurity and threat intelligencepriority:P1HighHighpriority:P1-highBlocks a milestoneBlocks a milestonestatus:in-reviewPR openPR opentier:0-trustTrust: security, isolation, evidence integrityTrust: security, isolation, evidence integritytype:securitySecurity defect or hardeningSecurity defect or hardening
Activity
fix(auth): add a shared store for MFA challenge attempts (#689)
fix(auth): limit MFA challenge attempts per token and per account (#689)
fix(auth): mail the owner when MFA sign-in gets locked (#689)
fix(auth): enforce MFA challenge limits over HTTP (#689)
fix(auth): revoke the access token on logout (#689)
docs(auth): mark the #689 plan done (#689)
alex-dembele commentedon Oct 2, 2026
backend — 2026-10-02
Done
docs/689_MFA_CHALLENGE_LIMITS.md, with the owner's decisions: limits as proposed, a lock email, and the frontend copy split into fix(frontend): the MFA code screen does not tell users to sign in again or wait after too many wrong codes #872.backend/internal/infrastructure/authmfa/attempts.go, plus theMFAAttemptStoreandTokenRevokerports inbackend/internal/application/auth/mfa_limits.go.AllowRate, because a 5-minute token can straddle two of its wall-clock buckets.ChallengeMFAUseCase(mfa_usecase.go,mfa_limits.go).backend/internal/infrastructure/authmail/reset_mailer.go,async.go.backend/internal/handler/auth/mfa_handler.goanswers401 MFA_CHALLENGE_EXHAUSTEDand429 MFA_LOCKEDwithRetry-After. The lock is audited once (mfa_locked). Inbackend/cmd/server/main.go, the route has a per-IP limit of 20 per 5 min, in its own prefixed bucket, ahead of the token middleware.backend/internal/application/auth/logout.goandbackend/internal/handler/auth/handler.go.Verified
go build ./... && go vet ./... && go test ./... -raceon the final tip (f4b34f32):gate-exit=0, 80 packagesok, 0FAIL.internal/handler:TestRealtimeStream_HelloDocumentsTheRevocationBoundhit a data race (a test spy against a stream goroutine).-count=10on a master worktree), so it is an intermittent flake, unrelated to this branch. Tracked in fix(test): data race in the realtime stream revocation test makes go test -race fail at random #873.TestMFAChallenge_TokenDiesAfterFiveFailures,_UserBackoffAcrossTokens,_BackupCodesCounted,_Success,_NotFound,_Unauthorized,_SuccessClearsFailureCount,_LockSendsOneEmail,_StoreErrorFailsClosed;TestLogout_AccessTokenRejectedAfterLogout(over HTTP, plus a cookie variant and a forged-token case), andTestLogout_Success,_NotFound,_Unauthorized,_BlacklistOutageStillRevokesRefreshToken;TestMFAChallengeE2E_*(4);authmfastore (4) andauthmaillock email (2).expected: 401, actual: 200), so it catches the bug.Criteria: 1 ✅ · 2 ✅ · 3 ✅ · 4 ✅ · 5 ✅ · 6 ✅ (the four named tests exist and pass)
Next: owner review of the PR. Then #872, so the sign-in screen tells users to start over or wait instead of showing "invalid code".
Blocked on: nothing.
Not done, deliberately:
Branch note:
4f544415,POST /users) had landed on the original689-securityauth-…branch from a parallel session in the shared working tree.870-…branch, where nothing is lost, and rebased security(auth): MFA challenge codes can be guessed without limit — no attempt counter, no rate limit #689 without it.689-mfa-challenge-limits, and the PR comes from there. The old remote branch689-securityauth-…still carries the fix(users): POST /users creates an account that can never sign in (no default_org_id) #870 commit. Delete it by hand once the PR is open.688-auth-throttleis stacked on the old security(auth): MFA challenge codes can be guessed without limit — no attempt counter, no rate limit #689 commits. Rebase it with:git rebase --onto origin/689-mfa-challenge-limits fff5eace 688-auth-throttlefix(frontend): tell users to sign in again or wait after too many MFA…