[cristim]

Summary

The MFA lifecycle endpoints (/api/auth/mfa/setup, /enable, /disable, /regenerate-recovery-codes) now go through the existing rate limiter, keyed per IP and per session user, fail-closed.

Root cause

None of the four MFA handlers called checkRateLimit/checkRateLimitStrict, and getDefaultRateLimits had no MFA bucket. A holder of a stolen session could POST unlimited TOTP guesses to regenerate-recovery-codes (about 333k expected requests with the 3-window verifier) and receive fresh recovery codes.

Fix

• New mfa bucket in getDefaultRateLimits: 10 attempts / 15 min (setup plus enable plus a few typos fit comfortably), registered in assertRateLimitKeysKnown.
• New requireMFASession in middleware.go: checkRateLimitStrict(..., "mfa") per IP (same helper as login/change-password), then requireSession, then AllowWithUser(..., "mfa"). Limiter errors return 503 and 429 on exhaustion, matching the other credential endpoints. The per-user key stops a single session from resetting its budget by rotating source IPs.
• The four MFA handlers call requireMFASession instead of requireSession.
• Login's TOTP leg is already covered by the existing login bucket (strict, per IP), unchanged here.

No new mechanism: this reuses RateLimiterInterface (AllowWithIP/AllowWithUser) and checkRateLimitStrict.

Interaction with #109

The in-memory limiter is per instance, so on a multi-instance deploy the effective budget is 10 x instance count per window. That is still a bounded, low rate (the brute-force estimate goes from minutes to years per account on a handful of instances), but it is not a global limit until #109 is resolved. Not fixed here.

Out of scope (filed as #442)

The TOTP replay window appended to #94 (audit A03-016: no last-accepted-counter tracking) and wiring MFA failures into the FailedLoginAttempts lockout are separate mechanisms; tracked in #442.

Regression test

internal/api/handler_auth_test.go:

• TestHandler_mfaEndpoints_RateLimited: for each of the 4 endpoints, with a real InMemoryRateLimiter, attempts 1..10 reach the service (400 invalid code) and attempt 11 gets 429. Run twice per endpoint: fixed IP, and a different source IP per attempt (proves the per-user key).
• TestHandler_mfaEnable_RateLimiterErrorFailsClosed: limiter error returns 503.

Proof it fails pre-fix: stashed the four non-test files and ran both tests; both FAIL (expected: 429 on attempt 11, and the fail-closed test fails). With the fix, all 10 (sub)tests pass.

Verification

• GOTOOLCHAIN=go1.26.6 GOWORK=off go build ./...: ok
• go vet ./internal/api/: ok
• go test -race -short -count=1 ./internal/api/: ok
• golangci-lint v2.10.1 run ./internal/api/... (CI pin): 0 issues
• go mod tidy -diff: empty
• pre-commit hooks: passed

Closes #94

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 5 billable files and costs up to $1.25.}

• Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 51 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 63 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml

Review profile: CHILL

Plan: Essentials

Run ID: 0faa54e3-f506-4e23-8c5f-8c17f7c6d923

📥 Commits

Reviewing files that changed from the base of the PR and between 1848880 and 5d1f5a8.

📒 Files selected for processing (5)

• internal/api/handler.go
• internal/api/handler_auth.go
• internal/api/handler_auth_test.go
• internal/api/middleware.go
• internal/api/rate_limiter.go

---

_{Comment @coderabbitai help to get the list of available commands.}

[cristim]

Independent adversarial review (Opus 5.5) at 5d1f5a8: MERGE. Every TOTP/recovery-code verifier is throttled: login under the existing strict 'login' bucket; setup, enable, disable and regenerate now via requireMFASession ('mfa' bucket, 10 per 15 min, per IP then per session user, fail-closed 503 only on a limiter error). Brute force is capped at about 960 guesses/day/user. Pre-fix: both new tests fail. go test -race -short api/auth 3212 pass, golangci-lint v2.10.1 clean; CI 25/25. Minor follow-ups added to #442.