diff --git a/backend/.env.example b/backend/.env.example index b0fbc725..8b1b175d 100644 --- a/backend/.env.example +++ b/backend/.env.example @@ -190,7 +190,12 @@ PAYMENT_RAIL_FAILOVER= # Payments confirmation pipeline (issue #1571) # Shared secret for verifying the sandbox rail's webhook HMAC signature +# (computed over `timestamp + '.' + rawBody`, issue #1938). PAYMENT_WEBHOOK_SECRET=change-me-in-every-environment +# Replay-protection window (seconds) for signed webhook timestamps. A +# delivery older than this is rejected and logged as `replay`. Defaults to +# 300 (5 minutes) when unset or invalid. +PAYMENT_WEBHOOK_TOLERANCE_SECONDS=300 # Bounded timeout (ms) for the synchronous verify-on-return fast path — # a timeout falls back to the webhook/real-time channel, it never marks a # payment CONFIRMED on its own. diff --git a/backend/src/admin-audit/admin-action-log.entity.ts b/backend/src/admin-audit/admin-action-log.entity.ts index 6aef066c..a73a968d 100644 --- a/backend/src/admin-audit/admin-action-log.entity.ts +++ b/backend/src/admin-audit/admin-action-log.entity.ts @@ -36,6 +36,14 @@ export class AdminActionLog { @Column({ type: 'text', nullable: true }) detail: string | null; + /** Caller IP captured from the request context, where one was available. */ + @Column({ type: 'varchar', length: 64, nullable: true }) + ip: string | null; + + /** Caller User-Agent captured from the request context. */ + @Column({ type: 'varchar', length: 512, name: 'user_agent', nullable: true }) + userAgent: string | null; + @CreateDateColumn({ name: 'created_at' }) createdAt: Date; } diff --git a/backend/src/admin-audit/admin-action-log.service.spec.ts b/backend/src/admin-audit/admin-action-log.service.spec.ts index 0c8ae9df..572dfe28 100644 --- a/backend/src/admin-audit/admin-action-log.service.spec.ts +++ b/backend/src/admin-audit/admin-action-log.service.spec.ts @@ -1,5 +1,7 @@ import { AdminActionLogService } from './admin-action-log.service'; import { AdminActionType } from './admin-action-type.enum'; +import { runWithRequestContext } from '../common/request-context'; +import { Between, LessThanOrEqual, MoreThanOrEqual } from 'typeorm'; function makeLogRepository(seed: any[] = []) { const rows = [...seed]; @@ -43,6 +45,99 @@ describe('AdminActionLogService', () => { expect(log.detail).toBe('DISTRIBUTION'); }); + it('captures IP and User-Agent from the active request context', async () => { + const repo = makeLogRepository(); + const service = new AdminActionLogService(repo as any); + + const log = await runWithRequestContext( + { requestId: 'req-1', ip: '203.0.113.7', userAgent: 'jest-agent' }, + () => + service.record({ + actorId: 'actor-1', + action: AdminActionType.PAYMENT_VOID, + targetType: 'Payment', + }), + ); + + expect(log.ip).toBe('203.0.113.7'); + expect(log.userAgent).toBe('jest-agent'); + }); + + it('stores explicit metadata over the request context', async () => { + const repo = makeLogRepository(); + const service = new AdminActionLogService(repo as any); + + const log = await runWithRequestContext( + { requestId: 'req-1', ip: '203.0.113.7', userAgent: 'context-agent' }, + () => + service.record({ + actorId: 'actor-1', + action: AdminActionType.PAYMENT_VOID, + targetType: 'Payment', + ip: '198.51.100.4', + userAgent: 'explicit-agent', + }), + ); + + expect(log.ip).toBe('198.51.100.4'); + expect(log.userAgent).toBe('explicit-agent'); + }); + + it('records null metadata outside a request context', async () => { + const repo = makeLogRepository(); + const service = new AdminActionLogService(repo as any); + + const log = await service.record({ + actorId: 'actor-1', + action: AdminActionType.PAYMENT_VOID, + targetType: 'Payment', + }); + + expect(log.ip).toBeNull(); + expect(log.userAgent).toBeNull(); + }); + + it('caps and range-filters the export query', async () => { + const repo = makeLogRepository(); + const service = new AdminActionLogService(repo as any); + const from = new Date('2025-01-01T00:00:00Z'); + const to = new Date('2025-02-01T00:00:00Z'); + + await service.listForExport({ + from, + to, + action: AdminActionType.PAYMENT_VOID, + limit: 10000, + }); + + expect(repo.find).toHaveBeenCalledWith({ + where: { action: 'payment_void', createdAt: Between(from, to) }, + order: { createdAt: 'DESC' }, + take: 10000, + }); + }); + + it('supports an open-ended export range', async () => { + const repo = makeLogRepository(); + const service = new AdminActionLogService(repo as any); + const from = new Date('2025-01-01T00:00:00Z'); + + await service.listForExport({ from, limit: 50 }); + expect(repo.find).toHaveBeenLastCalledWith({ + where: { createdAt: MoreThanOrEqual(from) }, + order: { createdAt: 'DESC' }, + take: 50, + }); + + const to = new Date('2025-02-01T00:00:00Z'); + await service.listForExport({ to, limit: 50 }); + expect(repo.find).toHaveBeenLastCalledWith({ + where: { createdAt: LessThanOrEqual(to) }, + order: { createdAt: 'DESC' }, + take: 50, + }); + }); + it('lists logs newest first', async () => { const repo = makeLogRepository([ { id: 'a', action: 'payment_void', createdAt: new Date('2025-01-01T00:00:00Z') }, diff --git a/backend/src/admin-audit/admin-action-log.service.ts b/backend/src/admin-audit/admin-action-log.service.ts index 0ebcb948..26be1e96 100644 --- a/backend/src/admin-audit/admin-action-log.service.ts +++ b/backend/src/admin-audit/admin-action-log.service.ts @@ -1,8 +1,15 @@ import { Injectable } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; -import { Repository } from 'typeorm'; +import { + Between, + FindOptionsWhere, + LessThanOrEqual, + MoreThanOrEqual, + Repository, +} from 'typeorm'; import { AdminActionLog } from './admin-action-log.entity'; import { AdminActionType } from './admin-action-type.enum'; +import { currentRequestMetadata } from '../common/request-context'; export interface RecordAdminActionInput { actorId: string; @@ -10,6 +17,20 @@ export interface RecordAdminActionInput { targetType: string; targetId?: string | null; detail?: string | null; + /** + * Explicit forensic metadata. When omitted the value is taken from the + * active request context (populated by RequestContextMiddleware), so + * controllers do not have to thread the request through every call. + */ + ip?: string | null; + userAgent?: string | null; +} + +export interface ListForExportQuery { + from?: Date; + to?: Date; + action?: AdminActionType; + limit: number; } @Injectable() @@ -20,12 +41,15 @@ export class AdminActionLogService { ) {} async record(input: RecordAdminActionInput): Promise { + const requestMetadata = currentRequestMetadata(); const entry = this.logRepository.create({ actorId: input.actorId, action: input.action, targetType: input.targetType, targetId: input.targetId ?? null, detail: input.detail ?? null, + ip: input.ip ?? requestMetadata.ip, + userAgent: input.userAgent ?? requestMetadata.userAgent, }); return this.logRepository.save(entry); } @@ -37,6 +61,32 @@ export class AdminActionLogService { }); } + /** + * Newest-first slice used by the CSV export. Applies the same action + * filter as `list`, plus an optional inclusive createdAt range and a hard + * row cap so a compliance export cannot be asked to materialise an + * unbounded result. + */ + listForExport(query: ListForExportQuery): Promise { + const where: FindOptionsWhere = {}; + if (query.action) { + where.action = query.action; + } + if (query.from && query.to) { + where.createdAt = Between(query.from, query.to); + } else if (query.from) { + where.createdAt = MoreThanOrEqual(query.from); + } else if (query.to) { + where.createdAt = LessThanOrEqual(query.to); + } + + return this.logRepository.find({ + where, + order: { createdAt: 'DESC' }, + take: query.limit, + }); + } + async count(): Promise { return this.logRepository.count(); } diff --git a/backend/src/admin-audit/admin-action-type.enum.ts b/backend/src/admin-audit/admin-action-type.enum.ts index 2c4546f7..dbb95884 100644 --- a/backend/src/admin-audit/admin-action-type.enum.ts +++ b/backend/src/admin-audit/admin-action-type.enum.ts @@ -68,4 +68,13 @@ export enum AdminActionType { * is the Payment id and `detail` is the required operator reason. */ PAYMENT_VOID = 'payment_void', + + /** + * Recorded by `GET /admin/audit/actions/export` when an operator exports + * the audit trail as CSV. The target is `AdminActionLog`, `targetId` is + * null (the export is not scoped to one entry), and `detail` is + * `exported=` so the audit trail records that the trail itself + * was read. + */ + AUDIT_EXPORT = 'audit_export', } diff --git a/backend/src/admin-audit/admin-audit-export.util.spec.ts b/backend/src/admin-audit/admin-audit-export.util.spec.ts new file mode 100644 index 00000000..43bda236 --- /dev/null +++ b/backend/src/admin-audit/admin-audit-export.util.spec.ts @@ -0,0 +1,73 @@ +import { + ADMIN_AUDIT_CSV_COLUMNS, + AdminAuditEntry, + escapeCsvValue, + exportAdminAuditToCsv, +} from './admin-audit-export.util'; + +function makeEntry(overrides: Partial = {}): AdminAuditEntry { + return { + id: 'log-1', + actorId: 'actor-1', + action: 'payment_void', + targetType: 'Payment', + targetId: 'payment-1', + detail: 'reason', + ip: '203.0.113.7', + userAgent: 'jest-agent', + createdAt: new Date('2025-01-01T00:00:00.000Z'), + ...overrides, + }; +} + +describe('escapeCsvValue', () => { + it('neutralises formula-prefixed values', () => { + expect(escapeCsvValue('=1+1')).toBe("'=1+1"); + expect(escapeCsvValue('+1')).toBe("'+1"); + expect(escapeCsvValue('-1')).toBe("'-1"); + expect(escapeCsvValue('@SUM(A1)')).toBe("'@SUM(A1)"); + }); + + it('quotes values containing separators or quotes', () => { + expect(escapeCsvValue('a,b')).toBe('"a,b"'); + expect(escapeCsvValue('say "hi"')).toBe('"say ""hi"""'); + expect(escapeCsvValue('line\nbreak')).toBe('"line\nbreak"'); + }); + + it('neutralises then quotes a formula that also contains a comma', () => { + expect(escapeCsvValue('=cmd,quoted')).toBe('"\'=cmd,quoted"'); + }); + + it('renders null and undefined as empty cells', () => { + expect(escapeCsvValue(null)).toBe(''); + expect(escapeCsvValue(undefined)).toBe(''); + }); + + it('leaves ordinary values untouched', () => { + expect(escapeCsvValue('payment_void')).toBe('payment_void'); + }); +}); + +describe('exportAdminAuditToCsv', () => { + it('writes a stable header followed by one row per entry', () => { + const csv = exportAdminAuditToCsv([makeEntry()]); + const lines = csv.split('\n'); + + expect(lines[0]).toBe(ADMIN_AUDIT_CSV_COLUMNS.join(',')); + expect(lines[1]).toBe( + 'log-1,actor-1,payment_void,Payment,payment-1,reason,203.0.113.7,jest-agent,2025-01-01T00:00:00.000Z', + ); + }); + + it('neutralises a formula-prefixed detail in a full row', () => { + const csv = exportAdminAuditToCsv([ + makeEntry({ detail: '=HYPERLINK("evil")' }), + ]); + + expect(csv.split('\n')[1]).toContain('"\'=HYPERLINK(""evil"")"'); + }); + + it('returns just the header for an empty result set', () => { + expect(exportAdminAuditToCsv([])).toBe(ADMIN_AUDIT_CSV_COLUMNS.join(',')); + }); +}); diff --git a/backend/src/admin-audit/admin-audit-export.util.ts b/backend/src/admin-audit/admin-audit-export.util.ts index 6d831626..759077d3 100644 --- a/backend/src/admin-audit/admin-audit-export.util.ts +++ b/backend/src/admin-audit/admin-audit-export.util.ts @@ -1,28 +1,80 @@ -// Builds a CSV export of admin-audit entries for a date range, for -// compliance audits. Formatting helper meant to back an export endpoint -// on admin-audit.controller.ts. +// Builds a CSV export of admin-audit entries for the compliance export +// endpoint on admin-audit.controller.ts. The column order is fixed here so +// the export is stable and machine-parsable. export interface AdminAuditEntry { id: string; - adminId: string; + actorId: string; action: string; + targetType?: string | null; + targetId?: string | null; + detail?: string | null; + ip?: string | null; + userAgent?: string | null; createdAt: Date; } -const CSV_HEADER = 'id,adminId,action,createdAt'; +/** Stable column order shared by the header row and every data row. */ +export const ADMIN_AUDIT_CSV_COLUMNS = [ + 'id', + 'actorId', + 'action', + 'targetType', + 'targetId', + 'detail', + 'ip', + 'userAgent', + 'createdAt', +] as const; -function escapeCsv(value: string): string { - return /[",\n]/.test(value) ? `"${value.replace(/"/g, '""')}"` : value; +/** Hard cap on exported rows so a compliance pull can never be unbounded. */ +export const ADMIN_AUDIT_EXPORT_MAX_ROWS = 10_000; + +const CSV_HEADER = ADMIN_AUDIT_CSV_COLUMNS.join(','); + +// A leading one of these turns a cell into a formula in spreadsheet apps +// (CSV injection). Neutralise it with a leading single quote before the +// normal quoting rules are applied. +const FORMULA_PREFIXES = ['=', '+', '-', '@'] as const; + +function neutraliseFormula(value: string): string { + return FORMULA_PREFIXES.some((prefix) => value.startsWith(prefix)) + ? `'${value}` + : value; +} + +/** Escapes a single CSV cell, neutralising formula-prefixed values. */ +export function escapeCsvValue(value: unknown): string { + const raw = value === null || value === undefined ? '' : String(value); + const escaped = neutraliseFormula(raw); + return /[",\n\r]/.test(escaped) + ? `"${escaped.replace(/"/g, '""')}"` + : escaped; } export function exportAdminAuditToCsv(entries: AdminAuditEntry[]): string { const rows = entries.map((entry) => - [entry.id, entry.adminId, entry.action, entry.createdAt.toISOString()] - .map(escapeCsv) - .join(','), + ADMIN_AUDIT_CSV_COLUMNS.map((column) => { + const value = + column === 'createdAt' ? entry.createdAt.toISOString() : entry[column]; + return escapeCsvValue(value); + }).join(','), ); return [CSV_HEADER, ...rows].join('\n'); } +/** + * Parses an optional ISO date-range query parameter, rejecting anything the + * Date constructor cannot interpret so a malformed range fails loudly at the + * edge instead of silently exporting the wrong window. + */ +export function parseAuditDate(value: string, field: string): Date { + const parsed = new Date(value); + if (Number.isNaN(parsed.getTime())) { + throw new Error(`Invalid "${field}" date: "${value}"`); + } + return parsed; +} + export function filterAdminAuditByDateRange( entries: AdminAuditEntry[], start: Date, diff --git a/backend/src/admin-audit/admin-audit.controller.spec.ts b/backend/src/admin-audit/admin-audit.controller.spec.ts new file mode 100644 index 00000000..c5c123e9 --- /dev/null +++ b/backend/src/admin-audit/admin-audit.controller.spec.ts @@ -0,0 +1,126 @@ +import { BadRequestException } from '@nestjs/common'; +import { AdminAuditController } from './admin-audit.controller'; +import { AdminActionLogService } from './admin-action-log.service'; +import { AdminActionType } from './admin-action-type.enum'; + +function makeLog(overrides: Record = {}) { + return { + id: 'log-1', + actorId: 'actor-1', + action: 'payment_void', + targetType: 'Payment', + targetId: 'payment-1', + detail: 'reason', + ip: '203.0.113.7', + userAgent: 'jest-agent', + createdAt: new Date('2025-01-01T00:00:00.000Z'), + ...overrides, + } as any; +} + +async function readStream(file: { getStream: () => NodeJS.ReadableStream }) { + const chunks: Buffer[] = []; + for await (const chunk of file.getStream() as AsyncIterable) { + chunks.push(Buffer.from(chunk)); + } + return Buffer.concat(chunks).toString('utf8'); +} + +describe('AdminAuditController', () => { + let audit: { + list: jest.Mock; + listForExport: jest.Mock; + record: jest.Mock; + }; + let controller: AdminAuditController; + + beforeEach(() => { + audit = { + list: jest.fn(), + listForExport: jest.fn().mockResolvedValue([]), + record: jest.fn().mockResolvedValue(undefined), + }; + controller = new AdminAuditController(audit as unknown as AdminActionLogService); + }); + + it('lists actions and includes ip/userAgent in the DTO', async () => { + audit.list.mockResolvedValueOnce([makeLog()]); + + const result = await controller.list(); + + expect(result).toHaveLength(1); + expect(result[0].ip).toBe('203.0.113.7'); + expect(result[0].userAgent).toBe('jest-agent'); + }); + + it('exports the filtered rows as CSV in a stable column order', async () => { + audit.listForExport.mockResolvedValueOnce([makeLog()]); + + const file = await controller.export(undefined, undefined, undefined, { + id: 'admin-1', + } as any); + const csv = await readStream(file); + + expect(audit.listForExport).toHaveBeenCalledWith({ + from: undefined, + to: undefined, + action: undefined, + limit: expect.any(Number), + }); + expect(csv.split('\n')[0]).toBe( + 'id,actorId,action,targetType,targetId,detail,ip,userAgent,createdAt', + ); + expect(csv.split('\n')[1]).toContain('log-1,actor-1,payment_void'); + expect(file.getHeaders().type).toBe('text/csv'); + }); + + it('records the export itself as an audit event', async () => { + audit.listForExport.mockResolvedValueOnce([makeLog(), makeLog({ id: 'log-2' })]); + + await controller.export(undefined, undefined, undefined, { + id: 'admin-1', + } as any); + + expect(audit.record).toHaveBeenCalledWith({ + actorId: 'admin-1', + action: AdminActionType.AUDIT_EXPORT, + targetType: 'AdminActionLog', + detail: 'exported=2', + }); + }); + + it('passes the date range and action filter through to the query', async () => { + const from = '2025-01-01T00:00:00.000Z'; + const to = '2025-02-01T00:00:00.000Z'; + + await controller.export(from, to, AdminActionType.PAYMENT_VOID, { + id: 'admin-1', + } as any); + + expect(audit.listForExport).toHaveBeenCalledWith({ + from: new Date(from), + to: new Date(to), + action: AdminActionType.PAYMENT_VOID, + limit: expect.any(Number), + }); + }); + + it('rejects an unparseable date', async () => { + await expect( + controller.export('not-a-date', undefined, undefined, undefined), + ).rejects.toThrow(BadRequestException); + expect(audit.listForExport).not.toHaveBeenCalled(); + }); + + it('rejects a reversed date range', async () => { + await expect( + controller.export( + '2025-02-01T00:00:00.000Z', + '2025-01-01T00:00:00.000Z', + undefined, + undefined, + ), + ).rejects.toThrow(BadRequestException); + expect(audit.listForExport).not.toHaveBeenCalled(); + }); +}); diff --git a/backend/src/admin-audit/admin-audit.controller.ts b/backend/src/admin-audit/admin-audit.controller.ts index f3833782..30331cd1 100644 --- a/backend/src/admin-audit/admin-audit.controller.ts +++ b/backend/src/admin-audit/admin-audit.controller.ts @@ -1,12 +1,33 @@ -import { Controller, Get, Query, UseGuards } from '@nestjs/common'; -import { ApiBearerAuth, ApiOperation, ApiTags } from '@nestjs/swagger'; +import { + BadRequestException, + Controller, + Get, + Query, + StreamableFile, + UseGuards, +} from '@nestjs/common'; +import { + ApiBearerAuth, + ApiOperation, + ApiProduces, + ApiQuery, + ApiTags, +} from '@nestjs/swagger'; +import { Readable } from 'stream'; import { JwtAuthGuard } from '../auth/guards/jwt-auth.guard'; import { RolesGuard } from '../auth/guards/roles.guard'; import { Roles } from '../auth/decorators/roles.decorator'; +import { CurrentUser } from '../auth/decorators/current-user.decorator'; +import { RequestUser } from '../auth/interfaces/authenticated-request.interface'; import { UserRole } from '../auth/enums/user-role.enum'; import { AdminActionLogService } from './admin-action-log.service'; import { AdminActionLogResponseDto } from './dto/admin-action-log.response.dto'; import { AdminActionType } from './admin-action-type.enum'; +import { + ADMIN_AUDIT_EXPORT_MAX_ROWS, + exportAdminAuditToCsv, + parseAuditDate, +} from './admin-audit-export.util'; /** * Read-only surface for the structured admin action audit trail. Every @@ -35,4 +56,70 @@ export class AdminAuditController { const logs = await this.audit.list(action); return logs.map((log) => AdminActionLogResponseDto.fromEntity(log)); } + + @Get('actions/export') + @ApiProduces('text/csv') + @ApiOperation({ + summary: 'Export recorded admin actions as CSV', + description: + `Streams the same rows as GET /admin/audit/actions as a CSV file, ` + + `optionally narrowed to an inclusive [from, to] date range and/or a ` + + `single action type. Capped at ${ADMIN_AUDIT_EXPORT_MAX_ROWS} rows, ` + + `newest first. The export itself is recorded as an audit event.`, + }) + @ApiQuery({ name: 'from', required: false, description: 'Inclusive ISO start' }) + @ApiQuery({ name: 'to', required: false, description: 'Inclusive ISO end' }) + @ApiQuery({ + name: 'action', + required: false, + enum: AdminActionType, + }) + async export( + @Query('from') from?: string, + @Query('to') to?: string, + @Query('action') action?: AdminActionType, + @CurrentUser() currentUser?: RequestUser, + ): Promise { + let fromDate: Date | undefined; + let toDate: Date | undefined; + try { + if (from) { + fromDate = parseAuditDate(from, 'from'); + } + if (to) { + toDate = parseAuditDate(to, 'to'); + } + } catch (error) { + throw new BadRequestException( + error instanceof Error ? error.message : 'Invalid date range', + ); + } + if (fromDate && toDate && fromDate > toDate) { + throw new BadRequestException( + '"from" must be earlier than or equal to "to"', + ); + } + + const logs = await this.audit.listForExport({ + from: fromDate, + to: toDate, + action, + limit: ADMIN_AUDIT_EXPORT_MAX_ROWS, + }); + const csv = exportAdminAuditToCsv(logs); + + if (currentUser) { + await this.audit.record({ + actorId: currentUser.id, + action: AdminActionType.AUDIT_EXPORT, + targetType: 'AdminActionLog', + detail: `exported=${logs.length}`, + }); + } + + return new StreamableFile(Readable.from([csv]), { + type: 'text/csv', + disposition: 'attachment; filename="admin-audit-export.csv"', + }); + } } diff --git a/backend/src/admin-audit/dto/admin-action-log.response.dto.ts b/backend/src/admin-audit/dto/admin-action-log.response.dto.ts index cb7b3926..18aedae5 100644 --- a/backend/src/admin-audit/dto/admin-action-log.response.dto.ts +++ b/backend/src/admin-audit/dto/admin-action-log.response.dto.ts @@ -21,6 +21,12 @@ export class AdminActionLogResponseDto { @ApiProperty({ nullable: true }) detail: string | null; + @ApiProperty({ nullable: true, description: "Caller's IP address" }) + ip: string | null; + + @ApiProperty({ nullable: true, description: "Caller's User-Agent" }) + userAgent: string | null; + @ApiProperty() createdAt: Date; @@ -32,6 +38,8 @@ export class AdminActionLogResponseDto { dto.targetType = log.targetType; dto.targetId = log.targetId; dto.detail = log.detail; + dto.ip = log.ip ?? null; + dto.userAgent = log.userAgent ?? null; dto.createdAt = log.createdAt; return dto; } diff --git a/backend/src/admin-audit/request-metadata.util.spec.ts b/backend/src/admin-audit/request-metadata.util.spec.ts new file mode 100644 index 00000000..fab1d058 --- /dev/null +++ b/backend/src/admin-audit/request-metadata.util.spec.ts @@ -0,0 +1,89 @@ +import { extractRequestMetadata } from './request-metadata.util'; + +interface FakeRequestOptions { + headers?: Record; + ip?: string; + remoteAddress?: string; + trustProxy?: unknown; +} + +function makeRequest({ + headers = {}, + ip, + remoteAddress, + trustProxy = false, +}: FakeRequestOptions): any { + return { + headers, + ip, + socket: remoteAddress ? { remoteAddress } : undefined, + app: { + get: (key: string) => (key === 'trust proxy' ? trustProxy : undefined), + }, + }; +} + +describe('extractRequestMetadata', () => { + it('trusts X-Forwarded-For when the app is behind a trusted proxy', () => { + const req = makeRequest({ + headers: { 'x-forwarded-for': '203.0.113.7', 'user-agent': 'jest-agent' }, + remoteAddress: '10.0.0.1', + trustProxy: true, + }); + + expect(extractRequestMetadata(req)).toEqual({ + ip: '203.0.113.7', + userAgent: 'jest-agent', + }); + }); + + it('takes the first hop from a multi-value X-Forwarded-For header', () => { + const req = makeRequest({ + headers: { 'x-forwarded-for': '203.0.113.7, 10.0.0.9, 10.0.0.1' }, + remoteAddress: '10.0.0.1', + trustProxy: 2, + }); + + expect(extractRequestMetadata(req).ip).toBe('203.0.113.7'); + }); + + it('ignores a spoofed X-Forwarded-For when no proxy is trusted', () => { + const req = makeRequest({ + headers: { 'x-forwarded-for': '203.0.113.7' }, + remoteAddress: '10.0.0.1', + trustProxy: false, + }); + + expect(extractRequestMetadata(req).ip).toBe('10.0.0.1'); + }); + + it("prefers Express's resolved req.ip", () => { + const req = makeRequest({ + headers: { 'x-forwarded-for': '203.0.113.7' }, + ip: '198.51.100.4', + remoteAddress: '10.0.0.1', + trustProxy: true, + }); + + expect(extractRequestMetadata(req).ip).toBe('198.51.100.4'); + }); + + it('falls back to the socket peer address', () => { + const req = makeRequest({ remoteAddress: '10.0.0.1' }); + expect(extractRequestMetadata(req).ip).toBe('10.0.0.1'); + }); + + it('returns nulls when nothing is available', () => { + const req = makeRequest({}); + expect(extractRequestMetadata(req)).toEqual({ ip: null, userAgent: null }); + }); + + it('handles an array user-agent header', () => { + const req = makeRequest({ + headers: { 'user-agent': ['first', 'second'] }, + remoteAddress: '10.0.0.1', + }); + + expect(extractRequestMetadata(req).userAgent).toBe('first'); + }); +}); diff --git a/backend/src/admin-audit/request-metadata.util.ts b/backend/src/admin-audit/request-metadata.util.ts index f5dacaee..9cfdba52 100644 --- a/backend/src/admin-audit/request-metadata.util.ts +++ b/backend/src/admin-audit/request-metadata.util.ts @@ -7,15 +7,57 @@ export interface RequestMetadata { userAgent: string | null; } +/** + * Whether Express is configured to trust a front proxy for this request. + * Only then may the client-supplied `X-Forwarded-For` header be believed; + * otherwise it is attacker-controlled and must be ignored in favour of the + * transport-level peer address. + */ +function isTrustProxyEnabled(req: Request): boolean { + const app = req?.app; + if (!app || typeof app.get !== 'function') { + return false; + } + let setting: unknown; + try { + setting = app.get('trust proxy'); + } catch { + return false; + } + if (setting === true) { + return true; + } + if (typeof setting === 'number') { + return setting > 0; + } + if (typeof setting === 'string') { + return setting !== '' && setting !== 'false'; + } + return false; +} + export function extractRequestMetadata(req: Request): RequestMetadata { - const forwardedFor = req.headers['x-forwarded-for']; + const headers = req?.headers ?? {}; + + const userAgentHeader = headers['user-agent']; + const userAgent = Array.isArray(userAgentHeader) + ? (userAgentHeader[0] ?? null) + : (userAgentHeader ?? null); + + const forwardedFor = headers['x-forwarded-for']; + const forwardedIp = (Array.isArray(forwardedFor) ? forwardedFor[0] : forwardedFor) + ?.split(',')[0] + ?.trim(); + + // `req.ip` is the value Express itself resolved against the app's + // `trust proxy` setting, so it is preferred. `X-Forwarded-For` is only + // consulted when a trusted proxy is configured; a direct request falls + // back to the socket peer address. const ip = - (Array.isArray(forwardedFor) ? forwardedFor[0] : forwardedFor?.split(',')[0]?.trim()) || - req.socket?.remoteAddress || - req.ip || + req?.ip || + (isTrustProxyEnabled(req) ? forwardedIp : null) || + req?.socket?.remoteAddress || null; - const userAgent = req.headers['user-agent'] ?? null; - - return { ip, userAgent: userAgent as string | null }; + return { ip: ip || null, userAgent: userAgent || null }; } diff --git a/backend/src/auth/auth.controller.ts b/backend/src/auth/auth.controller.ts index c2c576a1..f01da287 100644 --- a/backend/src/auth/auth.controller.ts +++ b/backend/src/auth/auth.controller.ts @@ -29,7 +29,7 @@ const COOKIE_OPTIONS: CookieOptions = { secure: process.env.NODE_ENV === 'production', path: '/', maxAge: 7 * 24 * 60 * 60 * 1000, -}; +} as const; @ApiTags('auth') @Controller('auth') diff --git a/backend/src/common/admin-identity.tsx b/backend/src/common/admin-identity.tsx deleted file mode 100644 index 5ba3e707..00000000 --- a/backend/src/common/admin-identity.tsx +++ /dev/null @@ -1,30 +0,0 @@ -'use client'; - -import { useSession } from 'next-auth/react'; - -/** - * Displays the currently logged-in admin's name and email in the header. - * Intended for use inside `app/admin/layout.tsx`. - * Closes #1863 - */ -export function AdminIdentity() { - const { data: session } = useSession(); - const admin = session?.user; - - if (!admin) return null; - - return ( -
- -
-

