From d2df8b3f5398bdc267bf00d6ab4f648345fabe50 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 11 Aug 2026 12:07:35 -0700 Subject: [PATCH] fix(auth): rate limit the password reset endpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `/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. --- .../api/auth/forget-password/route.test.ts | 65 +++++++++++++++++++ .../sim/app/api/auth/forget-password/route.ts | 26 ++++++++ .../app/api/auth/reset-password/route.test.ts | 29 +++++++++ apps/sim/app/api/auth/reset-password/route.ts | 18 +++++ apps/sim/lib/core/rate-limiter/index.ts | 1 + .../lib/core/rate-limiter/route-helpers.ts | 23 +++++++ 6 files changed, 162 insertions(+) diff --git a/apps/sim/app/api/auth/forget-password/route.test.ts b/apps/sim/app/api/auth/forget-password/route.test.ts index 7b43e1fbc5a..44e40cda51c 100644 --- a/apps/sim/app/api/auth/forget-password/route.test.ts +++ b/apps/sim/app/api/auth/forget-password/route.test.ts @@ -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', diff --git a/apps/sim/app/api/auth/forget-password/route.ts b/apps/sim/app/api/auth/forget-password/route.ts index 2c4538782a7..43e2e2e0c60 100644 --- a/apps/sim/app/api/auth/forget-password/route.ts +++ b/apps/sim/app/api/auth/forget-password/route.ts @@ -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, diff --git a/apps/sim/app/api/auth/reset-password/route.test.ts b/apps/sim/app/api/auth/reset-password/route.test.ts index c92971a460a..5535380a52e 100644 --- a/apps/sim/app/api/auth/reset-password/route.test.ts +++ b/apps/sim/app/api/auth/reset-password/route.test.ts @@ -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', diff --git a/apps/sim/app/api/auth/reset-password/route.ts b/apps/sim/app/api/auth/reset-password/route.ts index 602c8fb3598..268992e07a7 100644 --- a/apps/sim/app/api/auth/reset-password/route.ts +++ b/apps/sim/app/api/auth/reset-password/route.ts @@ -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, diff --git a/apps/sim/lib/core/rate-limiter/index.ts b/apps/sim/lib/core/rate-limiter/index.ts index 9afd3cb231f..3c8a19d2e78 100644 --- a/apps/sim/lib/core/rate-limiter/index.ts +++ b/apps/sim/lib/core/rate-limiter/index.ts @@ -12,6 +12,7 @@ export { DEFAULT_PUBLIC_IP_ROUTE_LIMIT, DEFAULT_USER_ROUTE_LIMIT, enforceIpRateLimit, + enforceRecipientRateLimit, enforceUserOrIpRateLimit, enforceUserRateLimit, } from './route-helpers' diff --git a/apps/sim/lib/core/rate-limiter/route-helpers.ts b/apps/sim/lib/core/rate-limiter/route-helpers.ts index f71115bf532..26e267c8748 100644 --- a/apps/sim/lib/core/rate-limiter/route-helpers.ts +++ b/apps/sim/lib/core/rate-limiter/route-helpers.ts @@ -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 { + 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