feat: add per-IP login rate limit alongside per-account lockout - #181
jakubfilinger-b wants to merge 1 commit into
Conversation
…kout
Adds a coarse per-IP throttle on /identity/login, separate from the
existing per-account lockout and per-email request throttle. Protects
against credential stuffing spread across many accounts from one
source, which the per-email counter can't see since each guess lands
on a different key.
- New RATE_LIMIT_KEYS.LOGIN_IP ('login-ip') key prefix.
- IdentityServiceOptions.loginIpRateLimit lets an operator tune or
disable the threshold; default is 100/15min, well above the 10/5min
per-email budget so a shared network (office NAT) is never blocked.
- Fails closed (onUnavailable: 'deny'), matching the other
credential-guessing keys.
- Skipped when the request has no IP, rather than bucketing under a
shared 'unknown' key - that shared key would let one IP-less
client's traffic exhaust the budget for every other IP-less client.
- Not reset on successful login (unlike the per-email key), so a
successful login on one account can't be used to clear the IP's
failure budget.
Tests cover: per-IP exhaustion across different accounts, per-IP
bucket isolation, the no-IP skip, the enabled:false override, the
fail-closed behavior on backend unavailability, and that locking one
account never blocks a different account signing in from the same IP.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
b6dac81 to
9b388fe
Compare
jakubfilinger-b
changed the title
| // a shared 'unknown' key - that shared key would let one client's traffic exhaust the | ||
| // budget for every other client with no IP, which is a self-inflicted DoS. | ||
| const ipRateLimitOptions = this.options?.loginIpRateLimit; | ||
| if (ip && (ipRateLimitOptions?.enabled ?? true)) { |
There was a problem hiding this comment.
This limiter uses an IP extracted from request headers. The runtime preserves a client-supplied X-Real-IP, so an attacker can rotate it on every attempt and create a fresh login-ip:* bucket. Sending only X-Forwarded-For also leaves the IP null, which skips this gate. Establish the IP at the trusted proxy/socket boundary before this path consumes it, and add an E2E proving spoofed headers cannot evade the limit.
| // Skipped when the IP is unknown (no trusted proxy header) rather than bucketed under | ||
| // a shared 'unknown' key - that shared key would let one client's traffic exhaust the | ||
| // budget for every other client with no IP, which is a self-inflicted DoS. | ||
| const ipRateLimitOptions = this.options?.loginIpRateLimit; |
There was a problem hiding this comment.
loginIpRateLimit accepts arbitrary numeric values. With windowMs: -1, Redis deletes the key after each INCR, so every request starts at count 1 and this limiter never exhausts. Validate positive, finite integer limit and windowMs values at the configuration boundary, with hostile-value coverage.
| * per-account threshold so a shared network (office NAT) never blocks legitimate | ||
| * users signing into their own accounts. | ||
| */ | ||
| export type IdentityLoginIpRateLimitOptions = { |
There was a problem hiding this comment.
This new public option type is not re-exported from @openora/core/contracts, unlike IdentityLockoutOptions. Please add it to the contracts barrel so consumers can name the configuration type through the public subpath.
damianrzepka
left a comment
damianrzepka
left a comment
There was a problem hiding this comment.
NO-GO — the per-IP limiter can be bypassed through client-controlled IP headers, and invalid configuration values can disable it.
|
Superseded by #184 (same change, renamed the branch). |
3 participants
What
Adds a coarse per-IP throttle on
/identity/login, separate from the existing per-accountlockout (
user.failedLoginAttempts/lockoutUntil) and the existing per-email requestthrottle (
login:<email>, 10/5min).Why
The per-email throttle and per-account lockout both key on the email/account being attacked,
so they see repeated guessing against one account but are blind to credential stuffing
spread thin across many accounts from the same source (a botnet, a leaked-credential list).
A coarse per-IP counter closes that gap without touching the per-account behavior at all.
How
RATE_LIMIT_KEYS.LOGIN_IP('login-ip') added to the rate-limit key registry.IdentityServiceOptions.loginIpRateLimit(enabled?,limit?,windowMs?) lets adownstream operator tune or disable the threshold without a core change. Default is
100 requests / 15 min - well above the 10/5min per-email budget so a shared network
(office NAT) is never blocked.
onUnavailable: 'deny'), matching the other credential-guessing keys(
login:,pwreset:,verify2fa:, ...) - an unthrottled window during a limiter outageis worse than a transient 429 on this surface.
'unknown'key - that shared key would let one IP-less client's traffic exhaust thebudget for every other IP-less client, a self-inflicted DoS.
account can't be used to clear the IP's failure budget.
Tests
identity.rate-limit.int.test.ts(real Redis): per-IP exhaustion across different accounts,bucket isolation between IPs, the no-IP skip, the
enabled: falseoverride, fail-closedbehavior on backend unavailability.
identity.service.int.test.ts(real Postgres + Redis): locking one account never blocks adifferent account signing in from the same IP.
99/99 tests green (
pnpm -F @openora/core test:integration).check:types,check:lint,check:format,check:shapeclean.Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com