fix(auth): rate limit the password reset endpoints (#6553)

`/api/auth/forget-password` calls `auth.api.requestPasswordReset` without
headers, which bypasses Better Auth's rate limiter entirely — that limiter
lives in the HTTP router, not the endpoint. The route was an unthrottled
email-send amplifier keyed on any address a caller chose.

Add two dimensions via the existing route-helper family: a per-IP budget
before parsing (cheap pre-parse gate), and a per-recipient budget, since no
per-IP limit can stop a distributed attempt to bomb one mailbox. The recipient
key is normalized and hashed, so the bucket store never holds an address and
long inputs cannot inflate key cardinality. It is enforced before any user
lookup and identically whether or not the account exists, so a 429 is not an
account-existence oracle.

Also throttle `/api/auth/reset-password`, which had none and is an online
token-guessing surface.

Passing headers to `auth.api.*` is deliberately not the fix: Better Auth's
limiter throws an APIError that these routes' catch blocks project as a 500,
and it cannot express the per-recipient dimension.
This commit is contained in:
Waleed
2026-08-11 12:36:21 -07:00
committed by GitHub
parent 061ecd34e3
commit 31bfcdd640
6 changed files with 162 additions and 0 deletions
@@ -6,6 +6,34 @@
import { createMockRequest, resetEnvMock, setEnv } from '@sim/testing'
import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
const { mockCheckRateLimitDirect } = vi.hoisted(() => ({
mockCheckRateLimitDirect: vi.fn(),
}))
/**
* Mocked at the storage boundary rather than at `route-helpers`, so the real
* key derivation (normalize + hash) is exercised through the route.
*/
vi.mock('@/lib/core/rate-limiter/rate-limiter', () => ({
RateLimiter: class {
checkRateLimitDirect = mockCheckRateLimitDirect
},
}))
const allowAll = () => ({ allowed: true, resetAt: new Date(Date.now() + 60_000) })
function exhaust(dimension: string, resetAt: Date) {
mockCheckRateLimitDirect.mockImplementation(async (key: string) =>
key.includes(dimension) ? { allowed: false, resetAt } : allowAll()
)
}
function recipientKeys(): string[] {
return mockCheckRateLimitDirect.mock.calls
.map(([key]) => key as string)
.filter((key) => key.includes(':recipient:'))
}
const { mockRequestPasswordReset, mockLogger } = vi.hoisted(() => {
const logger = {
info: vi.fn(),
@@ -42,6 +70,7 @@ describe('Forget Password API Route', () => {
vi.clearAllMocks()
setEnv({ NEXT_PUBLIC_APP_URL: 'https://app.example.com' })
mockRequestPasswordReset.mockResolvedValue(undefined)
mockCheckRateLimitDirect.mockImplementation(async () => allowAll())
})
afterAll(() => {
@@ -73,6 +102,42 @@ describe('Forget Password API Route', () => {
})
})
it('rejects with 429 once the recipient budget is spent, without sending mail', async () => {
const resetAt = new Date(Date.now() + 900_000)
exhaust(':recipient:', resetAt)
const response = await POST(createMockRequest('POST', { email: 'test@example.com' }))
expect(response.status).toBe(429)
expect(response.headers.get('Retry-After')).toBe('900')
expect(mockRequestPasswordReset).not.toHaveBeenCalled()
})
it('buckets addresses that normalize to the same recipient together', async () => {
await POST(createMockRequest('POST', { email: 'Test@Example.com' }))
await POST(createMockRequest('POST', { email: 'test@example.com' }))
const [first, second] = recipientKeys()
expect(first).toBe(second)
})
it('keys the recipient bucket by hash, never the raw address', async () => {
await POST(createMockRequest('POST', { email: 'test@example.com' }))
const [key] = recipientKeys()
expect(key).toMatch(/^route:forget-password:recipient:[0-9a-f]{64}$/)
})
it('short-circuits on the per-IP budget before spending the recipient budget', async () => {
exhaust(':ip:', new Date(Date.now() + 60_000))
const response = await POST(createMockRequest('POST', { email: 'test@example.com' }))
expect(response.status).toBe(429)
expect(recipientKeys()).toHaveLength(0)
expect(mockRequestPasswordReset).not.toHaveBeenCalled()
})
it('should reject external redirectTo URL', async () => {
const req = createMockRequest('POST', {
email: 'test@example.com',
@@ -7,14 +7,29 @@ import { type NextRequest, NextResponse } from 'next/server'
import { forgetPasswordContract } from '@/lib/api/contracts'
import { getValidationErrorMessage, parseRequest } from '@/lib/api/server'
import { auth } from '@/lib/auth'
import {
enforceIpRateLimit,
enforceRecipientRateLimit,
type TokenBucketConfig,
} from '@/lib/core/rate-limiter'
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
export const dynamic = 'force-dynamic'
const logger = createLogger('ForgetPasswordAPI')
/** Sized to absorb a frustrated user retrying, not to be tight. */
const RESET_EMAIL_RATE_LIMIT: TokenBucketConfig = {
maxTokens: 5,
refillRate: 5,
refillIntervalMs: 15 * 60_000,
}
export const POST = withRouteHandler(async (request: NextRequest) => {
try {
const ipRateLimited = await enforceIpRateLimit('forget-password', request)
if (ipRateLimited) return ipRateLimited
const parsed = await parseRequest(
forgetPasswordContract,
request,
@@ -33,6 +48,17 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
const { email, redirectTo } = parsed.data.body
/**
* Enforced before any lookup, and identically whether or not the account
* exists, so a 429 discloses nothing the success response doesn't already.
*/
const recipientRateLimited = await enforceRecipientRateLimit(
'forget-password',
email,
RESET_EMAIL_RATE_LIMIT
)
if (recipientRateLimited) return recipientRateLimited
await auth.api.requestPasswordReset({
body: {
email,
@@ -6,6 +6,16 @@
import { createMockRequest } from '@sim/testing'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
const { mockCheckRateLimitDirect } = vi.hoisted(() => ({
mockCheckRateLimitDirect: vi.fn(),
}))
vi.mock('@/lib/core/rate-limiter/rate-limiter', () => ({
RateLimiter: class {
checkRateLimitDirect = mockCheckRateLimitDirect
},
}))
const { mockResetPassword, mockLogger } = vi.hoisted(() => {
const logger = {
info: vi.fn(),
@@ -41,12 +51,31 @@ describe('Reset Password API Route', () => {
beforeEach(() => {
vi.clearAllMocks()
mockResetPassword.mockResolvedValue(undefined)
mockCheckRateLimitDirect.mockResolvedValue({
allowed: true,
resetAt: new Date(Date.now() + 60_000),
})
})
afterEach(() => {
vi.clearAllMocks()
})
it('rejects with 429 once the per-IP budget is spent, without consuming the token', async () => {
mockCheckRateLimitDirect.mockResolvedValue({
allowed: false,
resetAt: new Date(Date.now() + 900_000),
})
const response = await POST(
createMockRequest('POST', { token: 'guess', newPassword: 'newSecurePassword123!' })
)
expect(response.status).toBe(429)
expect(response.headers.get('Retry-After')).toBe('900')
expect(mockResetPassword).not.toHaveBeenCalled()
})
it('should reset password successfully', async () => {
const req = createMockRequest('POST', {
token: 'valid-reset-token',
@@ -3,14 +3,32 @@ import { type NextRequest, NextResponse } from 'next/server'
import { resetPasswordContract } from '@/lib/api/contracts'
import { parseRequest } from '@/lib/api/server'
import { auth } from '@/lib/auth'
import { enforceIpRateLimit, type TokenBucketConfig } from '@/lib/core/rate-limiter'
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
export const dynamic = 'force-dynamic'
const logger = createLogger('PasswordResetAPI')
/**
* Submitting reset tokens without a session is guessing; a legitimate user
* submits once. Tighter than the public default for that reason.
*/
const RESET_PASSWORD_RATE_LIMIT: TokenBucketConfig = {
maxTokens: 10,
refillRate: 10,
refillIntervalMs: 15 * 60_000,
}
export const POST = withRouteHandler(async (request: NextRequest) => {
try {
const rateLimited = await enforceIpRateLimit(
'reset-password',
request,
RESET_PASSWORD_RATE_LIMIT
)
if (rateLimited) return rateLimited
const parsed = await parseRequest(
resetPasswordContract,
request,
+1
View File
@@ -12,6 +12,7 @@ export {
DEFAULT_PUBLIC_IP_ROUTE_LIMIT,
DEFAULT_USER_ROUTE_LIMIT,
enforceIpRateLimit,
enforceRecipientRateLimit,
enforceUserOrIpRateLimit,
enforceUserRateLimit,
} from './route-helpers'
@@ -1,4 +1,6 @@
import { createLogger } from '@sim/logger'
import { sha256Hex } from '@sim/security/hash'
import { normalizeEmail } from '@sim/utils/string'
import { type NextRequest, NextResponse } from 'next/server'
import { RateLimiter } from '@/lib/core/rate-limiter/rate-limiter'
import type { TokenBucketConfig } from '@/lib/core/rate-limiter/storage'
@@ -72,6 +74,27 @@ export async function enforceIpRateLimit(
return buildRateLimitResponse(resetAt)
}
/**
* Apply a per-recipient token bucket to a route that mails an address the
* caller chooses. A per-IP bucket cannot stop a distributed attempt to bomb one
* mailbox, so the address needs a budget of its own.
*
* The address is normalized (case variants must not each buy a fresh budget)
* and hashed, so the store never holds an address and long inputs cannot
* inflate key cardinality.
*/
export async function enforceRecipientRateLimit(
bucketName: string,
email: string,
config: TokenBucketConfig
): Promise<NextResponse | null> {
const key = `route:${bucketName}:recipient:${sha256Hex(normalizeEmail(email))}`
const { allowed, resetAt } = await rateLimiter.checkRateLimitDirect(key, config)
if (allowed) return null
logger.warn('Recipient rate limit exceeded', { bucket: bucketName })
return buildRateLimitResponse(resetAt)
}
/**
* Apply a per-workspace token bucket. Use for routes whose cost is borne by the
* workspace rather than the acting user — a shared budget any member spends