From 9e791b2ceac184a8b4b38d69e019e63b9f1271a8 Mon Sep 17 00:00:00 2001 From: Matheus Andrade Date: Wed, 7 Oct 2026 12:13:45 -0300 Subject: [PATCH 1/3] :sparkles: feat(auth-permission): accept a list of headers in apiKeyHeader Co-Authored-By: Claude Opus 5.5 --- .../express-register-permission-auth.spec.ts | 55 +++++++++ .../express-register-permission-auth.ts | 20 +++- .../fastify/rate-limit/rate-limit-key.ts | 4 +- .../fastify/register-permission-auth.spec.ts | 109 ++++++++++++++++++ .../fastify/register-permission-auth.ts | 22 +++- src/auth-permission/shared/api-key-headers.ts | 30 +++++ .../shared/read-credentials.ts | 14 ++- 7 files changed, 240 insertions(+), 14 deletions(-) create mode 100644 src/auth-permission/shared/api-key-headers.ts diff --git a/src/auth-permission/express/express-register-permission-auth.spec.ts b/src/auth-permission/express/express-register-permission-auth.spec.ts index 95778cf..27fcc96 100644 --- a/src/auth-permission/express/express-register-permission-auth.spec.ts +++ b/src/auth-permission/express/express-register-permission-auth.spec.ts @@ -85,6 +85,7 @@ interface BuildAppOptions { sessionTokenVerifier?: SessionTokenVerifier; routes: Readonly>; publicPaths?: string[]; + apiKeyHeader?: string | readonly string[]; } function buildTestApp(options: BuildAppOptions): Express { @@ -95,6 +96,7 @@ function buildTestApp(options: BuildAppOptions): Express { sessionTokenVerifier: options.sessionTokenVerifier, routes: options.routes, publicPaths: options.publicPaths, + apiKeyHeader: options.apiKeyHeader, }); app.use(middleware); @@ -173,6 +175,59 @@ describe('expressRegisterPermissionAuth', () => { }); }); + describe('apiKeyHeader', () => { + it('reads the key from any header in a list, case-insensitively', async () => { + const validator = authorizes(['tim']); + const app = buildTestApp({ + validator, + routes: { 'GET /balances': { permissionName: PERMISSION } }, + apiKeyHeader: ['x-api-key', 'Authorization'], + }); + const listening = await listen(app); + server = listening.server; + + const viaAuthorization = await inject(listening.baseUrl, '/balances', { authorization: API_KEY }); + const viaApiKey = await inject(listening.baseUrl, '/balances', { [API_KEY_HEADER]: API_KEY }); + + expect(viaAuthorization.statusCode).toBe(200); + expect(viaApiKey.statusCode).toBe(200); + expect(validator.calls.map((call) => call.apiKey)).toEqual([API_KEY, API_KEY]); + }); + + it('rejects a header outside the configured list (401)', async () => { + const validator = authorizes(); + const app = buildTestApp({ + validator, + routes: { 'GET /balances': { permissionName: PERMISSION } }, + apiKeyHeader: 'authorization', + }); + const listening = await listen(app); + server = listening.server; + + const res = await inject(listening.baseUrl, '/balances', { [API_KEY_HEADER]: API_KEY }); + + expect(res.statusCode).toBe(401); + expect(validator.calls).toHaveLength(0); + }); + + it('throws synchronously when the authorization header is an API key header and a route accepts session tokens', () => { + expect(() => + expressRegisterPermissionAuth({ + validator: authorizes(), + sessionTokenVerifier: verifiesAs({ sub: 'key-1', brokers: ['tim'] }), + routes: { 'GET /session-scoped': { permissionName: PERMISSION, acceptsSessionToken: true } }, + apiKeyHeader: [API_KEY_HEADER, 'Authorization'], + }), + ).toThrow(/acceptsSessionToken/); + }); + + it('throws synchronously on an empty list', () => { + expect(() => + expressRegisterPermissionAuth({ validator: authorizes(), routes: {}, apiKeyHeader: [] }), + ).toThrow(/empty apiKeyHeader/); + }); + }); + describe('authentication', () => { it('rejects a mapped route without a key (401), without calling the validator', async () => { const validator = authorizes(); diff --git a/src/auth-permission/express/express-register-permission-auth.ts b/src/auth-permission/express/express-register-permission-auth.ts index a5540d9..72778a8 100644 --- a/src/auth-permission/express/express-register-permission-auth.ts +++ b/src/auth-permission/express/express-register-permission-auth.ts @@ -6,6 +6,7 @@ import { PermissionDeniedError } from '../errors/permission-denied-error'; import type { RouteProperties } from '../route-properties'; import { checkApiKeyPermission } from '../api-key/check-api-key-permission'; import { authenticateWithSessionToken } from '../session-token/authenticate-with-session-token'; +import { resolveApiKeyHeaders } from '../shared/api-key-headers'; import { readApiKey, readBearerToken } from '../shared/read-credentials'; import { isPublicPath } from '../shared/is-public-path'; import { compileRoutes, matchRoute } from './route-matcher'; @@ -40,7 +41,12 @@ export interface ExpressPermissionAuthOptions { * includes `'/'` would silently make that router's own root path public. */ publicPaths?: string[]; - apiKeyHeader?: string; + /** + * Header(s) the API key is read from, case-insensitively. With a list, the + * first one present wins. May only include `authorizationHeader` when no + * route sets `acceptsSessionToken`. + */ + apiKeyHeader?: string | readonly string[]; /** Required when any route in `routes` sets `acceptsSessionToken: true`. */ sessionTokenVerifier?: SessionTokenVerifier; authorizationHeader?: string; @@ -88,7 +94,6 @@ export function expressRegisterPermissionAuth( options: ExpressPermissionAuthOptions, ): ExpressPermissionAuth { const publicPaths = options.publicPaths ?? []; - const apiKeyHeader = options.apiKeyHeader ?? API_KEY_HEADER; const authorizationHeader = options.authorizationHeader ?? AUTHORIZATION_HEADER; const brokerIdHeader = options.brokerIdHeader ?? BROKER_ID_HEADER; const brokerIdMaxLength = options.brokerIdMaxLength ?? BROKER_ID_MAX_LENGTH; @@ -101,10 +106,17 @@ export function expressRegisterPermissionAuth( } } + const apiKeyHeaders = resolveApiKeyHeaders( + options.apiKeyHeader ?? API_KEY_HEADER, + authorizationHeader, + options.routes, + 'expressRegisterPermissionAuth', + ); + const compiledRoutes = compileRoutes(options.routes); const assertHasPermission: ExpressAssertHasPermission = async (request, route) => { - const apiKey = readApiKey(request, apiKeyHeader); + const apiKey = readApiKey(request, apiKeyHeaders); if (apiKey !== undefined) { const ctx = await checkApiKeyPermission( request, @@ -149,7 +161,7 @@ export function expressRegisterPermissionAuth( } const hasCredential = - readApiKey(request, apiKeyHeader) !== undefined || + readApiKey(request, apiKeyHeaders) !== undefined || readBearerToken(request, authorizationHeader) !== undefined; if (!hasCredential) { next(new PermissionValidatorUnauthorizedError()); diff --git a/src/auth-permission/fastify/rate-limit/rate-limit-key.ts b/src/auth-permission/fastify/rate-limit/rate-limit-key.ts index 823a99d..8b73b3d 100644 --- a/src/auth-permission/fastify/rate-limit/rate-limit-key.ts +++ b/src/auth-permission/fastify/rate-limit/rate-limit-key.ts @@ -12,13 +12,13 @@ import { toWaveRequest } from '../to-wave-request'; * (still useful to cap unauthenticated request floods). */ export function credentialRateLimitKey( - apiKeyHeader: string, + apiKeyHeaders: readonly string[], authorizationHeader: string, ): (request: FastifyRequest) => string { return (request) => { const waveRequest = toWaveRequest(request); return ( - readApiKey(waveRequest, apiKeyHeader) ?? + readApiKey(waveRequest, apiKeyHeaders) ?? readBearerToken(waveRequest, authorizationHeader) ?? request.ip ); diff --git a/src/auth-permission/fastify/register-permission-auth.spec.ts b/src/auth-permission/fastify/register-permission-auth.spec.ts index 550fe8c..d4a1f12 100644 --- a/src/auth-permission/fastify/register-permission-auth.spec.ts +++ b/src/auth-permission/fastify/register-permission-auth.spec.ts @@ -115,6 +115,7 @@ interface BuildAppOptions { publicPaths?: string[]; brokerIdMaxLength?: number; rateLimit?: CreateRateLimitOptions; + apiKeyHeader?: string | readonly string[]; } function buildTestApp(options: BuildAppOptions): FastifyInstance { @@ -138,6 +139,7 @@ function buildTestApp(options: BuildAppOptions): FastifyInstance { publicPaths: options.publicPaths, brokerIdMaxLength: options.brokerIdMaxLength, rateLimit: options.rateLimit, + apiKeyHeader: options.apiKeyHeader, }); app.get('/', () => ({ status: 'ok' })); @@ -271,6 +273,76 @@ describe('registerPermissionAuth', () => { }); }); + describe('apiKeyHeader', () => { + it('reads the key from any header in a list, case-insensitively', async () => { + const validator = authorizes(); + app = buildTestApp({ validator, apiKeyHeader: ['x-api-key', 'Authorization'] }); + + const viaAuthorization = await app.inject({ + method: 'POST', + url: '/protected', + headers: { authorization: API_KEY }, + }); + const viaApiKey = await app.inject({ + method: 'POST', + url: '/protected', + headers: { [API_KEY_HEADER]: API_KEY }, + }); + + expect(viaAuthorization.statusCode).toBe(200); + expect(viaApiKey.statusCode).toBe(200); + expect(validator.calls.map((call) => call.apiKey)).toEqual([API_KEY, API_KEY]); + }); + + it('prefers the earliest header in the list when several are present', async () => { + const validator = authorizes(); + app = buildTestApp({ validator, apiKeyHeader: ['authorization', 'x-api-key'] }); + + await app.inject({ + method: 'POST', + url: '/protected', + headers: { authorization: API_KEY, [API_KEY_HEADER]: 'other-key' }, + }); + + expect(validator.calls.map((call) => call.apiKey)).toEqual([API_KEY]); + }); + + it('rejects a header outside the configured list (401)', async () => { + const validator = authorizes(); + app = buildTestApp({ validator, apiKeyHeader: 'Authorization' }); + + const res = await app.inject({ + method: 'POST', + url: '/protected', + headers: { [API_KEY_HEADER]: API_KEY }, + }); + + expect(res.statusCode).toBe(401); + expect(validator.calls).toHaveLength(0); + }); + + it('throws synchronously on an empty list', () => { + app = Fastify({ logger: false }); + + expect(() => + registerPermissionAuth(app, { validator: authorizes(), routes: {}, apiKeyHeader: [] }), + ).toThrow(/empty apiKeyHeader/); + }); + + it('throws synchronously when the authorization header is an API key header and a route accepts session tokens', () => { + app = Fastify({ logger: false }); + + expect(() => + registerPermissionAuth(app, { + validator: authorizes(), + sessionTokenVerifier: verifiesAs({ sub: 'key-1', brokers: ['tim'] }), + routes: { 'GET /session-scoped': { permissionName: PERMISSION, acceptsSessionToken: true } }, + apiKeyHeader: ['x-api-key', 'Authorization'], + }), + ).toThrow(/acceptsSessionToken/); + }); + }); + describe('deny by default', () => { it('rejects a route Fastify knows about but that has no entry in `routes` (403), without calling the validator', async () => { const validator = authorizes(); @@ -537,6 +609,26 @@ describe('registerPermissionAuth', () => { }); describe('assertHasPermission returned by registerPermissionAuth', () => { + it('reads the key from the configured apiKeyHeader list', async () => { + const validator = authorizes(['tim']); + app = buildTestApp({ + validator, + publicPaths: ['/', '/management/health', '/webhook'], + apiKeyHeader: ['x-api-key', 'Authorization'], + }); + + const res = await app.inject({ + method: 'POST', + url: '/webhook', + headers: { authorization: API_KEY }, + }); + + expect(res.statusCode).toBe(200); + expect(validator.calls).toEqual([ + { apiKey: API_KEY, permission: 'toutbox.delivery_order.webhook' }, + ]); + }); + it('authenticates and authorizes a permission outside the route map', async () => { const validator = authorizes(['tim']); app = buildTestApp({ validator, publicPaths: ['/', '/management/health', '/webhook'] }); @@ -842,6 +934,23 @@ describe('registerPermissionAuth', () => { expect((await injectWith('a-different-api-key')).statusCode).toBe(200); }); + it('buckets by a key read from any configured apiKeyHeader', async () => { + const validator = authorizes(); + app = buildTestApp({ + validator, + apiKeyHeader: ['x-api-key', 'authorization'], + rateLimit: { max: 1, timeWindow: '1 minute' }, + }); + await app.ready(); + + const injectWith = (apiKey: string): Promise<{ statusCode: number }> => + app.inject({ method: 'POST', url: '/protected', headers: { authorization: apiKey } }); + + expect((await injectWith(API_KEY)).statusCode).toBe(200); + expect((await injectWith(API_KEY)).statusCode).toBe(429); + expect((await injectWith('a-different-api-key')).statusCode).toBe(200); + }); + it('honours a caller-supplied keyGenerator over the credential-based default', async () => { const validator = authorizes(); app = buildTestApp({ diff --git a/src/auth-permission/fastify/register-permission-auth.ts b/src/auth-permission/fastify/register-permission-auth.ts index 440f053..7e88955 100644 --- a/src/auth-permission/fastify/register-permission-auth.ts +++ b/src/auth-permission/fastify/register-permission-auth.ts @@ -7,6 +7,7 @@ import { PermissionDeniedError } from '../errors/permission-denied-error'; import type { RouteProperties } from '../route-properties'; import { checkApiKeyPermission } from '../api-key/check-api-key-permission'; import { authenticateWithSessionToken } from '../session-token/authenticate-with-session-token'; +import { resolveApiKeyHeaders } from '../shared/api-key-headers'; import { readApiKey, readBearerToken } from '../shared/read-credentials'; import { credentialRateLimitKey } from './rate-limit/rate-limit-key'; import { createRateLimitCheck } from './rate-limit/rate-limit-check'; @@ -30,7 +31,12 @@ export interface PermissionAuthOptions { routes: Readonly>; /** Paths that bypass authentication, matched by exact value or as a prefix. */ publicPaths?: string[]; - apiKeyHeader?: string; + /** + * Header(s) the API key is read from, case-insensitively. With a list, the + * first one present wins. May only include `authorizationHeader` when no + * route sets `acceptsSessionToken`. + */ + apiKeyHeader?: string | readonly string[]; /** Required when any route in `routes` sets `acceptsSessionToken: true`. */ sessionTokenVerifier?: SessionTokenVerifier; authorizationHeader?: string; @@ -116,7 +122,6 @@ export function registerPermissionAuth( options: PermissionAuthOptions, ): AssertHasPermission { const publicPaths = options.publicPaths ?? PUBLIC_PATHS; - const apiKeyHeader = options.apiKeyHeader ?? API_KEY_HEADER; const authorizationHeader = options.authorizationHeader ?? AUTHORIZATION_HEADER; const brokerIdHeader = options.brokerIdHeader ?? BROKER_ID_HEADER; const brokerIdMaxLength = options.brokerIdMaxLength ?? BROKER_ID_MAX_LENGTH; @@ -129,6 +134,13 @@ export function registerPermissionAuth( } } + const apiKeyHeaders = resolveApiKeyHeaders( + options.apiKeyHeader ?? API_KEY_HEADER, + authorizationHeader, + options.routes, + 'registerPermissionAuth', + ); + // `global: false`: this hook does its own per-request check via // `createRateLimit` (see rate-limit-check.ts) instead of the plugin's // automatic per-route wiring, which doesn't reach routes a caller @@ -137,7 +149,7 @@ export function registerPermissionAuth( options.rateLimit === undefined ? undefined : createRateLimitCheck(app, { - keyGenerator: credentialRateLimitKey(apiKeyHeader, authorizationHeader), + keyGenerator: credentialRateLimitKey(apiKeyHeaders, authorizationHeader), ...options.rateLimit, }); if (checkRateLimit !== undefined) { @@ -146,7 +158,7 @@ export function registerPermissionAuth( const assertHasPermission: AssertHasPermission = async (request, route) => { const waveRequest = toWaveRequest(request); - const apiKey = readApiKey(waveRequest, apiKeyHeader); + const apiKey = readApiKey(waveRequest, apiKeyHeaders); if (apiKey !== undefined) { const ctx = await checkApiKeyPermission( waveRequest, @@ -201,7 +213,7 @@ export function registerPermissionAuth( const waveRequest = toWaveRequest(request); const hasCredential = - readApiKey(waveRequest, apiKeyHeader) !== undefined || + readApiKey(waveRequest, apiKeyHeaders) !== undefined || readBearerToken(waveRequest, authorizationHeader) !== undefined; if (!hasCredential) { throw new PermissionValidatorUnauthorizedError(); diff --git a/src/auth-permission/shared/api-key-headers.ts b/src/auth-permission/shared/api-key-headers.ts new file mode 100644 index 0000000..2357b08 --- /dev/null +++ b/src/auth-permission/shared/api-key-headers.ts @@ -0,0 +1,30 @@ +import type { RouteProperties } from '../route-properties'; + +export function resolveApiKeyHeaders( + apiKeyHeader: string | readonly string[], + authorizationHeader: string, + routes: Readonly>, + registrar: string, +): readonly string[] { + const configured = typeof apiKeyHeader === 'string' ? [apiKeyHeader] : apiKeyHeader; + // Node exposes `request.headers` with lowercased names, so a header + // configured as `'Authorization'` would otherwise never match. + const headers = [...new Set(configured.map((header) => header.toLowerCase()))]; + if (headers.length === 0) { + throw new Error(`${registrar} was given an empty apiKeyHeader list`); + } + + // A `Bearer ` would be read as an API key first and sent to the + // validator, so the session token path could never be reached. + if (headers.includes(authorizationHeader.toLowerCase())) { + for (const [routeKey, route] of Object.entries(routes)) { + if (route.acceptsSessionToken === true) { + throw new Error( + `${routeKey} sets acceptsSessionToken but ${registrar} reads the API key from the authorization header "${authorizationHeader}"`, + ); + } + } + } + + return headers; +} diff --git a/src/auth-permission/shared/read-credentials.ts b/src/auth-permission/shared/read-credentials.ts index a781f8d..6e24dcd 100644 --- a/src/auth-permission/shared/read-credentials.ts +++ b/src/auth-permission/shared/read-credentials.ts @@ -1,9 +1,17 @@ import { countRawHeader } from './raw-headers'; import type { WaveRequest } from './wave-request'; -export function readApiKey(request: WaveRequest, apiKeyHeader: string): string | undefined { - const header = request.headers[apiKeyHeader]; - return typeof header === 'string' && header.length > 0 ? header : undefined; +export function readApiKey( + request: WaveRequest, + apiKeyHeaders: readonly string[], +): string | undefined { + for (const apiKeyHeader of apiKeyHeaders) { + const header = request.headers[apiKeyHeader]; + if (typeof header === 'string' && header.length > 0) { + return header; + } + } + return undefined; } /** From 3bccf91a3d6174b206a616e688e046245ffc72ee Mon Sep 17 00:00:00 2001 From: Matheus Andrade Date: Wed, 7 Oct 2026 12:25:58 -0300 Subject: [PATCH 2/3] :sparkles: feat(auth-permission): accept the request when any configured API key header is valid Co-Authored-By: Claude Opus 5.5 --- .../api-key/check-api-key-permission.ts | 41 ++++++++++++++++ .../express-register-permission-auth.spec.ts | 44 +++++++++++++++++ .../express-register-permission-auth.ts | 18 +++---- .../fastify/rate-limit/rate-limit-key.ts | 12 ++--- .../fastify/register-permission-auth.spec.ts | 48 ++++++++++++++++++- .../fastify/register-permission-auth.ts | 18 +++---- .../shared/read-credentials.ts | 16 ++----- 7 files changed, 161 insertions(+), 36 deletions(-) diff --git a/src/auth-permission/api-key/check-api-key-permission.ts b/src/auth-permission/api-key/check-api-key-permission.ts index 9d9fd0d..5dfc99f 100644 --- a/src/auth-permission/api-key/check-api-key-permission.ts +++ b/src/auth-permission/api-key/check-api-key-permission.ts @@ -1,6 +1,7 @@ import type { BrokerContext } from '../broker-context'; import type { PermissionValidationResult, PermissionValidator } from './permission-validator'; import { PermissionDeniedError } from '../errors/permission-denied-error'; +import { PermissionValidatorUnauthorizedError } from '../errors/permission-validator-unauthorized-error'; import type { RouteProperties } from '../route-properties'; import { readBrokerId, resolveBrokerScope } from '../shared/broker-scope-resolution'; import type { WaveRequest } from '../shared/wave-request'; @@ -48,3 +49,43 @@ export async function checkApiKeyPermission( `The API key does not have the ${route.permissionName} permission`, ); } + +/** + * Tries each presented API key in order and resolves with the first one that + * passes. Only a rejection of the key itself moves on to the next one; any + * other failure (validator unavailable, malformed broker header) is thrown + * immediately. When every key is rejected, the first rejection is thrown. + */ +export async function checkAnyApiKeyPermission( + request: WaveRequest, + route: RouteProperties, + apiKeys: readonly string[], + validator: PermissionValidator, + brokerIdHeader: string, + brokerIdMaxLength: number, +): Promise { + let firstRejection: Error = new PermissionValidatorUnauthorizedError(); + for (const [index, apiKey] of apiKeys.entries()) { + try { + return await checkApiKeyPermission( + request, + route, + apiKey, + validator, + brokerIdHeader, + brokerIdMaxLength, + ); + } catch (error) { + const isKeyRejection = + error instanceof PermissionValidatorUnauthorizedError || + error instanceof PermissionDeniedError; + if (!isKeyRejection) { + throw error; + } + if (index === 0) { + firstRejection = error; + } + } + } + throw firstRejection; +} diff --git a/src/auth-permission/express/express-register-permission-auth.spec.ts b/src/auth-permission/express/express-register-permission-auth.spec.ts index 27fcc96..37ad5d4 100644 --- a/src/auth-permission/express/express-register-permission-auth.spec.ts +++ b/src/auth-permission/express/express-register-permission-auth.spec.ts @@ -194,6 +194,50 @@ describe('expressRegisterPermissionAuth', () => { expect(validator.calls.map((call) => call.apiKey)).toEqual([API_KEY, API_KEY]); }); + it('accepts the request when any presented key is valid', async () => { + const validator = new FakePermissionValidator((request) => + request.apiKey === API_KEY + ? { authorized: true, brokers: ['tim'] } + : new PermissionValidatorUnauthorizedError(), + ); + const app = buildTestApp({ + validator, + routes: { 'GET /balances': { permissionName: PERMISSION } }, + apiKeyHeader: ['x-api-key', 'authorization'], + }); + const listening = await listen(app); + server = listening.server; + + const res = await inject(listening.baseUrl, '/balances', { + [API_KEY_HEADER]: 'invalid-key', + authorization: API_KEY, + }); + + expect(res.statusCode).toBe(200); + expect(validator.calls.map((call) => call.apiKey)).toEqual(['invalid-key', API_KEY]); + }); + + it('rejects (401) only when no presented key is valid', async () => { + const validator = new FakePermissionValidator( + () => new PermissionValidatorUnauthorizedError(), + ); + const app = buildTestApp({ + validator, + routes: { 'GET /balances': { permissionName: PERMISSION } }, + apiKeyHeader: ['x-api-key', 'authorization'], + }); + const listening = await listen(app); + server = listening.server; + + const res = await inject(listening.baseUrl, '/balances', { + [API_KEY_HEADER]: 'invalid-key', + authorization: 'another-invalid-key', + }); + + expect(res.statusCode).toBe(401); + expect(validator.calls).toHaveLength(2); + }); + it('rejects a header outside the configured list (401)', async () => { const validator = authorizes(); const app = buildTestApp({ diff --git a/src/auth-permission/express/express-register-permission-auth.ts b/src/auth-permission/express/express-register-permission-auth.ts index 72778a8..0434b7c 100644 --- a/src/auth-permission/express/express-register-permission-auth.ts +++ b/src/auth-permission/express/express-register-permission-auth.ts @@ -4,10 +4,10 @@ import type { SessionTokenVerifier } from '../session-token/session-token-verifi import { PermissionValidatorUnauthorizedError } from '../errors/permission-validator-unauthorized-error'; import { PermissionDeniedError } from '../errors/permission-denied-error'; import type { RouteProperties } from '../route-properties'; -import { checkApiKeyPermission } from '../api-key/check-api-key-permission'; +import { checkAnyApiKeyPermission } from '../api-key/check-api-key-permission'; import { authenticateWithSessionToken } from '../session-token/authenticate-with-session-token'; import { resolveApiKeyHeaders } from '../shared/api-key-headers'; -import { readApiKey, readBearerToken } from '../shared/read-credentials'; +import { readApiKeys, readBearerToken } from '../shared/read-credentials'; import { isPublicPath } from '../shared/is-public-path'; import { compileRoutes, matchRoute } from './route-matcher'; import './express-request'; @@ -43,8 +43,8 @@ export interface ExpressPermissionAuthOptions { publicPaths?: string[]; /** * Header(s) the API key is read from, case-insensitively. With a list, the - * first one present wins. May only include `authorizationHeader` when no - * route sets `acceptsSessionToken`. + * request passes if any key presented is valid. May only include + * `authorizationHeader` when no route sets `acceptsSessionToken`. */ apiKeyHeader?: string | readonly string[]; /** Required when any route in `routes` sets `acceptsSessionToken: true`. */ @@ -116,12 +116,12 @@ export function expressRegisterPermissionAuth( const compiledRoutes = compileRoutes(options.routes); const assertHasPermission: ExpressAssertHasPermission = async (request, route) => { - const apiKey = readApiKey(request, apiKeyHeaders); - if (apiKey !== undefined) { - const ctx = await checkApiKeyPermission( + const apiKeys = readApiKeys(request, apiKeyHeaders); + if (apiKeys.length > 0) { + const ctx = await checkAnyApiKeyPermission( request, route, - apiKey, + apiKeys, options.validator, brokerIdHeader, brokerIdMaxLength, @@ -161,7 +161,7 @@ export function expressRegisterPermissionAuth( } const hasCredential = - readApiKey(request, apiKeyHeaders) !== undefined || + readApiKeys(request, apiKeyHeaders).length > 0 || readBearerToken(request, authorizationHeader) !== undefined; if (!hasCredential) { next(new PermissionValidatorUnauthorizedError()); diff --git a/src/auth-permission/fastify/rate-limit/rate-limit-key.ts b/src/auth-permission/fastify/rate-limit/rate-limit-key.ts index 8b73b3d..c5e6f17 100644 --- a/src/auth-permission/fastify/rate-limit/rate-limit-key.ts +++ b/src/auth-permission/fastify/rate-limit/rate-limit-key.ts @@ -1,5 +1,5 @@ import type { FastifyRequest } from 'fastify'; -import { readApiKey, readBearerToken } from '../../shared/read-credentials'; +import { readApiKeys, readBearerToken } from '../../shared/read-credentials'; import { toWaveRequest } from '../to-wave-request'; /** @@ -17,10 +17,10 @@ export function credentialRateLimitKey( ): (request: FastifyRequest) => string { return (request) => { const waveRequest = toWaveRequest(request); - return ( - readApiKey(waveRequest, apiKeyHeaders) ?? - readBearerToken(waveRequest, authorizationHeader) ?? - request.ip - ); + const apiKeys = readApiKeys(waveRequest, apiKeyHeaders); + if (apiKeys.length > 0) { + return apiKeys[0]; + } + return readBearerToken(waveRequest, authorizationHeader) ?? request.ip; }; } diff --git a/src/auth-permission/fastify/register-permission-auth.spec.ts b/src/auth-permission/fastify/register-permission-auth.spec.ts index d4a1f12..91da6fc 100644 --- a/src/auth-permission/fastify/register-permission-auth.spec.ts +++ b/src/auth-permission/fastify/register-permission-auth.spec.ts @@ -294,7 +294,25 @@ describe('registerPermissionAuth', () => { expect(validator.calls.map((call) => call.apiKey)).toEqual([API_KEY, API_KEY]); }); - it('prefers the earliest header in the list when several are present', async () => { + it('accepts the request when any presented key is valid', async () => { + const validator = new FakePermissionValidator((request) => + request.apiKey === API_KEY + ? { authorized: true, brokers: ['tim'] } + : new PermissionValidatorUnauthorizedError(), + ); + app = buildTestApp({ validator, apiKeyHeader: ['x-api-key', 'authorization'] }); + + const res = await app.inject({ + method: 'POST', + url: '/protected', + headers: { [API_KEY_HEADER]: 'invalid-key', authorization: API_KEY }, + }); + + expect(res.statusCode).toBe(200); + expect(validator.calls.map((call) => call.apiKey)).toEqual(['invalid-key', API_KEY]); + }); + + it('stops at the first valid key', async () => { const validator = authorizes(); app = buildTestApp({ validator, apiKeyHeader: ['authorization', 'x-api-key'] }); @@ -307,6 +325,34 @@ describe('registerPermissionAuth', () => { expect(validator.calls.map((call) => call.apiKey)).toEqual([API_KEY]); }); + it('rejects (401) only when no presented key is valid', async () => { + const validator = throwing(new PermissionValidatorUnauthorizedError()); + app = buildTestApp({ validator, apiKeyHeader: ['x-api-key', 'authorization'] }); + + const res = await app.inject({ + method: 'POST', + url: '/protected', + headers: { [API_KEY_HEADER]: 'invalid-key', authorization: 'another-invalid-key' }, + }); + + expect(res.statusCode).toBe(401); + expect(validator.calls).toHaveLength(2); + }); + + it('fails closed (503) when the validator is unavailable, without trying the next key', async () => { + const validator = throwing(new PermissionValidatorUnavailableError('timeout')); + app = buildTestApp({ validator, apiKeyHeader: ['x-api-key', 'authorization'] }); + + const res = await app.inject({ + method: 'POST', + url: '/protected', + headers: { [API_KEY_HEADER]: API_KEY, authorization: 'another-key' }, + }); + + expect(res.statusCode).toBe(503); + expect(validator.calls).toHaveLength(1); + }); + it('rejects a header outside the configured list (401)', async () => { const validator = authorizes(); app = buildTestApp({ validator, apiKeyHeader: 'Authorization' }); diff --git a/src/auth-permission/fastify/register-permission-auth.ts b/src/auth-permission/fastify/register-permission-auth.ts index 7e88955..aad58fb 100644 --- a/src/auth-permission/fastify/register-permission-auth.ts +++ b/src/auth-permission/fastify/register-permission-auth.ts @@ -5,10 +5,10 @@ import type { SessionTokenVerifier } from '../session-token/session-token-verifi import { PermissionValidatorUnauthorizedError } from '../errors/permission-validator-unauthorized-error'; import { PermissionDeniedError } from '../errors/permission-denied-error'; import type { RouteProperties } from '../route-properties'; -import { checkApiKeyPermission } from '../api-key/check-api-key-permission'; +import { checkAnyApiKeyPermission } from '../api-key/check-api-key-permission'; import { authenticateWithSessionToken } from '../session-token/authenticate-with-session-token'; import { resolveApiKeyHeaders } from '../shared/api-key-headers'; -import { readApiKey, readBearerToken } from '../shared/read-credentials'; +import { readApiKeys, readBearerToken } from '../shared/read-credentials'; import { credentialRateLimitKey } from './rate-limit/rate-limit-key'; import { createRateLimitCheck } from './rate-limit/rate-limit-check'; import { toWaveRequest } from './to-wave-request'; @@ -33,8 +33,8 @@ export interface PermissionAuthOptions { publicPaths?: string[]; /** * Header(s) the API key is read from, case-insensitively. With a list, the - * first one present wins. May only include `authorizationHeader` when no - * route sets `acceptsSessionToken`. + * request passes if any key presented is valid. May only include + * `authorizationHeader` when no route sets `acceptsSessionToken`. */ apiKeyHeader?: string | readonly string[]; /** Required when any route in `routes` sets `acceptsSessionToken: true`. */ @@ -158,12 +158,12 @@ export function registerPermissionAuth( const assertHasPermission: AssertHasPermission = async (request, route) => { const waveRequest = toWaveRequest(request); - const apiKey = readApiKey(waveRequest, apiKeyHeaders); - if (apiKey !== undefined) { - const ctx = await checkApiKeyPermission( + const apiKeys = readApiKeys(waveRequest, apiKeyHeaders); + if (apiKeys.length > 0) { + const ctx = await checkAnyApiKeyPermission( waveRequest, route, - apiKey, + apiKeys, options.validator, brokerIdHeader, brokerIdMaxLength, @@ -213,7 +213,7 @@ export function registerPermissionAuth( const waveRequest = toWaveRequest(request); const hasCredential = - readApiKey(waveRequest, apiKeyHeaders) !== undefined || + readApiKeys(waveRequest, apiKeyHeaders).length > 0 || readBearerToken(waveRequest, authorizationHeader) !== undefined; if (!hasCredential) { throw new PermissionValidatorUnauthorizedError(); diff --git a/src/auth-permission/shared/read-credentials.ts b/src/auth-permission/shared/read-credentials.ts index 6e24dcd..18d9876 100644 --- a/src/auth-permission/shared/read-credentials.ts +++ b/src/auth-permission/shared/read-credentials.ts @@ -1,17 +1,11 @@ import { countRawHeader } from './raw-headers'; import type { WaveRequest } from './wave-request'; -export function readApiKey( - request: WaveRequest, - apiKeyHeaders: readonly string[], -): string | undefined { - for (const apiKeyHeader of apiKeyHeaders) { - const header = request.headers[apiKeyHeader]; - if (typeof header === 'string' && header.length > 0) { - return header; - } - } - return undefined; +export function readApiKeys(request: WaveRequest, apiKeyHeaders: readonly string[]): string[] { + const apiKeys = apiKeyHeaders + .map((apiKeyHeader) => request.headers[apiKeyHeader]) + .filter((header): header is string => typeof header === 'string' && header.length > 0); + return [...new Set(apiKeys)]; } /** From 28e252db025b40ef2cc74407ca23002db8480937 Mon Sep 17 00:00:00 2001 From: Matheus Andrade Date: Wed, 7 Oct 2026 13:10:09 -0300 Subject: [PATCH 3/3] :lock: fix(auth-permission): rate-limit every presented API key and keep trying keys on per-key broker errors Co-Authored-By: Claude Opus 5.5 --- .../api-key/check-api-key-permission.ts | 23 +++--- .../express-register-permission-auth.spec.ts | 26 ++++++ .../express-register-permission-auth.ts | 3 +- .../fastify/rate-limit/rate-limit-check.ts | 21 ++++- .../fastify/rate-limit/rate-limit-key.ts | 12 +-- .../fastify/register-permission-auth.spec.ts | 81 +++++++++++++++++++ .../fastify/register-permission-auth.ts | 16 ++-- 7 files changed, 155 insertions(+), 27 deletions(-) diff --git a/src/auth-permission/api-key/check-api-key-permission.ts b/src/auth-permission/api-key/check-api-key-permission.ts index 5dfc99f..198d99e 100644 --- a/src/auth-permission/api-key/check-api-key-permission.ts +++ b/src/auth-permission/api-key/check-api-key-permission.ts @@ -1,5 +1,6 @@ import type { BrokerContext } from '../broker-context'; import type { PermissionValidationResult, PermissionValidator } from './permission-validator'; +import { AmbiguousBrokerTargetError } from '../errors/ambiguous-broker-target-error'; import { PermissionDeniedError } from '../errors/permission-denied-error'; import { PermissionValidatorUnauthorizedError } from '../errors/permission-validator-unauthorized-error'; import type { RouteProperties } from '../route-properties'; @@ -50,12 +51,7 @@ export async function checkApiKeyPermission( ); } -/** - * Tries each presented API key in order and resolves with the first one that - * passes. Only a rejection of the key itself moves on to the next one; any - * other failure (validator unavailable, malformed broker header) is thrown - * immediately. When every key is rejected, the first rejection is thrown. - */ +/** Resolves with the first key that passes; rejects only when every key is rejected. */ export async function checkAnyApiKeyPermission( request: WaveRequest, route: RouteProperties, @@ -64,8 +60,8 @@ export async function checkAnyApiKeyPermission( brokerIdHeader: string, brokerIdMaxLength: number, ): Promise { - let firstRejection: Error = new PermissionValidatorUnauthorizedError(); - for (const [index, apiKey] of apiKeys.entries()) { + let rejection: Error | undefined; + for (const apiKey of apiKeys) { try { return await checkApiKeyPermission( request, @@ -78,14 +74,17 @@ export async function checkAnyApiKeyPermission( } catch (error) { const isKeyRejection = error instanceof PermissionValidatorUnauthorizedError || - error instanceof PermissionDeniedError; + error instanceof PermissionDeniedError || + error instanceof AmbiguousBrokerTargetError; if (!isKeyRejection) { throw error; } - if (index === 0) { - firstRejection = error; + // A key that is genuine but lacks the permission explains the failure + // better than one that is simply unknown. + if (rejection === undefined || rejection instanceof PermissionValidatorUnauthorizedError) { + rejection = error; } } } - throw firstRejection; + throw rejection ?? new PermissionValidatorUnauthorizedError(); } diff --git a/src/auth-permission/express/express-register-permission-auth.spec.ts b/src/auth-permission/express/express-register-permission-auth.spec.ts index 37ad5d4..465d1e6 100644 --- a/src/auth-permission/express/express-register-permission-auth.spec.ts +++ b/src/auth-permission/express/express-register-permission-auth.spec.ts @@ -11,6 +11,7 @@ import { PermissionValidatorUnauthorizedError } from '../errors/permission-valid import { PermissionDeniedError } from '../errors/permission-denied-error'; import { AmbiguousBrokerTargetError } from '../errors/ambiguous-broker-target-error'; import { SessionTokenInvalidError } from '../errors/session-token-invalid-error'; +import { PermissionValidatorUnavailableError } from '../errors/permission-validator-unavailable-error'; import type { PermissionValidationRequest, PermissionValidationResult, @@ -77,6 +78,10 @@ function testErrorHandler(error: Error, req: Request, res: Response, next: NextF res.status(401).json({ type: 'session-token-invalid', message: error.message }); return; } + if (error instanceof PermissionValidatorUnavailableError) { + res.status(503).json({ type: 'unavailable', message: error.message }); + return; + } res.status(500).json({ type: 'unexpected', message: error.message }); } @@ -238,6 +243,27 @@ describe('expressRegisterPermissionAuth', () => { expect(validator.calls).toHaveLength(2); }); + it('fails closed (503) when the validator is unavailable, without trying the next key', async () => { + const validator = new FakePermissionValidator( + () => new PermissionValidatorUnavailableError('timeout'), + ); + const app = buildTestApp({ + validator, + routes: { 'GET /balances': { permissionName: PERMISSION } }, + apiKeyHeader: ['x-api-key', 'authorization'], + }); + const listening = await listen(app); + server = listening.server; + + const res = await inject(listening.baseUrl, '/balances', { + [API_KEY_HEADER]: API_KEY, + authorization: 'another-key', + }); + + expect(res.statusCode).toBe(503); + expect(validator.calls).toHaveLength(1); + }); + it('rejects a header outside the configured list (401)', async () => { const validator = authorizes(); const app = buildTestApp({ diff --git a/src/auth-permission/express/express-register-permission-auth.ts b/src/auth-permission/express/express-register-permission-auth.ts index 0434b7c..5a59f56 100644 --- a/src/auth-permission/express/express-register-permission-auth.ts +++ b/src/auth-permission/express/express-register-permission-auth.ts @@ -43,7 +43,8 @@ export interface ExpressPermissionAuthOptions { publicPaths?: string[]; /** * Header(s) the API key is read from, case-insensitively. With a list, the - * request passes if any key presented is valid. May only include + * request passes if any key presented is valid, at the cost of one + * validator call per distinct key tried. May only include * `authorizationHeader` when no route sets `acceptsSessionToken`. */ apiKeyHeader?: string | readonly string[]; diff --git a/src/auth-permission/fastify/rate-limit/rate-limit-check.ts b/src/auth-permission/fastify/rate-limit/rate-limit-check.ts index 362390e..c523e2e 100644 --- a/src/auth-permission/fastify/rate-limit/rate-limit-check.ts +++ b/src/auth-permission/fastify/rate-limit/rate-limit-check.ts @@ -25,12 +25,17 @@ export type RateLimitCheck = (request: FastifyRequest) => Promise; export function createRateLimitCheck( app: FastifyInstance, options: CreateRateLimitOptions, + credentialKeys: (request: FastifyRequest) => readonly string[], ): RateLimitCheck { + const currentKey = new WeakMap(); let checkRateLimit: ReturnType | undefined; - return async (request) => { + const checkKey = async (request: FastifyRequest): Promise => { if (checkRateLimit === undefined) { - checkRateLimit = app.createRateLimit(options); + checkRateLimit = app.createRateLimit({ + keyGenerator: (keyed) => currentKey.get(keyed) ?? keyed.ip, + ...options, + }); } const verdict = await checkRateLimit(request); @@ -38,4 +43,16 @@ export function createRateLimitCheck( throw new TooManyRequestsError('Rate limit exceeded for this credential'); } }; + + return async (request) => { + if (options.keyGenerator !== undefined) { + await checkKey(request); + return; + } + + for (const key of credentialKeys(request)) { + currentKey.set(request, key); + await checkKey(request); + } + }; } diff --git a/src/auth-permission/fastify/rate-limit/rate-limit-key.ts b/src/auth-permission/fastify/rate-limit/rate-limit-key.ts index c5e6f17..43d99b5 100644 --- a/src/auth-permission/fastify/rate-limit/rate-limit-key.ts +++ b/src/auth-permission/fastify/rate-limit/rate-limit-key.ts @@ -9,18 +9,20 @@ import { toWaveRequest } from '../to-wave-request'; * the same NAT/gateway into one bucket and let a single leaked credential * roam free across IPs; grouping by credential does the opposite of both. * Falls back to `request.ip` only when no credential was presented at all - * (still useful to cap unauthenticated request floods). + * (still useful to cap unauthenticated request floods). Every API key + * presented gets its own bucket checked, since any one of them may be the + * one that authenticates. */ -export function credentialRateLimitKey( +export function credentialRateLimitKeys( apiKeyHeaders: readonly string[], authorizationHeader: string, -): (request: FastifyRequest) => string { +): (request: FastifyRequest) => string[] { return (request) => { const waveRequest = toWaveRequest(request); const apiKeys = readApiKeys(waveRequest, apiKeyHeaders); if (apiKeys.length > 0) { - return apiKeys[0]; + return apiKeys; } - return readBearerToken(waveRequest, authorizationHeader) ?? request.ip; + return [readBearerToken(waveRequest, authorizationHeader) ?? request.ip]; }; } diff --git a/src/auth-permission/fastify/register-permission-auth.spec.ts b/src/auth-permission/fastify/register-permission-auth.spec.ts index 91da6fc..9c0b339 100644 --- a/src/auth-permission/fastify/register-permission-auth.spec.ts +++ b/src/auth-permission/fastify/register-permission-auth.spec.ts @@ -353,6 +353,67 @@ describe('registerPermissionAuth', () => { expect(validator.calls).toHaveLength(1); }); + it('moves on to the next key when one resolves an ambiguous broker scope', async () => { + const validator = new FakePermissionValidator((request) => ({ + authorized: true, + brokers: request.apiKey === API_KEY ? ['tim'] : ['tim', 'other-broker'], + })); + app = buildTestApp({ validator, apiKeyHeader: ['x-api-key', 'authorization'] }); + + const res = await app.inject({ + method: 'POST', + url: '/protected', + headers: { [API_KEY_HEADER]: 'multi-broker-key', authorization: API_KEY }, + }); + + expect(res.statusCode).toBe(200); + expect(validator.calls).toHaveLength(2); + }); + + it('reports a denied permission (403) over an unknown key (401) when every key fails', async () => { + const validator = new FakePermissionValidator((request) => + request.apiKey === API_KEY + ? { authorized: false, brokers: [] } + : new PermissionValidatorUnauthorizedError(), + ); + app = buildTestApp({ validator, apiKeyHeader: ['x-api-key', 'authorization'] }); + + const res = await app.inject({ + method: 'POST', + url: '/protected', + headers: { [API_KEY_HEADER]: 'unknown-key', authorization: API_KEY }, + }); + + expect(res.statusCode).toBe(403); + }); + + it('validates a key repeated across headers only once', async () => { + const validator = authorizes(); + app = buildTestApp({ validator, apiKeyHeader: ['x-api-key', 'authorization'] }); + + await app.inject({ + method: 'POST', + url: '/protected', + headers: { [API_KEY_HEADER]: API_KEY, authorization: API_KEY }, + }); + + expect(validator.calls).toHaveLength(1); + }); + + it('rejects a malformed x-broker-id (400) without validating any key', async () => { + const validator = authorizes(); + app = buildTestApp({ validator, apiKeyHeader: ['x-api-key', 'authorization'] }); + + const res = await app.inject({ + method: 'POST', + url: '/protected', + headers: { [API_KEY_HEADER]: API_KEY, authorization: 'another-key', [BROKER_ID_HEADER]: '' }, + }); + + expect(res.statusCode).toBe(400); + expect(validator.calls).toHaveLength(0); + }); + it('rejects a header outside the configured list (401)', async () => { const validator = authorizes(); app = buildTestApp({ validator, apiKeyHeader: 'Authorization' }); @@ -997,6 +1058,26 @@ describe('registerPermissionAuth', () => { expect((await injectWith('a-different-api-key')).statusCode).toBe(200); }); + it('checks the budget of every presented key, so a random extra key cannot dodge it', async () => { + const validator = authorizes(); + app = buildTestApp({ + validator, + apiKeyHeader: ['x-api-key', 'authorization'], + rateLimit: { max: 1, timeWindow: '1 minute' }, + }); + await app.ready(); + + const injectWith = (decoy: string): Promise<{ statusCode: number }> => + app.inject({ + method: 'POST', + url: '/protected', + headers: { [API_KEY_HEADER]: decoy, authorization: API_KEY }, + }); + + expect((await injectWith('decoy-1')).statusCode).toBe(200); + expect((await injectWith('decoy-2')).statusCode).toBe(429); + }); + it('honours a caller-supplied keyGenerator over the credential-based default', async () => { const validator = authorizes(); app = buildTestApp({ diff --git a/src/auth-permission/fastify/register-permission-auth.ts b/src/auth-permission/fastify/register-permission-auth.ts index aad58fb..141b2b3 100644 --- a/src/auth-permission/fastify/register-permission-auth.ts +++ b/src/auth-permission/fastify/register-permission-auth.ts @@ -9,7 +9,7 @@ import { checkAnyApiKeyPermission } from '../api-key/check-api-key-permission'; import { authenticateWithSessionToken } from '../session-token/authenticate-with-session-token'; import { resolveApiKeyHeaders } from '../shared/api-key-headers'; import { readApiKeys, readBearerToken } from '../shared/read-credentials'; -import { credentialRateLimitKey } from './rate-limit/rate-limit-key'; +import { credentialRateLimitKeys } from './rate-limit/rate-limit-key'; import { createRateLimitCheck } from './rate-limit/rate-limit-check'; import { toWaveRequest } from './to-wave-request'; import { isPublicPath } from '../shared/is-public-path'; @@ -33,7 +33,8 @@ export interface PermissionAuthOptions { publicPaths?: string[]; /** * Header(s) the API key is read from, case-insensitively. With a list, the - * request passes if any key presented is valid. May only include + * request passes if any key presented is valid, at the cost of one + * validator call per distinct key tried. May only include * `authorizationHeader` when no route sets `acceptsSessionToken`. */ apiKeyHeader?: string | readonly string[]; @@ -46,7 +47,7 @@ export interface PermissionAuthOptions { * Opts every route this hook protects into rate limiting (backed by * `@fastify/rate-limit`'s `createRateLimit`, see `rate-limit-check.ts`), * bucketed by the presented credential rather than `request.ip` by - * default (see `credentialRateLimitKey`) — pass a `keyGenerator` to + * default (see `credentialRateLimitKeys`) — pass a `keyGenerator` to * override that. A request over budget throws `TooManyRequestsError` * (`429`). Omitted entirely by default: this hook runs inside a shared * framework used by many independently deployed services with very @@ -148,10 +149,11 @@ export function registerPermissionAuth( const checkRateLimit = options.rateLimit === undefined ? undefined - : createRateLimitCheck(app, { - keyGenerator: credentialRateLimitKey(apiKeyHeaders, authorizationHeader), - ...options.rateLimit, - }); + : createRateLimitCheck( + app, + options.rateLimit, + credentialRateLimitKeys(apiKeyHeaders, authorizationHeader), + ); if (checkRateLimit !== undefined) { void app.register(fastifyRateLimit, { global: false }); }