Skip to content

Commit 31bfcdd

Browse files
waleedlatif1waleedlatif1
authored
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.
1 parent 061ecd3 commit 31bfcdd

6 files changed

Lines changed: 162 additions & 0 deletions

File tree

‎apps/sim/app/api/auth/forget-password/route.test.ts‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,34 @@
66
import { createMockRequest, resetEnvMock, setEnv } from '@sim/testing'
77
import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
88

9+
const { mockCheckRateLimitDirect } = vi.hoisted(() => ({
10+
mockCheckRateLimitDirect: vi.fn(),
11+
}))
12+
13+
/**
14+
* Mocked at the storage boundary rather than at `route-helpers`, so the real
15+
* key derivation (normalize + hash) is exercised through the route.
16+
*/
17+
vi.mock('@/lib/core/rate-limiter/rate-limiter', () => ({
18+
RateLimiter: class {
19+
checkRateLimitDirect = mockCheckRateLimitDirect
20+
},
21+
}))
22+
23+
const allowAll = () => ({ allowed: true, resetAt: new Date(Date.now() + 60_000) })
24+
25+
function exhaust(dimension: string, resetAt: Date) {
26+
mockCheckRateLimitDirect.mockImplementation(async (key: string) =>
27+
key.includes(dimension) ? { allowed: false, resetAt } : allowAll()
28+
)
29+
}
30+
31+
function recipientKeys(): string[] {
32+
return mockCheckRateLimitDirect.mock.calls
33+
.map(([key]) => key as string)
34+
.filter((key) => key.includes(':recipient:'))
35+
}
36+
937
const { mockRequestPasswordReset, mockLogger } = vi.hoisted(() => {
1038
const logger = {
1139
info: vi.fn(),
@@ -42,6 +70,7 @@ describe('Forget Password API Route', () => {
4270
vi.clearAllMocks()
4371
setEnv({ NEXT_PUBLIC_APP_URL: 'https://app.example.com' })
4472
mockRequestPasswordReset.mockResolvedValue(undefined)
73+
mockCheckRateLimitDirect.mockImplementation(async () => allowAll())
4574
})
4675

4776
afterAll(() => {
@@ -73,6 +102,42 @@ describe('Forget Password API Route', () => {
73102
})
74103
})
75104

105+
it('rejects with 429 once the recipient budget is spent, without sending mail', async () => {
106+
const resetAt = new Date(Date.now() + 900_000)
107+
exhaust(':recipient:', resetAt)
108+
109+
const response = await POST(createMockRequest('POST', { email: 'test@example.com' }))
110+
111+
expect(response.status).toBe(429)
112+
expect(response.headers.get('Retry-After')).toBe('900')
113+
expect(mockRequestPasswordReset).not.toHaveBeenCalled()
114+
})
115+
116+
it('buckets addresses that normalize to the same recipient together', async () => {
117+
await POST(createMockRequest('POST', { email: 'Test@Example.com' }))
118+
await POST(createMockRequest('POST', { email: 'test@example.com' }))
119+
120+
const [first, second] = recipientKeys()
121+
expect(first).toBe(second)
122+
})
123+
124+
it('keys the recipient bucket by hash, never the raw address', async () => {
125+
await POST(createMockRequest('POST', { email: 'test@example.com' }))
126+
127+
const [key] = recipientKeys()
128+
expect(key).toMatch(/^route:forget-password:recipient:[0-9a-f]{64}$/)
129+
})
130+
131+
it('short-circuits on the per-IP budget before spending the recipient budget', async () => {
132+
exhaust(':ip:', new Date(Date.now() + 60_000))
133+
134+
const response = await POST(createMockRequest('POST', { email: 'test@example.com' }))
135+
136+
expect(response.status).toBe(429)
137+
expect(recipientKeys()).toHaveLength(0)
138+
expect(mockRequestPasswordReset).not.toHaveBeenCalled()
139+
})
140+
76141
it('should reject external redirectTo URL', async () => {
77142
const req = createMockRequest('POST', {
78143
email: 'test@example.com',

‎apps/sim/app/api/auth/forget-password/route.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,29 @@ import { type NextRequest, NextResponse } from 'next/server'
77
import { forgetPasswordContract } from '@/lib/api/contracts'
88
import { getValidationErrorMessage, parseRequest } from '@/lib/api/server'
99
import { auth } from '@/lib/auth'
10+
import {
11+
enforceIpRateLimit,
12+
enforceRecipientRateLimit,
13+
type TokenBucketConfig,
14+
} from '@/lib/core/rate-limiter'
1015
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1116

1217
export const dynamic = 'force-dynamic'
1318

1419
const logger = createLogger('ForgetPasswordAPI')
1520

21+
/** Sized to absorb a frustrated user retrying, not to be tight. */
22+
const RESET_EMAIL_RATE_LIMIT: TokenBucketConfig = {
23+
maxTokens: 5,
24+
refillRate: 5,
25+
refillIntervalMs: 15 * 60_000,
26+
}
27+
1628
export const POST = withRouteHandler(async (request: NextRequest) => {
1729
try {
30+
const ipRateLimited = await enforceIpRateLimit('forget-password', request)
31+
if (ipRateLimited) return ipRateLimited
32+
1833
const parsed = await parseRequest(
1934
forgetPasswordContract,
2035
request,
@@ -33,6 +48,17 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
3348

3449
const { email, redirectTo } = parsed.data.body
3550

51+
/**
52+
* Enforced before any lookup, and identically whether or not the account
53+
* exists, so a 429 discloses nothing the success response doesn't already.
54+
*/
55+
const recipientRateLimited = await enforceRecipientRateLimit(
56+
'forget-password',
57+
email,
58+
RESET_EMAIL_RATE_LIMIT
59+
)
60+
if (recipientRateLimited) return recipientRateLimited
61+
3662
await auth.api.requestPasswordReset({
3763
body: {
3864
email,

‎apps/sim/app/api/auth/reset-password/route.test.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,16 @@
66
import { createMockRequest } from '@sim/testing'
77
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
88

9+
const { mockCheckRateLimitDirect } = vi.hoisted(() => ({
10+
mockCheckRateLimitDirect: vi.fn(),
11+
}))
12+
13+
vi.mock('@/lib/core/rate-limiter/rate-limiter', () => ({
14+
RateLimiter: class {
15+
checkRateLimitDirect = mockCheckRateLimitDirect
16+
},
17+
}))
18+
919
const { mockResetPassword, mockLogger } = vi.hoisted(() => {
1020
const logger = {
1121
info: vi.fn(),
@@ -41,12 +51,31 @@ describe('Reset Password API Route', () => {
4151
beforeEach(() => {
4252
vi.clearAllMocks()
4353
mockResetPassword.mockResolvedValue(undefined)
54+
mockCheckRateLimitDirect.mockResolvedValue({
55+
allowed: true,
56+
resetAt: new Date(Date.now() + 60_000),
57+
})
4458
})
4559

4660
afterEach(() => {
4761
vi.clearAllMocks()
4862
})
4963

64+
it('rejects with 429 once the per-IP budget is spent, without consuming the token', async () => {
65+
mockCheckRateLimitDirect.mockResolvedValue({
66+
allowed: false,
67+
resetAt: new Date(Date.now() + 900_000),
68+
})
69+
70+
const response = await POST(
71+
createMockRequest('POST', { token: 'guess', newPassword: 'newSecurePassword123!' })
72+
)
73+
74+
expect(response.status).toBe(429)
75+
expect(response.headers.get('Retry-After')).toBe('900')
76+
expect(mockResetPassword).not.toHaveBeenCalled()
77+
})
78+
5079
it('should reset password successfully', async () => {
5180
const req = createMockRequest('POST', {
5281
token: 'valid-reset-token',

‎apps/sim/app/api/auth/reset-password/route.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,32 @@ import { type NextRequest, NextResponse } from 'next/server'
33
import { resetPasswordContract } from '@/lib/api/contracts'
44
import { parseRequest } from '@/lib/api/server'
55
import { auth } from '@/lib/auth'
6+
import { enforceIpRateLimit, type TokenBucketConfig } from '@/lib/core/rate-limiter'
67
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
78

89
export const dynamic = 'force-dynamic'
910

1011
const logger = createLogger('PasswordResetAPI')
1112

13+
/**
14+
* Submitting reset tokens without a session is guessing; a legitimate user
15+
* submits once. Tighter than the public default for that reason.
16+
*/
17+
const RESET_PASSWORD_RATE_LIMIT: TokenBucketConfig = {
18+
maxTokens: 10,
19+
refillRate: 10,
20+
refillIntervalMs: 15 * 60_000,
21+
}
22+
1223
export const POST = withRouteHandler(async (request: NextRequest) => {
1324
try {
25+
const rateLimited = await enforceIpRateLimit(
26+
'reset-password',
27+
request,
28+
RESET_PASSWORD_RATE_LIMIT
29+
)
30+
if (rateLimited) return rateLimited
31+
1432
const parsed = await parseRequest(
1533
resetPasswordContract,
1634
request,

‎apps/sim/lib/core/rate-limiter/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ export {
1212
DEFAULT_PUBLIC_IP_ROUTE_LIMIT,
1313
DEFAULT_USER_ROUTE_LIMIT,
1414
enforceIpRateLimit,
15+
enforceRecipientRateLimit,
1516
enforceUserOrIpRateLimit,
1617
enforceUserRateLimit,
1718
} from './route-helpers'

‎apps/sim/lib/core/rate-limiter/route-helpers.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import { createLogger } from '@sim/logger'
2+
import { sha256Hex } from '@sim/security/hash'
3+
import { normalizeEmail } from '@sim/utils/string'
24
import { type NextRequest, NextResponse } from 'next/server'
35
import { RateLimiter } from '@/lib/core/rate-limiter/rate-limiter'
46
import type { TokenBucketConfig } from '@/lib/core/rate-limiter/storage'
@@ -72,6 +74,27 @@ export async function enforceIpRateLimit(
7274
return buildRateLimitResponse(resetAt)
7375
}
7476

77+
/**
78+
* Apply a per-recipient token bucket to a route that mails an address the
79+
* caller chooses. A per-IP bucket cannot stop a distributed attempt to bomb one
80+
* mailbox, so the address needs a budget of its own.
81+
*
82+
* The address is normalized (case variants must not each buy a fresh budget)
83+
* and hashed, so the store never holds an address and long inputs cannot
84+
* inflate key cardinality.
85+
*/
86+
export async function enforceRecipientRateLimit(
87+
bucketName: string,
88+
email: string,
89+
config: TokenBucketConfig
90+
): Promise<NextResponse | null> {
91+
const key = `route:${bucketName}:recipient:${sha256Hex(normalizeEmail(email))}`
92+
const { allowed, resetAt } = await rateLimiter.checkRateLimitDirect(key, config)
93+
if (allowed) return null
94+
logger.warn('Recipient rate limit exceeded', { bucket: bucketName })
95+
return buildRateLimitResponse(resetAt)
96+
}
97+
7598
/**
7699
* Apply a per-workspace token bucket. Use for routes whose cost is borne by the
77100
* workspace rather than the acting user — a shared budget any member spends

0 commit comments

Comments
 (0)