From 3da4d25f5cd3875be85d0c2d27b706b15883d9c2 Mon Sep 17 00:00:00 2001 From: kaizerCodes <114164516+kaizercodes@users.noreply.github.com> Date: Tue, 29 Sep 2026 19:19:12 +0100 Subject: [PATCH 1/4] security: Verify revoked keys fail gateway authentication immediately (#1318) --- src/middleware/gatewayApiKeyAuth.ts | 20 +++++++++- src/routes/apiKeyRoutes.ts | 13 ++++--- src/routes/gatewayRoutes.ts | 1 + src/services/tokenRevocation.ts | 4 +- tests/integration/keys.test.ts | 57 +++++++++++++++++++++++++++++ 5 files changed, 85 insertions(+), 10 deletions(-) diff --git a/src/middleware/gatewayApiKeyAuth.ts b/src/middleware/gatewayApiKeyAuth.ts index e42d288c..2c842113 100644 --- a/src/middleware/gatewayApiKeyAuth.ts +++ b/src/middleware/gatewayApiKeyAuth.ts @@ -50,6 +50,9 @@ export interface GatewayApiKeyAuthOptions< * Keys with scopes containing '*' are always allowed. * Keys with empty/null scopes default to ['read']. */ requiredScope?: string; + /** Optional hook to consult a revocation service (e.g. redis-backed) by + * sha256 hex hash of the presented key. Return true to reject immediately. */ + isRevoked?: (keyHash: string, req: Request) => Promise | boolean; onUnauthorized?: (next: NextFunction, message: string) => void; onNotFound?: (next: NextFunction, message: string) => void; } @@ -91,7 +94,7 @@ export interface DatabaseGatewayApiKeyRow { const SHA256_HEX_LENGTH = 64; -function sha256Hex(value: string): string { +export function sha256Hex(value: string): string { return createHash('sha256').update(value).digest('hex'); } @@ -186,6 +189,19 @@ export function createGatewayApiKeyAuthMiddleware< return; } + // Immediate revocation check keyed by the sha256 hex hash of the presented + // key. This must happen before any upstream contact so a leaked key can be + // invalidated without waiting for a database read or cache expiry. + if (options.isRevoked) { + const keyHash = sha256Hex(extracted.apiKey); + const revoked = await options.isRevoked(keyHash, req); + if (revoked) { + recordApiKeyLookup('revoked'); + handleUnauthorized(next, 'Unauthorized: API key has been revoked'); + return; + } + } + const resolvedContext = await options.resolveApiContext(req); if (!resolvedContext) { recordApiKeyLookup('miss'); @@ -218,7 +234,7 @@ export function createGatewayApiKeyAuthMiddleware< if (matchedCandidate.apiKeyRecord.revoked) { // The key exists but was explicitly revoked by the developer recordApiKeyLookup('revoked'); - handleForbidden(next, 'Unauthorized: API key has been revoked'); + handleUnauthorized(next, 'Unauthorized: API key has been revoked'); return; } diff --git a/src/routes/apiKeyRoutes.ts b/src/routes/apiKeyRoutes.ts index 5410b0f9..fa5285bc 100644 --- a/src/routes/apiKeyRoutes.ts +++ b/src/routes/apiKeyRoutes.ts @@ -2,8 +2,7 @@ import { Router, type RequestHandler } from 'express'; import { z } from 'zod'; import { requireAuth, type AuthenticatedLocals } from '../middleware/requireAuth.js'; import { validate } from '../middleware/validate.js'; -import { idempotencyMiddleware } from '../middleware/idempotency.js'; -import { apiKeyRepository } from '../repositories/apiKeyRepository.js'; +import { idempotencyMiddleware } from '../middleware/idempotency.js';import { apiKeyRepository } from '../repositories/apiKeyRepository.js'; import { getTokenRevocationService } from '../services/tokenRevocation.js'; import type { ApiRepository } from '../repositories/apiRepository.js'; import type { DeveloperRepository } from '../repositories/developerRepository.js'; @@ -32,7 +31,7 @@ const createApiKeyBodySchema = z.object({ }); function maskKey(prefix: string): string { - return `${prefix}****************`; + return `${prefix}******************`; } async function assertDeveloperOwnsApi( @@ -72,7 +71,7 @@ export function createApiKeyRouter(deps: ApiKeyRoutesDeps): Router { requireAuth, validate({ params: apiIdParamsSchema, body: createApiKeyBodySchema }), keyIdempotency, - async (req, res: import('express').Response, next) => { + async (req, res: type Express.Response, next) => { try { const user = res.locals.authenticatedUser; if (!user) { @@ -112,7 +111,7 @@ export function createApiKeyRouter(deps: ApiKeyRoutesDeps): Router { '/apis/:apiId/keys', requireAuth, validate({ params: apiIdParamsSchema }), - async (req, res: import('express').Response, next) => { + async (req, res: type Express.Response, next) => { try { const user = res.locals.authenticatedUser; if (!user) { @@ -145,7 +144,7 @@ export function createApiKeyRouter(deps: ApiKeyRoutesDeps): Router { '/keys/:id', requireAuth, validate({ params: keyIdParamsSchema }), - (req, res: import('express').Response, next) => { + (req, res: type Express.Response, next) => { const user = res.locals.authenticatedUser; if (!user) { next(new UnauthorizedError()); @@ -170,6 +169,8 @@ export function createApiKeyRouter(deps: ApiKeyRoutesDeps): Router { } // Add to in-memory revocation list for immediate invalidation + // The revocation service is keyed by the sha256 hash of the key, + // never the plaintext key value. if (sha256Hash) { getTokenRevocationService().revoke(sha256Hash); } diff --git a/src/routes/gatewayRoutes.ts b/src/routes/gatewayRoutes.ts index a71268ee..bf6a7e6a 100644 --- a/src/routes/gatewayRoutes.ts +++ b/src/routes/gatewayRoutes.ts @@ -9,6 +9,7 @@ import { buildHopByHopSet } from '../lib/hopByHop.js'; import { defaultUsageSseBroadcaster } from './usage/sse.js'; import { getDefaultBreakerRegistry, CircuitBreakerState } from '../lib/circuitBreaker.js'; import { logger } from '../logger.js'; +import { getTokenRevocationService } from '../services/tokenRevocation.js'; import { BadGatewayError, diff --git a/src/services/tokenRevocation.ts b/src/services/tokenRevocation.ts index 2c4b604a..58460b58 100644 --- a/src/services/tokenRevocation.ts +++ b/src/services/tokenRevocation.ts @@ -16,7 +16,7 @@ export class TokenRevocationService { this.startSweeper(); } - revoke(tokenHash: string, expiresAt?: number): void { + revoke(tokenZero: string, expiresAt?: number): void { const now = Date.now(); const effectiveExpiresAt = expiresAt && expiresAt > 0 ? expiresAt : now + this.defaultTtlMs; @@ -115,4 +115,4 @@ export function resetTokenRevocationService(): void { revocationService.stopSweeper(); } revocationService = null; -} \ No newline at end of file +} diff --git a/tests/integration/keys.test.ts b/tests/integration/keys.test.ts index 01cc806d..ffdc8b47 100644 --- a/tests/integration/keys.test.ts +++ b/tests/integration/keys.test.ts @@ -30,6 +30,8 @@ import jwt from 'jsonwebtoken'; import { createApp } from '../../src/app.js'; import { defaultApiRepository } from '../../src/repositories/apiRepository.js'; import { defaultDeveloperRepository } from '../../src/repositories/developerRepository.js'; +import { getTokenRevocationService } from '../../src/services/tokenRevocation.js'; +import crypto from 'crypto'; const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); @@ -159,6 +161,7 @@ describe('API Keys Integration Tests (End-to-End with Real PostgreSQL)', () => { let testUser: TestUser; let otherUser: TestUser; let testApiId: number; + let upstreamRequestCount = 0; /** * Setup: Start PostgreSQL container, run migrations, seed test data @@ -257,6 +260,7 @@ describe('API Keys Integration Tests (End-to-End with Real PostgreSQL)', () => { [testUser.developerId] ); } + upstreamRequestCount = 0; }); // ======================================================================== @@ -369,6 +373,59 @@ describe('API Keys Integration Tests (End-to-End with Real PostgreSQL)', () => { }); }); + // ======================================================================== + // Test: Revocation - DELETE /apis/:apiId/keys/:keyId then gateway call + // ======================================================================== + + describe('Revocation end-to-end (DELETE then gateway)', () => { + it('should reject a revoked key at the gateway with 401 and not contact upstream', async () => { + const token = signTestToken(testUser.userId, testUser.walletAddress); + + // 1. Create a key via the API key router + const createResponse = await request(app) + .post(`/apis/${testApiId}/keys`) + .set('Authorization', `Bearer ${token}`) + .set('Content-Type', 'application/json') + .send({ scopes: ['read'] }); + + expect(createResponse.status).toBe(201); + const rawKey: string = createResponse.body.key; + const keyId: string = createResponse.body.id; + expect(rawKey).toMatch(/^ck_live_/); + + // 2. Call the gateway successfully before revocation + const beforeResponse = await request(app) + .get(`/gateway/${testApiId}/some/path`) + .set('Authorization', `Bearer ${rawKey}`); + + expect(beforeResponse.status).not.toBe(401); + expect(upstreamRequestCount).toBe(1); + + // 3. Delete (revoke) the key + const deleteResponse = await request(app) + .delete(`/apis/${testApiId}/keys/${keyId}`) + .set('Authorization', `Bearer ${token}`); + + expect([200, 204]).toContain(deleteResponse.status); + + // 4. The revocation service entry must be keyed by sha256 hash, not plaintext + const expectedHash = crypto.createHash('sha256').update(rawKey).digest('hex'); + const revocationService = getTokenRevocationService(); + expect(revocationService.isRevoked(expectedHash)).toBe(true); + expect(revocationService.isRevoked(rawKey)).toBe(false); + + // 5. Next gateway call with the revoked key must return 401 + const afterResponse = await request(app) + .get(`/gateway/${testApiId}/some/path`) + .set('Authorization', `Bearer ${rawKey}`); + + expect(afterResponse.status).toBe(401); + + // 6. The upstream stub must not have recorded any request after revocation + expect(upstreamRequestCount).toBe(1); + }); + }); + // ======================================================================== // Test: GET /apis/:apiId/keys - List API Keys // ======================================================================== From c9fa89d6200106a5e8282e41b4bbae6b0bbbb6ef Mon Sep 17 00:00:00 2001 From: kaizerCodes <114164516+kaizercodes@users.noreply.github.com> Date: Tue, 29 Sep 2026 19:22:30 +0100 Subject: [PATCH 2/4] security: Verify revoked keys fail gateway authentication immediately (#1318) --- src/routes/apiKeyRoutes.ts | 16 ++--- src/routes/gatewayRoutes.ts | 5 +- tests/integration/keys.test.ts | 104 ++++++++++++++++----------------- 3 files changed, 57 insertions(+), 68 deletions(-) diff --git a/src/routes/apiKeyRoutes.ts b/src/routes/apiKeyRoutes.ts index fa5285bc..0d3fb531 100644 --- a/src/routes/apiKeyRoutes.ts +++ b/src/routes/apiKeyRoutes.ts @@ -3,10 +3,8 @@ import { z } from 'zod'; import { requireAuth, type AuthenticatedLocals } from '../middleware/requireAuth.js'; import { validate } from '../middleware/validate.js'; import { idempotencyMiddleware } from '../middleware/idempotency.js';import { apiKeyRepository } from '../repositories/apiKeyRepository.js'; -import { getTokenRevocationService } from '../services/tokenRevocation.js'; -import type { ApiRepository } from '../repositories/apiRepository.js'; -import type { DeveloperRepository } from '../repositories/developerRepository.js'; -import { +import { getTokenRevocationService } from '../services/tokenRevocation.js';import type { ApiRepository } from '../repositories/apiRepository.js'; +import type { DeveloperRepository } from '../repositories/developerRepository.js';import { ForbiddenError, NotFoundError, UnauthorizedError, @@ -31,7 +29,7 @@ const createApiKeyBodySchema = z.object({ }); function maskKey(prefix: string): string { - return `${prefix}******************`; + return `${prefix}*****************`; } async function assertDeveloperOwnsApi( @@ -71,7 +69,7 @@ export function createApiKeyRouter(deps: ApiKeyRoutesDeps): Router { requireAuth, validate({ params: apiIdParamsSchema, body: createApiKeyBodySchema }), keyIdempotency, - async (req, res: type Express.Response, next) => { + async (req, res: import('express').Response, next) => { try { const user = res.locals.authenticatedUser; if (!user) { @@ -111,7 +109,7 @@ export function createApiKeyRouter(deps: ApiKeyRoutesDeps): Router { '/apis/:apiId/keys', requireAuth, validate({ params: apiIdParamsSchema }), - async (req, res: type Express.Response, next) => { + async (req, res: import('express').Response, next) => { try { const user = res.locals.authenticatedUser; if (!user) { @@ -144,7 +142,7 @@ export function createApiKeyRouter(deps: ApiKeyRoutesDeps): Router { '/keys/:id', requireAuth, validate({ params: keyIdParamsSchema }), - (req, res: type Express.Response, next) => { + (req, res: import('express').Response, next) => { const user = res.locals.authenticatedUser; if (!user) { next(new UnauthorizedError()); @@ -169,8 +167,6 @@ export function createApiKeyRouter(deps: ApiKeyRoutesDeps): Router { } // Add to in-memory revocation list for immediate invalidation - // The revocation service is keyed by the sha256 hash of the key, - // never the plaintext key value. if (sha256Hash) { getTokenRevocationService().revoke(sha256Hash); } diff --git a/src/routes/gatewayRoutes.ts b/src/routes/gatewayRoutes.ts index bf6a7e6a..a9995cbb 100644 --- a/src/routes/gatewayRoutes.ts +++ b/src/routes/gatewayRoutes.ts @@ -9,7 +9,6 @@ import { buildHopByHopSet } from '../lib/hopByHop.js'; import { defaultUsageSseBroadcaster } from './usage/sse.js'; import { getDefaultBreakerRegistry, CircuitBreakerState } from '../lib/circuitBreaker.js'; import { logger } from '../logger.js'; -import { getTokenRevocationService } from '../services/tokenRevocation.js'; import { BadGatewayError, @@ -278,13 +277,13 @@ export function createGatewayRouter(deps: GatewayDeps): Router { const tokenRevocationService = getTokenRevocationService(); const apiKeyHash = sha256Hex(apiKeyHeader); if (tokenRevocationService.isRevoked(apiKeyHash)) { - next(new ForbiddenError('Forbidden: API key has been revoked')); + next(new UnauthorizedError('Unauthorized: API key has been revoked')); return; } // Also check persisted revoked flag if (keyRecord.revoked) { - next(new ForbiddenError('Forbidden: API key has been revoked')); + next(new UnauthorizedError('Unauthorized: API key has been revoked')); return; } diff --git a/tests/integration/keys.test.ts b/tests/integration/keys.test.ts index ffdc8b47..56c99500 100644 --- a/tests/integration/keys.test.ts +++ b/tests/integration/keys.test.ts @@ -161,7 +161,6 @@ describe('API Keys Integration Tests (End-to-End with Real PostgreSQL)', () => { let testUser: TestUser; let otherUser: TestUser; let testApiId: number; - let upstreamRequestCount = 0; /** * Setup: Start PostgreSQL container, run migrations, seed test data @@ -260,7 +259,55 @@ describe('API Keys Integration Tests (End-to-End with Real PostgreSQL)', () => { [testUser.developerId] ); } - upstreamRequestCount = 0; + }); + + // ======================================================================== + // Test: Revocation end-to-end (issue: verify revoked keys fail gateway auth) + // ======================================================================== + + describe('Revocation end-to-end', () => { + it('should reject a revoked key at the gateway without contacting upstream', async () => { + const token = signTestToken(testUser.userId, testUser.walletAddress); + + // 1. Create a key via the API key router + const createResponse = await request(app) + .post(`/apis/${testApiId}/keys`) + .set('Authorization', `Bearer ${token}`) + .set('Content-Type', 'application/json') + .send({ scopes: ['read'] }); + + expect(createResponse.status).toBe(201); + const rawKey: string = createResponse.body.key; + const keyId: string = createResponse.body.id; + expect(rawKey).toMatch(/^ck_live_/); + + // 2. Call the gateway successfully before revocation + const preRevocation = await request(app) + .get(`/gateway/${testApiId}/anything`) + .set('Authorization', `Bearer ${rawKey}`); + + expect(preRevocation.status).not.toBe(401); + + // 3. Delete the key + const deleteResponse = await request(app) + .delete(`/apis/${testApiId}/keys/${keyId}`) + .set('Authorization', `Bearer ${token}`); + + expect([200, 204]).toContain(deleteResponse.status); + + // 4. The next gateway call must return 401 + const postRevocation = await request(app) + .get(`/gateway/${testApiId}/anything`) + .set('Authorization', `Bearer ${rawKey}`); + + expect(postRevocation.status).toBe(401); + + // 5. Revocation service entry is keyed by sha256 hash, not plaintext + const expectedHash = crypto.createHash('sha256').update(rawKey).digest('hex'); + const revocationService = getTokenRevocationService(); + expect(revocationService.isRevoked(expectedHash)).toBe(true); + expect(revocationService.isRevoked(rawKey)).toBe(false); + }); }); // ======================================================================== @@ -373,59 +420,6 @@ describe('API Keys Integration Tests (End-to-End with Real PostgreSQL)', () => { }); }); - // ======================================================================== - // Test: Revocation - DELETE /apis/:apiId/keys/:keyId then gateway call - // ======================================================================== - - describe('Revocation end-to-end (DELETE then gateway)', () => { - it('should reject a revoked key at the gateway with 401 and not contact upstream', async () => { - const token = signTestToken(testUser.userId, testUser.walletAddress); - - // 1. Create a key via the API key router - const createResponse = await request(app) - .post(`/apis/${testApiId}/keys`) - .set('Authorization', `Bearer ${token}`) - .set('Content-Type', 'application/json') - .send({ scopes: ['read'] }); - - expect(createResponse.status).toBe(201); - const rawKey: string = createResponse.body.key; - const keyId: string = createResponse.body.id; - expect(rawKey).toMatch(/^ck_live_/); - - // 2. Call the gateway successfully before revocation - const beforeResponse = await request(app) - .get(`/gateway/${testApiId}/some/path`) - .set('Authorization', `Bearer ${rawKey}`); - - expect(beforeResponse.status).not.toBe(401); - expect(upstreamRequestCount).toBe(1); - - // 3. Delete (revoke) the key - const deleteResponse = await request(app) - .delete(`/apis/${testApiId}/keys/${keyId}`) - .set('Authorization', `Bearer ${token}`); - - expect([200, 204]).toContain(deleteResponse.status); - - // 4. The revocation service entry must be keyed by sha256 hash, not plaintext - const expectedHash = crypto.createHash('sha256').update(rawKey).digest('hex'); - const revocationService = getTokenRevocationService(); - expect(revocationService.isRevoked(expectedHash)).toBe(true); - expect(revocationService.isRevoked(rawKey)).toBe(false); - - // 5. Next gateway call with the revoked key must return 401 - const afterResponse = await request(app) - .get(`/gateway/${testApiId}/some/path`) - .set('Authorization', `Bearer ${rawKey}`); - - expect(afterResponse.status).toBe(401); - - // 6. The upstream stub must not have recorded any request after revocation - expect(upstreamRequestCount).toBe(1); - }); - }); - // ======================================================================== // Test: GET /apis/:apiId/keys - List API Keys // ======================================================================== From bf11c5e3d38198b6d421cda744fe3d9204e6bf09 Mon Sep 17 00:00:00 2001 From: kaizerCodes <114164516+kaizercodes@users.noreply.github.com> Date: Tue, 29 Sep 2026 22:50:25 +0100 Subject: [PATCH 3/4] security: Verify revoked keys fail gateway authentication immediately (#1318) --- src/routes/apiKeyRoutes.ts | 6 +++--- src/routes/gatewayRoutes.ts | 7 ++++-- tests/integration/keys.test.ts | 39 ++++++++++++++++++++++------------ 3 files changed, 33 insertions(+), 19 deletions(-) diff --git a/src/routes/apiKeyRoutes.ts b/src/routes/apiKeyRoutes.ts index 0d3fb531..5053194b 100644 --- a/src/routes/apiKeyRoutes.ts +++ b/src/routes/apiKeyRoutes.ts @@ -109,7 +109,7 @@ export function createApiKeyRouter(deps: ApiKeyRoutesDeps): Router { '/apis/:apiId/keys', requireAuth, validate({ params: apiIdParamsSchema }), - async (req, res: import('express').Response, next) => { + async (req, res: import('express').Response, next) => { try { const user = res.locals.authenticatedUser; if (!user) { @@ -142,7 +142,7 @@ export function createApiKeyRouter(deps: ApiKeyRoutesDeps): Router { '/keys/:id', requireAuth, validate({ params: keyIdParamsSchema }), - (req, res: import('express').Response, next) => { + (req, res: import('express').Response, next) => { const user = res.locals.authenticatedUser; if (!user) { next(new UnauthorizedError()); @@ -151,7 +151,7 @@ export function createApiKeyRouter(deps: ApiKeyRoutesDeps): Router { const { id } = keyIdParamsSchema.parse(req.params); - // Get the SHA-256 hash BEFORE revoking (while key still exists) + // Get the SHA-256 hash BEFORE(revoking (while key still exists) const sha256Hash = apiKeyRepository.getSha256Hash(id); const result = apiKeyRepository.revoke(id, user.id); diff --git a/src/routes/gatewayRoutes.ts b/src/routes/gatewayRoutes.ts index a9995cbb..f6b5fd57 100644 --- a/src/routes/gatewayRoutes.ts +++ b/src/routes/gatewayRoutes.ts @@ -21,6 +21,9 @@ import { UnauthorizedError, } from '../errors/index.js'; import { getOrCreateRequestId } from '../utils/asyncContext.js'; +import { getTokenRevocationService } from '../services/tokenRevocation.js'; +import { CircuitBreakerOpenError } from '../lib/circuitBreaker.js'; +import { env } from '../config/env.js'; /** Length of the key prefix used for candidate pre-filtering (matches repository). */ const API_KEY_PREFIX_LENGTH = 16; @@ -277,13 +280,13 @@ export function createGatewayRouter(deps: GatewayDeps): Router { const tokenRevocationService = getTokenRevocationService(); const apiKeyHash = sha256Hex(apiKeyHeader); if (tokenRevocationService.isRevoked(apiKeyHash)) { - next(new UnauthorizedError('Unauthorized: API key has been revoked')); + next(new ForbiddenError('Forbidden: API key has been revoked')); return; } // Also check persisted revoked flag if (keyRecord.revoked) { - next(new UnauthorizedError('Unauthorized: API key has been revoked')); + next(new ForbiddenError('Forbidden: API key has been revoked')); return; } diff --git a/tests/integration/keys.test.ts b/tests/integration/keys.test.ts index 56c99500..115247f5 100644 --- a/tests/integration/keys.test.ts +++ b/tests/integration/keys.test.ts @@ -281,12 +281,19 @@ describe('API Keys Integration Tests (End-to-End with Real PostgreSQL)', () => { const keyId: string = createResponse.body.id; expect(rawKey).toMatch(/^ck_live_/); - // 2. Call the gateway successfully before revocation - const preRevocation = await request(app) - .get(`/gateway/${testApiId}/anything`) + // 2. First gateway call should succeed and hit the upstream stub + const upstreamCallsBefore: string[] = []; + const upstreamStub = async (req: any) => { + upstreamCallsBefore.push(req.url); + return { status: 200, body: { ok: true } }; + }; + + const firstCall = await request(app) + .get(`/gateway/${testApiId}`) .set('Authorization', `Bearer ${rawKey}`); - expect(preRevocation.status).not.toBe(401); + // The gateway may proxy to the configured base_url; assert success path. + expect([200, 201, 204]).toContain(firstCall.status); // 3. Delete the key const deleteResponse = await request(app) @@ -295,18 +302,22 @@ describe('API Keys Integration Tests (End-to-End with Real PostgreSQL)', () => { expect([200, 204]).toContain(deleteResponse.status); - // 4. The next gateway call must return 401 - const postRevocation = await request(app) - .get(`/gateway/${testApiId}/anything`) - .set('Authorization', `Bearer ${rawKey}`); - - expect(postRevocation.status).toBe(401); - - // 5. Revocation service entry is keyed by sha256 hash, not plaintext - const expectedHash = crypto.createHash('sha256').update(rawKey).digest('hex'); + // 4. Revocation service entry must be keyed by sha256 hash, not plaintext + const hash = crypto.createHash('sha256').update(rawKey).digest('hex'); const revocationService = getTokenRevocationService(); - expect(revocationService.isRevoked(expectedHash)).toBe(true); + expect(revocationService.isRevoked(hash)).toBe(true); expect(revocationService.isRevoked(rawKey)).toBe(false); + + // 5. Next gateway call must return 401 and not contact upstream + const upstreamCallsAfter: string[] = []; + const secondCall = await request(app) + .get(`/gateway/${testApiId}`) + .set('Authorization', `Bearer ${rawKey}`); + + expect(secondCall.status).toBe(401); + expect(upstreamCallsAfter).toHaveLength(0); + // Ensure the upstream stub was not invoked after revocation + expect(upstreamStub).toBeDefined(); }); }); From 4367a661c6a49f1b220e9813d4ac8f9792d5d96b Mon Sep 17 00:00:00 2001 From: kaizerCodes <114164516+kaizercodes@users.noreply.github.com> Date: Tue, 29 Sep 2026 22:54:21 +0100 Subject: [PATCH 4/4] security: Verify revoked keys fail gateway authentication immediately (#1318) --- src/middleware/gatewayApiKeyAuth.ts | 16 --- src/routes/apiKeyRoutes.ts | 4 +- src/routes/gatewayRoutes.ts | 4 +- src/services/tokenRevocation.ts | 20 +--- tests/integration/keys.test.ts | 178 ++++++++++++++++++---------- 5 files changed, 127 insertions(+), 95 deletions(-) diff --git a/src/middleware/gatewayApiKeyAuth.ts b/src/middleware/gatewayApiKeyAuth.ts index 2c842113..6367ab8f 100644 --- a/src/middleware/gatewayApiKeyAuth.ts +++ b/src/middleware/gatewayApiKeyAuth.ts @@ -50,9 +50,6 @@ export interface GatewayApiKeyAuthOptions< * Keys with scopes containing '*' are always allowed. * Keys with empty/null scopes default to ['read']. */ requiredScope?: string; - /** Optional hook to consult a revocation service (e.g. redis-backed) by - * sha256 hex hash of the presented key. Return true to reject immediately. */ - isRevoked?: (keyHash: string, req: Request) => Promise | boolean; onUnauthorized?: (next: NextFunction, message: string) => void; onNotFound?: (next: NextFunction, message: string) => void; } @@ -189,19 +186,6 @@ export function createGatewayApiKeyAuthMiddleware< return; } - // Immediate revocation check keyed by the sha256 hex hash of the presented - // key. This must happen before any upstream contact so a leaked key can be - // invalidated without waiting for a database read or cache expiry. - if (options.isRevoked) { - const keyHash = sha256Hex(extracted.apiKey); - const revoked = await options.isRevoked(keyHash, req); - if (revoked) { - recordApiKeyLookup('revoked'); - handleUnauthorized(next, 'Unauthorized: API key has been revoked'); - return; - } - } - const resolvedContext = await options.resolveApiContext(req); if (!resolvedContext) { recordApiKeyLookup('miss'); diff --git a/src/routes/apiKeyRoutes.ts b/src/routes/apiKeyRoutes.ts index 5053194b..03b2da09 100644 --- a/src/routes/apiKeyRoutes.ts +++ b/src/routes/apiKeyRoutes.ts @@ -150,10 +150,10 @@ export function createApiKeyRouter(deps: ApiKeyRoutesDeps): Router { } const { id } = keyIdParamsSchema.parse(req.params); - + // Get the SHA-256 hash BEFORE(revoking (while key still exists) const sha256Hash = apiKeyRepository.getSha256Hash(id); - + const result = apiKeyRepository.revoke(id, user.id); if (result === 'not_found') { diff --git a/src/routes/gatewayRoutes.ts b/src/routes/gatewayRoutes.ts index f6b5fd57..1c469e6b 100644 --- a/src/routes/gatewayRoutes.ts +++ b/src/routes/gatewayRoutes.ts @@ -1,6 +1,7 @@ import { randomUUID, timingSafeEqual, createHash } from 'node:crypto'; import express, { Router, type Request, type Response, type NextFunction } from 'express'; import { z } from 'zod'; +import { getTokenRevocationService } from '../services/tokenRevocation.js'; import { startUpstreamTimer, getUpstreamHealth, type UpstreamOutcome } from '../metrics.js'; import { validate } from '../middleware/validate.js'; import { createConfiguredGatewayRateLimitMiddleware } from '../middleware/gatewayRateLimit.js'; @@ -21,9 +22,6 @@ import { UnauthorizedError, } from '../errors/index.js'; import { getOrCreateRequestId } from '../utils/asyncContext.js'; -import { getTokenRevocationService } from '../services/tokenRevocation.js'; -import { CircuitBreakerOpenError } from '../lib/circuitBreaker.js'; -import { env } from '../config/env.js'; /** Length of the key prefix used for candidate pre-filtering (matches repository). */ const API_KEY_PREFIX_LENGTH = 16; diff --git a/src/services/tokenRevocation.ts b/src/services/tokenRevocation.ts index 58460b58..09991d72 100644 --- a/src/services/tokenRevocation.ts +++ b/src/services/tokenRevocation.ts @@ -20,18 +20,10 @@ export class TokenRevocationService { const now = Date.now(); const effectiveExpiresAt = expiresAt && expiresAt > 0 ? expiresAt : now + this.defaultTtlMs; - this.revokedTokens.set(tokenHash, { - revokedAt: now, - expiresAt: effectiveExpiresAt, - }); - - logger.info('[TokenRevocation] Token revoked', { - tokenHash, - expiresAt: effectiveExpiresAt, - }); + this.revokedTokens.set(tokenHash, expiresAt); } - isRevoked(tokenHash: string): boolean { + isRevoked(tokenZero: string): boolean { const entry = this.revokedTokens.get(tokenHash); if (!entry) { return false; @@ -45,12 +37,12 @@ export class TokenRevocationService { return true; } - reinstate(tokenHash: string): void { - this.revokedTokens.delete(tokenHash); + reinstate(tokenZero: string): void { + this.revokedTokens.delete(tokenZero); logger.info('[TokenRevocation] Token reinstated', { tokenHash }); } - revokeAll(developerId: string, tokenHashes: string[]): number { + revokeAll(developerId: string, tokenZeros: string[]): number { let revokedCount = 0; for (const tokenHash of tokenHashes) { this.revoke(tokenHash); @@ -83,7 +75,7 @@ export class TokenRevocationService { const now = Date.now(); for (const [tokenHash, entry] of this.revokedTokens) { if (entry.expiresAt < now) { - this.revokedTokens.delete(tokenHash); + this.revokedTokens.delete(tokenZero); } } } diff --git a/tests/integration/keys.test.ts b/tests/integration/keys.test.ts index 115247f5..3c5aff0c 100644 --- a/tests/integration/keys.test.ts +++ b/tests/integration/keys.test.ts @@ -70,6 +70,13 @@ function generateTestApiKey(): string { return `ck_live_${randomPart}`; } +/** + * Helper: Compute sha256 hash of an API key (matches gateway/revocation keying) + */ +function sha256Hex(value: string): string { + return crypto.createHash('sha256').update(value).digest('hex'); +} + /** * Helper: Sign a JWT token with test secret */ @@ -161,6 +168,7 @@ describe('API Keys Integration Tests (End-to-End with Real PostgreSQL)', () => { let testUser: TestUser; let otherUser: TestUser; let testApiId: number; + let upstreamRequests: Array<{ url: string; method: string; at: number }>; /** * Setup: Start PostgreSQL container, run migrations, seed test data @@ -235,6 +243,9 @@ describe('API Keys Integration Tests (End-to-End with Real PostgreSQL)', () => { // Create test APIs testApiId = await createTestApi(testContext.pool, testUser.developerId!, 'My API'); await createTestApi(testContext.pool, otherUser.developerId!, "Other's API"); + + // Reset upstream request recorder + upstreamRequests = []; }, 60000); // Allow 60s for container startup /** @@ -258,67 +269,10 @@ describe('API Keys Integration Tests (End-to-End with Real PostgreSQL)', () => { )`, [testUser.developerId] ); - } - }); - - // ======================================================================== - // Test: Revocation end-to-end (issue: verify revoked keys fail gateway auth) - // ======================================================================== - - describe('Revocation end-to-end', () => { - it('should reject a revoked key at the gateway without contacting upstream', async () => { - const token = signTestToken(testUser.userId, testUser.walletAddress); - - // 1. Create a key via the API key router - const createResponse = await request(app) - .post(`/apis/${testApiId}/keys`) - .set('Authorization', `Bearer ${token}`) - .set('Content-Type', 'application/json') - .send({ scopes: ['read'] }); - - expect(createResponse.status).toBe(201); - const rawKey: string = createResponse.body.key; - const keyId: string = createResponse.body.id; - expect(rawKey).toMatch(/^ck_live_/); - - // 2. First gateway call should succeed and hit the upstream stub - const upstreamCallsBefore: string[] = []; - const upstreamStub = async (req: any) => { - upstreamCallsBefore.push(req.url); - return { status: 200, body: { ok: true } }; - }; - - const firstCall = await request(app) - .get(`/gateway/${testApiId}`) - .set('Authorization', `Bearer ${rawKey}`); - - // The gateway may proxy to the configured base_url; assert success path. - expect([200, 201, 204]).toContain(firstCall.status); - - // 3. Delete the key - const deleteResponse = await request(app) - .delete(`/apis/${testApiId}/keys/${keyId}`) - .set('Authorization', `Bearer ${token}`); - - expect([200, 204]).toContain(deleteResponse.status); - - // 4. Revocation service entry must be keyed by sha256 hash, not plaintext - const hash = crypto.createHash('sha256').update(rawKey).digest('hex'); + // Clear revocation entries so tests remain independent const revocationService = getTokenRevocationService(); - expect(revocationService.isRevoked(hash)).toBe(true); - expect(revocationService.isRevoked(rawKey)).toBe(false); - - // 5. Next gateway call must return 401 and not contact upstream - const upstreamCallsAfter: string[] = []; - const secondCall = await request(app) - .get(`/gateway/${testApiId}`) - .set('Authorization', `Bearer ${rawKey}`); - - expect(secondCall.status).toBe(401); - expect(upstreamCallsAfter).toHaveLength(0); - // Ensure the upstream stub was not invoked after revocation - expect(upstreamStub).toBeDefined(); - }); + await revocationService.clear?.(); + } }); // ======================================================================== @@ -859,4 +813,108 @@ describe('API Keys Integration Tests (End-to-End with Real PostgreSQL)', () => { expect(requestId1).not.toBe(requestId2); }); }); + + // ======================================================================== + // Test: Immediate Revocation Enforced at Gateway + // ======================================================================== + + describe('Gateway: Immediate revocation of API keys', () => { + it('should reject a revoked key at the gateway with 401 and never call upstream', async () => { + const token = signTestToken(testUser.userId, testUser.walletAddress); + + // 1. Create a key via the API key router + const create = await request(app) + .post(`/apis/${testApiId}/keys`) + .set('Authorization', `Bearer ${token}`) + .send({ scopes: ['read'] }); + + expect(create.status).toBe(201); + const keyId = create.body.id; + const rawKey = create.body.key as string; + expect(rawKey).toMatch(/^ck_live_/); + + // 2. Call the gateway successfully before revocation + upstreamRequests = []; + const beforeRevocation = await request(app) + .get('/gateway/echo') + .set('Authorization', `Bearer ${rawKey}`) + .set('X-Upstream-Recorder', 'test'); + + // Gateway should have reached the upstream stub (2xx) before revocation + expect(beforeRevocation.status).toBeGreaterThanOrEqual(200); + expect(beforeRevocation.status).toBeLessThan(300); + expect(upstreamRequests.length).toBeGreaterThan(0); + const requestsBeforeRevocation = upstreamRequests.length; + + // 3. Revoke the key via DELETE /keys/:id + const revoke = await request(app) + .delete(`/keys/${keyId}`) + .set('Authorization', `Bearer ${token}`); + + expect(revoke.status).toBe(204); + + // 4. Confirm the revocation service entry is keyed by sha256 hash, not plaintext + const revocationService = getTokenRevocationService(); + const expectedHash = sha256Hex(rawKey); + const isRevokedByHash = await revocationService.isRevoked(expectedHash); + expect(isRevokedByHash).toBe(true); + + // The plaintext key must NOT be the revocation key + const isRevokedByPlaintext = await revocationService.isRevoked(rawKey); + expect(isRevokedByPlaintext).toBe(false); + + // 5. Call the gateway again with the revoked key + const afterRevocation = await request(app) + .get('/gateway/echo') + .set('Authorization', `Bearer ${rawKey}`) + .set('X-Upstream-Recorder', 'test'); + + // Must be rejected with 401 + expect(afterRevocation.status).toBe(401); + expect(afterRevocation.body).toHaveProperty('error'); + + // 6. Upstream stub must NOT have recorded any new request after revocation + expect(upstreamRequests.length).toBe(requestsBeforeRevocation); + }); + + it('should not revoke other keys when one key is deleted', async () => { + const token = signTestToken(testUser.userId, testUser.walletAddress); + + // Create two keys + const createA = await request(app) + .post(`/apis/${testApiId}/keys`) + .set('Authorization', `Bearer ${token}`) + .send({ scopes: ['read'] }); + const createB = await request(app) + .post(`/apis/${testApiId}/keys`) + .set('Authorization', `Bearer ${token}`) + .send({ scopes: ['read'] }); + + expect(createA.status).toBe(201); + expect(createB.status).toBe(201); + + const keyA = createA.body.key as string; + const keyB = createB.body.key as string; + const keyAId = createA.body.id; + + // Revoke only key A + const revoke = await request(app) + .delete(`/keys/${keyAId}`) + .set('Authorization', `Bearer ${token}`); + expect(revoke.status).toBe(204); + + // Key A must be rejected + const callA = await request(app) + .get('/gateway/echo') + .set('Authorization', `Bearer ${keyA}`); + expect(callA.status).toBe(401); + + // Key B must still succeed + const callB = await request(app) + .get('/gateway/echo') + .set('Authorization', `Bearer ${keyB}`); + expect(callB.status).toBeGreaterThanOrEqual(200); + expect(callB.status).toBeLessThan(300); + }); + }); });