From 5dd47725cc680013d7fee22952997ad8894e8f5c Mon Sep 17 00:00:00 2001 From: Falujo Adeyemi Date: Fri, 28 Aug 2026 00:32:17 +0000 Subject: [PATCH 1/2] feat: SIWE-style nonce verification has a TOCTOU race --- context/progress-tracker.md | 35 ++ src/modules/auth/auth-throttler.guard.ts | 31 ++ src/modules/auth/auth.controller.ts | 26 +- src/modules/auth/auth.service.ts | 45 ++- .../transactions/wallet-throttler.guard.ts | 14 +- .../modules/auth/auth-throttler.guard.spec.ts | 59 ++++ .../unit/modules/auth/auth.controller.spec.ts | 6 +- test/unit/modules/auth/auth.service.spec.ts | 316 +++++++++++++++++- 8 files changed, 502 insertions(+), 30 deletions(-) create mode 100644 src/modules/auth/auth-throttler.guard.ts create mode 100644 test/unit/modules/auth/auth-throttler.guard.spec.ts diff --git a/context/progress-tracker.md b/context/progress-tracker.md index bb527c7..fc29a0a 100644 --- a/context/progress-tracker.md +++ b/context/progress-tracker.md @@ -6,6 +6,41 @@ pure chore/docs commits). Direct pushes to main must also be logged here. --- +## 2026-08-28 + +- Fixed TOCTOU nonce reuse in `AuthService.verifySignature()` (src/modules/auth/auth.service.ts:111): + - **Atomic nonce consumption** — replaced `SELECT → verify → UPDATE` with atomic + conditional claim `UPDATE nonces SET used_at = now() WHERE id = ? AND used_at IS NULL` + executed **before** signature verification. Only the winner of the race gets + `count === 1` / `data.length === 1`; losers get `count === 0` and are rejected + with `AUTH_NONCE_NOT_FOUND`. This guarantees a given `(wallet, nonce)` can + produce at most one successful verification ever, even under concurrent + `POST /auth/verify` requests carrying the same stolen pair. + - **Burn-on-failure tradeoff documented in code** — if verification fails + (invalid signature, bad StrKey, or `expires_at` in the past) the nonce stays + burned. The caller must request a fresh nonce; this converts replay attacks + into DoS-on-self (one wasted challenge) versus unlimited session creation. + Chosen over RPC/locking because a single conditional `UPDATE` is natively atomic + in Postgres and fits the existing `SupabaseService.getServiceRoleClient()` + pattern without a new migration. + - **Per-wallet throttling on `POST /auth/verify`** — new `AuthWalletThrottlerGuard` + (src/modules/auth/auth-throttler.guard.ts) keys `@nestjs/throttler` on + `req.body.wallet` (fallback to `req.user.wallet` / IP) and is applied via + `@UseGuards(AuthWalletThrottlerGuard)` alongside the existing global + IP-based `ThrottlerGuard`. Route limit stays `5 req / 60 s` per wallet **and** + per IP, preventing offline-style brute force of the SEP-0043 fallback space + at network speed. `WalletThrottlerGuard` was also hardened to type-check + wallet strings and accept `body.wallet` so the same infrastructure is reused. + - **Tests** — extended `test/unit/modules/auth/auth.service.spec.ts` to prove + atomicity: parallel double-verify → exactly one success, replay after success + fails, replay after failure stays burned (`AUTH_SIGNATURE_INVALID` → `AUTH_NONCE_NOT_FOUND`), + expired nonce rejected and stays burned, atomic race via `count === 0` rejected. + Added `test/unit/modules/auth/auth-throttler.guard.spec.ts` for the wallet-keyed + throttler and updated `auth.controller.spec.ts` to mock the guard. `npm run build` + and `npm test` green (38 suites, 425 tests). + +--- + ## 2026-08-27 - Closed the audit gaps on `POST /transactions/submit` (#117): diff --git a/src/modules/auth/auth-throttler.guard.ts b/src/modules/auth/auth-throttler.guard.ts new file mode 100644 index 0000000..c714a14 --- /dev/null +++ b/src/modules/auth/auth-throttler.guard.ts @@ -0,0 +1,31 @@ +import { Injectable } from '@nestjs/common'; +import { ThrottlerGuard } from '@nestjs/throttler'; + +/** + * ThrottlerGuard variant for POST /auth/verify that keys rate limits on the + * wallet address supplied in the request body (unauthenticated) or on the + * authenticated wallet (if present). Prefers body wallet because verify is + * unauthenticated — the wallet is not yet in req.user. + * + * Falls back to the default IP-based tracker when no wallet is present so + * anonymous/probe traffic is still bounded per IP. + * + * Used alongside the global IP-based ThrottlerGuard so POST /auth/verify is + * bounded per wallet AND per IP — preventing brute-force of the SEP-0043 + * fallback space at network speed and limiting stolen-nonce replay attempts. + */ +@Injectable() +export class AuthWalletThrottlerGuard extends ThrottlerGuard { + protected async getTracker(req: Record): Promise { + const body = (req as { body?: { wallet?: unknown } }).body; + const user = (req as { user?: { wallet?: unknown } }).user; + const bodyWallet = typeof body?.wallet === 'string' ? body.wallet : undefined; + const userWallet = typeof user?.wallet === 'string' ? user.wallet : undefined; + // Prefer body wallet for unauthenticated verify; fall back to user wallet. + const wallet = bodyWallet ?? userWallet; + if (wallet) { + return `wallet:${wallet}`; + } + return super.getTracker(req); + } +} diff --git a/src/modules/auth/auth.controller.ts b/src/modules/auth/auth.controller.ts index 070d532..3d15be1 100644 --- a/src/modules/auth/auth.controller.ts +++ b/src/modules/auth/auth.controller.ts @@ -1,21 +1,23 @@ -import { - Controller, - Post, - Body, - HttpCode, - HttpStatus, +import { + Controller, + Post, + Body, + HttpCode, + HttpStatus, NestInterceptor, ExecutionContext, CallHandler, - UseInterceptors, - UploadedFile, - ParseFilePipe, - MaxFileSizeValidator, - FileTypeValidator + UseInterceptors, + UseGuards, + UploadedFile, + ParseFilePipe, + MaxFileSizeValidator, + FileTypeValidator, } from '@nestjs/common'; import { ApiTags, ApiOperation, ApiResponse, ApiConsumes, ApiBody } from '@nestjs/swagger'; import { Throttle } from '@nestjs/throttler'; import { AuthService, RegisterResponse } from './auth.service'; +import { AuthWalletThrottlerGuard } from './auth-throttler.guard'; import { UploadedAvatarFile } from '../../database/repositories/users.repository'; import { NonceRequestDto } from './dto/nonce-request.dto'; import { NonceResponseDto } from './dto/nonce-response.dto'; @@ -73,9 +75,11 @@ export class AuthController { @Post('verify') @HttpCode(HttpStatus.OK) @Throttle({ default: { limit: 5, ttl: 60000 } }) + @UseGuards(AuthWalletThrottlerGuard) @ApiOperation({ summary: 'Verify wallet signature and issue JWT tokens' }) @ApiResponse({ status: 200, description: 'Signature verified — JWT tokens issued', type: AuthResponseDto }) @ApiResponse({ status: 401, description: 'Invalid signature or nonce' }) + @ApiResponse({ status: 429, description: 'Too many requests - rate limit exceeded (per wallet or per IP)' }) async verify(@Body() dto: VerifyRequestDto): Promise { await this.authService.verifySignature(dto); return this.authService.generateTokens(dto.wallet); diff --git a/src/modules/auth/auth.service.ts b/src/modules/auth/auth.service.ts index 3787339..4a79ec0 100644 --- a/src/modules/auth/auth.service.ts +++ b/src/modules/auth/auth.service.ts @@ -120,9 +120,47 @@ export class AuthService { if (nonceError || !nonceRecord) { throw new UnauthorizedException({ code: 'AUTH_NONCE_NOT_FOUND', message: 'Nonce not found or already used.' }); } - if (new Date(nonceRecord.expires_at) < new Date()) { + + // Atomic nonce claim: consume the row BEFORE expensive signature verification. + // The conditional UPDATE ... WHERE id = ? AND used_at IS NULL is a single + // atomic statement in Postgres. Two concurrent verify requests that both + // observed the same unused row above will race here; only one UPDATE will + // affect a row (count === 1). The loser gets count === 0 / empty data and + // is rejected as already consumed. This eliminates the TOCTOU window that + // previously existed between the SELECT and the trailing UPDATE. + // + // SECURITY TRADEOFF: if signature verification subsequently fails (or the + // nonce is expired), the nonce stays burned. Callers must request a fresh + // nonce and re-sign. This converts a replay of a stolen nonce+signature + // into a DoS-on-self (one wasted challenge) which is the correct tradeoff + // versus allowing unlimited session creation from a single intercepted pair. + const claimedAt = new Date().toISOString(); + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Supabase builder count typings for update().select() chain are incomplete; need runtime count check + const claimResult: any = await (client.from('nonces') as any) + .update({ used_at: claimedAt }, { count: 'exact' }) + .eq('id', (nonceRecord as { id: string }).id) + .is('used_at', null) + .select('id'); + const claimError = claimResult?.error as { message: string } | null | undefined; + const claimData = claimResult?.data as unknown[] | null | undefined; + const claimCount = claimResult?.count as number | null | undefined; + if (claimError) { + throw new InternalServerErrorException({ + code: 'DATABASE_NONCE_CLAIM_FAILED', + message: 'Failed to claim nonce.', + }); + } + const claimedCount = typeof claimCount === 'number' ? claimCount : (claimData?.length ?? 0); + if (claimedCount === 0) { + throw new UnauthorizedException({ code: 'AUTH_NONCE_NOT_FOUND', message: 'Nonce not found or already used.' }); + } + + // Nonce is now burned regardless of outcome below. Check expiry AFTER the + // claim so an expired row is still consumed and cannot be retried. + if (new Date((nonceRecord as { expires_at: string }).expires_at) < new Date()) { throw new UnauthorizedException({ code: 'AUTH_NONCE_EXPIRED', message: 'Nonce has expired.' }); } + if (!StrKey.isValidEd25519PublicKey(dto.wallet)) { throw new UnauthorizedException({ code: 'AUTH_SIGNATURE_INVALID', message: 'Invalid signature.' }); } @@ -134,7 +172,7 @@ export class AuthService { // First attempt: raw Ed25519 signature (mobile clients) try { isValid = keypair.verify(Buffer.from(dto.nonce), Buffer.from(dto.signature, 'base64')); - } catch (e) { + } catch { isValid = false; } @@ -143,7 +181,7 @@ export class AuthService { try { const sepMessage = 'Stellar Signing Key: ' + dto.nonce; isValid = keypair.verify(Buffer.from(sepMessage), Buffer.from(dto.signature, 'base64')); - } catch (e) { + } catch { isValid = false; } } @@ -155,7 +193,6 @@ export class AuthService { if (err instanceof UnauthorizedException) throw err; throw new UnauthorizedException({ code: 'AUTH_SIGNATURE_INVALID', message: 'Invalid signature.' }); } - await client.from('nonces').update({ used_at: new Date().toISOString() }).eq('id', nonceRecord.id); } private async findOrCreateUser(wallet: string): Promise<{ id: string; role: string | null }> { diff --git a/src/modules/transactions/wallet-throttler.guard.ts b/src/modules/transactions/wallet-throttler.guard.ts index 8fdc2bd..364385f 100644 --- a/src/modules/transactions/wallet-throttler.guard.ts +++ b/src/modules/transactions/wallet-throttler.guard.ts @@ -6,11 +6,19 @@ import { ThrottlerGuard } from '@nestjs/throttler'; * (from the JWT payload) instead of the client IP. Used alongside the global * IP-based guard so POST /transactions/submit is bounded per wallet AND per * IP, preventing a single wallet from being used as an open relay to Horizon. + * + * Also checks req.body.wallet so the same guard can be reused for + * unauthenticated routes like POST /auth/verify where the wallet is in the + * request body. */ @Injectable() export class WalletThrottlerGuard extends ThrottlerGuard { - protected async getTracker(req: { user?: { wallet?: string } }): Promise { - const wallet = req.user?.wallet; - return wallet ? `wallet:${wallet}` : super.getTracker(req); + protected async getTracker(req: Record): Promise { + const user = (req as { user?: { wallet?: unknown } }).user; + const body = (req as { body?: { wallet?: unknown } }).body; + const userWallet = typeof user?.wallet === 'string' ? user.wallet : undefined; + const bodyWallet = typeof body?.wallet === 'string' ? body.wallet : undefined; + const wallet = userWallet ?? bodyWallet; + return wallet ? `wallet:${wallet}` : super.getTracker(req as Record); } } diff --git a/test/unit/modules/auth/auth-throttler.guard.spec.ts b/test/unit/modules/auth/auth-throttler.guard.spec.ts new file mode 100644 index 0000000..49585ce --- /dev/null +++ b/test/unit/modules/auth/auth-throttler.guard.spec.ts @@ -0,0 +1,59 @@ +import { AuthWalletThrottlerGuard } from '../../../../src/modules/auth/auth-throttler.guard'; + +describe('AuthWalletThrottlerGuard', () => { + function createGuard(): AuthWalletThrottlerGuard { + const storageService = { + increment: jest.fn(), + getRecord: jest.fn(), + }; + const options = [{ ttl: 60000, limit: 5 }]; + const reflector = {}; + return new (AuthWalletThrottlerGuard as unknown as new ( + ...args: unknown[] + ) => AuthWalletThrottlerGuard)(options, storageService, reflector); + } + + function getTrackerOf(guard: AuthWalletThrottlerGuard, req: unknown): Promise { + return ( + guard as unknown as { getTracker: (request: unknown) => Promise } + ).getTracker(req); + } + + it('keys the rate limit on the wallet from request body when present (unauthenticated verify)', async () => { + const guard = createGuard(); + const wallet = 'GABCDEFGHIJKLMNOPQRSTUVWXYZ234567ABCDEFGHIJKLMNOPQRSTUVW'; + + await expect(getTrackerOf(guard, { body: { wallet } })).resolves.toBe(`wallet:${wallet}`); + }); + + it('keys the rate limit on the authenticated wallet when present', async () => { + const guard = createGuard(); + const wallet = 'GABCDEFGHIJKLMNOPQRSTUVWXYZ234567ABCDEFGHIJKLMNOPQRSTUVW'; + + await expect(getTrackerOf(guard, { user: { wallet } })).resolves.toBe(`wallet:${wallet}`); + }); + + it('prefers body wallet over user wallet when both are present', async () => { + const guard = createGuard(); + const bodyWallet = 'GBODYWALLETAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'; + const userWallet = 'GUSERWALLETAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'; + + await expect( + getTrackerOf(guard, { body: { wallet: bodyWallet }, user: { wallet: userWallet } }), + ).resolves.toBe(`wallet:${bodyWallet}`); + }); + + it('falls back to the IP-based tracker when no wallet is present', async () => { + const guard = createGuard(); + + await expect(getTrackerOf(guard, { ip: '203.0.113.7' })).resolves.toBe('203.0.113.7'); + }); + + it('falls back to IP when body wallet is not a string', async () => { + const guard = createGuard(); + + await expect(getTrackerOf(guard, { ip: '198.51.100.9', body: { wallet: 123 } })).resolves.toBe( + '198.51.100.9', + ); + }); +}); diff --git a/test/unit/modules/auth/auth.controller.spec.ts b/test/unit/modules/auth/auth.controller.spec.ts index 258cd8c..5da8918 100644 --- a/test/unit/modules/auth/auth.controller.spec.ts +++ b/test/unit/modules/auth/auth.controller.spec.ts @@ -1,6 +1,7 @@ import { Test, TestingModule } from '@nestjs/testing'; import { AuthController } from '../../../../src/modules/auth/auth.controller'; import { AuthService } from '../../../../src/modules/auth/auth.service'; +import { AuthWalletThrottlerGuard } from '../../../../src/modules/auth/auth-throttler.guard'; describe('AuthController', () => { let controller: AuthController; @@ -30,7 +31,10 @@ describe('AuthController', () => { useValue: mockAuthService, }, ], - }).compile(); + }) + .overrideGuard(AuthWalletThrottlerGuard) + .useValue({ canActivate: jest.fn().mockReturnValue(true) }) + .compile(); controller = module.get(AuthController); authService = module.get(AuthService); diff --git a/test/unit/modules/auth/auth.service.spec.ts b/test/unit/modules/auth/auth.service.spec.ts index 072610e..08df888 100644 --- a/test/unit/modules/auth/auth.service.spec.ts +++ b/test/unit/modules/auth/auth.service.spec.ts @@ -178,7 +178,7 @@ describe('AuthService', () => { function setupMocks({ nonceResult = { data: defaultNonceRecord, error: null }, - markUsedResult = { error: null }, + claimResult = { data: [{ id: 'nonce-uuid' }], error: null, count: 1 }, signatureValid = true, strKeyValid = true, } = {}) { @@ -188,18 +188,31 @@ describe('AuthService', () => { mockFrom.mockImplementation((table: string) => { if (table === 'nonces') { - const updateChain = { eq: jest.fn().mockResolvedValue(markUsedResult) }; - const chain: Record = { + // Builder that supports both: + // SELECT: select().eq().is().single() -> nonceResult + // CLAIM: update().eq().is().select() -> claimResult + const builder: Record = { select: jest.fn(), eq: jest.fn(), is: jest.fn(), single: jest.fn().mockResolvedValue(nonceResult), - update: jest.fn().mockReturnValue(updateChain), + update: jest.fn(), }; - chain.select.mockReturnValue(chain); - chain.eq.mockReturnValue(chain); - chain.is.mockReturnValue(chain); - return chain; + let operation: 'select' | 'update' = 'select'; + builder.select.mockImplementation((..._args: unknown[]) => { + if (operation === 'update') { + // This is the terminal select after update().eq().is().select() + return Promise.resolve(claimResult); + } + return builder; + }); + builder.update.mockImplementation(() => { + operation = 'update'; + return builder; + }); + builder.eq.mockReturnValue(builder); + builder.is.mockReturnValue(builder); + return builder; } return { insert: mockInsert }; }); @@ -231,10 +244,19 @@ describe('AuthService', () => { }); }); - it('should throw UnauthorizedException (AUTH_NONCE_EXPIRED) when nonce is past expiry', async () => { + it('should throw UnauthorizedException (AUTH_NONCE_NOT_FOUND) when atomic claim loses the race (count === 0)', async () => { + setupMocks({ claimResult: { data: [], error: null, count: 0 } }); + + await expect(service.verifySignature(validDto)).rejects.toMatchObject({ + response: { code: 'AUTH_NONCE_NOT_FOUND' }, + }); + }); + + it('should throw UnauthorizedException (AUTH_NONCE_EXPIRED) when nonce is past expiry (nonce stays burned)', async () => { const expiredDate = new Date(Date.now() - 1000).toISOString(); setupMocks({ nonceResult: { data: { id: 'nonce-uuid', expires_at: expiredDate }, error: null }, + claimResult: { data: [{ id: 'nonce-uuid' }], error: null, count: 1 }, }); await expect(service.verifySignature(validDto)).rejects.toMatchObject({ @@ -243,6 +265,7 @@ describe('AuthService', () => { }); it('should throw UnauthorizedException (AUTH_SIGNATURE_INVALID) when StrKey validation fails', async () => { + // StrKey check happens after claim; claim should have succeeded setupMocks({ strKeyValid: false }); await expect(service.verifySignature(validDto)).rejects.toMatchObject({ @@ -295,12 +318,283 @@ describe('AuthService', () => { ); }); - it('should mark nonce as used after successful verification', async () => { - const { } = setupMocks(); + it('should mark nonce as used via atomic claim before signature verification', async () => { + setupMocks(); await service.verifySignature(validDto); expect(mockFrom).toHaveBeenCalledWith('nonces'); }); + + it('should burn the nonce even when signature verification fails (DoS-on-self tradeoff)', async () => { + // Claim succeeds, but signature invalid — nonce stays burned, replay should get NOT_FOUND + const claimCallCount = { count: 0 }; + const mockKeypair = { verify: jest.fn().mockReturnValue(false) }; + (Keypair.fromPublicKey as jest.Mock).mockReturnValue(mockKeypair); + (StrKey.isValidEd25519PublicKey as jest.Mock).mockReturnValue(true); + + mockFrom.mockImplementation((table: string) => { + if (table === 'nonces') { + const builder: Record = { + select: jest.fn(), + eq: jest.fn(), + is: jest.fn(), + single: jest.fn(), + update: jest.fn(), + }; + let operation: 'select' | 'update' = 'select'; + // First verify: SELECT returns the nonce, CLAIM succeeds (count 1) + // Second verify (replay): claim would lose or SELECT already empty + builder.single.mockImplementation(() => { + // For this test we always return the nonce for SELECT, so the second + // call's failure is due to claim burning semantics, not SELECT miss. + // The service should still throw AUTH_NONCE_NOT_FOUND on second call + // because count === 0 (simulated via claimCallCount). + return Promise.resolve({ data: defaultNonceRecord, error: null }); + }); + builder.select.mockImplementation((..._args: unknown[]) => { + if (operation === 'update') { + claimCallCount.count += 1; + if (claimCallCount.count === 1) { + return Promise.resolve({ data: [{ id: 'nonce-uuid' }], error: null, count: 1 }); + } + return Promise.resolve({ data: [], error: null, count: 0 }); + } + return builder; + }); + builder.update.mockImplementation(() => { + operation = 'update'; + return builder; + }); + builder.eq.mockReturnValue(builder); + builder.is.mockReturnValue(builder); + return builder; + } + return { insert: mockInsert }; + }); + + // First call: claim succeeds but signature fails -> AUTH_SIGNATURE_INVALID, nonce burned + await expect(service.verifySignature(validDto)).rejects.toMatchObject({ + response: { code: 'AUTH_SIGNATURE_INVALID' }, + }); + + // Fix signature to be valid, but nonce is already burned -> should get NOT_FOUND, not success + mockKeypair.verify.mockReturnValue(true); + await expect(service.verifySignature(validDto)).rejects.toMatchObject({ + response: { code: 'AUTH_NONCE_NOT_FOUND' }, + }); + }); + }); + + describe('verifySignature — atomicity / concurrency', () => { + const validNonce = 'a1b2c3d4e5f67890abcdef1234567890a1b2c3d4e5f67890abcdef1234567890'; + const validSignature = Buffer.alloc(64).toString('base64'); + const futureExpiry = new Date(Date.now() + 5 * 60 * 1000).toISOString(); + const defaultNonceRecord = { id: 'nonce-uuid', expires_at: futureExpiry }; + const validDto = { wallet: validWallet, nonce: validNonce, signature: validSignature }; + + it('parallel double-verify yields exactly one success (atomic claim)', async () => { + const mockKeypair = { verify: jest.fn().mockReturnValue(true) }; + (Keypair.fromPublicKey as jest.Mock).mockReturnValue(mockKeypair); + (StrKey.isValidEd25519PublicKey as jest.Mock).mockReturnValue(true); + + let claimAttempts = 0; + mockFrom.mockImplementation((table: string) => { + if (table === 'nonces') { + const builder: Record = { + select: jest.fn(), + eq: jest.fn(), + is: jest.fn(), + single: jest.fn().mockResolvedValue({ data: defaultNonceRecord, error: null }), + update: jest.fn(), + }; + let op: 'select' | 'update' = 'select'; + builder.select.mockImplementation((..._args: unknown[]) => { + if (op === 'update') { + claimAttempts += 1; + if (claimAttempts === 1) { + return Promise.resolve({ data: [{ id: 'nonce-uuid' }], error: null, count: 1 }); + } + return Promise.resolve({ data: [], error: null, count: 0 }); + } + return builder; + }); + builder.update.mockImplementation(() => { + op = 'update'; + return builder; + }); + builder.eq.mockReturnValue(builder); + builder.is.mockReturnValue(builder); + return builder; + } + return { insert: mockInsert }; + }); + + const results = await Promise.allSettled([ + service.verifySignature(validDto), + service.verifySignature(validDto), + ]); + + const fulfilled = results.filter((r) => r.status === 'fulfilled'); + const rejected = results.filter((r) => r.status === 'rejected'); + + expect(fulfilled).toHaveLength(1); + expect(rejected).toHaveLength(1); + const rejectedReason = (rejected[0] as PromiseRejectedResult).reason as { response: { code: string } }; + expect(rejectedReason.response.code).toBe('AUTH_NONCE_NOT_FOUND'); + }); + + it('replay after success fails with AUTH_NONCE_NOT_FOUND', async () => { + const mockKeypair = { verify: jest.fn().mockReturnValue(true) }; + (Keypair.fromPublicKey as jest.Mock).mockReturnValue(mockKeypair); + (StrKey.isValidEd25519PublicKey as jest.Mock).mockReturnValue(true); + + // First call: both SELECT and CLAIM succeed + mockFrom.mockImplementation((table: string) => { + if (table === 'nonces') { + const builder: Record = { + select: jest.fn(), + eq: jest.fn(), + is: jest.fn(), + single: jest.fn().mockResolvedValue({ data: defaultNonceRecord, error: null }), + update: jest.fn(), + }; + let op: 'select' | 'update' = 'select'; + builder.select.mockImplementation((..._args: unknown[]) => { + if (op === 'update') return Promise.resolve({ data: [{ id: 'nonce-uuid' }], error: null, count: 1 }); + return builder; + }); + builder.update.mockImplementation(() => { + op = 'update'; + return builder; + }); + builder.eq.mockReturnValue(builder); + builder.is.mockReturnValue(builder); + return builder; + } + return { insert: mockInsert }; + }); + + await expect(service.verifySignature(validDto)).resolves.toBeUndefined(); + + // Second call: nonce already used — SELECT returns no rows (or CLAIM count 0) + mockFrom.mockImplementation((table: string) => { + if (table === 'nonces') { + const builder: Record = { + select: jest.fn(), + eq: jest.fn(), + is: jest.fn(), + single: jest.fn().mockResolvedValue({ data: null, error: { message: 'No rows found' } }), + update: jest.fn(), + }; + builder.select.mockReturnValue(builder); + builder.eq.mockReturnValue(builder); + builder.is.mockReturnValue(builder); + builder.update.mockReturnValue(builder); + return builder; + } + return { insert: mockInsert }; + }); + + await expect(service.verifySignature(validDto)).rejects.toMatchObject({ + response: { code: 'AUTH_NONCE_NOT_FOUND' }, + }); + }); + + it('replay during failure burns the nonce (invalid signature leaves it consumed)', async () => { + const mockKeypair = { verify: jest.fn().mockReturnValue(false) }; + (Keypair.fromPublicKey as jest.Mock).mockReturnValue(mockKeypair); + (StrKey.isValidEd25519PublicKey as jest.Mock).mockReturnValue(true); + + let firstClaim = true; + mockFrom.mockImplementation((table: string) => { + if (table === 'nonces') { + const builder: Record = { + select: jest.fn(), + eq: jest.fn(), + is: jest.fn(), + single: jest.fn().mockResolvedValue({ data: defaultNonceRecord, error: null }), + update: jest.fn(), + }; + let op: 'select' | 'update' = 'select'; + builder.select.mockImplementation((..._args: unknown[]) => { + if (op === 'update') { + if (firstClaim) { + firstClaim = false; + return Promise.resolve({ data: [{ id: 'nonce-uuid' }], error: null, count: 1 }); + } + return Promise.resolve({ data: [], error: null, count: 0 }); + } + return builder; + }); + builder.update.mockImplementation(() => { + op = 'update'; + return builder; + }); + builder.eq.mockReturnValue(builder); + builder.is.mockReturnValue(builder); + return builder; + } + return { insert: mockInsert }; + }); + + await expect(service.verifySignature(validDto)).rejects.toMatchObject({ + response: { code: 'AUTH_SIGNATURE_INVALID' }, + }); + + // Even with a now-valid signature, replay should fail because nonce was burned + mockKeypair.verify.mockReturnValue(true); + await expect(service.verifySignature(validDto)).rejects.toMatchObject({ + response: { code: 'AUTH_NONCE_NOT_FOUND' }, + }); + }); + + it('expired nonce is rejected and stays burned', async () => { + const expiredRecord = { id: 'nonce-uuid', expires_at: new Date(Date.now() - 1000).toISOString() }; + const mockKeypair = { verify: jest.fn().mockReturnValue(true) }; + (Keypair.fromPublicKey as jest.Mock).mockReturnValue(mockKeypair); + (StrKey.isValidEd25519PublicKey as jest.Mock).mockReturnValue(true); + + let claimOnce = true; + mockFrom.mockImplementation((table: string) => { + if (table === 'nonces') { + const builder: Record = { + select: jest.fn(), + eq: jest.fn(), + is: jest.fn(), + single: jest.fn().mockResolvedValue({ data: expiredRecord, error: null }), + update: jest.fn(), + }; + let op: 'select' | 'update' = 'select'; + builder.select.mockImplementation((..._args: unknown[]) => { + if (op === 'update') { + if (claimOnce) { + claimOnce = false; + return Promise.resolve({ data: [{ id: 'nonce-uuid' }], error: null, count: 1 }); + } + return Promise.resolve({ data: [], error: null, count: 0 }); + } + return builder; + }); + builder.update.mockImplementation(() => { + op = 'update'; + return builder; + }); + builder.eq.mockReturnValue(builder); + builder.is.mockReturnValue(builder); + return builder; + } + return { insert: mockInsert }; + }); + + await expect(service.verifySignature(validDto)).rejects.toMatchObject({ + response: { code: 'AUTH_NONCE_EXPIRED' }, + }); + + // Second attempt should be NOT_FOUND because expired nonce was consumed + await expect(service.verifySignature(validDto)).rejects.toMatchObject({ + response: { code: 'AUTH_NONCE_NOT_FOUND' }, + }); + }); }); // --------------------------------------------------------------------------- From 6a52a0d3921309b00f9466ecd5d91cbe23f2311a Mon Sep 17 00:00:00 2001 From: Falujo Adeyemi Date: Fri, 28 Aug 2026 00:45:50 +0000 Subject: [PATCH 2/2] feat: ApiKeyGuard hammers Supabase per request --- context/progress-tracker.md | 8 + src/app.module.ts | 10 +- src/auth/guards/api-key.guard.ts | 149 ++++-- src/modules/vendors/vendors.service.ts | 21 +- test/unit/modules/auth/api-key.guard.spec.ts | 436 ++++++++++++------ .../modules/vendors/vendors.service.spec.ts | 86 ++++ 6 files changed, 542 insertions(+), 168 deletions(-) diff --git a/context/progress-tracker.md b/context/progress-tracker.md index fc29a0a..8858878 100644 --- a/context/progress-tracker.md +++ b/context/progress-tracker.md @@ -39,6 +39,14 @@ pure chore/docs commits). Direct pushes to main must also be logged here. throttler and updated `auth.controller.spec.ts` to mock the guard. `npm run build` and `npm test` green (38 suites, 425 tests). +- Hardened `ApiKeyGuard` hot path (`src/auth/guards/api-key.guard.ts:29`): + - **Cache key records by hash** — `CACHE_MANAGER` (Redis via `cache-manager` + `ioredis`, same pattern as `src/modules/liquidity/liquidity.service.ts:54` and `src/modules/transactions/transactions.service.ts:121`) stores `ApiKeyRecord` under `apikey:record:` (never the raw key) with `60s` TTL. Steady-state vendor traffic now causes ≤1 `SELECT` per TTL per key instead of 2 DB round-trips per request (lookup + unconditional `last_used_at` update). Negative lookups are not cached to avoid polluting the store; enumeration is handled by unified errors. + - **Collapsed `last_used_at` writes** — cache-guarded dirty flag `apikey:last_used:` with `300s` TTL ensures at-most-once-per-5-minutes-per-key DB `UPDATE`, eliminating 1:1 write amplification. Fire-and-forget `maybeUpdateLastUsed()` logs but never blocks the request. + - **Normalized failure responses** — `API_KEY_INVALID`, `API_KEY_INACTIVE`, `API_KEY_EXPIRED`, and missing/malformed headers all map to a single `API_KEY_UNAUTHORIZED` (401) with `message: 'Invalid API key.'`. Server-side `Logger.warn` retains distinct reasons (`hash 8-char prefix`, `keyId`) for forensics, preventing enumeration of revoked vs expired vs nonexistent keys. `API_KEY_INSUFFICIENT_PERMISSIONS` (403) and `API_KEY_RATE_LIMITED` (429) remain distinct. + - **Per-key sliding-window rate limiting** — cache-backed counter `apikey:rate:` with `60s` window and `60` req limit (structured `429` `API_KEY_RATE_LIMITED` via `HttpException`). Wired through the repo's established `CACHE_MANAGER` guard pattern (not a new BullMQ queue), consistent with `ThrottlerGuard` per-wallet limits. Trips and resets with TTL are tested. + - **Revocation invalidation** — `VendorsService.revokeApiKey()` (`src/modules/vendors/vendors.service.ts:636`) now selects `key_hash` alongside `id`, performs the `is_active=false` update, then `await cacheManager.del` for `apikey:record:`, `apikey:rate:`, and `apikey:last_used:`, guaranteeing visibility within one TTL. `VendorsService` now injects `CACHE_MANAGER` (`@Inject(CACHE_MANAGER)`) and `src/app.module.ts:13` registers a global `CacheModule` (`isGlobal: true`) via `getRedisConfig` so `ApiKeyGuard` and `VendorsService` share the same Redis/in-memory store. + - **Tests** — rewrote `test/unit/modules/auth/api-key.guard.spec.ts:7` to assert cache hit avoids DB (mock `select` call counts and `never store full keys`), revocation invalidation (manual `del` then DB re-check), rate-limit trips (`60` → `429`) and resets after TTL, and enumeration uniformity (missing/invalid/inactive/expired all `API_KEY_UNAUTHORIZED`). Updated `test/unit/modules/vendors/vendors.service.spec.ts:14` to provide `CACHE_MANAGER` mock and verify `revokeApiKey` deletes the three cache keys and tolerates cache failures. `npm run build` and `npm test` green (38 suites, 434 tests). + --- ## 2026-08-27 diff --git a/src/app.module.ts b/src/app.module.ts index ef812c6..0104f74 100644 --- a/src/app.module.ts +++ b/src/app.module.ts @@ -1,8 +1,10 @@ import { MiddlewareConsumer, Module, NestModule, OnModuleInit } from '@nestjs/common'; import { APP_GUARD, APP_FILTER, APP_INTERCEPTOR } from '@nestjs/core'; -import { ConfigModule } from '@nestjs/config'; +import { ConfigModule, ConfigService } from '@nestjs/config'; +import { CacheModule } from '@nestjs/cache-manager'; import { ThrottlerModule, ThrottlerGuard } from '@nestjs/throttler'; import { ScheduleModule } from '@nestjs/schedule'; +import { getRedisConfig } from './config/redis.config'; import { SentryModule, SentryGlobalFilter } from '@sentry/nestjs/setup'; import { AuthModule } from './modules/auth/auth.module'; import { HealthModule } from './modules/health/health.module'; @@ -36,6 +38,12 @@ import { AuditInterceptor } from './common/interceptors/audit.interceptor'; @Module({ imports: [ ConfigModule.forRoot({ isGlobal: true }), + CacheModule.registerAsync({ + isGlobal: true, + imports: [ConfigModule], + inject: [ConfigService], + useFactory: getRedisConfig, + }), SentryModule.forRoot(), ScheduleModule.forRoot(), LoggerModule, diff --git a/src/auth/guards/api-key.guard.ts b/src/auth/guards/api-key.guard.ts index b357fad..1ffdaf3 100644 --- a/src/auth/guards/api-key.guard.ts +++ b/src/auth/guards/api-key.guard.ts @@ -4,9 +4,14 @@ import { ExecutionContext, UnauthorizedException, ForbiddenException, + HttpException, + HttpStatus, Inject, + Logger, } from '@nestjs/common'; import { Reflector } from '@nestjs/core'; +import { CACHE_MANAGER } from '@nestjs/cache-manager'; +import { Cache } from 'cache-manager'; import { createHash } from 'crypto'; import { SupabaseService } from '../../database/supabase.client'; import { API_KEY_PERMISSIONS_KEY } from './api-key-permissions.decorator'; @@ -25,9 +30,36 @@ interface ApiKeyRecord { updated_at: string; } +/** + * Cache and rate-limit constants. + * + * - API_KEY_CACHE_TTL: short TTL for key records (≤1 DB lookup per TTL per key in steady state). + * - API_KEY_LAST_USED_TTL: collapse last_used_at writes to at-most-once-per-N-minutes-per-key. + * - API_KEY_RATE_LIMIT_* : per-key sliding-window (counts live in cache-manager, not DB). + */ +const API_KEY_CACHE_TTL_SECONDS = 60; +const API_KEY_LAST_USED_TTL_SECONDS = 300; +const API_KEY_RATE_LIMIT_WINDOW_SECONDS = 60; +const API_KEY_RATE_LIMIT_MAX_REQUESTS = 60; + +function getRecordCacheKey(keyHash: string): string { + return `apikey:record:${keyHash}`; +} + +function getLastUsedCacheKey(keyId: string): string { + return `apikey:last_used:${keyId}`; +} + +function getRateLimitCacheKey(keyId: string): string { + return `apikey:rate:${keyId}`; +} + @Injectable() export class ApiKeyGuard implements CanActivate { + private readonly logger = new Logger(ApiKeyGuard.name); + constructor( + @Inject(CACHE_MANAGER) private readonly cacheManager: Cache, private readonly supabaseService: SupabaseService, private readonly reflector: Reflector, ) {} @@ -41,44 +73,75 @@ export class ApiKeyGuard implements CanActivate { const apiKeyHeader = request.headers['x-api-key']; if (!apiKeyHeader || typeof apiKeyHeader !== 'string') { + this.logger.warn('API key missing or malformed header'); throw new UnauthorizedException({ - code: 'API_KEY_MISSING', - message: 'X-API-Key header is required.', + code: 'API_KEY_UNAUTHORIZED', + message: 'Invalid API key.', }); } const keyHash = createHash('sha256').update(apiKeyHeader).digest('hex'); + const recordCacheKey = getRecordCacheKey(keyHash); - const client = this.supabaseService.getServiceRoleClient(); - const { data, error } = await client - .from('api_keys') - .select('*') - .eq('key_hash', keyHash) - .single(); + let keyRecord: ApiKeyRecord | undefined; - if (error || !data) { - throw new UnauthorizedException({ - code: 'API_KEY_INVALID', - message: 'Invalid API key.', - }); + try { + keyRecord = await this.cacheManager.get(recordCacheKey); + } catch (error) { + this.logger.warn(`API key cache read failed for ${keyHash.slice(0, 8)}...: ${(error as Error).message}`); } - const keyRecord = data as unknown as ApiKeyRecord; + if (!keyRecord) { + const client = this.supabaseService.getServiceRoleClient(); + const { data, error } = await client.from('api_keys').select('*').eq('key_hash', keyHash).single(); + + if (error || !data) { + this.logger.warn( + `API key lookup failed for hash ${keyHash.slice(0, 8)}...: ${error?.message ?? 'not found'}`, + ); + throw new UnauthorizedException({ + code: 'API_KEY_UNAUTHORIZED', + message: 'Invalid API key.', + }); + } + + keyRecord = data as unknown as ApiKeyRecord; + } + // Unified validation: is_active and expires_at both map to the same + // API_KEY_UNAUTHORIZED response to prevent enumeration of revoked vs + // expired vs nonexistent keys. Details are logged server-side only. if (!keyRecord.is_active) { + this.logger.warn(`API key inactive: ${keyRecord.id} (hash ${keyHash.slice(0, 8)}...)`); throw new UnauthorizedException({ - code: 'API_KEY_INACTIVE', - message: 'API key has been revoked.', + code: 'API_KEY_UNAUTHORIZED', + message: 'Invalid API key.', }); } if (keyRecord.expires_at && new Date(keyRecord.expires_at) < new Date()) { + this.logger.warn(`API key expired: ${keyRecord.id} (hash ${keyHash.slice(0, 8)}...)`); throw new UnauthorizedException({ - code: 'API_KEY_EXPIRED', - message: 'API key has expired.', + code: 'API_KEY_UNAUTHORIZED', + message: 'Invalid API key.', }); } + // Cache the validated record for steady-state traffic (≤1 lookup per TTL per key). + // Only cache after successful validation so inactive/expired records are not + // served from cache; revocation explicitly invalidates via VendorsService. + try { + const cached = await this.cacheManager.get(recordCacheKey); + if (!cached) { + await this.cacheManager.set(recordCacheKey, keyRecord, API_KEY_CACHE_TTL_SECONDS); + } + } catch (error) { + this.logger.warn(`API key cache write failed for ${keyRecord.id}: ${(error as Error).message}`); + } + + // Per-key sliding-window rate limit (cache-backed, not DB). + await this.enforceRateLimit(keyRecord.id, keyHash); + const requiredPermissions = this.reflector.get( API_KEY_PERMISSIONS_KEY, context.getHandler(), @@ -95,21 +158,55 @@ export class ApiKeyGuard implements CanActivate { } } - this.updateLastUsed(keyRecord.id); + // Throttled last_used_at: at-most-once-per-N-minutes-per-key. + // Fire-and-forget is intentionally not awaited to avoid adding latency to + // the hot path; errors are logged. + void this.maybeUpdateLastUsed(keyRecord.id); request.apiKey = keyRecord; return true; } - private async updateLastUsed(keyId: string): Promise { + private async enforceRateLimit(keyId: string, keyHash: string): Promise { + const rateKey = getRateLimitCacheKey(keyId); try { + const current = (await this.cacheManager.get(rateKey)) ?? 0; + if (current >= API_KEY_RATE_LIMIT_MAX_REQUESTS) { + this.logger.warn( + `API key rate limited: ${keyId} (hash ${keyHash.slice(0, 8)}...) — ${current}/${API_KEY_RATE_LIMIT_MAX_REQUESTS} per ${API_KEY_RATE_LIMIT_WINDOW_SECONDS}s`, + ); + throw new HttpException( + { + code: 'API_KEY_RATE_LIMITED', + message: 'Too many requests for this API key. Please retry after a short delay.', + }, + HttpStatus.TOO_MANY_REQUESTS, + ); + } + const next = current + 1; + // Sliding window: each increment resets TTL to full window. For a fixed + // window we would preserve the original TTL, but sliding is simpler and + // matches the per-key burst protection needed here. + await this.cacheManager.set(rateKey, next, API_KEY_RATE_LIMIT_WINDOW_SECONDS); + } catch (error) { + if (error instanceof HttpException) throw error; + // Cache failures should not block legitimate traffic; log and allow. + this.logger.warn(`API key rate-limit cache error for ${keyId}: ${(error as Error).message}`); + } + } + + private async maybeUpdateLastUsed(keyId: string): Promise { + const lastUsedKey = getLastUsedCacheKey(keyId); + try { + const flagged = await this.cacheManager.get(lastUsedKey); + if (flagged) { + return; + } const client = this.supabaseService.getServiceRoleClient(); - await client - .from('api_keys') - .update({ last_used_at: new Date().toISOString() }) - .eq('id', keyId); - } catch { - // Fire-and-forget — failure to update last_used_at should not block the request + await client.from('api_keys').update({ last_used_at: new Date().toISOString() }).eq('id', keyId); + await this.cacheManager.set(lastUsedKey, true, API_KEY_LAST_USED_TTL_SECONDS); + } catch (error) { + this.logger.warn(`Failed to update last_used_at for ${keyId}: ${(error as Error).message}`); } } } diff --git a/src/modules/vendors/vendors.service.ts b/src/modules/vendors/vendors.service.ts index ce947ed..b1fa712 100644 --- a/src/modules/vendors/vendors.service.ts +++ b/src/modules/vendors/vendors.service.ts @@ -5,7 +5,10 @@ import { ConflictException, ForbiddenException, InternalServerErrorException, + Inject, } from '@nestjs/common'; +import { CACHE_MANAGER } from '@nestjs/cache-manager'; +import { Cache } from 'cache-manager'; import { createHash, randomBytes } from 'crypto'; import { SupabaseService } from '../../database/supabase.client'; import { VendorsRepository, VendorDetailRecord } from '../../database/repositories/vendors.repository'; @@ -105,6 +108,7 @@ export class VendorsService { private readonly supabaseService: SupabaseService, private readonly vendorsRepository: VendorsRepository, private readonly vendorRegistryClient: VendorRegistryContractClient, + @Inject(CACHE_MANAGER) private readonly cacheManager: Cache, ) {} async getAll(type?: VendorType): Promise { @@ -641,7 +645,7 @@ export class VendorsService { const client = this.supabaseService.getServiceRoleClient(); const { data: existing, error: fetchError } = await client .from('api_keys') - .select('id') + .select('id, key_hash') .eq('id', keyId) .eq('vendor_id', vendor.id) .single(); @@ -665,6 +669,21 @@ export class VendorsService { message: 'Failed to revoke API key.', }); } + + // Invalidate cached key record so the next request hits the DB and + // observes is_active = false within one TTL. Never store full keys; + // cache is keyed by hash. + try { + const row = existing as unknown as { key_hash: string }; + const keyHash = row.key_hash; + if (keyHash) { + await this.cacheManager.del(`apikey:record:${keyHash}`); + } + await this.cacheManager.del(`apikey:rate:${keyId}`); + await this.cacheManager.del(`apikey:last_used:${keyId}`); + } catch (error) { + this.logger.warn(`Failed to invalidate API key cache for ${keyId}: ${(error as Error).message}`); + } } private mapToDto(data: VendorRow): VendorResponseDto { diff --git a/test/unit/modules/auth/api-key.guard.spec.ts b/test/unit/modules/auth/api-key.guard.spec.ts index 113c043..b68d439 100644 --- a/test/unit/modules/auth/api-key.guard.spec.ts +++ b/test/unit/modules/auth/api-key.guard.spec.ts @@ -1,6 +1,8 @@ import { Test, TestingModule } from '@nestjs/testing'; -import { UnauthorizedException, ForbiddenException } from '@nestjs/common'; +import { CACHE_MANAGER } from '@nestjs/cache-manager'; +import { UnauthorizedException, ForbiddenException, HttpException } from '@nestjs/common'; import { Reflector } from '@nestjs/core'; +import { Cache } from 'cache-manager'; import { ApiKeyGuard } from '../../../../src/auth/guards/api-key.guard'; import { SupabaseService } from '../../../../src/database/supabase.client'; @@ -9,7 +11,17 @@ describe('ApiKeyGuard', () => { let mockSupabaseClient: Record; let mockSupabaseService: { getServiceRoleClient: jest.Mock }; let mockReflector: { get: jest.Mock }; - let mockContext: any; + let mockCacheManager: { + get: jest.Mock; + set: jest.Mock; + del: jest.Mock; + store: Map; + }; + let mockContext: { + switchToHttp: jest.Mock; + getRequest: jest.Mock; + getHandler: jest.Mock; + }; const activeKeyRecord = { id: 'key-uuid', @@ -27,10 +39,51 @@ describe('ApiKeyGuard', () => { const validApiKey = 'sfi_' + 'a'.repeat(64); + function createMockCache(): typeof mockCacheManager { + const store = new Map(); + return { + store, + get: jest.fn(async (key: string) => { + const entry = store.get(key); + if (!entry) return undefined; + if (entry.expiresAt && Date.now() > entry.expiresAt) { + store.delete(key); + return undefined; + } + return entry.value; + }), + set: jest.fn(async (key: string, value: unknown, ttlSeconds?: number) => { + const expiresAt = ttlSeconds ? Date.now() + ttlSeconds * 1000 : undefined; + store.set(key, { value, expiresAt }); + }), + del: jest.fn(async (key: string) => { + store.delete(key); + }), + }; + } + + function createSupabaseMock(result: { data: unknown; error: unknown }) { + const singleFn = jest.fn().mockResolvedValue(result); + const eqFn = jest.fn().mockReturnValue({ single: singleFn }); + const selectFn = jest.fn().mockReturnValue({ eq: eqFn }); + const updateEqFn = jest.fn().mockResolvedValue({ error: null }); + const updateFn = jest.fn().mockReturnValue({ eq: updateEqFn }); + mockSupabaseClient.from.mockImplementation((table: string) => { + if (table === 'api_keys') { + return { + select: selectFn, + update: updateFn, + }; + } + return { select: selectFn, update: updateFn } as unknown as ReturnType; + }); + return { singleFn, eqFn, selectFn, updateFn, updateEqFn }; + } + beforeEach(async () => { mockSupabaseClient = { from: jest.fn(), - }; + } as unknown as Record; mockSupabaseService = { getServiceRoleClient: jest.fn(() => mockSupabaseClient), @@ -40,11 +93,14 @@ describe('ApiKeyGuard', () => { get: jest.fn().mockReturnValue(null), }; + mockCacheManager = createMockCache(); + const module: TestingModule = await Test.createTestingModule({ providers: [ ApiKeyGuard, { provide: SupabaseService, useValue: mockSupabaseService }, { provide: Reflector, useValue: mockReflector }, + { provide: CACHE_MANAGER, useValue: mockCacheManager }, ], }).compile(); @@ -53,273 +109,373 @@ describe('ApiKeyGuard', () => { mockContext = { switchToHttp: jest.fn().mockReturnThis(), getRequest: jest.fn(), - getHandler: jest.fn(), - }; + getHandler: jest.fn().mockReturnValue(() => {}), + } as unknown as typeof mockContext; }); afterEach(() => { jest.clearAllMocks(); }); + function setupRequest(headers: Record) { + const request: Record = { headers }; + mockContext.switchToHttp.mockReturnValue({ + getRequest: jest.fn().mockReturnValue(request), + }); + return request as { headers: Record; apiKey?: unknown }; + } + // --------------------------------------------------------------------------- // canActivate — basic scenarios // --------------------------------------------------------------------------- - describe('canActivate', () => { - function setupRequest(headers: Record) { - mockContext.switchToHttp.mockReturnValue({ - getRequest: jest.fn().mockReturnValue({ headers }), - }); - } - - function setupDbQuery(result: { data: any; error: any }) { - const singleFn = jest.fn().mockResolvedValue(result); - const eqFn = jest.fn().mockReturnValue({ single: singleFn }); - const selectFn = jest.fn().mockReturnValue({ eq: eqFn }); - const updateFn = jest.fn().mockResolvedValue({ error: null }); - const updateEqFn = jest.fn().mockReturnValue(updateFn); - - mockSupabaseClient.from.mockReturnValue({ - select: selectFn, - update: jest.fn().mockReturnValue({ eq: updateEqFn }), - }); - } - + describe('canActivate — basic and enumeration uniformity', () => { it('should return true when X-API-Key is valid and active', async () => { setupRequest({ 'x-api-key': validApiKey }); - setupDbQuery({ data: activeKeyRecord, error: null }); - mockContext.getHandler.mockReturnValue(() => {}); + createSupabaseMock({ data: activeKeyRecord, error: null }); - const result = await guard.canActivate(mockContext); + const result = await guard.canActivate(mockContext as unknown as never); expect(result).toBe(true); }); - it('should throw UnauthorizedException (API_KEY_MISSING) when X-API-Key header is absent', async () => { + it('should throw API_KEY_UNAUTHORIZED when X-API-Key header is absent (normalized)', async () => { setupRequest({}); - await expect(guard.canActivate(mockContext)).rejects.toMatchObject({ - response: { code: 'API_KEY_MISSING' }, + await expect(guard.canActivate(mockContext as unknown as never)).rejects.toMatchObject({ + response: { code: 'API_KEY_UNAUTHORIZED' }, }); }); - it('should throw UnauthorizedException (API_KEY_MISSING) when X-API-Key is not a string', async () => { - setupRequest({ 'x-api-key': ['key1', 'key2'] } as any); + it('should throw API_KEY_UNAUTHORIZED when X-API-Key is not a string (normalized)', async () => { + setupRequest({ 'x-api-key': ['key1', 'key2'] as unknown as string }); - await expect(guard.canActivate(mockContext)).rejects.toMatchObject({ - response: { code: 'API_KEY_MISSING' }, + await expect(guard.canActivate(mockContext as unknown as never)).rejects.toMatchObject({ + response: { code: 'API_KEY_UNAUTHORIZED' }, }); }); - it('should throw UnauthorizedException (API_KEY_INVALID) when key_hash not found', async () => { + it('should throw API_KEY_UNAUTHORIZED when key_hash not found (normalized)', async () => { setupRequest({ 'x-api-key': validApiKey }); - setupDbQuery({ data: null, error: { message: 'No rows found' } }); - mockContext.getHandler.mockReturnValue(() => {}); + createSupabaseMock({ data: null, error: { message: 'No rows found' } }); - await expect(guard.canActivate(mockContext)).rejects.toMatchObject({ - response: { code: 'API_KEY_INVALID' }, + await expect(guard.canActivate(mockContext as unknown as never)).rejects.toMatchObject({ + response: { code: 'API_KEY_UNAUTHORIZED' }, }); }); - it('should throw UnauthorizedException (API_KEY_INVALID) when database error occurs', async () => { + it('should throw API_KEY_UNAUTHORIZED when database error occurs (normalized, logged server-side)', async () => { setupRequest({ 'x-api-key': validApiKey }); - setupDbQuery({ data: null, error: { message: 'DB error' } }); - mockContext.getHandler.mockReturnValue(() => {}); + createSupabaseMock({ data: null, error: { message: 'DB error' } }); - await expect(guard.canActivate(mockContext)).rejects.toMatchObject({ - response: { code: 'API_KEY_INVALID' }, + await expect(guard.canActivate(mockContext as unknown as never)).rejects.toMatchObject({ + response: { code: 'API_KEY_UNAUTHORIZED' }, }); }); - it('should throw UnauthorizedException (API_KEY_INACTIVE) when key is revoked', async () => { + it('should throw API_KEY_UNAUTHORIZED when key is revoked (normalized, was API_KEY_INACTIVE)', async () => { setupRequest({ 'x-api-key': validApiKey }); - setupDbQuery({ + createSupabaseMock({ data: { ...activeKeyRecord, is_active: false }, error: null, }); - mockContext.getHandler.mockReturnValue(() => {}); - await expect(guard.canActivate(mockContext)).rejects.toMatchObject({ - response: { code: 'API_KEY_INACTIVE' }, + await expect(guard.canActivate(mockContext as unknown as never)).rejects.toMatchObject({ + response: { code: 'API_KEY_UNAUTHORIZED' }, }); }); - it('should throw UnauthorizedException (API_KEY_EXPIRED) when key is past expiry', async () => { + it('should throw API_KEY_UNAUTHORIZED when key is past expiry (normalized, was API_KEY_EXPIRED)', async () => { setupRequest({ 'x-api-key': validApiKey }); - setupDbQuery({ + createSupabaseMock({ data: { ...activeKeyRecord, expires_at: new Date(Date.now() - 86400000).toISOString(), }, error: null, }); - mockContext.getHandler.mockReturnValue(() => {}); - await expect(guard.canActivate(mockContext)).rejects.toMatchObject({ - response: { code: 'API_KEY_EXPIRED' }, + await expect(guard.canActivate(mockContext as unknown as never)).rejects.toMatchObject({ + response: { code: 'API_KEY_UNAUTHORIZED' }, }); }); + it('enumeration uniformity: missing, invalid, inactive, expired all yield API_KEY_UNAUTHORIZED', async () => { + const cases: Array<{ headers: Record; db: { data: unknown; error: unknown } | null }> = [ + { headers: {}, db: null }, + { headers: { 'x-api-key': validApiKey }, db: { data: null, error: { message: 'No rows found' } } }, + { + headers: { 'x-api-key': validApiKey }, + db: { data: { ...activeKeyRecord, is_active: false }, error: null }, + }, + { + headers: { 'x-api-key': validApiKey }, + db: { + data: { ...activeKeyRecord, expires_at: new Date(Date.now() - 1000).toISOString() }, + error: null, + }, + }, + ]; + + for (const c of cases) { + // Reset cache between cases to avoid cross-contamination + mockCacheManager.store.clear(); + setupRequest(c.headers); + if (c.db) createSupabaseMock(c.db); + await expect(guard.canActivate(mockContext as unknown as never)).rejects.toMatchObject({ + response: { code: 'API_KEY_UNAUTHORIZED' }, + }); + } + }); + it('should not reject when expiry is in the future', async () => { setupRequest({ 'x-api-key': validApiKey }); - setupDbQuery({ + createSupabaseMock({ data: { ...activeKeyRecord, expires_at: new Date(Date.now() + 86400000).toISOString(), }, error: null, }); - mockContext.getHandler.mockReturnValue(() => {}); - const result = await guard.canActivate(mockContext); + const result = await guard.canActivate(mockContext as unknown as never); expect(result).toBe(true); }); it('should set request.apiKey with the key record', async () => { - const request = { headers: { 'x-api-key': validApiKey } }; - mockContext.switchToHttp.mockReturnValue({ - getRequest: jest.fn().mockReturnValue(request), - }); - setupDbQuery({ data: activeKeyRecord, error: null }); - mockContext.getHandler.mockReturnValue(() => {}); + const request = setupRequest({ 'x-api-key': validApiKey }); + createSupabaseMock({ data: activeKeyRecord, error: null }); - await guard.canActivate(mockContext); + await guard.canActivate(mockContext as unknown as never); expect(request).toHaveProperty('apiKey'); - expect((request as any).apiKey.id).toBe('key-uuid'); + expect((request as unknown as { apiKey: { id: string } }).apiKey.id).toBe('key-uuid'); }); }); // --------------------------------------------------------------------------- - // canActivate — permission enforcement + // Caching — steady state ≤1 lookup per TTL per key // --------------------------------------------------------------------------- - describe('permission enforcement', () => { - function setupRequest(headers: Record) { - mockContext.switchToHttp.mockReturnValue({ - getRequest: jest.fn().mockReturnValue({ headers }), + describe('caching', () => { + it('cache hit avoids DB call (mock assertions)', async () => { + setupRequest({ 'x-api-key': validApiKey }); + const { selectFn } = createSupabaseMock({ data: activeKeyRecord, error: null }); + + // First request hits DB and populates cache + await expect(guard.canActivate(mockContext as unknown as never)).resolves.toBe(true); + expect(selectFn).toHaveBeenCalledTimes(1); + expect(mockCacheManager.set).toHaveBeenCalledWith( + expect.stringContaining('apikey:record:'), + expect.objectContaining({ id: 'key-uuid' }), + expect.any(Number), + ); + + // Second request with same key should hit cache and not hit DB + const request2 = setupRequest({ 'x-api-key': validApiKey }); + // Reset select mock to detect new calls + const secondSelectFn = jest.fn().mockReturnValue({ + eq: jest.fn().mockReturnValue({ single: jest.fn().mockResolvedValue({ data: activeKeyRecord, error: null }) }), }); - } + mockSupabaseClient.from.mockReturnValue({ + select: secondSelectFn, + update: jest.fn().mockReturnValue({ eq: jest.fn().mockResolvedValue({ error: null }) }), + } as unknown as ReturnType); + + await expect(guard.canActivate(mockContext as unknown as never)).resolves.toBe(true); + expect(secondSelectFn).not.toHaveBeenCalled(); + // apiKey still set correctly from cache + expect(request2).toHaveProperty('apiKey'); + }); - function setupDbQuery(result: { data: any; error: any }) { - const singleFn = jest.fn().mockResolvedValue(result); - const eqFn = jest.fn().mockReturnValue({ single: singleFn }); - const selectFn = jest.fn().mockReturnValue({ eq: eqFn }); - const updateFn = jest.fn().mockResolvedValue({ error: null }); - const updateEqFn = jest.fn().mockReturnValue(updateFn); + it('revocation invalidates within one TTL (vendors service deletes cache)', async () => { + // This test documents the contract: VendorsService.revokeApiKey deletes + // `apikey:record:` plus rate/last_used keys. Here we verify the + // guard respects a manual del (simulating revocation). + setupRequest({ 'x-api-key': validApiKey }); + createSupabaseMock({ data: activeKeyRecord, error: null }); + await guard.canActivate(mockContext as unknown as never); + expect(mockCacheManager.store.size).toBeGreaterThan(0); - mockSupabaseClient.from.mockReturnValue({ - select: selectFn, - update: jest.fn().mockReturnValue({ eq: updateEqFn }), + // Simulate revocation invalidation + const hash = require('crypto').createHash('sha256').update(validApiKey).digest('hex'); + await mockCacheManager.del(`apikey:record:${hash}`); + expect(await mockCacheManager.get(`apikey:record:${hash}`)).toBeUndefined(); + + // Next request should miss cache and hit DB again (which will now see inactive) + setupRequest({ 'x-api-key': validApiKey }); + createSupabaseMock({ + data: { ...activeKeyRecord, is_active: false }, + error: null, + }); + await expect(guard.canActivate(mockContext as unknown as never)).rejects.toMatchObject({ + response: { code: 'API_KEY_UNAUTHORIZED' }, }); - } + }); + + it('never stores full keys in cache (only hash-derived keys)', async () => { + setupRequest({ 'x-api-key': validApiKey }); + createSupabaseMock({ data: activeKeyRecord, error: null }); + await guard.canActivate(mockContext as unknown as never); + + const cacheKeys = Array.from(mockCacheManager.store.keys()); + const hasRawKey = cacheKeys.some((k) => k.includes(validApiKey)); + expect(hasRawKey).toBe(false); + const hasHashKey = cacheKeys.some((k) => k.startsWith('apikey:record:')); + expect(hasHashKey).toBe(true); + }); + }); + + // --------------------------------------------------------------------------- + // Rate limiting — per-key sliding window + // --------------------------------------------------------------------------- + describe('rate limiting', () => { + it('rate limit trips and returns structured 429', async () => { + // Pre-fill rate counter to just below limit + const hash = require('crypto').createHash('sha256').update(validApiKey).digest('hex'); + // We need to know the keyId to build rate key; guard uses keyRecord.id + // Simulate 60 requests already counted + const rateKey = `apikey:rate:${activeKeyRecord.id}`; + await mockCacheManager.set(rateKey, 60, 60); + setupRequest({ 'x-api-key': validApiKey }); + createSupabaseMock({ data: activeKeyRecord, error: null }); + + await expect(guard.canActivate(mockContext as unknown as never)).rejects.toMatchObject({ + status: 429, + response: { code: 'API_KEY_RATE_LIMITED' }, + }); + }); + + it('rate limit resets after TTL', async () => { + const rateKey = `apikey:rate:${activeKeyRecord.id}`; + await mockCacheManager.set(rateKey, 60, 1); // 1 second TTL + // Wait for expiry + await new Promise((r) => setTimeout(r, 1100)); + + setupRequest({ 'x-api-key': validApiKey }); + createSupabaseMock({ data: activeKeyRecord, error: null }); + + await expect(guard.canActivate(mockContext as unknown as never)).resolves.toBe(true); + }); + + it('successful requests increment rate counter', async () => { + setupRequest({ 'x-api-key': validApiKey }); + createSupabaseMock({ data: activeKeyRecord, error: null }); + + await guard.canActivate(mockContext as unknown as never); + const rateKey = `apikey:rate:${activeKeyRecord.id}`; + const count = (await mockCacheManager.get(rateKey)) as unknown as number; + expect(count).toBe(1); + + // Second request increments + setupRequest({ 'x-api-key': validApiKey }); + // Need to keep cache for record, but rate key should increment + // guard will hit cache for record, so no DB needed, but we still need mock for DB fallback (should not be called) + await guard.canActivate(mockContext as unknown as never); + const count2 = (await mockCacheManager.get(rateKey)) as unknown as number; + expect(count2).toBe(2); + }); + }); + + // --------------------------------------------------------------------------- + // Permission enforcement + // --------------------------------------------------------------------------- + describe('permission enforcement', () => { it('should pass when required permissions match key permissions', async () => { setupRequest({ 'x-api-key': validApiKey }); - setupDbQuery({ data: activeKeyRecord, error: null }); + createSupabaseMock({ data: activeKeyRecord, error: null }); mockReflector.get.mockReturnValue(['loans:read']); - mockContext.getHandler.mockReturnValue(() => {}); - const result = await guard.canActivate(mockContext); + const result = await guard.canActivate(mockContext as unknown as never); expect(result).toBe(true); }); it('should pass when key has any of the required permissions', async () => { setupRequest({ 'x-api-key': validApiKey }); - setupDbQuery({ data: activeKeyRecord, error: null }); + createSupabaseMock({ data: activeKeyRecord, error: null }); mockReflector.get.mockReturnValue(['loans:write', 'transactions:read']); - mockContext.getHandler.mockReturnValue(() => {}); - const result = await guard.canActivate(mockContext); + const result = await guard.canActivate(mockContext as unknown as never); expect(result).toBe(true); }); it('should throw ForbiddenException (API_KEY_INSUFFICIENT_PERMISSIONS) when key lacks required permissions', async () => { setupRequest({ 'x-api-key': validApiKey }); - setupDbQuery({ data: activeKeyRecord, error: null }); + createSupabaseMock({ data: activeKeyRecord, error: null }); mockReflector.get.mockReturnValue(['admin:write']); - mockContext.getHandler.mockReturnValue(() => {}); - await expect(guard.canActivate(mockContext)).rejects.toMatchObject({ + await expect(guard.canActivate(mockContext as unknown as never)).rejects.toMatchObject({ response: { code: 'API_KEY_INSUFFICIENT_PERMISSIONS' }, }); }); it('should pass when no permissions are required on the endpoint', async () => { setupRequest({ 'x-api-key': validApiKey }); - setupDbQuery({ data: activeKeyRecord, error: null }); + createSupabaseMock({ data: activeKeyRecord, error: null }); mockReflector.get.mockReturnValue(null); - mockContext.getHandler.mockReturnValue(() => {}); - - const result = await guard.canActivate(mockContext); - expect(result).toBe(true); - }); - - it('should pass when required permissions is an empty array', async () => { - setupRequest({ 'x-api-key': validApiKey }); - setupDbQuery({ data: activeKeyRecord, error: null }); - mockReflector.get.mockReturnValue([]); - mockContext.getHandler.mockReturnValue(() => {}); - const result = await guard.canActivate(mockContext); + const result = await guard.canActivate(mockContext as unknown as never); expect(result).toBe(true); }); }); // --------------------------------------------------------------------------- - // last_used_at update (fire-and-forget) + // last_used_at update — throttled to at-most-once-per-5-minutes-per-key // --------------------------------------------------------------------------- - describe('last_used_at update', () => { - function setupRequest(headers: Record) { - mockContext.switchToHttp.mockReturnValue({ - getRequest: jest.fn().mockReturnValue({ headers }), - }); - } - - it('should update last_used_at on successful authentication', async () => { + describe('last_used_at throttling', () => { + it('should trigger last_used_at update on first successful authentication', async () => { setupRequest({ 'x-api-key': validApiKey }); + const { updateFn } = createSupabaseMock({ data: activeKeyRecord, error: null }); + + await guard.canActivate(mockContext as unknown as never); + // Allow fire-and-forget to complete + await new Promise((r) => setImmediate(r)); + + // update should have been called once (plus the rate-limit cache, but we check from mock) + // The mock's from was called for select and for update + expect(mockSupabaseClient.from).toHaveBeenCalledWith('api_keys'); + // We can verify that update was attempted by checking the mock's call count + // The updateFn is for last_used_at + expect(updateFn).toHaveBeenCalled(); + }); - const singleFn = jest.fn().mockResolvedValue({ data: activeKeyRecord, error: null }); - const eqFn = jest.fn().mockReturnValue({ single: singleFn }); - const selectFn = jest.fn().mockReturnValue({ eq: eqFn }); - const updateEqFn = jest.fn().mockResolvedValue({ error: null }); - const updateFn = jest.fn().mockReturnValue({ eq: updateEqFn }); - - mockSupabaseClient.from.mockImplementation((table: string) => { - if (table === 'api_keys') { - return { - select: selectFn, - update: jest.fn().mockReturnValue({ eq: updateEqFn }), - }; - } - return { insert: jest.fn() }; - }); + it('should collapse last_used_at writes within TTL (second request does not hit DB for update)', async () => { + setupRequest({ 'x-api-key': validApiKey }); + createSupabaseMock({ data: activeKeyRecord, error: null }); + await guard.canActivate(mockContext as unknown as never); + await new Promise((r) => setImmediate(r)); - mockContext.getHandler.mockReturnValue(() => {}); + const callCountAfterFirst = mockSupabaseClient.from.mock.calls.length; - await guard.canActivate(mockContext); - expect(mockSupabaseService.getServiceRoleClient).toHaveBeenCalledTimes(2); + // Second request should hit cache for record and skip last_used update due to dirty flag + setupRequest({ 'x-api-key': validApiKey }); + // Keep cache, so DB not hit for record; but we need to ensure update not called again + await guard.canActivate(mockContext as unknown as never); + await new Promise((r) => setImmediate(r)); + + // from should not have been called again for update (only maybe for select if cache miss, but we have cache hit) + // So call count should not increase by 1 for update + // Since we use cache hit for record, no DB lookup, and last_used is throttled, no update + expect(mockSupabaseClient.from.mock.calls.length).toBe(callCountAfterFirst); }); - it('should not throw when last_used_at update fails (fire-and-forget)', async () => { + it('should not throw when last_used_at update fails', async () => { setupRequest({ 'x-api-key': validApiKey }); - const singleFn = jest.fn().mockResolvedValue({ data: activeKeyRecord, error: null }); const eqFn = jest.fn().mockReturnValue({ single: singleFn }); const selectFn = jest.fn().mockReturnValue({ eq: eqFn }); const updateEqFn = jest.fn().mockRejectedValue(new Error('Network error')); - const updateFn = jest.fn().mockReturnValue({ eq: updateEqFn }); - mockSupabaseClient.from.mockImplementation((table: string) => { if (table === 'api_keys') { return { select: selectFn, update: jest.fn().mockReturnValue({ eq: updateEqFn }), - }; + } as unknown as ReturnType; } - return { insert: jest.fn() }; + return { select: selectFn } as unknown as ReturnType; }); - mockContext.getHandler.mockReturnValue(() => {}); - - const result = await guard.canActivate(mockContext); + const result = await guard.canActivate(mockContext as unknown as never); expect(result).toBe(true); + await new Promise((r) => setImmediate(r)); + // Still returns true despite update failure }); }); }); diff --git a/test/unit/modules/vendors/vendors.service.spec.ts b/test/unit/modules/vendors/vendors.service.spec.ts index fd802bc..cd9de59 100644 --- a/test/unit/modules/vendors/vendors.service.spec.ts +++ b/test/unit/modules/vendors/vendors.service.spec.ts @@ -1,4 +1,5 @@ import { Test, TestingModule } from '@nestjs/testing'; +import { CACHE_MANAGER } from '@nestjs/cache-manager'; import { ConflictException, NotFoundException, ForbiddenException, UnauthorizedException, ExecutionContext } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; import { VendorsService } from '../../../../src/modules/vendors/vendors.service'; @@ -42,6 +43,7 @@ describe('VendorsModule', () => { verified: true, }; + let mockCacheManager: { get: jest.Mock; set: jest.Mock; del: jest.Mock }; beforeEach(async () => { const mockQueryBuilder = { select: jest.fn().mockReturnThis(), @@ -49,6 +51,9 @@ describe('VendorsModule', () => { order: jest.fn().mockReturnThis(), single: jest.fn(), update: jest.fn().mockReturnThis(), + delete: jest.fn().mockReturnThis(), + in: jest.fn().mockReturnThis(), + range: jest.fn().mockReturnThis(), }; mockSupabaseService = { @@ -77,6 +82,12 @@ describe('VendorsModule', () => { }), }; + mockCacheManager = { + get: jest.fn().mockResolvedValue(undefined), + set: jest.fn().mockResolvedValue(undefined), + del: jest.fn().mockResolvedValue(undefined), + }; + const module: TestingModule = await Test.createTestingModule({ controllers: [VendorsController], providers: [ @@ -88,6 +99,7 @@ describe('VendorsModule', () => { { provide: VendorsRepository, useValue: { findByWallet: jest.fn() } }, { provide: VendorRegistryContractClient, useValue: mockVendorRegistryClient }, { provide: ConfigService, useValue: mockConfigService }, + { provide: CACHE_MANAGER, useValue: mockCacheManager }, ], }).compile(); @@ -230,4 +242,78 @@ describe('VendorsModule', () => { expect(() => adminGuard.canActivate(ctx)).toThrow(UnauthorizedException); }); }); + + describe('VendorsService.revokeApiKey — cache invalidation', () => { + const mockVendor = { id: mockVendorId, wallet_address: 'GAVENDOR' } as unknown as ReturnType extends Promise ? T : never; + const mockKeyHash = 'abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890'; + + it('should invalidate cached key record and rate/last_used keys on revocation', async () => { + const vendorsRepo = (service as unknown as { vendorsRepository: VendorsRepository }).vendorsRepository as unknown as { findByWallet: jest.Mock }; + vendorsRepo.findByWallet.mockResolvedValue(mockVendor); + + const qb = mockSupabaseService._queryBuilder; + qb.single + .mockResolvedValueOnce({ data: { id: 'key-id', key_hash: mockKeyHash }, error: null }) // fetch existing + .mockResolvedValueOnce({ data: null, error: null }); // not used + + // mock update chain + qb.update.mockReturnValue({ + eq: jest.fn().mockReturnValue({ + eq: jest.fn().mockResolvedValue({ error: null }), + }), + } as unknown as ReturnType); + // Actually our service does: .update({is_active:false}).eq('id', keyId) — single eq + const mockUpdateSingleEq = jest.fn().mockResolvedValue({ error: null }); + mockSupabaseService.getServiceRoleClient.mockReturnValue({ + from: jest.fn().mockImplementation((table: string) => { + if (table === 'api_keys') { + return { + select: jest.fn().mockReturnValue({ + eq: jest.fn().mockReturnValue({ + eq: jest.fn().mockReturnValue({ + single: jest.fn().mockResolvedValue({ data: { id: 'key-id', key_hash: mockKeyHash }, error: null }), + }), + }), + }), + update: jest.fn().mockReturnValue({ eq: mockUpdateSingleEq }), + }; + } + return { select: jest.fn() } as unknown as ReturnType['from']; + }), + } as unknown as typeof mockSupabaseService.getServiceRoleClient extends () => infer R ? R : never); + + await service.revokeApiKey('GAVENDOR', 'key-id'); + + expect(mockCacheManager.del).toHaveBeenCalledWith(`apikey:record:${mockKeyHash}`); + expect(mockCacheManager.del).toHaveBeenCalledWith(`apikey:rate:key-id`); + expect(mockCacheManager.del).toHaveBeenCalledWith(`apikey:last_used:key-id`); + }); + + it('should not throw when cache invalidation fails', async () => { + const vendorsRepo = (service as unknown as { vendorsRepository: VendorsRepository }).vendorsRepository as unknown as { findByWallet: jest.Mock }; + vendorsRepo.findByWallet.mockResolvedValue(mockVendor); + + mockCacheManager.del.mockRejectedValueOnce(new Error('Redis down')); + + mockSupabaseService.getServiceRoleClient.mockReturnValue({ + from: jest.fn().mockImplementation((table: string) => { + if (table === 'api_keys') { + return { + select: jest.fn().mockReturnValue({ + eq: jest.fn().mockReturnValue({ + eq: jest.fn().mockReturnValue({ + single: jest.fn().mockResolvedValue({ data: { id: 'key-id', key_hash: mockKeyHash }, error: null }), + }), + }), + }), + update: jest.fn().mockReturnValue({ eq: jest.fn().mockResolvedValue({ error: null }) }), + }; + } + return { select: jest.fn() } as never; + }), + } as never); + + await expect(service.revokeApiKey('GAVENDOR', 'key-id')).resolves.toBeUndefined(); + }); + }); });