From a7358cd25106dcbf3539fb93106a8ae7d8f5c277 Mon Sep 17 00:00:00 2001 From: wobsoriano Date: Mon, 11 May 2026 10:13:27 -0700 Subject: [PATCH 01/15] feat(backend): add CfConnectingIp, ForwardedFor, and RealIp header constants --- packages/backend/src/constants.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/backend/src/constants.ts b/packages/backend/src/constants.ts index 2d5f49625f8..f5e294fd3a5 100644 --- a/packages/backend/src/constants.ts +++ b/packages/backend/src/constants.ts @@ -56,10 +56,13 @@ const Headers = { ContentSecurityPolicy: 'content-security-policy', ContentSecurityPolicyReportOnly: 'content-security-policy-report-only', EnableDebug: 'x-clerk-debug', + CfConnectingIp: 'cf-connecting-ip', + ForwardedFor: 'x-forwarded-for', ForwardedHost: 'x-forwarded-host', ForwardedPort: 'x-forwarded-port', ForwardedProto: 'x-forwarded-proto', Host: 'host', + RealIp: 'x-real-ip', Location: 'location', Nonce: 'x-nonce', Origin: 'origin', From c0558ccd02e40c744ea6a0b011abe47d331b9c3f Mon Sep 17 00:00:00 2001 From: wobsoriano Date: Mon, 11 May 2026 10:13:48 -0700 Subject: [PATCH 02/15] feat(backend): add MachineTokenRateLimit to AuthErrorReason --- packages/backend/src/tokens/authStatus.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/backend/src/tokens/authStatus.ts b/packages/backend/src/tokens/authStatus.ts index 421c7bd61f4..fcb16c8d47b 100644 --- a/packages/backend/src/tokens/authStatus.ts +++ b/packages/backend/src/tokens/authStatus.ts @@ -118,6 +118,7 @@ export const AuthErrorReason = { SessionTokenWithoutClientUAT: 'session-token-but-no-client-uat', ActiveOrganizationMismatch: 'active-organization-mismatch', TokenTypeMismatch: 'token-type-mismatch', + MachineTokenRateLimit: 'machine-token-rate-limit', UnexpectedError: 'unexpected-error', } as const; From af973b76eab77cf79a5c1bf7b0757378f22db0d1 Mon Sep 17 00:00:00 2001 From: wobsoriano Date: Mon, 11 May 2026 10:15:01 -0700 Subject: [PATCH 03/15] feat(backend): add per-IP token-bucket rate limiter for machine auth --- .../__tests__/machineTokenRateLimiter.test.ts | 79 +++++++++++++++++++ .../src/tokens/machineTokenRateLimiter.ts | 27 +++++++ 2 files changed, 106 insertions(+) create mode 100644 packages/backend/src/tokens/__tests__/machineTokenRateLimiter.test.ts create mode 100644 packages/backend/src/tokens/machineTokenRateLimiter.ts diff --git a/packages/backend/src/tokens/__tests__/machineTokenRateLimiter.test.ts b/packages/backend/src/tokens/__tests__/machineTokenRateLimiter.test.ts new file mode 100644 index 00000000000..4ca18828973 --- /dev/null +++ b/packages/backend/src/tokens/__tests__/machineTokenRateLimiter.test.ts @@ -0,0 +1,79 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import { checkMachineTokenRateLimit, resetMachineTokenRateLimiter } from '../machineTokenRateLimiter'; + +afterEach(() => { + resetMachineTokenRateLimiter(); + vi.useRealTimers(); +}); + +describe('checkMachineTokenRateLimit', () => { + it('allows the first request from an IP', () => { + expect(checkMachineTokenRateLimit('1.2.3.4')).toBe(true); + }); + + it('allows up to MAX_BURST requests in a burst', () => { + const ip = '10.0.0.1'; + for (let i = 0; i < 20; i++) { + expect(checkMachineTokenRateLimit(ip), `request ${i + 1} should be allowed`).toBe(true); + } + }); + + it('blocks requests that exceed MAX_BURST', () => { + const ip = '10.0.0.2'; + for (let i = 0; i < 20; i++) { + checkMachineTokenRateLimit(ip); + } + expect(checkMachineTokenRateLimit(ip)).toBe(false); + }); + + it('allows requests again after tokens refill', () => { + vi.useFakeTimers(); + const ip = '10.0.0.3'; + for (let i = 0; i < 20; i++) { + checkMachineTokenRateLimit(ip); + } + expect(checkMachineTokenRateLimit(ip)).toBe(false); + + // Advance 2 seconds: at 10 tokens/s, 20 new tokens should be available + vi.advanceTimersByTime(2000); + expect(checkMachineTokenRateLimit(ip)).toBe(true); + }); + + it('tracks different IPs independently', () => { + const ipA = '192.168.1.1'; + const ipB = '192.168.1.2'; + for (let i = 0; i < 20; i++) { + checkMachineTokenRateLimit(ipA); + } + expect(checkMachineTokenRateLimit(ipA)).toBe(false); + expect(checkMachineTokenRateLimit(ipB)).toBe(true); + }); + + it('treats the unknown sentinel as a single IP', () => { + for (let i = 0; i < 20; i++) { + checkMachineTokenRateLimit('unknown'); + } + expect(checkMachineTokenRateLimit('unknown')).toBe(false); + }); + + it('clears all buckets and allows requests again when MAX_BUCKETS is reached', () => { + // Fill up to MAX_BUCKETS (10 000) unique IPs + for (let i = 0; i < 10_000; i++) { + checkMachineTokenRateLimit(`10.${Math.floor(i / 65536)}.${Math.floor((i % 65536) / 256)}.${i % 256}`); + } + // The 10 001st IP triggers the clear; the new IP gets a fresh bucket + const freshIp = '172.16.0.1'; + expect(checkMachineTokenRateLimit(freshIp)).toBe(true); + }); + + it('allows a previously blocked IP after resetMachineTokenRateLimiter', () => { + const ip = '5.5.5.5'; + for (let i = 0; i < 21; i++) { + checkMachineTokenRateLimit(ip); + } + expect(checkMachineTokenRateLimit(ip)).toBe(false); + resetMachineTokenRateLimiter(); + expect(checkMachineTokenRateLimit(ip)).toBe(true); + }); +}); diff --git a/packages/backend/src/tokens/machineTokenRateLimiter.ts b/packages/backend/src/tokens/machineTokenRateLimiter.ts new file mode 100644 index 00000000000..98ec38bdabc --- /dev/null +++ b/packages/backend/src/tokens/machineTokenRateLimiter.ts @@ -0,0 +1,27 @@ +const MAX_BURST = 20; +const REFILL_RATE = 10; // tokens per second +const MAX_BUCKETS = 10_000; + +type Bucket = { tokens: number; lastRefill: number }; +const buckets = new Map(); + +export function checkMachineTokenRateLimit(ip: string): boolean { + if (buckets.size >= MAX_BUCKETS) { + buckets.clear(); + } + const now = Date.now(); + const existing = buckets.get(ip); + const bucket: Bucket = existing ?? { tokens: MAX_BURST, lastRefill: now }; + const elapsed = (now - bucket.lastRefill) / 1000; + const refilled = Math.min(MAX_BURST, bucket.tokens + elapsed * REFILL_RATE); + if (refilled < 1) { + buckets.set(ip, { tokens: refilled, lastRefill: now }); + return false; + } + buckets.set(ip, { tokens: refilled - 1, lastRefill: now }); + return true; +} + +export function resetMachineTokenRateLimiter(): void { + buckets.clear(); +} From 7df18699e4541d2ab4f3ed83cfdee6d936560fd6 Mon Sep 17 00:00:00 2001 From: wobsoriano Date: Mon, 11 May 2026 10:19:59 -0700 Subject: [PATCH 04/15] test(backend): add failing integration tests for machine token rate limiting --- .../src/tokens/__tests__/request.test.ts | 114 ++++++++++++++++++ 1 file changed, 114 insertions(+) diff --git a/packages/backend/src/tokens/__tests__/request.test.ts b/packages/backend/src/tokens/__tests__/request.test.ts index e9b5fa6bfda..38990d70430 100644 --- a/packages/backend/src/tokens/__tests__/request.test.ts +++ b/packages/backend/src/tokens/__tests__/request.test.ts @@ -2,6 +2,7 @@ import { http, HttpResponse } from 'msw'; import { afterEach, beforeEach, describe, expect, it, test, vi } from 'vitest'; import { MachineTokenVerificationErrorCode, TokenVerificationErrorReason } from '../../errors'; +import { checkMachineTokenRateLimit, resetMachineTokenRateLimiter } from '../machineTokenRateLimiter'; import { mockExpiredJwt, mockInvalidSignatureJwt, @@ -1762,6 +1763,119 @@ describe('tokens.authenticateRequest(options)', () => { }); }); + describe('Rate limiting', () => { + afterEach(() => { + resetMachineTokenRateLimiter(); + }); + + const exhaustBucket = (ip: string) => { + for (let i = 0; i < 20; i++) { + checkMachineTokenRateLimit(ip); + } + }; + + test('blocks machine token request when IP exceeds burst limit', async () => { + server.use( + http.post(mockMachineAuthResponses.oauth_token.endpoint, () => + HttpResponse.json(mockVerificationResults.oauth_token), + ), + ); + const ip = '203.0.113.1'; + exhaustBucket(ip); + const rateLimited = await authenticateRequest( + mockRequest({ authorization: `Bearer ${mockTokens.oauth_token}`, 'cf-connecting-ip': ip }), + mockOptions({ acceptsToken: 'oauth_token' }), + ); + expect(rateLimited).toBeMachineUnauthenticated({ + tokenType: 'oauth_token', + reason: AuthErrorReason.MachineTokenRateLimit, + message: '', + }); + }); + + test('prefers cf-connecting-ip over x-forwarded-for for rate limit key', async () => { + server.use( + http.post(mockMachineAuthResponses.oauth_token.endpoint, () => + HttpResponse.json(mockVerificationResults.oauth_token), + ), + ); + const cfIp = '10.0.0.1'; + exhaustBucket(cfIp); + // cf-connecting-ip is exhausted; x-forwarded-for is a different IP and has full bucket + const rateLimited = await authenticateRequest( + mockRequest({ + authorization: `Bearer ${mockTokens.oauth_token}`, + 'cf-connecting-ip': cfIp, + 'x-forwarded-for': '99.99.99.99', + }), + mockOptions({ acceptsToken: 'oauth_token' }), + ); + expect(rateLimited).toBeMachineUnauthenticated({ + tokenType: 'oauth_token', + reason: AuthErrorReason.MachineTokenRateLimit, + message: '', + }); + // x-forwarded-for IP is untouched: a request using only that header must be allowed + const allowed = await authenticateRequest( + mockRequest({ + authorization: `Bearer ${mockTokens.oauth_token}`, + 'x-forwarded-for': '99.99.99.99', + }), + mockOptions({ acceptsToken: 'oauth_token' }), + ); + expect(allowed).toBeMachineAuthenticated(); + }); + + test('falls back to x-real-ip when cf-connecting-ip is absent', async () => { + server.use( + http.post(mockMachineAuthResponses.oauth_token.endpoint, () => + HttpResponse.json(mockVerificationResults.oauth_token), + ), + ); + const realIp = '192.168.10.20'; + exhaustBucket(realIp); + const rateLimited = await authenticateRequest( + mockRequest({ authorization: `Bearer ${mockTokens.oauth_token}`, 'x-real-ip': realIp }), + mockOptions({ acceptsToken: 'oauth_token' }), + ); + expect(rateLimited).toBeMachineUnauthenticated({ + tokenType: 'oauth_token', + reason: AuthErrorReason.MachineTokenRateLimit, + message: '', + }); + }); + + test('falls back to first value in x-forwarded-for when higher-priority headers are absent', async () => { + server.use( + http.post(mockMachineAuthResponses.oauth_token.endpoint, () => + HttpResponse.json(mockVerificationResults.oauth_token), + ), + ); + const ip = '172.16.5.5'; + exhaustBucket(ip); + const rateLimited = await authenticateRequest( + mockRequest({ + authorization: `Bearer ${mockTokens.oauth_token}`, + 'x-forwarded-for': `${ip}, 10.0.0.2`, + }), + mockOptions({ acceptsToken: 'oauth_token' }), + ); + expect(rateLimited).toBeMachineUnauthenticated({ + tokenType: 'oauth_token', + reason: AuthErrorReason.MachineTokenRateLimit, + message: '', + }); + }); + + test('session token path is not rate-limited', async () => { + server.use(http.get('https://api.clerk.test/v1/jwks', () => HttpResponse.json(mockJwks))); + // Exhaust the 'unknown' sentinel bucket (no IP headers on mockRequestWithHeaderAuth) + exhaustBucket('unknown'); + const result = await authenticateRequest(mockRequestWithHeaderAuth(), mockOptions()); + expect(result).toBeSignedIn(); + }); + }); + describe('Token Location Validation', () => { test.each(tokenTypes)('returns unauthenticated state when %s is in cookie instead of header', async tokenType => { const mockToken = mockTokens[tokenType]; From c67bca3ce5e350c3d1db8921ef41cfbc03e2925f Mon Sep 17 00:00:00 2001 From: wobsoriano Date: Mon, 11 May 2026 10:21:29 -0700 Subject: [PATCH 05/15] feat(backend): gate verifyMachineAuthToken behind per-IP token-bucket rate limiter --- packages/backend/src/tokens/request.ts | 35 ++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/packages/backend/src/tokens/request.ts b/packages/backend/src/tokens/request.ts index 415f1e4e0b6..f204fdb974c 100644 --- a/packages/backend/src/tokens/request.ts +++ b/packages/backend/src/tokens/request.ts @@ -15,12 +15,29 @@ import { createClerkRequest } from './clerkRequest'; import { getCookieName, getCookieValue } from './cookie'; import { HandshakeService } from './handshake'; import { getMachineTokenType, isMachineJwt, isMachineToken, isTokenTypeAccepted } from './machine'; +import { checkMachineTokenRateLimit } from './machineTokenRateLimiter'; import { OrganizationMatcher } from './organizationMatcher'; import type { MachineTokenType, SessionTokenType } from './tokenTypes'; import { TokenType } from './tokenTypes'; import type { AuthenticateRequestOptions } from './types'; import { verifyMachineAuthToken, verifyToken } from './verify'; +function extractCallerIp(request: Request): string { + const cfConnectingIp = request.headers.get(constants.Headers.CfConnectingIp); + if (cfConnectingIp) { + return cfConnectingIp; + } + const xRealIp = request.headers.get(constants.Headers.RealIp); + if (xRealIp) { + return xRealIp; + } + const xForwardedFor = request.headers.get(constants.Headers.ForwardedFor); + if (xForwardedFor) { + return xForwardedFor.split(',')[0]?.trim() ?? 'unknown'; + } + return 'unknown'; +} + export const RefreshTokenErrorReason = { NonEligibleNoCookie: 'non-eligible-no-refresh-cookie', NonEligibleNonGet: 'non-eligible-non-get', @@ -795,6 +812,15 @@ export const authenticateRequest: AuthenticateRequest = (async ( return mismatchState; } + if (!checkMachineTokenRateLimit(extractCallerIp(request))) { + return signedOut({ + tokenType: parsedTokenType, + authenticateContext, + reason: AuthErrorReason.MachineTokenRateLimit, + message: '', + }); + } + const { data, tokenType, errors } = await verifyMachineAuthToken(tokenInHeader, authenticateContext); if (errors) { return handleMachineError(tokenType, errors[0]); @@ -822,6 +848,15 @@ export const authenticateRequest: AuthenticateRequest = (async ( return mismatchState; } + if (!checkMachineTokenRateLimit(extractCallerIp(request))) { + return signedOut({ + tokenType: parsedTokenType, + authenticateContext, + reason: AuthErrorReason.MachineTokenRateLimit, + message: '', + }); + } + const { data, tokenType, errors } = await verifyMachineAuthToken(tokenInHeader, authenticateContext); if (errors) { return handleMachineError(tokenType, errors[0]); From 9530d3ac66ec54574b0e8d3f7528bb5802a5cc28 Mon Sep 17 00:00:00 2001 From: wobsoriano Date: Mon, 11 May 2026 11:28:38 -0700 Subject: [PATCH 06/15] fix(backend): exempt JWT-format OAuth and M2M tokens from IP rate limiter --- .../src/tokens/__tests__/request.test.ts | 46 +++++++++++++++++++ packages/backend/src/tokens/request.ts | 12 +++-- 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/packages/backend/src/tokens/__tests__/request.test.ts b/packages/backend/src/tokens/__tests__/request.test.ts index 38990d70430..9769538dc2d 100644 --- a/packages/backend/src/tokens/__tests__/request.test.ts +++ b/packages/backend/src/tokens/__tests__/request.test.ts @@ -9,6 +9,7 @@ import { mockJwks, mockJwt, mockJwtPayload, + mockM2MJwtPayload, signingJwks, } from '../../fixtures'; import { @@ -1874,6 +1875,51 @@ describe('tokens.authenticateRequest(options)', () => { const result = await authenticateRequest(mockRequestWithHeaderAuth(), mockOptions()); expect(result).toBeSignedIn(); }); + + test('OAuth JWT tokens bypass the rate limiter', async () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date(mockJwtPayload.iat * 1000)); + server.use(http.get('https://api.clerk.test/v1/jwks', () => HttpResponse.json(mockJwks))); + const ip = '203.0.113.2'; + exhaustBucket(ip); + const { data: oauthJwt } = await signJwt( + { + iss: 'https://clerk.oauth.example.test', + sub: 'user_2vYVtestTESTtestTESTtestTESTtest', + client_id: 'client_2VTWUzvGC5UhdJCNx6xG1D98edc', + scope: 'read:foo', + exp: mockJwtPayload.iat + 300, + iat: mockJwtPayload.iat, + nbf: mockJwtPayload.iat - 10, + }, + signingJwks, + { algorithm: 'RS256', header: { typ: 'at+jwt', kid: 'ins_2GIoQhbUpy0hX7B2cVkuTMinXoD' } }, + ); + const result = await authenticateRequest( + mockRequest({ authorization: `Bearer ${oauthJwt}`, 'cf-connecting-ip': ip }), + mockOptions({ acceptsToken: 'oauth_token' }), + ); + expect(result.reason).not.toBe(AuthErrorReason.MachineTokenRateLimit); + vi.useRealTimers(); + }); + + test('M2M JWT tokens bypass the rate limiter', async () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date(mockJwtPayload.iat * 1000)); + server.use(http.get('https://api.clerk.test/v1/jwks', () => HttpResponse.json(mockJwks))); + const ip = '203.0.113.3'; + exhaustBucket(ip); + const { data: m2mJwt } = await signJwt(mockM2MJwtPayload, signingJwks, { + algorithm: 'RS256', + header: { typ: 'JWT', kid: 'ins_2GIoQhbUpy0hX7B2cVkuTMinXoD' }, + }); + const result = await authenticateRequest( + mockRequest({ authorization: `Bearer ${m2mJwt}`, 'cf-connecting-ip': ip }), + mockOptions({ acceptsToken: 'm2m_token' }), + ); + expect(result.reason).not.toBe(AuthErrorReason.MachineTokenRateLimit); + vi.useRealTimers(); + }); }); describe('Token Location Validation', () => { diff --git a/packages/backend/src/tokens/request.ts b/packages/backend/src/tokens/request.ts index f204fdb974c..8fb0f13ece9 100644 --- a/packages/backend/src/tokens/request.ts +++ b/packages/backend/src/tokens/request.ts @@ -14,7 +14,13 @@ import { AuthErrorReason, handshake, signedIn, signedOut, signedOutInvalidToken import { createClerkRequest } from './clerkRequest'; import { getCookieName, getCookieValue } from './cookie'; import { HandshakeService } from './handshake'; -import { getMachineTokenType, isMachineJwt, isMachineToken, isTokenTypeAccepted } from './machine'; +import { + getMachineTokenType, + isMachineJwt, + isMachineToken, + isMachineTokenByPrefix, + isTokenTypeAccepted, +} from './machine'; import { checkMachineTokenRateLimit } from './machineTokenRateLimiter'; import { OrganizationMatcher } from './organizationMatcher'; import type { MachineTokenType, SessionTokenType } from './tokenTypes'; @@ -812,7 +818,7 @@ export const authenticateRequest: AuthenticateRequest = (async ( return mismatchState; } - if (!checkMachineTokenRateLimit(extractCallerIp(request))) { + if (isMachineTokenByPrefix(tokenInHeader) && !checkMachineTokenRateLimit(extractCallerIp(request))) { return signedOut({ tokenType: parsedTokenType, authenticateContext, @@ -848,7 +854,7 @@ export const authenticateRequest: AuthenticateRequest = (async ( return mismatchState; } - if (!checkMachineTokenRateLimit(extractCallerIp(request))) { + if (isMachineTokenByPrefix(tokenInHeader) && !checkMachineTokenRateLimit(extractCallerIp(request))) { return signedOut({ tokenType: parsedTokenType, authenticateContext, From ef15a10cd88c6f8dc4381baa8441da3cc7ba288d Mon Sep 17 00:00:00 2001 From: wobsoriano Date: Mon, 11 May 2026 11:57:48 -0700 Subject: [PATCH 07/15] chore: add integration tests --- integration/testUtils/machineAuthHelpers.ts | 96 +++++++++++++++++++++ integration/tests/next-machine.test.ts | 29 ++++++- 2 files changed, 124 insertions(+), 1 deletion(-) diff --git a/integration/testUtils/machineAuthHelpers.ts b/integration/testUtils/machineAuthHelpers.ts index ea541c2d0f2..14f48a11001 100644 --- a/integration/testUtils/machineAuthHelpers.ts +++ b/integration/testUtils/machineAuthHelpers.ts @@ -177,6 +177,10 @@ export type MachineAuthTestAdapter = { callbackPath: string; addRoutes: RouteBuilder; }; + rateLimit?: { + path: string; + addRoutes: RouteBuilder; + }; }; const createApiKeysEnv = (): EnvironmentConfig => appConfigs.envs.withAPIKeys.clone(); @@ -445,3 +449,95 @@ export const registerOAuthAuthTests = (adapter: MachineAuthTestAdapter): void => } }); }; + +export const registerRateLimitTests = (adapter: MachineAuthTestAdapter): void => { + if (!adapter.rateLimit) { + return; + } + + test.describe('Machine token rate limiting', () => { + test.describe.configure({ mode: 'serial' }); + let app: Application; + let fakeUser: FakeUser; + let fakeBapiUser: User; + let fakeAPIKey: FakeAPIKey; + + test.beforeAll(async () => { + test.setTimeout(120_000); + + app = await buildApp(adapter, adapter.rateLimit!.addRoutes); + await app.setup(); + await app.withEnv(createApiKeysEnv()); + await app.dev(); + + const u = createTestUtils({ app }); + fakeUser = u.services.users.createFakeUser(); + fakeBapiUser = await u.services.users.createBapiUser(fakeUser); + fakeAPIKey = await u.services.users.createFakeAPIKey(fakeBapiUser.id); + }); + + test.afterAll(async () => { + await fakeAPIKey?.revoke(); + await fakeUser?.deleteIfExists(); + await app?.teardown(); + }); + + test('rate-limits opaque machine tokens after burst exhaustion', async ({ request }) => { + const url = new URL(adapter.rateLimit!.path, app.serverUrl).toString(); + // Use a dedicated test IP so this test's bucket is isolated from others + const testIp = '203.0.113.42'; + + for (let i = 0; i < 20; i++) { + await request.get(url, { + headers: { + Authorization: `Bearer ${fakeAPIKey.secret}`, + 'x-forwarded-for': testIp, + }, + }); + } + + const res = await request.get(url, { + headers: { + Authorization: `Bearer ${fakeAPIKey.secret}`, + 'x-forwarded-for': testIp, + }, + }); + expect(res.status()).toBe(401); + const body = await res.json(); + expect(body.reason).toBe('machine-token-rate-limit'); + }); + + test('tracks different source IPs independently', async ({ request }) => { + const url = new URL(adapter.rateLimit!.path, app.serverUrl).toString(); + const ipA = '203.0.113.1'; + const ipB = '203.0.113.2'; + + for (let i = 0; i < 20; i++) { + await request.get(url, { + headers: { + Authorization: `Bearer ${fakeAPIKey.secret}`, + 'x-forwarded-for': ipA, + }, + }); + } + + const resA = await request.get(url, { + headers: { + Authorization: `Bearer ${fakeAPIKey.secret}`, + 'x-forwarded-for': ipA, + }, + }); + expect(resA.status()).toBe(401); + const bodyA = await resA.json(); + expect(bodyA.reason).toBe('machine-token-rate-limit'); + + const resB = await request.get(url, { + headers: { + Authorization: `Bearer ${fakeAPIKey.secret}`, + 'x-forwarded-for': ipB, + }, + }); + expect(resB.status()).toBe(200); + }); + }); +}; diff --git a/integration/tests/next-machine.test.ts b/integration/tests/next-machine.test.ts index 0a753cce476..1b2f1c5bac5 100644 --- a/integration/tests/next-machine.test.ts +++ b/integration/tests/next-machine.test.ts @@ -2,7 +2,12 @@ import { test } from '@playwright/test'; import { appConfigs } from '../presets'; import type { MachineAuthTestAdapter } from '../testUtils/machineAuthHelpers'; -import { registerApiKeyAuthTests, registerM2MAuthTests, registerOAuthAuthTests } from '../testUtils/machineAuthHelpers'; +import { + registerApiKeyAuthTests, + registerM2MAuthTests, + registerOAuthAuthTests, + registerRateLimitTests, +} from '../testUtils/machineAuthHelpers'; const adapter: MachineAuthTestAdapter = { baseConfig: appConfigs.next.appRouter, @@ -88,10 +93,32 @@ const adapter: MachineAuthTestAdapter = { `, ), }, + rateLimit: { + path: '/api/rate-limit-test', + addRoutes: config => + config.addFile( + 'src/app/api/rate-limit-test/route.ts', + () => ` + import { auth } from '@clerk/nextjs/server'; + + export async function GET(request: Request) { + const { userId, tokenType } = await auth({ acceptsToken: 'api_key' }); + + if (!userId) { + const reason = request.headers.get('x-clerk-auth-reason'); + return Response.json({ error: 'Unauthorized', reason }, { status: 401 }); + } + + return Response.json({ userId, tokenType }); + } + `, + ), + }, }; test.describe('Next.js machine authentication @machine', () => { registerApiKeyAuthTests(adapter); registerM2MAuthTests(adapter); registerOAuthAuthTests(adapter); + registerRateLimitTests(adapter); }); From 88413ea7010f056f53a55ba77652efad120ec2f1 Mon Sep 17 00:00:00 2001 From: wobsoriano Date: Mon, 11 May 2026 14:25:09 -0700 Subject: [PATCH 08/15] chore: revert e2e --- integration/testUtils/machineAuthHelpers.ts | 96 --------------------- integration/tests/next-machine.test.ts | 29 +------ 2 files changed, 1 insertion(+), 124 deletions(-) diff --git a/integration/testUtils/machineAuthHelpers.ts b/integration/testUtils/machineAuthHelpers.ts index 14f48a11001..ea541c2d0f2 100644 --- a/integration/testUtils/machineAuthHelpers.ts +++ b/integration/testUtils/machineAuthHelpers.ts @@ -177,10 +177,6 @@ export type MachineAuthTestAdapter = { callbackPath: string; addRoutes: RouteBuilder; }; - rateLimit?: { - path: string; - addRoutes: RouteBuilder; - }; }; const createApiKeysEnv = (): EnvironmentConfig => appConfigs.envs.withAPIKeys.clone(); @@ -449,95 +445,3 @@ export const registerOAuthAuthTests = (adapter: MachineAuthTestAdapter): void => } }); }; - -export const registerRateLimitTests = (adapter: MachineAuthTestAdapter): void => { - if (!adapter.rateLimit) { - return; - } - - test.describe('Machine token rate limiting', () => { - test.describe.configure({ mode: 'serial' }); - let app: Application; - let fakeUser: FakeUser; - let fakeBapiUser: User; - let fakeAPIKey: FakeAPIKey; - - test.beforeAll(async () => { - test.setTimeout(120_000); - - app = await buildApp(adapter, adapter.rateLimit!.addRoutes); - await app.setup(); - await app.withEnv(createApiKeysEnv()); - await app.dev(); - - const u = createTestUtils({ app }); - fakeUser = u.services.users.createFakeUser(); - fakeBapiUser = await u.services.users.createBapiUser(fakeUser); - fakeAPIKey = await u.services.users.createFakeAPIKey(fakeBapiUser.id); - }); - - test.afterAll(async () => { - await fakeAPIKey?.revoke(); - await fakeUser?.deleteIfExists(); - await app?.teardown(); - }); - - test('rate-limits opaque machine tokens after burst exhaustion', async ({ request }) => { - const url = new URL(adapter.rateLimit!.path, app.serverUrl).toString(); - // Use a dedicated test IP so this test's bucket is isolated from others - const testIp = '203.0.113.42'; - - for (let i = 0; i < 20; i++) { - await request.get(url, { - headers: { - Authorization: `Bearer ${fakeAPIKey.secret}`, - 'x-forwarded-for': testIp, - }, - }); - } - - const res = await request.get(url, { - headers: { - Authorization: `Bearer ${fakeAPIKey.secret}`, - 'x-forwarded-for': testIp, - }, - }); - expect(res.status()).toBe(401); - const body = await res.json(); - expect(body.reason).toBe('machine-token-rate-limit'); - }); - - test('tracks different source IPs independently', async ({ request }) => { - const url = new URL(adapter.rateLimit!.path, app.serverUrl).toString(); - const ipA = '203.0.113.1'; - const ipB = '203.0.113.2'; - - for (let i = 0; i < 20; i++) { - await request.get(url, { - headers: { - Authorization: `Bearer ${fakeAPIKey.secret}`, - 'x-forwarded-for': ipA, - }, - }); - } - - const resA = await request.get(url, { - headers: { - Authorization: `Bearer ${fakeAPIKey.secret}`, - 'x-forwarded-for': ipA, - }, - }); - expect(resA.status()).toBe(401); - const bodyA = await resA.json(); - expect(bodyA.reason).toBe('machine-token-rate-limit'); - - const resB = await request.get(url, { - headers: { - Authorization: `Bearer ${fakeAPIKey.secret}`, - 'x-forwarded-for': ipB, - }, - }); - expect(resB.status()).toBe(200); - }); - }); -}; diff --git a/integration/tests/next-machine.test.ts b/integration/tests/next-machine.test.ts index 1b2f1c5bac5..0a753cce476 100644 --- a/integration/tests/next-machine.test.ts +++ b/integration/tests/next-machine.test.ts @@ -2,12 +2,7 @@ import { test } from '@playwright/test'; import { appConfigs } from '../presets'; import type { MachineAuthTestAdapter } from '../testUtils/machineAuthHelpers'; -import { - registerApiKeyAuthTests, - registerM2MAuthTests, - registerOAuthAuthTests, - registerRateLimitTests, -} from '../testUtils/machineAuthHelpers'; +import { registerApiKeyAuthTests, registerM2MAuthTests, registerOAuthAuthTests } from '../testUtils/machineAuthHelpers'; const adapter: MachineAuthTestAdapter = { baseConfig: appConfigs.next.appRouter, @@ -93,32 +88,10 @@ const adapter: MachineAuthTestAdapter = { `, ), }, - rateLimit: { - path: '/api/rate-limit-test', - addRoutes: config => - config.addFile( - 'src/app/api/rate-limit-test/route.ts', - () => ` - import { auth } from '@clerk/nextjs/server'; - - export async function GET(request: Request) { - const { userId, tokenType } = await auth({ acceptsToken: 'api_key' }); - - if (!userId) { - const reason = request.headers.get('x-clerk-auth-reason'); - return Response.json({ error: 'Unauthorized', reason }, { status: 401 }); - } - - return Response.json({ userId, tokenType }); - } - `, - ), - }, }; test.describe('Next.js machine authentication @machine', () => { registerApiKeyAuthTests(adapter); registerM2MAuthTests(adapter); registerOAuthAuthTests(adapter); - registerRateLimitTests(adapter); }); From 2a1b3a8dcbd4e62b26ac7bc09a3f2da9671e7ce7 Mon Sep 17 00:00:00 2001 From: wobsoriano Date: Mon, 11 May 2026 14:51:26 -0700 Subject: [PATCH 09/15] chore: address coderabbit comments --- .../src/tokens/__tests__/machineTokenRateLimiter.test.ts | 4 ++-- packages/backend/src/tokens/machineTokenRateLimiter.ts | 7 ++++++- packages/backend/src/tokens/request.ts | 3 +++ 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/backend/src/tokens/__tests__/machineTokenRateLimiter.test.ts b/packages/backend/src/tokens/__tests__/machineTokenRateLimiter.test.ts index 4ca18828973..f392917ea0d 100644 --- a/packages/backend/src/tokens/__tests__/machineTokenRateLimiter.test.ts +++ b/packages/backend/src/tokens/__tests__/machineTokenRateLimiter.test.ts @@ -57,12 +57,12 @@ describe('checkMachineTokenRateLimit', () => { expect(checkMachineTokenRateLimit('unknown')).toBe(false); }); - it('clears all buckets and allows requests again when MAX_BUCKETS is reached', () => { + it('evicts the oldest bucket and allows a new IP when MAX_BUCKETS is reached', () => { // Fill up to MAX_BUCKETS (10 000) unique IPs for (let i = 0; i < 10_000; i++) { checkMachineTokenRateLimit(`10.${Math.floor(i / 65536)}.${Math.floor((i % 65536) / 256)}.${i % 256}`); } - // The 10 001st IP triggers the clear; the new IP gets a fresh bucket + // The 10 001st IP triggers eviction of the oldest entry; the new IP gets a fresh bucket const freshIp = '172.16.0.1'; expect(checkMachineTokenRateLimit(freshIp)).toBe(true); }); diff --git a/packages/backend/src/tokens/machineTokenRateLimiter.ts b/packages/backend/src/tokens/machineTokenRateLimiter.ts index 98ec38bdabc..6de2a6c4b68 100644 --- a/packages/backend/src/tokens/machineTokenRateLimiter.ts +++ b/packages/backend/src/tokens/machineTokenRateLimiter.ts @@ -7,7 +7,12 @@ const buckets = new Map(); export function checkMachineTokenRateLimit(ip: string): boolean { if (buckets.size >= MAX_BUCKETS) { - buckets.clear(); + // Evict the oldest entry rather than clearing all buckets to prevent an attacker + // from neutralizing rate limits by forcing key churn across many distinct IPs. + const oldest = buckets.keys().next().value; + if (oldest !== undefined) { + buckets.delete(oldest); + } } const now = Date.now(); const existing = buckets.get(ip); diff --git a/packages/backend/src/tokens/request.ts b/packages/backend/src/tokens/request.ts index 8fb0f13ece9..e968292d0b2 100644 --- a/packages/backend/src/tokens/request.ts +++ b/packages/backend/src/tokens/request.ts @@ -28,6 +28,9 @@ import { TokenType } from './tokenTypes'; import type { AuthenticateRequestOptions } from './types'; import { verifyMachineAuthToken, verifyToken } from './verify'; +// NOTE: IP headers like x-forwarded-for can be spoofed by clients not behind a trusted proxy. +// cf-connecting-ip (set by Cloudflare) and x-real-ip (set by Nginx) are more reliable when present. +// The rate limiter is defense-in-depth against BAPI quota exhaustion, not a security boundary. function extractCallerIp(request: Request): string { const cfConnectingIp = request.headers.get(constants.Headers.CfConnectingIp); if (cfConnectingIp) { From eca73d4b4b5ce1f01fca2a9d2e3efab9777a11c1 Mon Sep 17 00:00:00 2001 From: wobsoriano Date: Mon, 11 May 2026 15:41:35 -0700 Subject: [PATCH 10/15] chore: limit solution to oauth opaque tokens --- ....test.ts => oauthTokenRateLimiter.test.ts} | 44 ++++++++--------- .../src/tokens/__tests__/request.test.ts | 48 +++++++++++++++---- packages/backend/src/tokens/authStatus.ts | 2 +- packages/backend/src/tokens/machine.ts | 4 ++ ...ateLimiter.ts => oauthTokenRateLimiter.ts} | 4 +- packages/backend/src/tokens/request.ts | 12 ++--- 6 files changed, 74 insertions(+), 40 deletions(-) rename packages/backend/src/tokens/__tests__/{machineTokenRateLimiter.test.ts => oauthTokenRateLimiter.test.ts} (51%) rename packages/backend/src/tokens/{machineTokenRateLimiter.ts => oauthTokenRateLimiter.ts} (89%) diff --git a/packages/backend/src/tokens/__tests__/machineTokenRateLimiter.test.ts b/packages/backend/src/tokens/__tests__/oauthTokenRateLimiter.test.ts similarity index 51% rename from packages/backend/src/tokens/__tests__/machineTokenRateLimiter.test.ts rename to packages/backend/src/tokens/__tests__/oauthTokenRateLimiter.test.ts index f392917ea0d..8b93a7fdafa 100644 --- a/packages/backend/src/tokens/__tests__/machineTokenRateLimiter.test.ts +++ b/packages/backend/src/tokens/__tests__/oauthTokenRateLimiter.test.ts @@ -1,79 +1,79 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; -import { checkMachineTokenRateLimit, resetMachineTokenRateLimiter } from '../machineTokenRateLimiter'; +import { checkOAuthTokenRateLimit, resetOAuthTokenRateLimiter } from '../oauthTokenRateLimiter'; afterEach(() => { - resetMachineTokenRateLimiter(); + resetOAuthTokenRateLimiter(); vi.useRealTimers(); }); -describe('checkMachineTokenRateLimit', () => { +describe('checkOAuthTokenRateLimit', () => { it('allows the first request from an IP', () => { - expect(checkMachineTokenRateLimit('1.2.3.4')).toBe(true); + expect(checkOAuthTokenRateLimit('1.2.3.4')).toBe(true); }); it('allows up to MAX_BURST requests in a burst', () => { const ip = '10.0.0.1'; for (let i = 0; i < 20; i++) { - expect(checkMachineTokenRateLimit(ip), `request ${i + 1} should be allowed`).toBe(true); + expect(checkOAuthTokenRateLimit(ip), `request ${i + 1} should be allowed`).toBe(true); } }); it('blocks requests that exceed MAX_BURST', () => { const ip = '10.0.0.2'; for (let i = 0; i < 20; i++) { - checkMachineTokenRateLimit(ip); + checkOAuthTokenRateLimit(ip); } - expect(checkMachineTokenRateLimit(ip)).toBe(false); + expect(checkOAuthTokenRateLimit(ip)).toBe(false); }); it('allows requests again after tokens refill', () => { vi.useFakeTimers(); const ip = '10.0.0.3'; for (let i = 0; i < 20; i++) { - checkMachineTokenRateLimit(ip); + checkOAuthTokenRateLimit(ip); } - expect(checkMachineTokenRateLimit(ip)).toBe(false); + expect(checkOAuthTokenRateLimit(ip)).toBe(false); // Advance 2 seconds: at 10 tokens/s, 20 new tokens should be available vi.advanceTimersByTime(2000); - expect(checkMachineTokenRateLimit(ip)).toBe(true); + expect(checkOAuthTokenRateLimit(ip)).toBe(true); }); it('tracks different IPs independently', () => { const ipA = '192.168.1.1'; const ipB = '192.168.1.2'; for (let i = 0; i < 20; i++) { - checkMachineTokenRateLimit(ipA); + checkOAuthTokenRateLimit(ipA); } - expect(checkMachineTokenRateLimit(ipA)).toBe(false); - expect(checkMachineTokenRateLimit(ipB)).toBe(true); + expect(checkOAuthTokenRateLimit(ipA)).toBe(false); + expect(checkOAuthTokenRateLimit(ipB)).toBe(true); }); it('treats the unknown sentinel as a single IP', () => { for (let i = 0; i < 20; i++) { - checkMachineTokenRateLimit('unknown'); + checkOAuthTokenRateLimit('unknown'); } - expect(checkMachineTokenRateLimit('unknown')).toBe(false); + expect(checkOAuthTokenRateLimit('unknown')).toBe(false); }); it('evicts the oldest bucket and allows a new IP when MAX_BUCKETS is reached', () => { // Fill up to MAX_BUCKETS (10 000) unique IPs for (let i = 0; i < 10_000; i++) { - checkMachineTokenRateLimit(`10.${Math.floor(i / 65536)}.${Math.floor((i % 65536) / 256)}.${i % 256}`); + checkOAuthTokenRateLimit(`10.${Math.floor(i / 65536)}.${Math.floor((i % 65536) / 256)}.${i % 256}`); } // The 10 001st IP triggers eviction of the oldest entry; the new IP gets a fresh bucket const freshIp = '172.16.0.1'; - expect(checkMachineTokenRateLimit(freshIp)).toBe(true); + expect(checkOAuthTokenRateLimit(freshIp)).toBe(true); }); - it('allows a previously blocked IP after resetMachineTokenRateLimiter', () => { + it('allows a previously blocked IP after resetOAuthTokenRateLimiter', () => { const ip = '5.5.5.5'; for (let i = 0; i < 21; i++) { - checkMachineTokenRateLimit(ip); + checkOAuthTokenRateLimit(ip); } - expect(checkMachineTokenRateLimit(ip)).toBe(false); - resetMachineTokenRateLimiter(); - expect(checkMachineTokenRateLimit(ip)).toBe(true); + expect(checkOAuthTokenRateLimit(ip)).toBe(false); + resetOAuthTokenRateLimiter(); + expect(checkOAuthTokenRateLimit(ip)).toBe(true); }); }); diff --git a/packages/backend/src/tokens/__tests__/request.test.ts b/packages/backend/src/tokens/__tests__/request.test.ts index 9769538dc2d..31fab9c34a6 100644 --- a/packages/backend/src/tokens/__tests__/request.test.ts +++ b/packages/backend/src/tokens/__tests__/request.test.ts @@ -2,7 +2,7 @@ import { http, HttpResponse } from 'msw'; import { afterEach, beforeEach, describe, expect, it, test, vi } from 'vitest'; import { MachineTokenVerificationErrorCode, TokenVerificationErrorReason } from '../../errors'; -import { checkMachineTokenRateLimit, resetMachineTokenRateLimiter } from '../machineTokenRateLimiter'; +import { checkOAuthTokenRateLimit, resetOAuthTokenRateLimiter } from '../oauthTokenRateLimiter'; import { mockExpiredJwt, mockInvalidSignatureJwt, @@ -1766,12 +1766,12 @@ describe('tokens.authenticateRequest(options)', () => { describe('Rate limiting', () => { afterEach(() => { - resetMachineTokenRateLimiter(); + resetOAuthTokenRateLimiter(); }); const exhaustBucket = (ip: string) => { for (let i = 0; i < 20; i++) { - checkMachineTokenRateLimit(ip); + checkOAuthTokenRateLimit(ip); } }; @@ -1789,7 +1789,7 @@ describe('tokens.authenticateRequest(options)', () => { ); expect(rateLimited).toBeMachineUnauthenticated({ tokenType: 'oauth_token', - reason: AuthErrorReason.MachineTokenRateLimit, + reason: AuthErrorReason.OAuthTokenRateLimit, message: '', }); }); @@ -1813,7 +1813,7 @@ describe('tokens.authenticateRequest(options)', () => { ); expect(rateLimited).toBeMachineUnauthenticated({ tokenType: 'oauth_token', - reason: AuthErrorReason.MachineTokenRateLimit, + reason: AuthErrorReason.OAuthTokenRateLimit, message: '', }); // x-forwarded-for IP is untouched: a request using only that header must be allowed @@ -1841,7 +1841,7 @@ describe('tokens.authenticateRequest(options)', () => { ); expect(rateLimited).toBeMachineUnauthenticated({ tokenType: 'oauth_token', - reason: AuthErrorReason.MachineTokenRateLimit, + reason: AuthErrorReason.OAuthTokenRateLimit, message: '', }); }); @@ -1863,7 +1863,7 @@ describe('tokens.authenticateRequest(options)', () => { ); expect(rateLimited).toBeMachineUnauthenticated({ tokenType: 'oauth_token', - reason: AuthErrorReason.MachineTokenRateLimit, + reason: AuthErrorReason.OAuthTokenRateLimit, message: '', }); }); @@ -1899,7 +1899,7 @@ describe('tokens.authenticateRequest(options)', () => { mockRequest({ authorization: `Bearer ${oauthJwt}`, 'cf-connecting-ip': ip }), mockOptions({ acceptsToken: 'oauth_token' }), ); - expect(result.reason).not.toBe(AuthErrorReason.MachineTokenRateLimit); + expect(result.reason).not.toBe(AuthErrorReason.OAuthTokenRateLimit); vi.useRealTimers(); }); @@ -1917,9 +1917,39 @@ describe('tokens.authenticateRequest(options)', () => { mockRequest({ authorization: `Bearer ${m2mJwt}`, 'cf-connecting-ip': ip }), mockOptions({ acceptsToken: 'm2m_token' }), ); - expect(result.reason).not.toBe(AuthErrorReason.MachineTokenRateLimit); + expect(result.reason).not.toBe(AuthErrorReason.OAuthTokenRateLimit); vi.useRealTimers(); }); + + test('opaque api_key tokens bypass the rate limiter', async () => { + server.use( + http.post(mockMachineAuthResponses.api_key.endpoint, () => + HttpResponse.json(mockVerificationResults.api_key), + ), + ); + const ip = '203.0.113.4'; + exhaustBucket(ip); + const result = await authenticateRequest( + mockRequest({ authorization: `Bearer ${mockTokens.api_key}`, 'cf-connecting-ip': ip }), + mockOptions({ acceptsToken: 'api_key' }), + ); + expect(result.reason).not.toBe(AuthErrorReason.OAuthTokenRateLimit); + }); + + test('opaque m2m tokens bypass the rate limiter', async () => { + server.use( + http.post(mockMachineAuthResponses.m2m_token.endpoint, () => + HttpResponse.json(mockVerificationResults.m2m_token), + ), + ); + const ip = '203.0.113.5'; + exhaustBucket(ip); + const result = await authenticateRequest( + mockRequest({ authorization: `Bearer ${mockTokens.m2m_token}`, 'cf-connecting-ip': ip }), + mockOptions({ acceptsToken: 'm2m_token' }), + ); + expect(result.reason).not.toBe(AuthErrorReason.OAuthTokenRateLimit); + }); }); describe('Token Location Validation', () => { diff --git a/packages/backend/src/tokens/authStatus.ts b/packages/backend/src/tokens/authStatus.ts index fcb16c8d47b..6d374f7cb9e 100644 --- a/packages/backend/src/tokens/authStatus.ts +++ b/packages/backend/src/tokens/authStatus.ts @@ -118,7 +118,7 @@ export const AuthErrorReason = { SessionTokenWithoutClientUAT: 'session-token-but-no-client-uat', ActiveOrganizationMismatch: 'active-organization-mismatch', TokenTypeMismatch: 'token-type-mismatch', - MachineTokenRateLimit: 'machine-token-rate-limit', + OAuthTokenRateLimit: 'oauth-token-rate-limit', UnexpectedError: 'unexpected-error', } as const; diff --git a/packages/backend/src/tokens/machine.ts b/packages/backend/src/tokens/machine.ts index cfc055e96d3..70e67ba8e14 100644 --- a/packages/backend/src/tokens/machine.ts +++ b/packages/backend/src/tokens/machine.ts @@ -90,6 +90,10 @@ export function isMachineTokenByPrefix(token: string): boolean { return MACHINE_TOKEN_PREFIXES.some(prefix => token.startsWith(prefix)); } +export function isOAuthTokenByPrefix(token: string): boolean { + return token.startsWith(OAUTH_TOKEN_PREFIX); +} + /** * Checks if a token is a machine token by looking at its prefix or if it's an OAuth/M2M JWT. * diff --git a/packages/backend/src/tokens/machineTokenRateLimiter.ts b/packages/backend/src/tokens/oauthTokenRateLimiter.ts similarity index 89% rename from packages/backend/src/tokens/machineTokenRateLimiter.ts rename to packages/backend/src/tokens/oauthTokenRateLimiter.ts index 6de2a6c4b68..4ab5ee13378 100644 --- a/packages/backend/src/tokens/machineTokenRateLimiter.ts +++ b/packages/backend/src/tokens/oauthTokenRateLimiter.ts @@ -5,7 +5,7 @@ const MAX_BUCKETS = 10_000; type Bucket = { tokens: number; lastRefill: number }; const buckets = new Map(); -export function checkMachineTokenRateLimit(ip: string): boolean { +export function checkOAuthTokenRateLimit(ip: string): boolean { if (buckets.size >= MAX_BUCKETS) { // Evict the oldest entry rather than clearing all buckets to prevent an attacker // from neutralizing rate limits by forcing key churn across many distinct IPs. @@ -27,6 +27,6 @@ export function checkMachineTokenRateLimit(ip: string): boolean { return true; } -export function resetMachineTokenRateLimiter(): void { +export function resetOAuthTokenRateLimiter(): void { buckets.clear(); } diff --git a/packages/backend/src/tokens/request.ts b/packages/backend/src/tokens/request.ts index e968292d0b2..92d4bc0390e 100644 --- a/packages/backend/src/tokens/request.ts +++ b/packages/backend/src/tokens/request.ts @@ -18,10 +18,10 @@ import { getMachineTokenType, isMachineJwt, isMachineToken, - isMachineTokenByPrefix, + isOAuthTokenByPrefix, isTokenTypeAccepted, } from './machine'; -import { checkMachineTokenRateLimit } from './machineTokenRateLimiter'; +import { checkOAuthTokenRateLimit } from './oauthTokenRateLimiter'; import { OrganizationMatcher } from './organizationMatcher'; import type { MachineTokenType, SessionTokenType } from './tokenTypes'; import { TokenType } from './tokenTypes'; @@ -821,11 +821,11 @@ export const authenticateRequest: AuthenticateRequest = (async ( return mismatchState; } - if (isMachineTokenByPrefix(tokenInHeader) && !checkMachineTokenRateLimit(extractCallerIp(request))) { + if (isOAuthTokenByPrefix(tokenInHeader) && !checkOAuthTokenRateLimit(extractCallerIp(request))) { return signedOut({ tokenType: parsedTokenType, authenticateContext, - reason: AuthErrorReason.MachineTokenRateLimit, + reason: AuthErrorReason.OAuthTokenRateLimit, message: '', }); } @@ -857,11 +857,11 @@ export const authenticateRequest: AuthenticateRequest = (async ( return mismatchState; } - if (isMachineTokenByPrefix(tokenInHeader) && !checkMachineTokenRateLimit(extractCallerIp(request))) { + if (isOAuthTokenByPrefix(tokenInHeader) && !checkOAuthTokenRateLimit(extractCallerIp(request))) { return signedOut({ tokenType: parsedTokenType, authenticateContext, - reason: AuthErrorReason.MachineTokenRateLimit, + reason: AuthErrorReason.OAuthTokenRateLimit, message: '', }); } From 0ed7f13d82dc92872d4998effeafce77b0e4c47d Mon Sep 17 00:00:00 2001 From: wobsoriano Date: Tue, 12 May 2026 07:13:46 -0700 Subject: [PATCH 11/15] chore: improve tests --- integration/testUtils/machineAuthHelpers.ts | 42 +++++ .../__tests__/oauthNegativeCache.test.ts | 147 +++++++++++++++ .../__tests__/oauthTokenRateLimiter.test.ts | 79 -------- .../src/tokens/__tests__/request.test.ts | 168 +++++------------- packages/backend/src/tokens/authStatus.ts | 1 - .../backend/src/tokens/oauthNegativeCache.ts | 44 +++++ .../src/tokens/oauthTokenRateLimiter.ts | 32 ---- packages/backend/src/tokens/request.ts | 45 ++--- 8 files changed, 285 insertions(+), 273 deletions(-) create mode 100644 packages/backend/src/tokens/__tests__/oauthNegativeCache.test.ts delete mode 100644 packages/backend/src/tokens/__tests__/oauthTokenRateLimiter.test.ts create mode 100644 packages/backend/src/tokens/oauthNegativeCache.ts delete mode 100644 packages/backend/src/tokens/oauthTokenRateLimiter.ts diff --git a/integration/testUtils/machineAuthHelpers.ts b/integration/testUtils/machineAuthHelpers.ts index ea541c2d0f2..9a5034753f3 100644 --- a/integration/testUtils/machineAuthHelpers.ts +++ b/integration/testUtils/machineAuthHelpers.ts @@ -432,6 +432,48 @@ export const registerOAuthAuthTests = (adapter: MachineAuthTestAdapter): void => expect(res.status()).toBe(401); }); + test('consistently rejects the same invalid oat_ token across repeated requests', async ({ request }) => { + // After the first rejection the token is cached as invalid in-process. + // Subsequent requests with the same token must still return 401 (not be + // accidentally accepted due to cache state). + const url = new URL(adapter.oauth.verifyPath, app.serverUrl).toString(); + const invalidToken = `oat_integration_test_invalid_${Date.now()}`; + + for (let i = 0; i < 3; i++) { + const res = await request.get(url, { headers: { Authorization: `Bearer ${invalidToken}` } }); + expect(res.status()).toBe(401); + } + }); + + test('valid OAuth token is accepted after invalid tokens have been cached', async ({ page, context }) => { + const u = createTestUtils({ app, page, context }); + const url = new URL(adapter.oauth.verifyPath, app.serverUrl).toString(); + + // Seed the negative cache with a few invalid tokens + for (let i = 0; i < 3; i++) { + await u.page.request.get(url, { + headers: { Authorization: `Bearer oat_integration_cache_seed_${i}` }, + }); + } + + // A legitimate token obtained through the real OAuth flow must still work + const accessToken = await obtainOAuthAccessToken({ + page: u.page, + oAuthApp: fakeOAuth.oAuthApp, + redirectUri: new URL(adapter.oauth.callbackPath, app.serverUrl).toString(), + fakeUser, + signIn: u.po.signIn, + }); + + const res = await u.page.request.get(url, { + headers: { Authorization: `Bearer ${accessToken}` }, + }); + expect(res.status()).toBe(200); + const authData = await res.json(); + expect(authData.userId).toBeDefined(); + expect(authData.tokenType).toBe(TokenType.OAuthToken); + }); + for (const [tokenType, token] of [ ['API key', 'ak_test_mismatch'], ['M2M', 'mt_test_mismatch'], diff --git a/packages/backend/src/tokens/__tests__/oauthNegativeCache.test.ts b/packages/backend/src/tokens/__tests__/oauthNegativeCache.test.ts new file mode 100644 index 00000000000..0d57dd92392 --- /dev/null +++ b/packages/backend/src/tokens/__tests__/oauthNegativeCache.test.ts @@ -0,0 +1,147 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { MachineTokenVerificationError, MachineTokenVerificationErrorCode } from '../../errors'; +import { + isOAuthTokenCachedAsInvalid, + makeCachedInvalidOAuthTokenError, + maybeCacheOAuthTokenAsInvalid, + resetOAuthNegativeCache, +} from '../oauthNegativeCache'; + +const TOKEN = 'oat_abc123'; +const ANOTHER_TOKEN = 'oat_xyz789'; + +function makeTokenInvalidError() { + return new MachineTokenVerificationError({ + message: 'OAuth token not found', + code: MachineTokenVerificationErrorCode.TokenInvalid, + status: 404, + }); +} + +function makeOtherError() { + return new MachineTokenVerificationError({ + message: 'Invalid secret key', + code: MachineTokenVerificationErrorCode.InvalidSecretKey, + status: 401, + }); +} + +describe('oauthNegativeCache', () => { + beforeEach(() => { + resetOAuthNegativeCache(); + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + describe('isOAuthTokenCachedAsInvalid', () => { + it('returns false for a token that has never been cached', () => { + expect(isOAuthTokenCachedAsInvalid(TOKEN)).toBe(false); + }); + + it('returns true for a token cached as invalid', () => { + maybeCacheOAuthTokenAsInvalid(makeTokenInvalidError(), TOKEN); + expect(isOAuthTokenCachedAsInvalid(TOKEN)).toBe(true); + }); + + it('returns false for a different token not in the cache', () => { + maybeCacheOAuthTokenAsInvalid(makeTokenInvalidError(), TOKEN); + expect(isOAuthTokenCachedAsInvalid(ANOTHER_TOKEN)).toBe(false); + }); + + it('returns false and evicts the entry after TTL expires', () => { + maybeCacheOAuthTokenAsInvalid(makeTokenInvalidError(), TOKEN); + expect(isOAuthTokenCachedAsInvalid(TOKEN)).toBe(true); + + vi.advanceTimersByTime(30_001); + + expect(isOAuthTokenCachedAsInvalid(TOKEN)).toBe(false); + }); + + it('returns true just before TTL expires', () => { + maybeCacheOAuthTokenAsInvalid(makeTokenInvalidError(), TOKEN); + vi.advanceTimersByTime(29_999); + expect(isOAuthTokenCachedAsInvalid(TOKEN)).toBe(true); + }); + }); + + describe('maybeCacheOAuthTokenAsInvalid', () => { + it('caches when error is TokenInvalid', () => { + maybeCacheOAuthTokenAsInvalid(makeTokenInvalidError(), TOKEN); + expect(isOAuthTokenCachedAsInvalid(TOKEN)).toBe(true); + }); + + it('does not cache when error is a different MachineTokenVerificationError code', () => { + maybeCacheOAuthTokenAsInvalid(makeOtherError(), TOKEN); + expect(isOAuthTokenCachedAsInvalid(TOKEN)).toBe(false); + }); + + it('does not cache when error is not a MachineTokenVerificationError', () => { + maybeCacheOAuthTokenAsInvalid(new Error('network failure'), TOKEN); + expect(isOAuthTokenCachedAsInvalid(TOKEN)).toBe(false); + }); + + it('does not cache when error is null', () => { + maybeCacheOAuthTokenAsInvalid(null, TOKEN); + expect(isOAuthTokenCachedAsInvalid(TOKEN)).toBe(false); + }); + + it('updates the expiry when caching the same token again', () => { + maybeCacheOAuthTokenAsInvalid(makeTokenInvalidError(), TOKEN); + + vi.advanceTimersByTime(20_000); + maybeCacheOAuthTokenAsInvalid(makeTokenInvalidError(), TOKEN); + + vi.advanceTimersByTime(20_000); + // 40s total since first cache, but only 20s since re-cache; should still be valid + expect(isOAuthTokenCachedAsInvalid(TOKEN)).toBe(true); + + vi.advanceTimersByTime(10_001); + // 30s since re-cache; should now expire + expect(isOAuthTokenCachedAsInvalid(TOKEN)).toBe(false); + }); + }); + + describe('makeCachedInvalidOAuthTokenError', () => { + it('returns a MachineTokenVerificationError with TokenInvalid code', () => { + const err = makeCachedInvalidOAuthTokenError(); + expect(err).toBeInstanceOf(MachineTokenVerificationError); + expect(err.code).toBe(MachineTokenVerificationErrorCode.TokenInvalid); + }); + + it('returns an error with status 404', () => { + const err = makeCachedInvalidOAuthTokenError(); + expect(err.status).toBe(404); + }); + }); + + describe('resetOAuthNegativeCache', () => { + it('clears all cached entries', () => { + maybeCacheOAuthTokenAsInvalid(makeTokenInvalidError(), TOKEN); + maybeCacheOAuthTokenAsInvalid(makeTokenInvalidError(), ANOTHER_TOKEN); + resetOAuthNegativeCache(); + expect(isOAuthTokenCachedAsInvalid(TOKEN)).toBe(false); + expect(isOAuthTokenCachedAsInvalid(ANOTHER_TOKEN)).toBe(false); + }); + }); + + describe('capacity eviction', () => { + it('evicts the oldest entry when the cache reaches MAX_ENTRIES (10,000)', () => { + // Fill the cache to max capacity + for (let i = 0; i < 10_000; i++) { + maybeCacheOAuthTokenAsInvalid(makeTokenInvalidError(), `oat_token_${i}`); + } + + expect(isOAuthTokenCachedAsInvalid('oat_token_0')).toBe(true); + + // Adding one more should evict oat_token_0 (the oldest) + maybeCacheOAuthTokenAsInvalid(makeTokenInvalidError(), 'oat_overflow'); + + expect(isOAuthTokenCachedAsInvalid('oat_token_0')).toBe(false); + expect(isOAuthTokenCachedAsInvalid('oat_overflow')).toBe(true); + }); + }); +}); diff --git a/packages/backend/src/tokens/__tests__/oauthTokenRateLimiter.test.ts b/packages/backend/src/tokens/__tests__/oauthTokenRateLimiter.test.ts deleted file mode 100644 index 8b93a7fdafa..00000000000 --- a/packages/backend/src/tokens/__tests__/oauthTokenRateLimiter.test.ts +++ /dev/null @@ -1,79 +0,0 @@ -import { afterEach, describe, expect, it, vi } from 'vitest'; - -import { checkOAuthTokenRateLimit, resetOAuthTokenRateLimiter } from '../oauthTokenRateLimiter'; - -afterEach(() => { - resetOAuthTokenRateLimiter(); - vi.useRealTimers(); -}); - -describe('checkOAuthTokenRateLimit', () => { - it('allows the first request from an IP', () => { - expect(checkOAuthTokenRateLimit('1.2.3.4')).toBe(true); - }); - - it('allows up to MAX_BURST requests in a burst', () => { - const ip = '10.0.0.1'; - for (let i = 0; i < 20; i++) { - expect(checkOAuthTokenRateLimit(ip), `request ${i + 1} should be allowed`).toBe(true); - } - }); - - it('blocks requests that exceed MAX_BURST', () => { - const ip = '10.0.0.2'; - for (let i = 0; i < 20; i++) { - checkOAuthTokenRateLimit(ip); - } - expect(checkOAuthTokenRateLimit(ip)).toBe(false); - }); - - it('allows requests again after tokens refill', () => { - vi.useFakeTimers(); - const ip = '10.0.0.3'; - for (let i = 0; i < 20; i++) { - checkOAuthTokenRateLimit(ip); - } - expect(checkOAuthTokenRateLimit(ip)).toBe(false); - - // Advance 2 seconds: at 10 tokens/s, 20 new tokens should be available - vi.advanceTimersByTime(2000); - expect(checkOAuthTokenRateLimit(ip)).toBe(true); - }); - - it('tracks different IPs independently', () => { - const ipA = '192.168.1.1'; - const ipB = '192.168.1.2'; - for (let i = 0; i < 20; i++) { - checkOAuthTokenRateLimit(ipA); - } - expect(checkOAuthTokenRateLimit(ipA)).toBe(false); - expect(checkOAuthTokenRateLimit(ipB)).toBe(true); - }); - - it('treats the unknown sentinel as a single IP', () => { - for (let i = 0; i < 20; i++) { - checkOAuthTokenRateLimit('unknown'); - } - expect(checkOAuthTokenRateLimit('unknown')).toBe(false); - }); - - it('evicts the oldest bucket and allows a new IP when MAX_BUCKETS is reached', () => { - // Fill up to MAX_BUCKETS (10 000) unique IPs - for (let i = 0; i < 10_000; i++) { - checkOAuthTokenRateLimit(`10.${Math.floor(i / 65536)}.${Math.floor((i % 65536) / 256)}.${i % 256}`); - } - // The 10 001st IP triggers eviction of the oldest entry; the new IP gets a fresh bucket - const freshIp = '172.16.0.1'; - expect(checkOAuthTokenRateLimit(freshIp)).toBe(true); - }); - - it('allows a previously blocked IP after resetOAuthTokenRateLimiter', () => { - const ip = '5.5.5.5'; - for (let i = 0; i < 21; i++) { - checkOAuthTokenRateLimit(ip); - } - expect(checkOAuthTokenRateLimit(ip)).toBe(false); - resetOAuthTokenRateLimiter(); - expect(checkOAuthTokenRateLimit(ip)).toBe(true); - }); -}); diff --git a/packages/backend/src/tokens/__tests__/request.test.ts b/packages/backend/src/tokens/__tests__/request.test.ts index 31fab9c34a6..59b08ce631c 100644 --- a/packages/backend/src/tokens/__tests__/request.test.ts +++ b/packages/backend/src/tokens/__tests__/request.test.ts @@ -2,14 +2,13 @@ import { http, HttpResponse } from 'msw'; import { afterEach, beforeEach, describe, expect, it, test, vi } from 'vitest'; import { MachineTokenVerificationErrorCode, TokenVerificationErrorReason } from '../../errors'; -import { checkOAuthTokenRateLimit, resetOAuthTokenRateLimiter } from '../oauthTokenRateLimiter'; +import { isOAuthTokenCachedAsInvalid, resetOAuthNegativeCache } from '../oauthNegativeCache'; import { mockExpiredJwt, mockInvalidSignatureJwt, mockJwks, mockJwt, mockJwtPayload, - mockM2MJwtPayload, signingJwks, } from '../../fixtures'; import { @@ -1454,6 +1453,7 @@ describe('tokens.authenticateRequest(options)', () => { describe('Machine authentication', () => { afterEach(() => { vi.clearAllMocks(); + resetOAuthNegativeCache(); }); // Test each token type with parameterized tests @@ -1764,191 +1764,105 @@ describe('tokens.authenticateRequest(options)', () => { }); }); - describe('Rate limiting', () => { + describe('OAuth negative cache', () => { afterEach(() => { - resetOAuthTokenRateLimiter(); + resetOAuthNegativeCache(); }); - const exhaustBucket = (ip: string) => { - for (let i = 0; i < 20; i++) { - checkOAuthTokenRateLimit(ip); - } - }; - - test('blocks machine token request when IP exceeds burst limit', async () => { + test('rejects a previously invalid oat_ token from cache without calling BAPI again', async () => { server.use( - http.post(mockMachineAuthResponses.oauth_token.endpoint, () => - HttpResponse.json(mockVerificationResults.oauth_token), - ), + http.post(mockMachineAuthResponses.oauth_token.endpoint, () => HttpResponse.json({}, { status: 404 })), ); - const ip = '203.0.113.1'; - exhaustBucket(ip); - const rateLimited = await authenticateRequest( - mockRequest({ authorization: `Bearer ${mockTokens.oauth_token}`, 'cf-connecting-ip': ip }), + const token = 'oat_invalid_garbage_token'; + await authenticateRequest( + mockRequest({ authorization: `Bearer ${token}` }), mockOptions({ acceptsToken: 'oauth_token' }), ); - expect(rateLimited).toBeMachineUnauthenticated({ - tokenType: 'oauth_token', - reason: AuthErrorReason.OAuthTokenRateLimit, - message: '', - }); - }); - test('prefers cf-connecting-ip over x-forwarded-for for rate limit key', async () => { + // BAPI now returns 200, but the token should still be rejected from cache server.use( http.post(mockMachineAuthResponses.oauth_token.endpoint, () => HttpResponse.json(mockVerificationResults.oauth_token), ), ); - const cfIp = '10.0.0.1'; - exhaustBucket(cfIp); - // cf-connecting-ip is exhausted; x-forwarded-for is a different IP and has full bucket - const rateLimited = await authenticateRequest( - mockRequest({ - authorization: `Bearer ${mockTokens.oauth_token}`, - 'cf-connecting-ip': cfIp, - 'x-forwarded-for': '99.99.99.99', - }), + const second = await authenticateRequest( + mockRequest({ authorization: `Bearer ${token}` }), mockOptions({ acceptsToken: 'oauth_token' }), ); - expect(rateLimited).toBeMachineUnauthenticated({ + expect(second).toBeMachineUnauthenticated({ tokenType: 'oauth_token', - reason: AuthErrorReason.OAuthTokenRateLimit, - message: '', + reason: MachineTokenVerificationErrorCode.TokenInvalid, + message: 'OAuth token not found (code=token-invalid, status=404)', }); - // x-forwarded-for IP is untouched: a request using only that header must be allowed - const allowed = await authenticateRequest( - mockRequest({ - authorization: `Bearer ${mockTokens.oauth_token}`, - 'x-forwarded-for': '99.99.99.99', - }), - mockOptions({ acceptsToken: 'oauth_token' }), - ); - expect(allowed).toBeMachineAuthenticated(); }); - test('falls back to x-real-ip when cf-connecting-ip is absent', async () => { + test('does not cache valid oat_ tokens', async () => { server.use( http.post(mockMachineAuthResponses.oauth_token.endpoint, () => HttpResponse.json(mockVerificationResults.oauth_token), ), ); - const realIp = '192.168.10.20'; - exhaustBucket(realIp); - const rateLimited = await authenticateRequest( - mockRequest({ authorization: `Bearer ${mockTokens.oauth_token}`, 'x-real-ip': realIp }), + await authenticateRequest( + mockRequest({ authorization: `Bearer ${mockTokens.oauth_token}` }), mockOptions({ acceptsToken: 'oauth_token' }), ); - expect(rateLimited).toBeMachineUnauthenticated({ - tokenType: 'oauth_token', - reason: AuthErrorReason.OAuthTokenRateLimit, - message: '', - }); + expect(isOAuthTokenCachedAsInvalid(mockTokens.oauth_token)).toBe(false); }); - test('falls back to first value in x-forwarded-for when higher-priority headers are absent', async () => { + test('does not cache oat_ tokens that fail with non-TokenInvalid errors', async () => { + // 401 maps to InvalidSecretKey, not TokenInvalid server.use( - http.post(mockMachineAuthResponses.oauth_token.endpoint, () => - HttpResponse.json(mockVerificationResults.oauth_token), - ), + http.post(mockMachineAuthResponses.oauth_token.endpoint, () => HttpResponse.json({}, { status: 401 })), ); - const ip = '172.16.5.5'; - exhaustBucket(ip); - const rateLimited = await authenticateRequest( - mockRequest({ - authorization: `Bearer ${mockTokens.oauth_token}`, - 'x-forwarded-for': `${ip}, 10.0.0.2`, - }), + const token = 'oat_secret_key_error_token'; + await authenticateRequest( + mockRequest({ authorization: `Bearer ${token}` }), mockOptions({ acceptsToken: 'oauth_token' }), ); - expect(rateLimited).toBeMachineUnauthenticated({ - tokenType: 'oauth_token', - reason: AuthErrorReason.OAuthTokenRateLimit, - message: '', - }); - }); - - test('session token path is not rate-limited', async () => { - server.use(http.get('https://api.clerk.test/v1/jwks', () => HttpResponse.json(mockJwks))); - // Exhaust the 'unknown' sentinel bucket (no IP headers on mockRequestWithHeaderAuth) - exhaustBucket('unknown'); - const result = await authenticateRequest(mockRequestWithHeaderAuth(), mockOptions()); - expect(result).toBeSignedIn(); + expect(isOAuthTokenCachedAsInvalid(token)).toBe(false); }); - test('OAuth JWT tokens bypass the rate limiter', async () => { + test('re-verifies token after cache TTL expires', async () => { vi.useFakeTimers(); - vi.setSystemTime(new Date(mockJwtPayload.iat * 1000)); - server.use(http.get('https://api.clerk.test/v1/jwks', () => HttpResponse.json(mockJwks))); - const ip = '203.0.113.2'; - exhaustBucket(ip); - const { data: oauthJwt } = await signJwt( - { - iss: 'https://clerk.oauth.example.test', - sub: 'user_2vYVtestTESTtestTESTtestTESTtest', - client_id: 'client_2VTWUzvGC5UhdJCNx6xG1D98edc', - scope: 'read:foo', - exp: mockJwtPayload.iat + 300, - iat: mockJwtPayload.iat, - nbf: mockJwtPayload.iat - 10, - }, - signingJwks, - { algorithm: 'RS256', header: { typ: 'at+jwt', kid: 'ins_2GIoQhbUpy0hX7B2cVkuTMinXoD' } }, + server.use( + http.post(mockMachineAuthResponses.oauth_token.endpoint, () => HttpResponse.json({}, { status: 404 })), ); - const result = await authenticateRequest( - mockRequest({ authorization: `Bearer ${oauthJwt}`, 'cf-connecting-ip': ip }), + const token = 'oat_will_expire_from_cache'; + await authenticateRequest( + mockRequest({ authorization: `Bearer ${token}` }), mockOptions({ acceptsToken: 'oauth_token' }), ); - expect(result.reason).not.toBe(AuthErrorReason.OAuthTokenRateLimit); - vi.useRealTimers(); - }); + expect(isOAuthTokenCachedAsInvalid(token)).toBe(true); - test('M2M JWT tokens bypass the rate limiter', async () => { - vi.useFakeTimers(); - vi.setSystemTime(new Date(mockJwtPayload.iat * 1000)); - server.use(http.get('https://api.clerk.test/v1/jwks', () => HttpResponse.json(mockJwks))); - const ip = '203.0.113.3'; - exhaustBucket(ip); - const { data: m2mJwt } = await signJwt(mockM2MJwtPayload, signingJwks, { - algorithm: 'RS256', - header: { typ: 'JWT', kid: 'ins_2GIoQhbUpy0hX7B2cVkuTMinXoD' }, - }); - const result = await authenticateRequest( - mockRequest({ authorization: `Bearer ${m2mJwt}`, 'cf-connecting-ip': ip }), - mockOptions({ acceptsToken: 'm2m_token' }), - ); - expect(result.reason).not.toBe(AuthErrorReason.OAuthTokenRateLimit); + vi.advanceTimersByTime(30_001); + expect(isOAuthTokenCachedAsInvalid(token)).toBe(false); vi.useRealTimers(); }); - test('opaque api_key tokens bypass the rate limiter', async () => { + test('ak_ tokens are not affected by the cache', async () => { server.use( http.post(mockMachineAuthResponses.api_key.endpoint, () => HttpResponse.json(mockVerificationResults.api_key), ), ); - const ip = '203.0.113.4'; - exhaustBucket(ip); const result = await authenticateRequest( - mockRequest({ authorization: `Bearer ${mockTokens.api_key}`, 'cf-connecting-ip': ip }), + mockRequest({ authorization: `Bearer ${mockTokens.api_key}` }), mockOptions({ acceptsToken: 'api_key' }), ); - expect(result.reason).not.toBe(AuthErrorReason.OAuthTokenRateLimit); + expect(result).toBeMachineAuthenticated(); }); - test('opaque m2m tokens bypass the rate limiter', async () => { + test('mt_ tokens are not affected by the cache', async () => { server.use( http.post(mockMachineAuthResponses.m2m_token.endpoint, () => HttpResponse.json(mockVerificationResults.m2m_token), ), ); - const ip = '203.0.113.5'; - exhaustBucket(ip); const result = await authenticateRequest( - mockRequest({ authorization: `Bearer ${mockTokens.m2m_token}`, 'cf-connecting-ip': ip }), + mockRequest({ authorization: `Bearer ${mockTokens.m2m_token}` }), mockOptions({ acceptsToken: 'm2m_token' }), ); - expect(result.reason).not.toBe(AuthErrorReason.OAuthTokenRateLimit); + expect(result).toBeMachineAuthenticated(); }); }); diff --git a/packages/backend/src/tokens/authStatus.ts b/packages/backend/src/tokens/authStatus.ts index 6d374f7cb9e..421c7bd61f4 100644 --- a/packages/backend/src/tokens/authStatus.ts +++ b/packages/backend/src/tokens/authStatus.ts @@ -118,7 +118,6 @@ export const AuthErrorReason = { SessionTokenWithoutClientUAT: 'session-token-but-no-client-uat', ActiveOrganizationMismatch: 'active-organization-mismatch', TokenTypeMismatch: 'token-type-mismatch', - OAuthTokenRateLimit: 'oauth-token-rate-limit', UnexpectedError: 'unexpected-error', } as const; diff --git a/packages/backend/src/tokens/oauthNegativeCache.ts b/packages/backend/src/tokens/oauthNegativeCache.ts new file mode 100644 index 00000000000..b986c0b547c --- /dev/null +++ b/packages/backend/src/tokens/oauthNegativeCache.ts @@ -0,0 +1,44 @@ +import { MachineTokenVerificationError, MachineTokenVerificationErrorCode } from '../errors'; + +const TTL_MS = 30_000; +const MAX_ENTRIES = 10_000; + +type Entry = { expiresAt: number }; +const cache = new Map(); + +export function isOAuthTokenCachedAsInvalid(token: string): boolean { + const entry = cache.get(token); + if (!entry) { + return false; + } + if (Date.now() >= entry.expiresAt) { + cache.delete(token); + return false; + } + return true; +} + +export function maybeCacheOAuthTokenAsInvalid(err: unknown, token: string): void { + if (!(err instanceof MachineTokenVerificationError) || err.code !== MachineTokenVerificationErrorCode.TokenInvalid) { + return; + } + if (cache.size >= MAX_ENTRIES) { + const oldest = cache.keys().next().value; + if (oldest !== undefined) { + cache.delete(oldest); + } + } + cache.set(token, { expiresAt: Date.now() + TTL_MS }); +} + +export function makeCachedInvalidOAuthTokenError(): MachineTokenVerificationError { + return new MachineTokenVerificationError({ + message: 'OAuth token not found', + code: MachineTokenVerificationErrorCode.TokenInvalid, + status: 404, + }); +} + +export function resetOAuthNegativeCache(): void { + cache.clear(); +} diff --git a/packages/backend/src/tokens/oauthTokenRateLimiter.ts b/packages/backend/src/tokens/oauthTokenRateLimiter.ts deleted file mode 100644 index 4ab5ee13378..00000000000 --- a/packages/backend/src/tokens/oauthTokenRateLimiter.ts +++ /dev/null @@ -1,32 +0,0 @@ -const MAX_BURST = 20; -const REFILL_RATE = 10; // tokens per second -const MAX_BUCKETS = 10_000; - -type Bucket = { tokens: number; lastRefill: number }; -const buckets = new Map(); - -export function checkOAuthTokenRateLimit(ip: string): boolean { - if (buckets.size >= MAX_BUCKETS) { - // Evict the oldest entry rather than clearing all buckets to prevent an attacker - // from neutralizing rate limits by forcing key churn across many distinct IPs. - const oldest = buckets.keys().next().value; - if (oldest !== undefined) { - buckets.delete(oldest); - } - } - const now = Date.now(); - const existing = buckets.get(ip); - const bucket: Bucket = existing ?? { tokens: MAX_BURST, lastRefill: now }; - const elapsed = (now - bucket.lastRefill) / 1000; - const refilled = Math.min(MAX_BURST, bucket.tokens + elapsed * REFILL_RATE); - if (refilled < 1) { - buckets.set(ip, { tokens: refilled, lastRefill: now }); - return false; - } - buckets.set(ip, { tokens: refilled - 1, lastRefill: now }); - return true; -} - -export function resetOAuthTokenRateLimiter(): void { - buckets.clear(); -} diff --git a/packages/backend/src/tokens/request.ts b/packages/backend/src/tokens/request.ts index 92d4bc0390e..9fba39a4ed3 100644 --- a/packages/backend/src/tokens/request.ts +++ b/packages/backend/src/tokens/request.ts @@ -21,32 +21,17 @@ import { isOAuthTokenByPrefix, isTokenTypeAccepted, } from './machine'; -import { checkOAuthTokenRateLimit } from './oauthTokenRateLimiter'; +import { + isOAuthTokenCachedAsInvalid, + maybeCacheOAuthTokenAsInvalid, + makeCachedInvalidOAuthTokenError, +} from './oauthNegativeCache'; import { OrganizationMatcher } from './organizationMatcher'; import type { MachineTokenType, SessionTokenType } from './tokenTypes'; import { TokenType } from './tokenTypes'; import type { AuthenticateRequestOptions } from './types'; import { verifyMachineAuthToken, verifyToken } from './verify'; -// NOTE: IP headers like x-forwarded-for can be spoofed by clients not behind a trusted proxy. -// cf-connecting-ip (set by Cloudflare) and x-real-ip (set by Nginx) are more reliable when present. -// The rate limiter is defense-in-depth against BAPI quota exhaustion, not a security boundary. -function extractCallerIp(request: Request): string { - const cfConnectingIp = request.headers.get(constants.Headers.CfConnectingIp); - if (cfConnectingIp) { - return cfConnectingIp; - } - const xRealIp = request.headers.get(constants.Headers.RealIp); - if (xRealIp) { - return xRealIp; - } - const xForwardedFor = request.headers.get(constants.Headers.ForwardedFor); - if (xForwardedFor) { - return xForwardedFor.split(',')[0]?.trim() ?? 'unknown'; - } - return 'unknown'; -} - export const RefreshTokenErrorReason = { NonEligibleNoCookie: 'non-eligible-no-refresh-cookie', NonEligibleNonGet: 'non-eligible-non-get', @@ -821,17 +806,13 @@ export const authenticateRequest: AuthenticateRequest = (async ( return mismatchState; } - if (isOAuthTokenByPrefix(tokenInHeader) && !checkOAuthTokenRateLimit(extractCallerIp(request))) { - return signedOut({ - tokenType: parsedTokenType, - authenticateContext, - reason: AuthErrorReason.OAuthTokenRateLimit, - message: '', - }); + if (isOAuthTokenByPrefix(tokenInHeader) && isOAuthTokenCachedAsInvalid(tokenInHeader)) { + return handleMachineError(parsedTokenType, makeCachedInvalidOAuthTokenError()); } const { data, tokenType, errors } = await verifyMachineAuthToken(tokenInHeader, authenticateContext); if (errors) { + maybeCacheOAuthTokenAsInvalid(errors[0], tokenInHeader); return handleMachineError(tokenType, errors[0]); } return signedIn({ @@ -857,17 +838,13 @@ export const authenticateRequest: AuthenticateRequest = (async ( return mismatchState; } - if (isOAuthTokenByPrefix(tokenInHeader) && !checkOAuthTokenRateLimit(extractCallerIp(request))) { - return signedOut({ - tokenType: parsedTokenType, - authenticateContext, - reason: AuthErrorReason.OAuthTokenRateLimit, - message: '', - }); + if (isOAuthTokenByPrefix(tokenInHeader) && isOAuthTokenCachedAsInvalid(tokenInHeader)) { + return handleMachineError(parsedTokenType, makeCachedInvalidOAuthTokenError()); } const { data, tokenType, errors } = await verifyMachineAuthToken(tokenInHeader, authenticateContext); if (errors) { + maybeCacheOAuthTokenAsInvalid(errors[0], tokenInHeader); return handleMachineError(tokenType, errors[0]); } From a0ace6556a05b3812a831ae962cece4adf41c082 Mon Sep 17 00:00:00 2001 From: wobsoriano Date: Tue, 12 May 2026 14:56:54 -0700 Subject: [PATCH 12/15] chore: remove unused added constants --- packages/backend/src/constants.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/backend/src/constants.ts b/packages/backend/src/constants.ts index f5e294fd3a5..2d5f49625f8 100644 --- a/packages/backend/src/constants.ts +++ b/packages/backend/src/constants.ts @@ -56,13 +56,10 @@ const Headers = { ContentSecurityPolicy: 'content-security-policy', ContentSecurityPolicyReportOnly: 'content-security-policy-report-only', EnableDebug: 'x-clerk-debug', - CfConnectingIp: 'cf-connecting-ip', - ForwardedFor: 'x-forwarded-for', ForwardedHost: 'x-forwarded-host', ForwardedPort: 'x-forwarded-port', ForwardedProto: 'x-forwarded-proto', Host: 'host', - RealIp: 'x-real-ip', Location: 'location', Nonce: 'x-nonce', Origin: 'origin', From e90b4d4fa63752822bfad3bf16bcd47eaeb69149 Mon Sep 17 00:00:00 2001 From: wobsoriano Date: Tue, 12 May 2026 15:02:33 -0700 Subject: [PATCH 13/15] chore: only cache oauth tokens --- packages/backend/src/tokens/request.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/tokens/request.ts b/packages/backend/src/tokens/request.ts index 9fba39a4ed3..d0e40668a82 100644 --- a/packages/backend/src/tokens/request.ts +++ b/packages/backend/src/tokens/request.ts @@ -812,7 +812,9 @@ export const authenticateRequest: AuthenticateRequest = (async ( const { data, tokenType, errors } = await verifyMachineAuthToken(tokenInHeader, authenticateContext); if (errors) { - maybeCacheOAuthTokenAsInvalid(errors[0], tokenInHeader); + if (isOAuthTokenByPrefix(tokenInHeader)) { + maybeCacheOAuthTokenAsInvalid(errors[0], tokenInHeader); + } return handleMachineError(tokenType, errors[0]); } return signedIn({ @@ -844,7 +846,9 @@ export const authenticateRequest: AuthenticateRequest = (async ( const { data, tokenType, errors } = await verifyMachineAuthToken(tokenInHeader, authenticateContext); if (errors) { - maybeCacheOAuthTokenAsInvalid(errors[0], tokenInHeader); + if (isOAuthTokenByPrefix(tokenInHeader)) { + maybeCacheOAuthTokenAsInvalid(errors[0], tokenInHeader); + } return handleMachineError(tokenType, errors[0]); } From 21a2f11f0e57726f1b063e33e29bc6e38cee8c25 Mon Sep 17 00:00:00 2001 From: wobsoriano Date: Tue, 12 May 2026 15:21:03 -0700 Subject: [PATCH 14/15] chore: remove useless e2e test --- integration/testUtils/machineAuthHelpers.ts | 42 --------------------- packages/backend/src/tokens/request.ts | 2 +- 2 files changed, 1 insertion(+), 43 deletions(-) diff --git a/integration/testUtils/machineAuthHelpers.ts b/integration/testUtils/machineAuthHelpers.ts index 9a5034753f3..ea541c2d0f2 100644 --- a/integration/testUtils/machineAuthHelpers.ts +++ b/integration/testUtils/machineAuthHelpers.ts @@ -432,48 +432,6 @@ export const registerOAuthAuthTests = (adapter: MachineAuthTestAdapter): void => expect(res.status()).toBe(401); }); - test('consistently rejects the same invalid oat_ token across repeated requests', async ({ request }) => { - // After the first rejection the token is cached as invalid in-process. - // Subsequent requests with the same token must still return 401 (not be - // accidentally accepted due to cache state). - const url = new URL(adapter.oauth.verifyPath, app.serverUrl).toString(); - const invalidToken = `oat_integration_test_invalid_${Date.now()}`; - - for (let i = 0; i < 3; i++) { - const res = await request.get(url, { headers: { Authorization: `Bearer ${invalidToken}` } }); - expect(res.status()).toBe(401); - } - }); - - test('valid OAuth token is accepted after invalid tokens have been cached', async ({ page, context }) => { - const u = createTestUtils({ app, page, context }); - const url = new URL(adapter.oauth.verifyPath, app.serverUrl).toString(); - - // Seed the negative cache with a few invalid tokens - for (let i = 0; i < 3; i++) { - await u.page.request.get(url, { - headers: { Authorization: `Bearer oat_integration_cache_seed_${i}` }, - }); - } - - // A legitimate token obtained through the real OAuth flow must still work - const accessToken = await obtainOAuthAccessToken({ - page: u.page, - oAuthApp: fakeOAuth.oAuthApp, - redirectUri: new URL(adapter.oauth.callbackPath, app.serverUrl).toString(), - fakeUser, - signIn: u.po.signIn, - }); - - const res = await u.page.request.get(url, { - headers: { Authorization: `Bearer ${accessToken}` }, - }); - expect(res.status()).toBe(200); - const authData = await res.json(); - expect(authData.userId).toBeDefined(); - expect(authData.tokenType).toBe(TokenType.OAuthToken); - }); - for (const [tokenType, token] of [ ['API key', 'ak_test_mismatch'], ['M2M', 'mt_test_mismatch'], diff --git a/packages/backend/src/tokens/request.ts b/packages/backend/src/tokens/request.ts index d0e40668a82..642cc781ada 100644 --- a/packages/backend/src/tokens/request.ts +++ b/packages/backend/src/tokens/request.ts @@ -23,8 +23,8 @@ import { } from './machine'; import { isOAuthTokenCachedAsInvalid, - maybeCacheOAuthTokenAsInvalid, makeCachedInvalidOAuthTokenError, + maybeCacheOAuthTokenAsInvalid, } from './oauthNegativeCache'; import { OrganizationMatcher } from './organizationMatcher'; import type { MachineTokenType, SessionTokenType } from './tokenTypes'; From 5a74cb4c156819fd6a185d2d4ebc45d851194e6f Mon Sep 17 00:00:00 2001 From: wobsoriano Date: Tue, 12 May 2026 15:38:03 -0700 Subject: [PATCH 15/15] chore: use 'any' as mocked option for authenticateRequest --- packages/backend/src/tokens/__tests__/request.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/tokens/__tests__/request.test.ts b/packages/backend/src/tokens/__tests__/request.test.ts index 59b08ce631c..48b926d3a44 100644 --- a/packages/backend/src/tokens/__tests__/request.test.ts +++ b/packages/backend/src/tokens/__tests__/request.test.ts @@ -1776,7 +1776,7 @@ describe('tokens.authenticateRequest(options)', () => { const token = 'oat_invalid_garbage_token'; await authenticateRequest( mockRequest({ authorization: `Bearer ${token}` }), - mockOptions({ acceptsToken: 'oauth_token' }), + mockOptions({ acceptsToken: 'any' }), ); // BAPI now returns 200, but the token should still be rejected from cache @@ -1787,7 +1787,7 @@ describe('tokens.authenticateRequest(options)', () => { ); const second = await authenticateRequest( mockRequest({ authorization: `Bearer ${token}` }), - mockOptions({ acceptsToken: 'oauth_token' }), + mockOptions({ acceptsToken: 'any' }), ); expect(second).toBeMachineUnauthenticated({ tokenType: 'oauth_token',