fix(security): rate-limit login + 2FA challenge endpoints (LC-151) #184

Merged

nrupard merged 1 commit from feat/lc-151-login-rate-limit into main

2026-05-22 20:11:04 +02:00

nrupard commented

2026-05-22 20:06:36 +02:00

Owner

What

Closes the unthrottled-login gap from the LC-148 audit (finding S5). Registration was per-IP rate-limited but POST /login, POST /login/2fa, and POST /login/recovery were not, leaving online password + TOTP/recovery-code brute force unthrottled.

Fix

New RateLimitKind::Login bucket and a shared enforce_login_rate_limit helper, called at the top of all three handlers (before any password/code verify), so attempts count regardless of success.
The three endpoints share one per-IP budget, so guessing the password and guessing the second factor draw from the same allowance.
Operator sets the cap via a new rate_limit_logins setting (0 = disabled), surfaced on the admin anti-spam page next to the existing caps.
Per-IP, so effective only behind a trusted reverse proxy (client_ip_for_rate_limit returns None and no-ops otherwise) - the admin field help text says so. Consistent with the existing register/message limiters.

Test

routes_anti_spam: login_rate_limit_returns_429_after_cap (Nth+1 wrong-password attempt from one IP -> 429, different IP unaffected) and login_and_2fa_challenge_share_one_per_ip_budget (a /login/2fa attempt is throttled on the budget burned via /login, and the rate-limit check precedes the pending-cookie check so it 429s even with no valid challenge). just check / just test / just test-saas green (the routes_uploads upload-pipeline flake under concurrent-binary load is pre-existing and passes in isolation - this PR touches no upload code).

Part of the LC-159 post-audit story.

🤖 Generated with Claude Code

## What Closes the unthrottled-login gap from the LC-148 audit (finding S5). Registration was per-IP rate-limited but `POST /login`, `POST /login/2fa`, and `POST /login/recovery` were not, leaving online password + TOTP/recovery-code brute force unthrottled. ## Fix - New `RateLimitKind::Login` bucket and a shared `enforce_login_rate_limit` helper, called at the **top** of all three handlers (before any password/code verify), so attempts count regardless of success. - The three endpoints share **one** per-IP budget, so guessing the password and guessing the second factor draw from the same allowance. - Operator sets the cap via a new `rate_limit_logins` setting (`0` = disabled), surfaced on the admin anti-spam page next to the existing caps. - Per-IP, so effective only behind a trusted reverse proxy (`client_ip_for_rate_limit` returns `None` and no-ops otherwise) - the admin field help text says so. Consistent with the existing register/message limiters. ## Test `routes_anti_spam`: `login_rate_limit_returns_429_after_cap` (Nth+1 wrong-password attempt from one IP -> 429, different IP unaffected) and `login_and_2fa_challenge_share_one_per_ip_budget` (a `/login/2fa` attempt is throttled on the budget burned via `/login`, and the rate-limit check precedes the pending-cookie check so it 429s even with no valid challenge). `just check` / `just test` / `just test-saas` green (the `routes_uploads` upload-pipeline flake under concurrent-binary load is pre-existing and passes in isolation - this PR touches no upload code). Part of the LC-159 post-audit story. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

nrupard added 1 commit

2026-05-22 20:06:36 +02:00

fix(security): rate-limit login + 2FA challenge endpoints (LC-151)

check-secrets / TruffleHog (push) Successful in 7s

Details

check-secrets / Kingfisher (push) Successful in 10s

Details

check-secrets / Nosey parker (push) Successful in 7s

Details

check-secrets / Nosey parker (pull_request) Successful in 5s

Details

check-secrets / Kingfisher (pull_request) Successful in 10s

Details

check-secrets / TruffleHog (pull_request) Successful in 11s

Details

Check / clippy + fmt + tests (pull_request) Successful in 3m2s

Details

Create release / Create release from merged PR (pull_request) Has been skipped

Details

ebdc87928c

Registration was per-IP throttled but POST /login, POST /login/2fa, and POST /login/recovery were not, leaving online password and TOTP/recovery-code brute force unthrottled (audit S5).

Adds a RateLimitKind::Login bucket and a shared enforce_login_rate_limit helper, called at the top of all three handlers before any password/code verification, so attempts are counted regardless of outcome. The three endpoints share one per-IP budget so guessing the password and guessing the second factor draw from the same allowance. Operator sets the cap via a new rate_limit_logins setting (0 = disabled), exposed on the admin anti-spam page alongside the existing message/registration/password-reset caps.

Like the register limiter this is per-IP, so it is only effective behind a trusted reverse proxy (client_ip_for_rate_limit returns None and the check no-ops otherwise); the help text on the admin field says so.

Tests: login_rate_limit_returns_429_after_cap (Nth+1 wrong-password attempt from one IP gets 429, a different IP is unaffected) and login_and_2fa_challenge_share_one_per_ip_budget (a /login/2fa attempt is throttled on the budget burned via /login, and the rate-limit check precedes the pending-cookie check so it 429s even with no valid challenge).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>