feat(auth): per-IP signup rate limit with Redis-backed storage#4828
feat(auth): per-IP signup rate limit with Redis-backed storage#4828waleedlatif1 wants to merge 1 commit into
Conversation
better-auth's built-in rate limit only guards bursts (3 req / 10s on /sign-up*), which a slow drip of signups from a single IP sails past. Add a long-window per-IP cap on /sign-up/email (default 20 / hour, configurable via SIGNUP_RATE_LIMIT_MAX and SIGNUP_RATE_LIMIT_WINDOW_SECONDS) to bound sustained abuse without affecting legitimate use — an enterprise office behind one NAT stays well under it, and SSO / org-invite onboarding use different endpoints this rule does not touch. Back the limiter with Redis when REDIS_URL is set, via rateLimit.customStorage, so counts hold across replicas and survive deploys. Without Redis it falls back to better-auth's in-memory store (unchanged behavior for self-hosters). Reads and writes fail open: a Redis outage degrades to "allow" rather than locking users out of auth. Session storage is untouched (we do not set secondaryStorage). Set advanced.ipAddress.ipAddressHeaders to prefer cf-connecting-ip so the rate-limit key is the real client IP (set by Cloudflare's edge, not forgeable by the caller), falling back to x-forwarded-for for non-Cloudflare deployments. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview When
Vitest coverage was added for the Redis storage adapter; env schema documents the two new signup limit variables. Reviewed by Cursor Bugbot for commit 75c2c6d. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 75c2c6d. Configure here.
| try { | ||
| const redis = getRedisClient() | ||
| if (!redis) return | ||
| await redis.set(`${REDIS_KEY_PREFIX}${key}`, JSON.stringify(value), 'EX', REDIS_TTL_SECONDS) |
There was a problem hiding this comment.
Redis TTL ignores signup window
Medium Severity
REDIS_TTL_SECONDS is fixed at 3600 while /sign-up/email uses SIGNUP_RATE_LIMIT_WINDOW_SECONDS, which can be longer. When a Redis key expires before that window ends, get returns nothing and the per-IP counter resets, so the sustained signup cap is weaker than configured.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 75c2c6d. Configure here.
Greptile SummaryThis PR adds a Redis-backed per-IP rate limit (default 20 signups / hour) on the
Confidence Score: 3/5Safe to merge for the default 1-hour window, but operators who configure a longer window via SIGNUP_RATE_LIMIT_WINDOW_SECONDS will get a weaker rate limit than intended due to a TTL mismatch. The Redis TTL is hardcoded to 3600 s in rate-limit-storage.ts while the window is operator-configurable. Any deployment that increases SIGNUP_RATE_LIMIT_WINDOW_SECONDS beyond 3600 will see keys expire before the window closes, silently resetting the counter mid-window and allowing far more signups than the configured cap. The fail-open design means a Redis outage also completely bypasses the new limit, which is intentional but means this control is only reliable when Redis is healthy. apps/sim/lib/auth/rate-limit-storage.ts — the hardcoded REDIS_TTL_SECONDS constant and the lack of a ttlSeconds parameter are the primary concern. Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant BetterAuth
participant RedisStorage as Redis Storage
participant Redis
participant AuthHandler
Client->>BetterAuth: POST /sign-up/email
BetterAuth->>BetterAuth: Resolve client IP (cf-connecting-ip preferred)
BetterAuth->>RedisStorage: get(auth-rl:IP)
RedisStorage->>Redis: GET auth-rl:IP
alt Redis available
Redis-->>RedisStorage: JSON counter or null
RedisStorage-->>BetterAuth: RateLimitRecord or undefined
else Redis error (fail-open)
Redis-->>RedisStorage: Error
RedisStorage-->>BetterAuth: undefined
end
BetterAuth->>BetterAuth: Check count vs max using lastRequest timestamp
alt Under limit
BetterAuth->>RedisStorage: set(auth-rl:IP, count+1)
RedisStorage->>Redis: SET auth-rl:IP ... EX 3600
BetterAuth->>AuthHandler: Proceed with signup
AuthHandler-->>Client: 200 OK
else Over limit
BetterAuth-->>Client: 429 Too Many Requests
end
Reviews (1): Last reviewed commit: "feat(auth): add per-IP signup rate limit..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| /** | ||
| * TTL for stored counters. Correctness comes from the `lastRequest` timestamp | ||
| * comparison in better-auth's limiter, not from expiry — the TTL only bounds | ||
| * key growth. It must be at least as long as the largest configured window so a | ||
| * key never expires mid-window and under-counts. | ||
| */ | ||
| const REDIS_TTL_SECONDS = 3600 | ||
|
|
||
| /** | ||
| * Redis-backed storage for better-auth's rate limiter. | ||
| * | ||
| * Returns `undefined` when no Redis is configured, in which case better-auth | ||
| * falls back to its default in-memory store. That store is per-process and | ||
| * resets on deploy, so a long-window per-IP cap only holds across a multi-replica | ||
| * deployment when counts live in shared storage. Backing the limiter with Redis | ||
| * (rather than the primary database) keeps the per-request counter I/O off the | ||
| * Postgres primary. | ||
| * | ||
| * Fail-open: any Redis error resolves `get` to `undefined` (treated as no prior | ||
| * requests) and makes `set` a no-op, so a Redis outage degrades to "allow" | ||
| * rather than locking users out of authentication. | ||
| */ | ||
| export function buildRedisRateLimitStorage(): RateLimitStorage | undefined { |
There was a problem hiding this comment.
3600 but the rate-limit window is operator-configurable via SIGNUP_RATE_LIMIT_WINDOW_SECONDS. The code comment correctly states "it must be at least as long as the largest configured window so a key never expires mid-window and under-counts" — but the hardcoded value violates that guarantee for any window > 3600 s. If an operator sets SIGNUP_RATE_LIMIT_WINDOW_SECONDS=7200, keys expire after one hour, the counter resets mid-window, and the IP gets a fresh 20-signup budget in the second hour. Accepting ttlSeconds as a parameter (with a 3600 default) and threading signupRateLimitWindowSeconds from auth.ts into the call site fixes this.
| /** | |
| * TTL for stored counters. Correctness comes from the `lastRequest` timestamp | |
| * comparison in better-auth's limiter, not from expiry — the TTL only bounds | |
| * key growth. It must be at least as long as the largest configured window so a | |
| * key never expires mid-window and under-counts. | |
| */ | |
| const REDIS_TTL_SECONDS = 3600 | |
| /** | |
| * Redis-backed storage for better-auth's rate limiter. | |
| * | |
| * Returns `undefined` when no Redis is configured, in which case better-auth | |
| * falls back to its default in-memory store. That store is per-process and | |
| * resets on deploy, so a long-window per-IP cap only holds across a multi-replica | |
| * deployment when counts live in shared storage. Backing the limiter with Redis | |
| * (rather than the primary database) keeps the per-request counter I/O off the | |
| * Postgres primary. | |
| * | |
| * Fail-open: any Redis error resolves `get` to `undefined` (treated as no prior | |
| * requests) and makes `set` a no-op, so a Redis outage degrades to "allow" | |
| * rather than locking users out of authentication. | |
| */ | |
| export function buildRedisRateLimitStorage(): RateLimitStorage | undefined { | |
| /** | |
| * Redis-backed storage for better-auth's rate limiter. | |
| * | |
| * Returns `undefined` when no Redis is configured, in which case better-auth | |
| * falls back to its default in-memory store. That store is per-process and | |
| * resets on deploy, so a long-window per-IP cap only holds across a multi-replica | |
| * deployment when counts live in shared storage. Backing the limiter with Redis | |
| * (rather than the primary database) keeps the per-request counter I/O off the | |
| * Postgres primary. | |
| * | |
| * Fail-open: any Redis error resolves `get` to `undefined` (treated as no prior | |
| * requests) and makes `set` a no-op, so a Redis outage degrades to "allow" | |
| * rather than locking users out of authentication. | |
| * | |
| * @param ttlSeconds Redis key TTL. Must be ≥ the largest configured rate-limit | |
| * window so keys never expire mid-window and under-count. Defaults to 3600 s. | |
| */ | |
| export function buildRedisRateLimitStorage(ttlSeconds = 3600): RateLimitStorage | undefined { |
| try { | ||
| const redis = getRedisClient() | ||
| if (!redis) return | ||
| await redis.set(`${REDIS_KEY_PREFIX}${key}`, JSON.stringify(value), 'EX', REDIS_TTL_SECONDS) |
There was a problem hiding this comment.
set implementation to use the ttlSeconds parameter instead of the now-removed module constant.
| await redis.set(`${REDIS_KEY_PREFIX}${key}`, JSON.stringify(value), 'EX', REDIS_TTL_SECONDS) | |
| await redis.set(`${REDIS_KEY_PREFIX}${key}`, JSON.stringify(value), 'EX', ttlSeconds) |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| * counts hold across replicas and deploys; otherwise `undefined`, leaving | ||
| * better-auth on its default in-memory store. | ||
| */ | ||
| const authRateLimitStorage = buildRedisRateLimitStorage() |
There was a problem hiding this comment.
signupRateLimitWindowSeconds into buildRedisRateLimitStorage so the Redis TTL is always at least as long as the configured window, preventing mid-window key expiry when the window exceeds the current hardcoded 3600 s.
| const authRateLimitStorage = buildRedisRateLimitStorage() | |
| const authRateLimitStorage = buildRedisRateLimitStorage(signupRateLimitWindowSeconds) |
| // onboarding hit different endpoints that this rule does not touch. | ||
| '/sign-up/email': { window: signupRateLimitWindowSeconds, max: signupRateLimitMax }, | ||
| }, | ||
| ...(authRateLimitStorage ? { customStorage: authRateLimitStorage } : {}), |
There was a problem hiding this comment.
customStorage scopes wider than /sign-up/email
customStorage is a top-level rateLimit option in better-auth and applies to all rate-limit rules — including the built-in burst-protection rule (/sign-up* at 3 req / 10 s) and any other special rules the library applies globally. The PR description presents this as targeted signup storage, but in practice every auth-path rate-limit key hits Redis on every auth request. This is likely fine for the intended deployment, but worth being deliberate: if only the long-window signup counter should be Redis-backed, better-auth doesn't currently offer per-rule storage isolation (confirmed in issue #4497).
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| const signupRateLimitWindowSeconds = | ||
| Number(env.SIGNUP_RATE_LIMIT_WINDOW_SECONDS) || DEFAULT_SIGNUP_RATE_LIMIT_WINDOW_SECONDS | ||
| const signupRateLimitMax = Number(env.SIGNUP_RATE_LIMIT_MAX) || DEFAULT_SIGNUP_RATE_LIMIT_MAX |
There was a problem hiding this comment.
SIGNUP_RATE_LIMIT_MAX or SIGNUP_RATE_LIMIT_WINDOW_SECONDS is set to "0", Number("0") evaluates to 0, which is falsy, so the || fallback silently restores the default instead of respecting the operator's explicit zero.
| const signupRateLimitWindowSeconds = | |
| Number(env.SIGNUP_RATE_LIMIT_WINDOW_SECONDS) || DEFAULT_SIGNUP_RATE_LIMIT_WINDOW_SECONDS | |
| const signupRateLimitMax = Number(env.SIGNUP_RATE_LIMIT_MAX) || DEFAULT_SIGNUP_RATE_LIMIT_MAX | |
| const signupRateLimitWindowSeconds = | |
| (env.SIGNUP_RATE_LIMIT_WINDOW_SECONDS !== undefined | |
| ? Number(env.SIGNUP_RATE_LIMIT_WINDOW_SECONDS) | |
| : NaN) || DEFAULT_SIGNUP_RATE_LIMIT_WINDOW_SECONDS | |
| const signupRateLimitMax = | |
| (env.SIGNUP_RATE_LIMIT_MAX !== undefined ? Number(env.SIGNUP_RATE_LIMIT_MAX) : NaN) || | |
| DEFAULT_SIGNUP_RATE_LIMIT_MAX |
1 participant
What
Adds a long-window per-IP rate limit on email signup, backed by Redis so it holds across replicas and deploys.
Why
better-auth's built-in rate limit (already active in prod) only guards bursts — its default special rule caps
/sign-up*at 3 requests / 10s per IP. A signup-spam botnet that drips one request every few seconds from a single IP never trips that window, which is how the recent wave got through despite Turnstile + MX/domain denylists. There was no sustained per-IP cap.What changed
/sign-up/emailcustom rule: default 20 signups / hour / IP, configurable viaSIGNUP_RATE_LIMIT_MAXandSIGNUP_RATE_LIMIT_WINDOW_SECONDS. Deliberately generous — an enterprise office behind a single NAT stays well under it, and SSO / org-invite onboarding use different endpoints this rule does not touch.rateLimit.customStorage) whenREDIS_URLis set, so per-IP counts are consistent across ECS replicas and survive deploys. Without Redis it falls back to better-auth's in-memory store (unchanged for self-hosters). Session storage is untouched — we do not setsecondaryStorage, so this is isolated to rate limiting and adds no load to the Postgres primary.getreturn "no prior requests" andseta no-op, so a Redis outage degrades to allow rather than locking users out of auth.advanced.ipAddress.ipAddressHeaders: prefercf-connecting-ip(set by Cloudflare's edge, not forgeable by the caller) so the rate-limit key is the real client IP; fall back tox-forwarded-forfor non-Cloudflare deployments. This also closes a bypass where a caller could forgex-forwarded-forto mint a fresh key per request.Safety / non-goals
enabledmirrors better-auth's default (production only) so local/CI signup stays unthrottled.Tuning
SIGNUP_RATE_LIMIT_MAX20SIGNUP_RATE_LIMIT_WINDOW_SECONDS3600If an enterprise reports being blocked, raise
SIGNUP_RATE_LIMIT_MAX(no deploy needed beyond the env change).Tests
lib/auth/rate-limit-storage.test.ts— Redis read/parse, empty read, TTL-bounded write, fail-open on read/write errors, and undefined (memory fallback) when Redis is absent.🤖 Generated with Claude Code