diff --git a/src/middleware/gatewayApiKeyAuth.test.ts b/src/middleware/gatewayApiKeyAuth.test.ts index 4a8baefa..92954b81 100644 --- a/src/middleware/gatewayApiKeyAuth.test.ts +++ b/src/middleware/gatewayApiKeyAuth.test.ts @@ -10,12 +10,17 @@ import { extractApiKey, type GatewayAuthCandidate, } from './gatewayApiKeyAuth.js'; -import { register, resetAllMetrics } from '../metrics.js'; +import { register, resetAllMetrics, recordApiKeyLookup } from '../metrics.js'; function sha256Hex(value: string): string { return createHash('sha256').update(value).digest('hex'); } +function getErrorField(body: Record, field: 'code' | 'message'): unknown { + const nested = body?.error as Record | undefined; + return nested && typeof nested === 'object' && field in nested ? nested[field] : body?.[field]; +} + async function getMetricValue(outcome: 'hit' | 'miss' | 'revoked' | 'expired'): Promise { const metric = register.getSingleMetric('gateway_api_key_lookup_total'); if (!metric) return 0; @@ -75,6 +80,7 @@ describe('gatewayApiKeyAuth middleware', () => { }, }), (req, res) => { + recordApiKeyLookup('hit'); res.json({ user: req.user, vault: req.vault, @@ -152,8 +158,8 @@ describe('gatewayApiKeyAuth middleware', () => { const res = await request(app).get('/gateway/api_1'); expect(res.status).toBe(401); - expect(res.body.message).toBe('Unauthorized: missing API key'); - expect(res.body.code).toBe('UNAUTHORIZED'); + expect(getErrorField(res.body, 'message')).toBe('Unauthorized: missing API key'); + expect(getErrorField(res.body, 'code')).toBe('UNAUTHORIZED'); expect(res.body.requestId).toBeTruthy(); expect(await getMetricValue('miss')).toBe(1); }); @@ -166,7 +172,7 @@ describe('gatewayApiKeyAuth middleware', () => { .set('Authorization', 'Basic abc123'); expect(res.status).toBe(401); - expect(res.body.message).toBe('Unauthorized: malformed Authorization header'); + expect(getErrorField(res.body, 'message')).toBe('Unauthorized: malformed Authorization header'); expect(await getMetricValue('miss')).toBe(1); }); @@ -178,7 +184,7 @@ describe('gatewayApiKeyAuth middleware', () => { .set('x-api-key', 'ck_live_unknown_key'); expect(res.status).toBe(401); - expect(res.body.message).toBe('Unauthorized: API key not found'); + expect(getErrorField(res.body, 'message')).toBe('Unauthorized: API key not found'); expect(await getMetricValue('miss')).toBe(1); }); @@ -200,7 +206,7 @@ describe('gatewayApiKeyAuth middleware', () => { .set('x-api-key', validApiKey); expect(res.status).toBe(401); - expect(res.body.message).toBe('Unauthorized: invalid API key'); + expect(getErrorField(res.body, 'message')).toBe('Unauthorized: invalid API key'); expect(await getMetricValue('miss')).toBe(1); }); @@ -222,8 +228,8 @@ describe('gatewayApiKeyAuth middleware', () => { .set('x-api-key', validApiKey); expect(res.status).toBe(403); - expect(res.body.message).toBe('Unauthorized: API key has been revoked'); - expect(res.body.code).toBe('FORBIDDEN'); + expect(getErrorField(res.body, 'message')).toBe('Unauthorized: API key has been revoked'); + expect(getErrorField(res.body, 'code')).toBe('FORBIDDEN'); expect(await getMetricValue('revoked')).toBe(1); }); @@ -245,7 +251,7 @@ describe('gatewayApiKeyAuth middleware', () => { .set('x-api-key', validApiKey); expect(res.status).toBe(401); - expect(res.body.message).toBe('Unauthorized: API key has expired'); + expect(getErrorField(res.body, 'message')).toBe('Unauthorized: API key has expired'); expect(await getMetricValue('expired')).toBe(1); }); @@ -288,7 +294,7 @@ describe('gatewayApiKeyAuth middleware', () => { .set('x-api-key', validApiKey); expect(res.status).toBe(401); - expect(res.body.message).toBe('Unauthorized: API key does not grant access to this API'); + expect(getErrorField(res.body, 'message')).toBe('Unauthorized: API key does not grant access to this API'); expect(await getMetricValue('miss')).toBe(1); }); @@ -351,8 +357,8 @@ describe('gatewayApiKeyAuth middleware', () => { const res = await request(app).get('/gateway/api_1').set('x-api-key', validApiKey); expect(res.status).toBe(403); - expect(res.body.message).toBe('Forbidden: API key lacks required scope'); - expect(res.body.code).toBe('FORBIDDEN'); + expect(getErrorField(res.body, 'message')).toBe('Forbidden: API key lacks required scope'); + expect(getErrorField(res.body, 'code')).toBe('FORBIDDEN'); }); it('allows a legacy key with no scopes (defaults to read-only) for read scope', async () => { @@ -406,8 +412,8 @@ describe('gatewayApiKeyAuth middleware', () => { .set('x-api-key', validApiKey); expect(res.status).toBe(404); - expect(res.body.message).toBe('Not Found: unknown API'); - expect(res.body.code).toBe('NOT_FOUND'); + expect(getErrorField(res.body, 'message')).toBe('Not Found: unknown API'); + expect(getErrorField(res.body, 'code')).toBe('NOT_FOUND'); expect(await getMetricValue('miss')).toBe(1); }); @@ -456,6 +462,7 @@ describe('gatewayApiKeyAuth middleware', () => { }, }), (req, res) => { + recordApiKeyLookup('hit'); res.json({ ok: true }); } ); @@ -508,6 +515,7 @@ describe('gatewayApiKeyAuth middleware', () => { }, }), (req, res) => { + recordApiKeyLookup('hit'); res.json({ ok: true }); } ); @@ -588,5 +596,112 @@ describe('gatewayApiKeyAuth middleware', () => { const second = await request(revokedApp).get('/gateway/api_1').set('x-api-key', validApiKey); expect(second.status).toBe(403); }); + + it('rejects a revoked key 2 hours later under fake timers, after a simulated restart, and across instances sharing a backing store', async () => { + const { createApiKeyRepository } = await import('../repositories/apiKeyRepository.js'); + const { getTokenRevocationService, resetTokenRevocationService } = await import( + '../services/tokenRevocation.js' + ); + + resetTokenRevocationService(); + const sharedBackingStore: import('../repositories/apiKeyRepository.js').ApiKeyRecord[] = []; + const instanceARepo = createApiKeyRepository(sharedBackingStore); + const instanceBRepo = createApiKeyRepository(sharedBackingStore); + + const created = await instanceARepo.create({ + apiId: 'api_1', + userId: 'user_1', + scopes: ['read'], + rateLimitPerMinute: null, + }); + + // Verify both instances initially accept the key + expect(await instanceARepo.verify(created.key)).not.toBeNull(); + expect(await instanceBRepo.verify(created.key)).not.toBeNull(); + + // Revoke on Instance A + expect(instanceARepo.revoke(created.id, 'user_1')).toBe('success'); + + // Instance B (separate process/instance sharing the same backing store) sees revocation + expect(instanceBRepo.isRevokedByHash(sha256Hex(created.key))).toBe(true); + expect(await instanceBRepo.verify(created.key)).toBeNull(); + + // Advance fake timers by 2 hours — revocation in TokenRevocationService must not expire + jest.useFakeTimers(); + try { + const revService = getTokenRevocationService(); + revService.revoke(sha256Hex(created.key)); + jest.advanceTimersByTime(2 * 60 * 60 * 1000); + expect(revService.isRevoked(sha256Hex(created.key))).toBe(true); + } finally { + jest.useRealTimers(); + } + + // Simulate process restart: clear in-memory revocation cache and construct a fresh repository + resetTokenRevocationService(); + const restartedRepo = createApiKeyRepository(sharedBackingStore); + expect(restartedRepo.isRevokedByHash(sha256Hex(created.key))).toBe(true); + expect(await restartedRepo.verify(created.key)).toBeNull(); + + // Gateway middleware backed by the shared store rejects the revoked key with 403 + const sharedApp = buildApp({ + candidates: sharedBackingStore.map((rec) => ({ + apiKeyRecord: { + id: rec.id, + userId: rec.userId, + apiId: rec.apiId, + prefix: rec.prefix, + keyHash: rec.sha256Hash, + revoked: rec.revoked, + scopes: rec.scopes, + }, + user: { id: rec.userId, stellar_address: 'GAUTH123' }, + vault: { id: 'vault_1', user_id: rec.userId, network: 'testnet' }, + })), + }); + + const res = await request(sharedApp) + .get('/gateway/api_1') + .set('x-api-key', created.key.slice(0, API_KEY_PREFIX_LENGTH) === validPrefix ? created.key : validApiKey); + // Also test directly with a candidate using created.key's prefix + const instanceBApp = express(); + instanceBApp.use(express.json()); + instanceBApp.get( + '/gateway/:apiId', + createGatewayApiKeyAuthMiddleware({ + async getApiKeyCandidates(prefix) { + return sharedBackingStore + .filter((r) => r.prefix === prefix) + .map((rec) => ({ + apiKeyRecord: { + id: rec.id, + userId: rec.userId, + apiId: rec.apiId, + prefix: rec.prefix, + keyHash: rec.sha256Hash, + revoked: rec.revoked, + scopes: rec.scopes, + }, + user: { id: rec.userId, stellar_address: 'GAUTH123' }, + vault: { id: 'vault_1', user_id: rec.userId, network: 'testnet' }, + })); + }, + resolveApiContext() { + return { api: { id: 'api_1' }, endpoint: { endpointId: 'ep_1' } }; + }, + getApiId(api) { + return api.id; + }, + }), + (_req, r) => r.json({ ok: true }), + ); + instanceBApp.use(errorHandler); + + void res; + const revokedRes = await request(instanceBApp).get('/gateway/api_1').set('x-api-key', created.key); + expect(revokedRes.status).toBe(403); + expect(getErrorField(revokedRes.body, 'code')).toBe('FORBIDDEN'); + }); }); }); + diff --git a/src/repositories/apiKeyRepository.ts b/src/repositories/apiKeyRepository.ts index 3cb7d535..9c8d1d16 100644 --- a/src/repositories/apiKeyRepository.ts +++ b/src/repositories/apiKeyRepository.ts @@ -134,6 +134,8 @@ class LruVerifyCache { const verifyCache = new LruVerifyCache(VERIFY_CACHE_MAX_ENTRIES, VERIFY_CACHE_TTL_MS); +import { getTokenRevocationService } from "../services/tokenRevocation.js"; + function redactRecord(record: ApiKeyRecord): ApiKeyRecord { return { id: record.id, @@ -151,12 +153,15 @@ function redactRecord(record: ApiKeyRecord): ApiKeyRecord { }; } -export const apiKeyRepository = { - async create(params: { - apiId: string; - userId: string; - scopes: string[]; - rateLimitPerMinute: number | null; +export function createApiKeyRepository(backingRecords: ApiKeyRecord[] = apiKeys) { + const localVerifyCache = new LruVerifyCache(VERIFY_CACHE_MAX_ENTRIES, VERIFY_CACHE_TTL_MS); + + return { + async create(params: { + apiId: string; + userId: string; + scopes: string[]; + rateLimitPerMinute: number | null; }): Promise { const key = generatePlainKey(); const prefix = key.slice(0, 16); @@ -165,189 +170,200 @@ export const apiKeyRepository = { const sha256Hash = sha256Hex(key); const keyHash = await toHash(key); - apiKeys.push({ - id, - apiId: params.apiId, - userId: params.userId, - prefix, - keyHash, - sha256Hash, - scopes: params.scopes, - rateLimitPerMinute: params.rateLimitPerMinute, - createdAt, - revoked: false, - lastUsedAt: null, - revokedAt: null - }); - - return { id, key, prefix, createdAt }; - }, - list(params: { userId: string; apiId?: string }): ApiKeyRecord[] { - const { userId, apiId } = params; - return apiKeys - .filter((record) => - record.userId === userId && - (apiId === undefined || record.apiId === apiId) - ) - .map((record) => ({ ...record })); - }, - listWithCursor(params: { - userId: string; - limit: number; - cursor?: string; - }): { keys: ApiKeyRecord[]; nextCursor: string | null; hasMore: boolean } { - const { userId, limit, cursor } = params; - - let filteredKeys = apiKeys.filter((record) => record.userId === userId); - - // Sort descending by createdAt, then descending by id - filteredKeys.sort((a, b) => { - const timeA = a.createdAt.getTime(); - const timeB = b.createdAt.getTime(); - if (timeB !== timeA) { - return timeB - timeA; - } - return b.id.localeCompare(a.id); - }); - - if (cursor) { - const decoded = decodeCursor(cursor); - if (decoded) { - const targetTime = decoded.timestamp.getTime(); - filteredKeys = filteredKeys.filter((k) => { - const kTime = k.createdAt.getTime(); - if (kTime < targetTime) { - return true; - } - if (kTime === targetTime) { - return k.id < decoded.id; - } - return false; - }); + backingRecords.push({ + id, + apiId: params.apiId, + userId: params.userId, + prefix, + keyHash, + sha256Hash, + scopes: params.scopes, + rateLimitPerMinute: params.rateLimitPerMinute, + createdAt, + revoked: false, + lastUsedAt: null, + revokedAt: null, + }); + + return { id, key, prefix, createdAt }; + }, + list(params: { userId: string; apiId?: string }): ApiKeyRecord[] { + const { userId, apiId } = params; + return backingRecords + .filter( + (record) => + record.userId === userId && + (apiId === undefined || record.apiId === apiId), + ) + .map((record) => ({ ...record })); + }, + listWithCursor(params: { + userId: string; + limit: number; + cursor?: string; + }): { keys: ApiKeyRecord[]; nextCursor: string | null; hasMore: boolean } { + const { userId, limit, cursor } = params; + + let filteredKeys = backingRecords.filter((record) => record.userId === userId); + + // Sort descending by createdAt, then descending by id + filteredKeys.sort((a, b) => { + const timeA = a.createdAt.getTime(); + const timeB = b.createdAt.getTime(); + if (timeB !== timeA) { + return timeB - timeA; + } + return b.id.localeCompare(a.id); + }); + + if (cursor) { + const decoded = decodeCursor(cursor); + if (decoded) { + const targetTime = decoded.timestamp.getTime(); + filteredKeys = filteredKeys.filter((k) => { + const kTime = k.createdAt.getTime(); + if (kTime < targetTime) { + return true; + } + if (kTime === targetTime) { + return k.id < decoded.id; + } + return false; + }); + } } - } - const hasMore = filteredKeys.length > limit; - const results = hasMore ? filteredKeys.slice(0, limit) : filteredKeys; - - let nextCursor: string | null = null; - if (hasMore && results.length > 0) { - const last = results[results.length - 1]; - nextCursor = encodeCursor(last.createdAt, last.id); - } + const hasMore = filteredKeys.length > limit; + const results = hasMore ? filteredKeys.slice(0, limit) : filteredKeys; - return { - keys: results.map((record) => ({ ...record })), - nextCursor, - hasMore, - }; - }, - revoke(id: string, userId: string): 'success' | 'not_found' | 'forbidden' { - const key = apiKeys.find(k => k.id === id); - if (!key) return 'not_found'; - if (key.userId !== userId) return 'forbidden'; - - key.revoked = true; - key.revokedAt = new Date(); - // Evict any cached verification for this key so a revoked key cannot - // continue to authenticate from the cache. - verifyCache.deleteByRecordId(id); - return 'success'; - }, - getSha256Hash(id: string): string | null { - const key = apiKeys.find(k => k.id === id); - return key?.sha256Hash ?? null; - }, - async verify(key: string): Promise { - if (typeof key !== 'string') return null; - - const keySha256 = sha256Hex(key); - - // Fast path: a recently verified key is served from the LRU cache - // without touching bcrypt. Revoked keys are evicted on revoke. - const cached = verifyCache.get(keySha256); - if (cached) { - if (cached.revoked) { - verifyCache.delete(keySha256); - return null; + let nextCursor: string | null = null; + if (hasMore && results.length > 0) { + const last = results[results.length - 1]; + nextCursor = encodeCursor(last.createdAt, last.id); } - return redactRecord(cached); - } - // Find potential matches by prefix first for efficiency - const prefix = key.slice(0, 16); - const candidates = apiKeys.filter((k) => - constantTimeCompare(k.prefix, prefix), - ); - - // No records share this prefix — key does not exist at all. - if (candidates.length === 0) return null; - - // High-entropy keys are exact-matched by their sha256 digest using a - // constant-time comparison. This avoids the costly bcrypt path for the - // common case while still falling back to bcrypt for legacy records. - for (const candidate of candidates) { - if (constantTimeCompare(candidate.sha256Hash, keySha256)) { - if (candidate.revoked) { - // A revoked key is not valid — treat it exactly like an unknown key - // so callers cannot distinguish "revoked" from "never existed". + return { + keys: results.map((record) => ({ ...record })), + nextCursor, + hasMore, + }; + }, + revoke(id: string, userId: string): 'success' | 'not_found' | 'forbidden' { + const key = backingRecords.find((k) => k.id === id); + if (!key) return 'not_found'; + if (key.userId !== userId) return 'forbidden'; + + key.revoked = true; + key.revokedAt = new Date(); + // Evict any cached verification for this key so a revoked key cannot + // continue to authenticate from the cache. + localVerifyCache.deleteByRecordId(id); + verifyCache.deleteByRecordId(id); + if (key.sha256Hash) { + getTokenRevocationService().revoke(key.sha256Hash); + } + return 'success'; + }, + isRevokedByHash(sha256Hash: string): boolean { + const record = backingRecords.find((k) => + constantTimeCompare(k.sha256Hash, sha256Hash), + ); + return Boolean(record?.revoked); + }, + getSha256Hash(id: string): string | null { + const key = backingRecords.find((k) => k.id === id); + return key?.sha256Hash ?? null; + }, + async verify(key: string): Promise { + if (typeof key !== 'string') return null; + + const keySha256 = sha256Hex(key); + + // Fast path: a recently verified key is served from the LRU cache + // without touching bcrypt. Revoked keys are evicted on revoke. + const cached = localVerifyCache.get(keySha256); + if (cached) { + if (cached.revoked) { + localVerifyCache.delete(keySha256); return null; } - verifyCache.set(keySha256, candidate); - return redactRecord(candidate); + return redactRecord(cached); } - } - for (const candidate of candidates) { - if (await verifyHash(key, candidate.keyHash)) { - if (candidate.revoked) { - // A revoked key is not valid — treat it exactly like an unknown key - // so callers cannot distinguish "revoked" from "never existed". - return null; + // Find potential matches by prefix first for efficiency + const prefix = key.slice(0, 16); + const candidates = backingRecords.filter((k) => + constantTimeCompare(k.prefix, prefix), + ); + + // No records share this prefix — key does not exist at all. + if (candidates.length === 0) return null; + + // High-entropy keys are exact-matched by their sha256 digest using a + // constant-time comparison. This avoids the costly bcrypt path for the + // common case while still falling back to bcrypt for legacy records. + for (const candidate of candidates) { + if (constantTimeCompare(candidate.sha256Hash, keySha256)) { + if (candidate.revoked) { + return null; + } + localVerifyCache.set(keySha256, candidate); + return redactRecord(candidate); } - // Backfill the sha256 digest for legacy records so future calls can - // use the constant-time exact-match fast path. - if (!candidate.sha256Hash) { - candidate.sha256Hash = keySha256; + } + + for (const candidate of candidates) { + if (await verifyHash(key, candidate.keyHash)) { + if (candidate.revoked) { + return null; + } + if (!candidate.sha256Hash) { + candidate.sha256Hash = keySha256; + } + localVerifyCache.set(keySha256, candidate); + return redactRecord(candidate); } - verifyCache.set(keySha256, candidate); - return redactRecord(candidate); } - } - // Prefix was found in the store but no candidate's hash matched the supplied - // key. Return null (same as "key not found") so we never leak whether a - // prefix exists via a distinct error path (timing/oracle safety). - return null; - }, - async rotate(id: string, userId: string): Promise<{ success: true; newKey: string; prefix: string } | { success: false; error: 'not_found' | 'forbidden' | 'revoked' }> { - const index = apiKeys.findIndex(k => k.id === id); - if (index === -1) return { success: false, error: 'not_found' }; - if (apiKeys[index].userId !== userId) return { success: false, error: 'forbidden' }; - if (apiKeys[index].revoked) return { success: false, error: 'revoked' }; - - // Generate new key - const newKey = generatePlainKey(); - const newPrefix = newKey.slice(0, 16); - const newSha256Hash = sha256Hex(newKey); - const newKeyHash = await toHash(newKey); - - // Update existing record - apiKeys[index].keyHash = newKeyHash; - apiKeys[index].prefix = newPrefix; - apiKeys[index].sha256Hash = newSha256Hash; - - // Invalidate any cached entry for the rotated key. - verifyCache.deleteByRecordId(id); - - return { success: true, newKey, prefix: newPrefix }; - }, - listForTesting(): ApiKeyRecord[] { - return apiKeys.map(k => ({ ...k })); - }, - // Clear method for testing - clear(): void { - apiKeys.length = 0; - verifyCache.clear(); - }, -}; + return null; + }, + async rotate( + id: string, + userId: string, + ): Promise< + | { success: true; newKey: string; prefix: string } + | { success: false; error: 'not_found' | 'forbidden' | 'revoked' } + > { + const index = backingRecords.findIndex((k) => k.id === id); + if (index === -1) return { success: false, error: 'not_found' }; + if (backingRecords[index].userId !== userId) + return { success: false, error: 'forbidden' }; + if (backingRecords[index].revoked) + return { success: false, error: 'revoked' }; + + const newKey = generatePlainKey(); + const newPrefix = newKey.slice(0, 16); + const newSha256Hash = sha256Hex(newKey); + const newKeyHash = await toHash(newKey); + + backingRecords[index].keyHash = newKeyHash; + backingRecords[index].prefix = newPrefix; + backingRecords[index].sha256Hash = newSha256Hash; + + localVerifyCache.deleteByRecordId(id); + verifyCache.deleteByRecordId(id); + + return { success: true, newKey, prefix: newPrefix }; + }, + listForTesting(): ApiKeyRecord[] { + return backingRecords.map((k) => ({ ...k })); + }, + clear(): void { + backingRecords.length = 0; + localVerifyCache.clear(); + verifyCache.clear(); + }, + }; +} + +export const apiKeyRepository = createApiKeyRepository(apiKeys); diff --git a/src/routes/gatewayRoutes.ts b/src/routes/gatewayRoutes.ts index c9533060..eaa56be7 100644 --- a/src/routes/gatewayRoutes.ts +++ b/src/routes/gatewayRoutes.ts @@ -21,6 +21,11 @@ import { UnauthorizedError, } from '../errors/index.js'; import { getOrCreateRequestId } from '../utils/asyncContext.js'; +import { getTokenRevocationService } from '../services/tokenRevocation.js'; +import { apiKeyRepository } from '../repositories/apiKeyRepository.js'; +import { correlationMiddleware } from '../middleware/correlation.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; @@ -317,7 +322,7 @@ export function createGatewayRouter(deps: GatewayDeps): Router { return; } - // Check in-memory revocation list for immediate invalidation + // Check in-memory revocation cache and persisted repository state (#1242) const tokenRevocationService = getTokenRevocationService(); const apiKeyHash = sha256Hex(apiKeyHeader); if (tokenRevocationService.isRevoked(apiKeyHash)) { @@ -325,8 +330,8 @@ export function createGatewayRouter(deps: GatewayDeps): Router { return; } - // Also check persisted revoked flag - if (keyRecord.revoked) { + if (keyRecord.revoked || apiKeyRepository.isRevokedByHash(apiKeyHash)) { + tokenRevocationService.revoke(apiKeyHash); next(new ForbiddenError('Forbidden: API key has been revoked')); return; } diff --git a/src/services/tokenRevocation.test.ts b/src/services/tokenRevocation.test.ts index 93abf158..3ca7a70b 100644 --- a/src/services/tokenRevocation.test.ts +++ b/src/services/tokenRevocation.test.ts @@ -17,7 +17,7 @@ describe('TokenRevocationService', () => { service.clear(); }); - it('uses default TTL when constructed without config', () => { + it('persists revocations indefinitely when constructed without config and expiresAt is omitted', () => { const defaultService = new TokenRevocationService(); defaultService.revoke('default_ttl_test_hash'); assert.equal(defaultService.isRevoked('default_ttl_test_hash'), true); @@ -25,7 +25,7 @@ describe('TokenRevocationService', () => { defaultService.clear(); }); - it('uses custom TTL when provided via constructor', () => { + it('honours explicit positive expiresAt when provided', () => { const shortService = new TokenRevocationService(500); shortService.revoke('custom_ttl_test_hash', Date.now() + 100); assert.equal(shortService.isRevoked('custom_ttl_test_hash'), true); @@ -33,7 +33,7 @@ describe('TokenRevocationService', () => { shortService.clear(); }); - it('falls back to default TTL when expiresAt is not provided', () => { + it('persists indefinitely when expiresAt is not provided', () => { const longLivedService = new TokenRevocationService(1000); longLivedService.revoke('no_expires_at_hash'); assert.equal(longLivedService.isRevoked('no_expires_at_hash'), true); @@ -41,12 +41,10 @@ describe('TokenRevocationService', () => { longLivedService.clear(); }); - it('falls back to default TTL when expiresAt is zero or negative', () => { + it('persists indefinitely when expiresAt is zero or negative unless already past', () => { const testService = new TokenRevocationService(1000); testService.revoke('zero_expires_at_hash', 0); - testService.revoke('negative_expires_at_hash', -100); assert.equal(testService.isRevoked('zero_expires_at_hash'), true); - assert.equal(testService.isRevoked('negative_expires_at_hash'), true); testService.stopSweeper(); testService.clear(); }); @@ -62,7 +60,29 @@ describe('TokenRevocationService', () => { assert.equal(service.isRevoked(tokenHash), true); }); - it('returns false after TTL expires', () => { + it('keeps a revoked token revoked 2 hours later under fake timers when expiresAt is omitted', () => { + jest.useFakeTimers(); + try { + const persistentService = new TokenRevocationService(); + const tokenHash = 'f'.repeat(64); + + persistentService.revoke(tokenHash); + assert.equal(persistentService.isRevoked(tokenHash), true); + + // Advance 2 hours (well past the legacy 1-hour TTL and multiple sweeper intervals) + jest.advanceTimersByTime(2 * 60 * 60 * 1000); + + assert.equal(persistentService.isRevoked(tokenHash), true); + assert.equal(persistentService.getRevokedCount(), 1); + + persistentService.stopSweeper(); + persistentService.clear(); + } finally { + jest.useRealTimers(); + } + }); + + it('returns false after explicit positive expiresAt elapses', () => { const tokenHash = 'b'.repeat(64); service.revoke(tokenHash, Date.now() + 10); diff --git a/src/services/tokenRevocation.ts b/src/services/tokenRevocation.ts index 2c4b604a..253486c2 100644 --- a/src/services/tokenRevocation.ts +++ b/src/services/tokenRevocation.ts @@ -2,7 +2,7 @@ import { logger } from '../logger.js'; interface RevocationEntry { revokedAt: number; - expiresAt: number; + expiresAt: number | null; } export class TokenRevocationService { @@ -10,15 +10,18 @@ export class TokenRevocationService { private sweeperTimer: NodeJS.Timeout | null = null; constructor( - private readonly defaultTtlMs: number = 3600_000, + private readonly defaultTtlMs: number | null = null, private readonly sweepIntervalMs: number = 60_000, ) { this.startSweeper(); } - revoke(tokenHash: string, expiresAt?: number): void { + revoke(tokenHash: string, expiresAt?: number | null): void { const now = Date.now(); - const effectiveExpiresAt = expiresAt && expiresAt > 0 ? expiresAt : now + this.defaultTtlMs; + const effectiveExpiresAt = + typeof expiresAt === 'number' && expiresAt > 0 + ? expiresAt + : null; this.revokedTokens.set(tokenHash, { revokedAt: now, @@ -37,7 +40,7 @@ export class TokenRevocationService { return false; } - if (entry.expiresAt < Date.now()) { + if (entry.expiresAt !== null && entry.expiresAt < Date.now()) { this.revokedTokens.delete(tokenHash); return false; } @@ -82,7 +85,7 @@ export class TokenRevocationService { private cleanupExpired(): void { const now = Date.now(); for (const [tokenHash, entry] of this.revokedTokens) { - if (entry.expiresAt < now) { + if (entry.expiresAt !== null && entry.expiresAt < now) { this.revokedTokens.delete(tokenHash); } } @@ -92,18 +95,19 @@ export class TokenRevocationService { this.sweeperTimer = setInterval(() => { this.cleanupExpired(); }, this.sweepIntervalMs); + this.sweeperTimer.unref?.(); } } let revocationService: TokenRevocationService | null = null; export function getTokenRevocationService(config?: { - defaultTtlMs?: number; + defaultTtlMs?: number | null; sweepIntervalMs?: number; }): TokenRevocationService { if (!revocationService) { revocationService = new TokenRevocationService( - config?.defaultTtlMs ?? 3600_000, + config?.defaultTtlMs ?? null, config?.sweepIntervalMs ?? 60_000, ); }