{admin.name ?? 'Admin'}

-

{admin.email}

-
-
- ); -} diff --git a/backend/src/common/request-context.middleware.ts b/backend/src/common/request-context.middleware.ts index f098dc4b..94e0c0ee 100644 --- a/backend/src/common/request-context.middleware.ts +++ b/backend/src/common/request-context.middleware.ts @@ -2,6 +2,7 @@ import { Injectable, NestMiddleware } from '@nestjs/common'; import { randomUUID } from 'crypto'; import { NextFunction, Request, Response } from 'express'; import { runWithRequestContext } from './request-context'; +import { extractRequestMetadata } from '../admin-audit/request-metadata.util'; const REQUEST_ID_REGEX = /^[A-Za-z0-9._-]{8,64}$/; @@ -18,7 +19,9 @@ export class RequestContextMiddleware implements NestMiddleware { ? incomingRequestId : randomUUID(); + const { ip, userAgent } = extractRequestMetadata(req); + res.setHeader('x-request-id', requestId); - runWithRequestContext({ requestId }, () => next()); + runWithRequestContext({ requestId, ip, userAgent }, () => next()); } } diff --git a/backend/src/common/request-context.spec.ts b/backend/src/common/request-context.spec.ts index baf96936..caaf34fb 100644 --- a/backend/src/common/request-context.spec.ts +++ b/backend/src/common/request-context.spec.ts @@ -1,5 +1,6 @@ import { currentRequestId, + currentRequestMetadata, outboundRequestHeaders, runWithRequestContext, withRequestId, @@ -32,6 +33,19 @@ describe('request context', () => { expect(headers).toEqual({}); }); + it('exposes the captured request metadata inside a request', () => { + const metadata = runWithRequestContext( + { requestId: 'request-789', ip: '203.0.113.7', userAgent: 'jest' }, + () => currentRequestMetadata(), + ); + + expect(metadata).toEqual({ ip: '203.0.113.7', userAgent: 'jest' }); + }); + + it('returns null metadata outside a request context', () => { + expect(currentRequestMetadata()).toEqual({ ip: null, userAgent: null }); + }); + it('keeps the request id available to log formatting', () => { const message = runWithRequestContext( { requestId: 'request-456' }, diff --git a/backend/src/common/request-context.ts b/backend/src/common/request-context.ts index bccd19f8..5359a2b6 100644 --- a/backend/src/common/request-context.ts +++ b/backend/src/common/request-context.ts @@ -2,6 +2,13 @@ import { AsyncLocalStorage } from 'async_hooks'; export interface RequestContextState { requestId: string; + ip?: string | null; + userAgent?: string | null; +} + +export interface RequestMetadataContext { + ip: string | null; + userAgent: string | null; } const requestContext = new AsyncLocalStorage(); @@ -17,6 +24,20 @@ export function currentRequestId(): string | undefined { return requestContext.getStore()?.requestId; } +/** + * The caller's IP and User-Agent captured for the active request, if any. + * Returns nulls outside an HTTP request so callers (e.g. the admin audit + * trail) can persist the columns without inventing a value for background + * work. + */ +export function currentRequestMetadata(): RequestMetadataContext { + const store = requestContext.getStore(); + return { + ip: store?.ip ?? null, + userAgent: store?.userAgent ?? null, + }; +} + /** * Builds the correlation headers for an outbound call made by the current * request. The middleware already accepts `x-request-id` and diff --git a/backend/src/database/migrations/1790007000000-AddAdminActionLogRequestMetadata.ts b/backend/src/database/migrations/1790007000000-AddAdminActionLogRequestMetadata.ts new file mode 100644 index 00000000..fce1fdd2 --- /dev/null +++ b/backend/src/database/migrations/1790007000000-AddAdminActionLogRequestMetadata.ts @@ -0,0 +1,28 @@ +import { MigrationInterface, QueryRunner } from 'typeorm'; + +/** + * Issue #1939: admin audit records could not say where an action came from. + * Adds nullable forensic columns so every admin action written after this + * migration stores the caller's IP and User-Agent. + */ +export class AddAdminActionLogRequestMetadata1790007000000 + implements MigrationInterface +{ + name = 'AddAdminActionLogRequestMetadata1790007000000'; + + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.query(` + ALTER TABLE "admin_action_logs" + ADD COLUMN "ip" varchar(64) NULL, + ADD COLUMN "user_agent" varchar(512) NULL + `); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.query(` + ALTER TABLE "admin_action_logs" + DROP COLUMN "user_agent", + DROP COLUMN "ip" + `); + } +} diff --git a/backend/src/main.ts b/backend/src/main.ts index 3606edfc..8fca6db6 100644 --- a/backend/src/main.ts +++ b/backend/src/main.ts @@ -1,3 +1,4 @@ +import { randomUUID } from 'crypto'; import { NestFactory } from '@nestjs/core'; import { Logger, ValidationPipe } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; diff --git a/backend/src/payments/README.md b/backend/src/payments/README.md index f63557c9..bdc0916f 100644 --- a/backend/src/payments/README.md +++ b/backend/src/payments/README.md @@ -153,19 +153,27 @@ and available as `PAYMENT_WEBHOOK_CONTRACT_VERSION` in 1. **Transport authenticity** — each rail has a dedicated controller (`/payments/webhooks/`) that authenticates the caller at the - transport level (the sandbox rail uses an HMAC-SHA256 signature over the - raw body, secret `PAYMENT_WEBHOOK_SECRET`). A bad signature is rejected - with `401` before any parsing. -2. **Per-rail mapping** — the rail adapter's `parseWebhookPayload(rawBody)` + transport level (the sandbox rail uses an HMAC-SHA256 signature over + `timestamp + '.' + rawBody`, secret `PAYMENT_WEBHOOK_SECRET`). A bad + signature is rejected with `401` before any parsing. +2. **Timestamp tolerance (issue #1938)** — the signed timestamp + (`x-payment-timestamp`, epoch ms) must be within + `PAYMENT_WEBHOOK_TOLERANCE_SECONDS` (default `300`) of the server clock; + a request outside the window is rejected with `401` and logged as + `replay`. +3. **Per-rail mapping** — the rail adapter's `parseWebhookPayload(rawBody)` maps the provider's native event fields into the normalized shape, so provider-specific field names/layouts never leak past the adapter. -3. **Contract validation** — the controller then runs +4. **Contract validation** — the controller then runs `validateWebhookPayload` on the normalized payload. Malformed/missing fields → `400` with a field-specific message (logged as `malformed_payload`). -4. **Idempotency** — `PaymentConfirmationService.apply` looks the payment - up by `providerReference` and no-ops on an already-terminal payment, so - duplicate or replay webhooks are harmless by construction. +5. **Idempotency** — the event id (`x-payment-event-id`, falling back to the + payload hash) is claimed in Redis with a TTL; a duplicate inside the + window is acknowledged without being applied twice. Independently, + `PaymentConfirmationService.apply` looks the payment up by + `providerReference` and no-ops on an already-terminal payment, so + duplicate or replay webhooks remain harmless by construction. ### Realtime status push contract (issue #1812) @@ -201,6 +209,7 @@ operator (or a later admin surface) can review and replay them: | `reason` | Trigger | | ------------------- | ---------------------------------------------------------------------------------- | | `invalid_signature` | HMAC signature check failed — payload discarded before parsing. | +| `replay` | Signed timestamp outside `PAYMENT_WEBHOOK_TOLERANCE_SECONDS` (issue #1938). | | `malformed_payload` | Payload parsed but failed contract validation (`validateWebhookPayload`). | | `process_error` | `apply` threw while processing a validly signed, well-formed webhook. | diff --git a/backend/src/payments/adapters/sandbox-rail.adapter.spec.ts b/backend/src/payments/adapters/sandbox-rail.adapter.spec.ts index f26fb337..4dc36ae1 100644 --- a/backend/src/payments/adapters/sandbox-rail.adapter.spec.ts +++ b/backend/src/payments/adapters/sandbox-rail.adapter.spec.ts @@ -5,8 +5,14 @@ function makeConfig(secret: string | undefined = 'test-secret') { return { get: jest.fn(() => secret) }; } -function sign(secret: string, rawBody: Buffer): string { - return createHmac('sha256', secret).update(rawBody).digest('hex'); +const TIMESTAMP = '1700000000000'; + +function sign(secret: string, rawBody: Buffer, timestamp = TIMESTAMP): string { + const signedPayload = Buffer.concat([ + Buffer.from(`${timestamp}.`, 'utf8'), + rawBody, + ]); + return createHmac('sha256', secret).update(signedPayload).digest('hex'); } describe('SandboxRailAdapter', () => { @@ -25,7 +31,11 @@ describe('SandboxRailAdapter', () => { const signature = sign('shh', rawBody); expect( - adapter.verifyWebhookSignature({ rawBody, signatureHeader: signature }), + adapter.verifyWebhookSignature({ + rawBody, + signatureHeader: signature, + timestamp: TIMESTAMP, + }), ).toBe(true); }); @@ -37,6 +47,7 @@ describe('SandboxRailAdapter', () => { adapter.verifyWebhookSignature({ rawBody, signatureHeader: 'not-the-right-signature-value', + timestamp: TIMESTAMP, }), ).toBe(false); }); @@ -50,6 +61,21 @@ describe('SandboxRailAdapter', () => { adapter.verifyWebhookSignature({ rawBody, signatureHeader: wrongSignature, + timestamp: TIMESTAMP, + }), + ).toBe(false); + }); + + it('rejects a signature computed over the body alone (timestamp not covered)', () => { + const adapter = new SandboxRailAdapter(makeConfig('shh') as any); + const rawBody = Buffer.from(JSON.stringify({ a: 1 })); + const bodyOnly = createHmac('sha256', 'shh').update(rawBody).digest('hex'); + + expect( + adapter.verifyWebhookSignature({ + rawBody, + signatureHeader: bodyOnly, + timestamp: TIMESTAMP, }), ).toBe(false); }); @@ -62,6 +88,19 @@ describe('SandboxRailAdapter', () => { adapter.verifyWebhookSignature({ rawBody, signatureHeader: undefined, + timestamp: TIMESTAMP, + }), + ).toBe(false); + }); + + it('rejects when the timestamp is missing', () => { + const adapter = new SandboxRailAdapter(makeConfig('shh') as any); + const rawBody = Buffer.from(JSON.stringify({ a: 1 })); + + expect( + adapter.verifyWebhookSignature({ + rawBody, + signatureHeader: sign('shh', rawBody), }), ).toBe(false); }); @@ -74,6 +113,7 @@ describe('SandboxRailAdapter', () => { adapter.verifyWebhookSignature({ rawBody, signatureHeader: sign('shh', rawBody), + timestamp: TIMESTAMP, }), ).toBe(false); }); diff --git a/backend/src/payments/adapters/sandbox-rail.adapter.ts b/backend/src/payments/adapters/sandbox-rail.adapter.ts index 03f93ff9..87b897cc 100644 --- a/backend/src/payments/adapters/sandbox-rail.adapter.ts +++ b/backend/src/payments/adapters/sandbox-rail.adapter.ts @@ -39,8 +39,9 @@ export class SandboxRailAdapter implements PaymentRailAdapter { verifyWebhookSignature({ rawBody, signatureHeader, + timestamp, }: WebhookSignatureInput): boolean { - if (!signatureHeader) { + if (!signatureHeader || !timestamp) { return false; } const secret = this.config.get('PAYMENT_WEBHOOK_SECRET'); @@ -48,7 +49,16 @@ export class SandboxRailAdapter implements PaymentRailAdapter { return false; } - const expected = createHmac('sha256', secret).update(rawBody).digest('hex'); + // Sign `timestamp + '.' + body` (issue #1938): including the timestamp + // in the signed content means a captured request cannot be replayed + // outside the tolerance window without invalidating the signature. + const signedPayload = Buffer.concat([ + Buffer.from(`${timestamp}.`, 'utf8'), + rawBody, + ]); + const expected = createHmac('sha256', secret) + .update(signedPayload) + .digest('hex'); const expectedBuf = Buffer.from(expected, 'utf8'); const providedBuf = Buffer.from(signatureHeader, 'utf8'); diff --git a/backend/src/payments/interfaces/payment-rail-adapter.interface.ts b/backend/src/payments/interfaces/payment-rail-adapter.interface.ts index c910ea63..5eb925ed 100644 --- a/backend/src/payments/interfaces/payment-rail-adapter.interface.ts +++ b/backend/src/payments/interfaces/payment-rail-adapter.interface.ts @@ -15,6 +15,13 @@ export interface PaymentVerificationResult { export interface WebhookSignatureInput { rawBody: Buffer; signatureHeader: string | undefined; + /** + * The signed timestamp (from the transport header). The signature is + * computed over `timestamp + '.' + rawBody` so a captured payload cannot + * be replayed later — the timestamp is verified against a tolerance by + * the caller. Required for rails that carry a timestamp. + */ + timestamp?: string; } export interface WebhookPayload { diff --git a/backend/src/payments/payment-webhook-dead-letter.service.ts b/backend/src/payments/payment-webhook-dead-letter.service.ts index a7ce5312..0b09ed33 100644 --- a/backend/src/payments/payment-webhook-dead-letter.service.ts +++ b/backend/src/payments/payment-webhook-dead-letter.service.ts @@ -7,6 +7,7 @@ import { withRequestId } from '../common/request-context'; export type WebhookDeadLetterReason = | 'invalid_signature' | 'malformed_payload' + | 'replay' | 'process_error'; /** diff --git a/backend/src/payments/payment-webhook.controller.spec.ts b/backend/src/payments/payment-webhook.controller.spec.ts index 15062f00..574c2f5f 100644 --- a/backend/src/payments/payment-webhook.controller.spec.ts +++ b/backend/src/payments/payment-webhook.controller.spec.ts @@ -8,6 +8,8 @@ function makeRequest(bodyObject: unknown) { return { rawBody, body: bodyObject } as any; } +const FRESH_TIMESTAMP = String(Date.now()); + describe('PaymentWebhookController', () => { let railAdapter: { verifyWebhookSignature: jest.Mock; @@ -20,6 +22,10 @@ describe('PaymentWebhookController', () => { let deadLetterService: { enqueue: jest.Mock; }; + let replay: { + isWithinTolerance: jest.Mock; + claimEvent: jest.Mock; + }; let controller: PaymentWebhookController; beforeEach(() => { @@ -34,10 +40,15 @@ describe('PaymentWebhookController', () => { deadLetterService = { enqueue: jest.fn(), }; + replay = { + isWithinTolerance: jest.fn().mockReturnValue(true), + claimEvent: jest.fn().mockResolvedValue(true), + }; controller = new PaymentWebhookController( railAdapter as any, confirmationService as any, deadLetterService as any, + replay as any, ); }); @@ -48,9 +59,9 @@ describe('PaymentWebhookController', () => { outcome: 'confirmed', }); - await expect(controller.sandbox(req, 'bad-sig')).rejects.toThrow( - UnauthorizedException, - ); + await expect( + controller.sandbox(req, 'bad-sig', FRESH_TIMESTAMP), + ).rejects.toThrow(UnauthorizedException); expect(confirmationService.logRejectedWebhook).toHaveBeenCalledWith( expect.any(String), @@ -68,6 +79,56 @@ describe('PaymentWebhookController', () => { expect(confirmationService.apply).not.toHaveBeenCalled(); }); + it('rejects and logs a stale timestamp as replay, never reaching apply', async () => { + railAdapter.verifyWebhookSignature.mockReturnValueOnce(true); + replay.isWithinTolerance.mockReturnValueOnce(false); + const req = makeRequest({ + providerReference: 'ref-1', + outcome: 'confirmed', + }); + + await expect( + controller.sandbox(req, 'good-sig', '1000'), + ).rejects.toThrow(UnauthorizedException); + + expect(replay.isWithinTolerance).toHaveBeenCalledWith(1000); + expect(confirmationService.logRejectedWebhook).toHaveBeenCalledWith( + expect.any(String), + 'replay', + ); + expect(deadLetterService.enqueue).toHaveBeenCalledWith( + expect.objectContaining({ reason: 'replay' }), + ); + expect(confirmationService.apply).not.toHaveBeenCalled(); + }); + + it('acknowledges a duplicate event id without applying it twice', async () => { + railAdapter.verifyWebhookSignature.mockReturnValueOnce(true); + railAdapter.parseWebhookPayload.mockReturnValueOnce({ + providerReference: 'ref-1', + outcome: 'confirmed', + }); + replay.claimEvent.mockResolvedValueOnce(false); + const req = makeRequest({ + providerReference: 'ref-1', + outcome: 'confirmed', + }); + + const result = await controller.sandbox( + req, + 'good-sig', + FRESH_TIMESTAMP, + 'event-123', + ); + + expect(result).toEqual({ + received: true, + contractVersion: PAYMENT_WEBHOOK_CONTRACT_VERSION, + }); + expect(replay.claimEvent).toHaveBeenCalledWith('event-123'); + expect(confirmationService.apply).not.toHaveBeenCalled(); + }); + it('rejects and logs a malformed (but authentically signed) payload', async () => { railAdapter.verifyWebhookSignature.mockReturnValueOnce(true); railAdapter.parseWebhookPayload.mockImplementationOnce(() => { @@ -75,9 +136,9 @@ describe('PaymentWebhookController', () => { }); const req = makeRequest({ nonsense: true }); - await expect(controller.sandbox(req, 'good-sig')).rejects.toThrow( - BadRequestException, - ); + await expect( + controller.sandbox(req, 'good-sig', FRESH_TIMESTAMP), + ).rejects.toThrow(BadRequestException); expect(confirmationService.logRejectedWebhook).toHaveBeenCalledWith( expect.any(String), @@ -106,7 +167,9 @@ describe('PaymentWebhookController', () => { outcome: 'confirmed', }); - await expect(controller.sandbox(req, 'good-sig')).rejects.toThrow(); + await expect( + controller.sandbox(req, 'good-sig', FRESH_TIMESTAMP), + ).rejects.toThrow(); expect(deadLetterService.enqueue).toHaveBeenCalledWith( expect.objectContaining({ @@ -128,7 +191,7 @@ describe('PaymentWebhookController', () => { outcome: 'confirmed', }); - const result = await controller.sandbox(req, 'good-sig'); + const result = await controller.sandbox(req, 'good-sig', FRESH_TIMESTAMP); expect(result).toEqual({ received: true, diff --git a/backend/src/payments/payment-webhook.controller.ts b/backend/src/payments/payment-webhook.controller.ts index eef173df..7b5eea91 100644 --- a/backend/src/payments/payment-webhook.controller.ts +++ b/backend/src/payments/payment-webhook.controller.ts @@ -19,6 +19,7 @@ import { } from './webhook-contract'; import { PaymentConfirmationService } from './payment-confirmation.service'; import { PaymentWebhookDeadLetterService } from './payment-webhook-dead-letter.service'; +import { WebhookReplayService } from './webhook-replay.service'; interface RawBodyRequest extends Request { rawBody?: Buffer; @@ -41,6 +42,7 @@ export class PaymentWebhookController { private readonly railAdapter: SandboxRailAdapter, private readonly confirmationService: PaymentConfirmationService, private readonly deadLetterService: PaymentWebhookDeadLetterService, + private readonly replay: WebhookReplayService, ) {} @Post('sandbox') @@ -75,6 +77,8 @@ export class PaymentWebhookController { async sandbox( @Req() req: RawBodyRequest, @Headers('x-payment-signature') signature?: string, + @Headers('x-payment-timestamp') timestamp?: string, + @Headers('x-payment-event-id') eventId?: string, ): Promise<{ received: boolean; contractVersion: string }> { const rawBody = req.rawBody ?? Buffer.from(JSON.stringify(req.body ?? {})); const rawPayloadHash = PaymentConfirmationService.hashPayload(rawBody); @@ -83,6 +87,7 @@ export class PaymentWebhookController { const isValid = this.railAdapter.verifyWebhookSignature({ rawBody, signatureHeader: signature, + timestamp, }); if (!isValid) { await this.confirmationService.logRejectedWebhook( @@ -97,6 +102,24 @@ export class PaymentWebhookController { throw new UnauthorizedException('Invalid webhook signature'); } + // Timestamp tolerance (issue #1938): the signature covers + // `timestamp + '.' + body`, so a captured request re-sent once the + // window has passed is a replay and must be refused. + if (!this.replay.isWithinTolerance(Number(timestamp))) { + await this.confirmationService.logRejectedWebhook( + rawPayloadHash, + 'replay', + ); + await this.deadLetterService.enqueue({ + rawPayloadHash, + rawPayload: rawPayloadText, + reason: 'replay', + }); + throw new UnauthorizedException( + 'Webhook timestamp outside tolerance window', + ); + } + let payload; try { payload = this.railAdapter.parseWebhookPayload(rawBody); @@ -117,6 +140,18 @@ export class PaymentWebhookController { throw new BadRequestException(message); } + // Idempotency (issue #1938): the event id (or the payload hash when the + // provider sends none) is claimed in Redis. A duplicate inside the + // window is acknowledged without being applied a second time. + const idempotencyKey = eventId ?? rawPayloadHash; + const firstSeen = await this.replay.claimEvent(idempotencyKey); + if (!firstSeen) { + return { + received: true, + contractVersion: PAYMENT_WEBHOOK_CONTRACT_VERSION, + }; + } + try { await this.confirmationService.apply( payload.providerReference, diff --git a/backend/src/payments/payments.module.ts b/backend/src/payments/payments.module.ts index 2b0b7e18..3f92a0fd 100644 --- a/backend/src/payments/payments.module.ts +++ b/backend/src/payments/payments.module.ts @@ -11,6 +11,7 @@ import { PaymentWebhookDeadLetter } from './entities/payment-webhook-dead-letter import { PaymentsService } from './payments.service'; import { PaymentConfirmationService } from './payment-confirmation.service'; import { PaymentWebhookDeadLetterService } from './payment-webhook-dead-letter.service'; +import { WebhookReplayService } from './webhook-replay.service'; import { ReconciliationService } from './reconciliation.service'; import { RefundsService } from './refunds.service'; import { PaymentRailRegistry } from './payment-rail-registry'; @@ -65,6 +66,7 @@ import { PaymentsService, PaymentConfirmationService, PaymentWebhookDeadLetterService, + WebhookReplayService, ReconciliationService, RefundsService, PaymentRailRegistry, diff --git a/backend/src/payments/webhook-idempotency-cache.util.ts b/backend/src/payments/webhook-idempotency-cache.util.ts deleted file mode 100644 index 6f784b87..00000000 --- a/backend/src/payments/webhook-idempotency-cache.util.ts +++ /dev/null @@ -1,31 +0,0 @@ -// Replay protection for payment-webhook.controller.ts: enforces a -// timestamp tolerance window plus a short-lived idempotency-key cache -// so a captured, valid webhook payload can't be replayed. -export class WebhookIdempotencyCache { - private seen = new Map(); - - constructor( - private readonly ttlMs = 5 * 60 * 1000, - private readonly maxClockSkewMs = 5 * 60 * 1000, - ) {} - - isWithinTolerance(timestampMs: number): boolean { - return Math.abs(Date.now() - timestampMs) <= this.maxClockSkewMs; - } - - private prune(): void { - const now = Date.now(); - for (const [key, expiresAt] of this.seen) { - if (expiresAt <= now) this.seen.delete(key); - } - } - - isReplay(idempotencyKey: string): boolean { - this.prune(); - return this.seen.has(idempotencyKey); - } - - markSeen(idempotencyKey: string): void { - this.seen.set(idempotencyKey, Date.now() + this.ttlMs); - } -} diff --git a/backend/src/payments/webhook-payload.validator.ts b/backend/src/payments/webhook-payload.validator.ts index 3bf6c2c0..a3ff2272 100644 --- a/backend/src/payments/webhook-payload.validator.ts +++ b/backend/src/payments/webhook-payload.validator.ts @@ -1,18 +1,20 @@ // Validates an incoming webhook payload against the shape defined in // webhook-contract.ts, so payment-webhook.controller.ts can't silently // drift from the contract it's supposed to honor. -import { WebhookContract } from './webhook-contract'; +import { WebhookPayload } from './interfaces/payment-rail-adapter.interface'; + +const REQUIRED_WEBHOOK_KEYS: ReadonlyArray = [ + 'providerReference', + 'outcome', +]; export function assertMatchesWebhookContract( payload: unknown, -): asserts payload is WebhookContract { +): asserts payload is WebhookPayload { if (typeof payload !== 'object' || payload === null) { throw new Error('Webhook payload must be an object'); } - const requiredKeys: (keyof WebhookContract)[] = Object.keys( - {} as WebhookContract, - ) as (keyof WebhookContract)[]; - for (const key of requiredKeys) { + for (const key of REQUIRED_WEBHOOK_KEYS) { if (!(key in payload)) { throw new Error(`Webhook payload missing required field "${String(key)}"`); } diff --git a/backend/src/payments/webhook-replay.service.spec.ts b/backend/src/payments/webhook-replay.service.spec.ts new file mode 100644 index 00000000..15cfba9d --- /dev/null +++ b/backend/src/payments/webhook-replay.service.spec.ts @@ -0,0 +1,89 @@ +import { + DEFAULT_WEBHOOK_TOLERANCE_SECONDS, + WebhookReplayService, +} from './webhook-replay.service'; + +function makeConfig(values: Record = {}) { + return { + get: jest.fn((key: string, fallback?: unknown) => + key in values ? values[key] : fallback, + ), + }; +} + +describe('WebhookReplayService', () => { + describe('tolerance', () => { + it('defaults to five minutes', () => { + const service = new WebhookReplayService(makeConfig() as any); + expect(service.toleranceMs()).toBe( + DEFAULT_WEBHOOK_TOLERANCE_SECONDS * 1000, + ); + }); + + it('honours a configured tolerance', () => { + const service = new WebhookReplayService( + makeConfig({ PAYMENT_WEBHOOK_TOLERANCE_SECONDS: 60 }) as any, + ); + expect(service.toleranceMs()).toBe(60 * 1000); + }); + + it('falls back to the default for an invalid configured value', () => { + const service = new WebhookReplayService( + makeConfig({ PAYMENT_WEBHOOK_TOLERANCE_SECONDS: 'not-a-number' }) as any, + ); + expect(service.toleranceMs()).toBe( + DEFAULT_WEBHOOK_TOLERANCE_SECONDS * 1000, + ); + }); + }); + + describe('isWithinTolerance', () => { + it('accepts a fresh timestamp', () => { + const service = new WebhookReplayService(makeConfig() as any); + expect(service.isWithinTolerance(Date.now())).toBe(true); + }); + + it('rejects a stale timestamp', () => { + const service = new WebhookReplayService(makeConfig() as any); + expect(service.isWithinTolerance(Date.now() - 10 * 60 * 1000)).toBe(false); + }); + + it('rejects a non-numeric timestamp', () => { + const service = new WebhookReplayService(makeConfig() as any); + expect(service.isWithinTolerance(Number.NaN)).toBe(false); + }); + }); + + describe('claimEvent', () => { + it('returns true when Redis records the event id', async () => { + const service = new WebhookReplayService(makeConfig() as any); + const set = jest.fn().mockResolvedValue('OK'); + jest.spyOn(service as any, 'getClient').mockReturnValue({ set }); + + await expect(service.claimEvent('event-1')).resolves.toBe(true); + expect(set).toHaveBeenCalledWith( + 'payment-webhook:seen:event-1', + '1', + 'EX', + DEFAULT_WEBHOOK_TOLERANCE_SECONDS, + 'NX', + ); + }); + + it('returns false for a duplicate inside the window', async () => { + const service = new WebhookReplayService(makeConfig() as any); + const set = jest.fn().mockResolvedValue(null); + jest.spyOn(service as any, 'getClient').mockReturnValue({ set }); + + await expect(service.claimEvent('event-1')).resolves.toBe(false); + }); + + it('fails open when Redis is unreachable', async () => { + const service = new WebhookReplayService(makeConfig() as any); + const set = jest.fn().mockRejectedValue(new Error('redis down')); + jest.spyOn(service as any, 'getClient').mockReturnValue({ set }); + + await expect(service.claimEvent('event-1')).resolves.toBe(true); + }); + }); +}); diff --git a/backend/src/payments/webhook-replay.service.ts b/backend/src/payments/webhook-replay.service.ts new file mode 100644 index 00000000..5cdb5bec --- /dev/null +++ b/backend/src/payments/webhook-replay.service.ts @@ -0,0 +1,103 @@ +import { Injectable, Logger, OnModuleDestroy } from '@nestjs/common'; +import { ConfigService } from '@nestjs/config'; +import Redis from 'ioredis'; + +/** Default tolerance between a webhook's signed timestamp and "now". */ +export const DEFAULT_WEBHOOK_TOLERANCE_SECONDS = 300; + +/** + * Replay protection for payment-webhook.controller.ts (issue #1938). + * + * Two independent guards: + * - a timestamp tolerance so a captured, validly-signed request cannot be + * re-sent once the window has passed; and + * - an event-id ledger in Redis (with a TTL) so a duplicate delivery inside + * the window is acknowledged without being applied twice. + * + * The event ledger is deliberately in Redis rather than an in-process Map so + * every instance behind a load balancer shares the same replay state. + */ +@Injectable() +export class WebhookReplayService implements OnModuleDestroy { + private readonly logger = new Logger(WebhookReplayService.name); + private client: Redis | null = null; + + constructor(private readonly config: ConfigService) {} + + private getClient(): Redis { + if (!this.client) { + this.client = new Redis({ + host: this.config.get('REDIS_HOST', 'localhost'), + port: this.config.get('REDIS_PORT', 6379), + password: this.config.get('REDIS_PASSWORD') || undefined, + db: this.config.get('REDIS_DB', 0), + lazyConnect: true, + maxRetriesPerRequest: 1, + }); + } + return this.client; + } + + /** Configured tolerance in milliseconds (default 5 minutes). */ + toleranceMs(): number { + const configured = Number( + this.config.get( + 'PAYMENT_WEBHOOK_TOLERANCE_SECONDS', + DEFAULT_WEBHOOK_TOLERANCE_SECONDS, + ), + ); + const seconds = + Number.isFinite(configured) && configured > 0 + ? configured + : DEFAULT_WEBHOOK_TOLERANCE_SECONDS; + return seconds * 1000; + } + + isWithinTolerance(timestampMs: number): boolean { + if (!Number.isFinite(timestampMs)) { + return false; + } + return Math.abs(Date.now() - timestampMs) <= this.toleranceMs(); + } + + /** + * Atomically records an event id, returning `true` when it had not been + * seen before (and is now recorded) or `false` for a duplicate inside the + * TTL window. + */ + async claimEvent(eventId: string): Promise { + const key = `payment-webhook:seen:${eventId}`; + const ttlSeconds = Math.max(1, Math.ceil(this.toleranceMs() / 1000)); + try { + const result = await this.getClient().set( + key, + '1', + 'EX', + ttlSeconds, + 'NX', + ); + return result === 'OK'; + } catch (error) { + // Redis is the shared replay ledger. If it is unreachable, fail open + // rather than drop legitimate webhooks; the timestamp tolerance still + // bounds how long any captured request stays replayable. + this.logger.warn( + `Could not record webhook event id in Redis (${eventId}): ${ + error instanceof Error ? error.message : String(error) + }`, + ); + return true; + } + } + + async onModuleDestroy(): Promise { + if (this.client) { + try { + await this.client.quit(); + } catch { + // best effort cleanup + } + this.client = null; + } + } +} diff --git a/backend/tsconfig.build.json b/backend/tsconfig.build.json index 64f86c6b..d0ed4d12 100644 --- a/backend/tsconfig.build.json +++ b/backend/tsconfig.build.json @@ -1,4 +1,4 @@ { "extends": "./tsconfig.json", - "exclude": ["node_modules", "test", "dist", "**/*spec.ts"] + "exclude": ["node_modules", "test", "dist", "**/*spec.ts", "**/*.tsx"] } diff --git a/backend/tsconfig.json b/backend/tsconfig.json index ec85a1c7..b14f3766 100644 --- a/backend/tsconfig.json +++ b/backend/tsconfig.json @@ -20,5 +20,5 @@ "typeRoots": ["./node_modules/@types", "./src/types"] }, "include": ["src/**/*", "test/**/*"], - "exclude": ["node_modules", "dist"] + "exclude": ["node_modules", "dist", "**/*.tsx"] } diff --git a/frontend/app/admin/layout.tsx b/frontend/app/admin/layout.tsx index 95644443..dca57495 100644 --- a/frontend/app/admin/layout.tsx +++ b/frontend/app/admin/layout.tsx @@ -9,6 +9,7 @@ import { Coins, } from "lucide-react"; import { AdminProviders } from "@/lib/providers"; +import { AdminBreadcrumbs } from "@/components/admin/admin-breadcrumbs"; import { cn } from "@/lib/utils"; export const metadata: Metadata = { @@ -53,7 +54,10 @@ export default function AdminLayout({ ))} -
{children}
+
+ + {children} +
diff --git a/frontend/app/layout.tsx b/frontend/app/layout.tsx index 69078e6a..4609d2d7 100644 --- a/frontend/app/layout.tsx +++ b/frontend/app/layout.tsx @@ -1,6 +1,7 @@ import type { Metadata } from "next"; import { Geist, Geist_Mono } from "next/font/google"; import { Providers } from "@/lib/providers/providers"; +import { OfflineBanner } from "@/components/admin/offline-banner"; import "./globals.css"; const geistSans = Geist({ @@ -116,6 +117,7 @@ export default function RootLayout({ + = { success: 'bg-green-600', - error: 'bg-red-600', - info: 'bg-blue-600', + error: 'bg-red-600', + info: 'bg-blue-600', }; /** diff --git a/backend/src/common/offline-banner.tsx b/frontend/components/admin/offline-banner.tsx similarity index 100% rename from backend/src/common/offline-banner.tsx rename to frontend/components/admin/offline-banner.tsx