diff --git a/ENVIRONMENT.md b/ENVIRONMENT.md index 5ea0373d0d8..cd6268d1ee4 100644 --- a/ENVIRONMENT.md +++ b/ENVIRONMENT.md @@ -142,7 +142,7 @@ ordinary local startup does not require this credential. - `CALLBACK_TOKEN_SECRET` - Secret for signing callback tokens. Required for local development. `[SECRET]` - `INTERNAL_SECRET` - Alias/fallback for `INTERNAL_API_SECRET`; used in KiloClaw E2E scripts (`services/kiloclaw/e2e/`). `[SECRET]` - Native (mobile) sign-in rate limits are Vercel Firewall rules configured in the project's firewall configuration, not environment variables. `native-token-address` counts token exchanges per caller address before the admission gate; `native-token-user` counts them per account after it. Both belong in that configuration, and `apps/web/src/lib/auth/native-token-rate-limit.ts` reports a rule that is not configured at most once a minute. The address rule must answer: when it is absent, the exchange is refused before any provider call. -- Sign-in code email rate limits are Vercel Firewall rules configured in the project's firewall configuration, not environment variables. `magic-link-email` counts sends per normalized address; `sign-in-code-email-ip` counts sends per caller address. `apps/web/src/lib/auth/email-signin-eligibility.ts` reports a rule that is not configured to Sentry at most once a minute, so both rules belong in that configuration. +- Sign-in code email rate limits are Vercel Firewall rules configured in the project's firewall configuration, not environment variables. `magic-link-email` counts sends per normalized address; `sign-in-code-email-ip` counts sends per caller address. `apps/web/src/lib/auth/email-signin-eligibility.ts` sends a magic link or sign-in code only when both rules answer that the caller may proceed; an unconfigured or unevaluable rule refuses the send and is reported to Sentry at most once a minute, so both rules are required configuration. - Native admission rate limiting is a Vercel Firewall rule configured in the project's firewall configuration, not an environment variable. `native-admission-challenge` counts challenge issuance per caller address at `POST /api/auth/native/admission-challenge`; it belongs in that configuration, because `apps/web/src/lib/auth/native-admission.ts` reports an unconfigured rule to Sentry at most once a minute and refuses the request. ### Social OAuth Clients diff --git a/apps/web/src/lib/auth/email-signin-eligibility.test.ts b/apps/web/src/lib/auth/email-signin-eligibility.test.ts index 2ac780966a5..d76d0795aae 100644 --- a/apps/web/src/lib/auth/email-signin-eligibility.test.ts +++ b/apps/web/src/lib/auth/email-signin-eligibility.test.ts @@ -31,6 +31,7 @@ import { isNewAccountEligibleForMagicLink, checkEmailSignInEligibility, } from './email-signin-eligibility'; +import { __test__ as rateLimitTest } from './firewall-rate-limit'; const mockCheckRateLimit = jest.mocked(checkRateLimit); const mockCaptureMessage = jest.mocked(captureMessage); @@ -116,8 +117,16 @@ describe('isNewAccountEligibleForMagicLink', () => { }); describe('checkEmailSignInEligibility send budgets', () => { + const refusedSend = { + ok: false, + status: 429, + errorCode: 'SIGNUP-RATE-LIMITED', + body: { success: false, error: 'Rate limit exceeded. Please try again later.' }, + } as const; + beforeEach(() => { jest.clearAllMocks(); + rateLimitTest.resetReportThrottle(); mockCheckRateLimit.mockResolvedValue({ rateLimited: false }); mockIsEmailBlacklistedByDomainAsync.mockResolvedValue(false); mockIsBlockedTLD.mockResolvedValue(false); @@ -177,41 +186,62 @@ describe('checkEmailSignInEligibility send budgets', () => { }); }); - it('reports a rule that is not configured once per interval and continues', async () => { + it('refuses the send when the per-caller rule is not configured', async () => { mockCheckRateLimit.mockImplementation(async id => { if (id === 'sign-in-code-email-ip') return { rateLimited: false, error: 'not-found' }; return { rateLimited: false }; }); - await expect(checkEmailSignInEligibility('user@example.com', request())).resolves.toEqual({ - ok: true, - }); - await checkEmailSignInEligibility('user@example.com', request()); + await expect(checkEmailSignInEligibility('user@example.com', request())).resolves.toEqual( + refusedSend + ); + expect(mockCheckRateLimit).toHaveBeenCalledTimes(2); + }); + + it('reports an unconfigured rule once per interval while refusing every send', async () => { + mockCheckRateLimit.mockResolvedValue({ rateLimited: false, error: 'not-found' }); + + await expect(checkEmailSignInEligibility('user@example.com', request())).resolves.toEqual( + refusedSend + ); + await expect(checkEmailSignInEligibility('user@example.com', request())).resolves.toEqual( + refusedSend + ); expect(mockCaptureMessage).toHaveBeenCalledTimes(1); expect(mockCaptureMessage).toHaveBeenCalledWith( - "Firewall rate limit 'sign-in-code-email-ip' is not configured", - expect.objectContaining({ level: 'error' }) + "Firewall rate limit 'magic-link-email' is not configured", + { level: 'error', tags: { source: 'email_signin_eligibility' } } ); }); - it('continues when the per-address rule is not configured and still applies the caller rule', async () => { - mockCheckRateLimit.mockImplementation(async id => { - if (id === 'magic-link-email') return { rateLimited: false, error: 'not-found' }; - return { rateLimited: false }; - }); + it('stops at the refused per-address rule without spending the caller allowance', async () => { + mockCheckRateLimit.mockResolvedValue({ rateLimited: false, error: 'not-found' }); - await expect(checkEmailSignInEligibility('user@example.com', request())).resolves.toEqual({ - ok: true, - }); + await checkEmailSignInEligibility('user@example.com', request()); - expect(mockCheckRateLimit).toHaveBeenCalledTimes(2); + expect(mockCheckRateLimit).toHaveBeenCalledTimes(1); + expect(mockCheckRateLimit).toHaveBeenCalledWith('magic-link-email', expect.any(Object)); + }); + + it('refuses the send when the limiter cannot evaluate a rule', async () => { + mockCheckRateLimit.mockResolvedValue({ rateLimited: false, error: 'blocked' }); + + await expect(checkEmailSignInEligibility('user@example.com', request())).resolves.toEqual( + refusedSend + ); expect(mockCaptureMessage).toHaveBeenCalledWith( - "Firewall rate limit 'magic-link-email' is not configured", - expect.objectContaining({ level: 'error' }) + "Firewall rate limit 'magic-link-email' could not be evaluated", + { level: 'error', tags: { source: 'email_signin_eligibility' } } ); }); + it('allows the send when both rules answer', async () => { + await expect(checkEmailSignInEligibility('user@example.com', request())).resolves.toEqual({ + ok: true, + }); + }); + it('checks caller budgets before the domain rules', async () => { mockIsEmailBlacklistedByDomainAsync.mockResolvedValue(true); diff --git a/apps/web/src/lib/auth/email-signin-eligibility.ts b/apps/web/src/lib/auth/email-signin-eligibility.ts index 2f814912023..81e53ddaf5c 100644 --- a/apps/web/src/lib/auth/email-signin-eligibility.ts +++ b/apps/web/src/lib/auth/email-signin-eligibility.ts @@ -1,7 +1,6 @@ import type { NextRequest } from 'next/server'; -import { checkRateLimit } from '@vercel/firewall'; -import { captureMessage } from '@sentry/nextjs'; import { createHmac } from 'node:crypto'; +import { checkFirewallRateLimit } from '@/lib/auth/firewall-rate-limit'; import { findUserByNormalizedEmail, getWorkOSOrganization } from '@/lib/user'; import { validateMagicLinkSignupEmail } from '@/lib/schemas/email'; import { isEmailSignupDisabled } from '@/lib/auth/email-signup-switch'; @@ -19,9 +18,10 @@ const SIGN_IN_CODE_EMAIL_IP_RATE_LIMIT_ID = 'sign-in-code-email-ip'; /** * Vercel Firewall rate limits that apply to sign-in code email volume. Each - * rule lives in the project's firewall configuration; a rule that is not - * configured is skipped and the gap is reported to Sentry at most once a - * minute. + * rule lives in the project's firewall configuration; a send runs only when + * every rule answers, so a rule that is not configured refuses the send and + * the gap is reported to Sentry at most once a minute through the shared + * firewall reporter. * * - `magic-link-email` counts sends per normalized address. * - `sign-in-code-email-ip` counts sends per caller address. @@ -49,31 +49,15 @@ function clientIpAddress(request: NextRequest): string { return request.headers.get('x-real-ip') || 'unknown'; } -const missingRuleReportedAt: Record = {}; -const MISSING_RULE_REPORT_INTERVAL_MS = 60_000; - -/** - * One report a minute is enough to raise the alarm for a rule that is missing - * from the project's firewall configuration; reporting per request would flood - * Sentry. - */ -function reportMissingRule(ruleId: string): void { - const now = Date.now(); - if (now - (missingRuleReportedAt[ruleId] ?? 0) >= MISSING_RULE_REPORT_INTERVAL_MS) { - missingRuleReportedAt[ruleId] = now; - captureMessage(`Firewall rate limit '${ruleId}' is not configured`, { - level: 'error', - tags: { source: 'email_signin_eligibility' }, - }); - } -} - /** - * Applies the per-address and per-caller firewall rules and reports whether a - * count is exhausted. A rule that is not configured is skipped. + * Applies the per-address and per-caller firewall rules and reports whether the + * send is refused. Every rule must answer `allowed`: an exhausted budget, a + * rule that is not configured, and a limiter that cannot answer all refuse, so + * an email leaves only when the budget was confirmed. The first refusal stops + * the checks, leaving the remaining allowances unspent. * Runs before the eligibility rules: these refusals cost nothing downstream. */ -async function isSendBudgetExhausted(email: string, request: NextRequest): Promise { +async function isSendBudgetRefused(email: string, request: NextRequest): Promise { const checks = [ { id: MAGIC_LINK_EMAIL_RATE_LIMIT_ID, @@ -86,15 +70,12 @@ async function isSendBudgetExhausted(email: string, request: NextRequest): Promi ] as const; for (const check of checks) { - const { rateLimited, error } = await checkRateLimit(check.id, { + const result = await checkFirewallRateLimit(check.id, { + source: 'email_signin_eligibility', request, rateLimitKey: check.key, }); - if (error === 'not-found') { - reportMissingRule(check.id); - continue; - } - if (rateLimited) { + if (result !== 'allowed') { return true; } } @@ -207,7 +188,7 @@ export async function checkEmailSignInEligibility( email: string, request: NextRequest ): Promise { - if (await isSendBudgetExhausted(email, request)) { + if (await isSendBudgetRefused(email, request)) { return { ok: false, status: 429,