From 261203ec5a9f71ca5cc70812df74f4101f6c6234 Mon Sep 17 00:00:00 2001 From: Julien Gautier Date: Thu, 14 May 2026 16:08:58 +0200 Subject: [PATCH] feat(portal-bff): admin audit-log read endpoint + audit_reader-locked path (#132) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary First real consumer of the admin module — `GET /api/admin/audit`, the paginated audit-log viewer named in [ADR-0020](docs/decisions/0020-portal-admin-app.md)'s v1 catalogue. Gated by `@RequireAdmin`, reads through the `audit_reader` Postgres role only, and emits `admin.audit.query` on every call as the "fishing expedition" deterrent ADR-0020 calls out (§"Read actions are also captured … to deter fishing expeditions"). This is the BFF half of the audit-viewer chantier — the SPA screen lands later. ## What ships ### [`AuditReader`](apps/portal-bff/src/admin/audit-reader.service.ts) - Wraps every read in a transaction whose first statement is `SET LOCAL ROLE audit_reader`. Symmetric with `AuditWriter`'s `SET LOCAL ROLE audit_writer`: the runtime role contract holds even when the BFF connection is otherwise privileged. UPDATE / INSERT / DELETE on `audit.events` fail at the Postgres level regardless of what gets through the application layer. - Parameterised SELECT only — filter values flow into `$queryRawUnsafe`'s positional params, never concatenated into the SQL string. Subject-prefix filter uses `LIKE` with explicit `ESCAPE '\\'` and escapes `%` / `_` / `\` in the literal so an admin-side wildcard can't masquerade as a meta-character. - COUNT(\*) + `LIMIT` / `OFFSET` pagination. Ordering is `created_at DESC, id DESC` for deterministic page boundaries on identical timestamps (UUIDs break ties). - Hard caps `limit` at `MAX_LIMIT` (200) even if a caller bypasses the DTO — defense in depth on the BFF's event loop. ### [`AdminAuditQueryDto`](apps/portal-bff/src/admin/audit-query.dto.ts) | Filter | Type | Notes | | --- | --- | --- | | `eventType` | string ≤128 | Exact match (e.g. `auth.sign_in`). | | `actorIdHash` | string ≤128 | Exact match on the salted hash from the writer. | | `audience` | enum | `workforce` \| `customer`. | | `outcome` | enum | `success` \| `failure` \| `denied`. | | `subjectPrefix` | string ≤128 | `LIKE 'prefix%'`, escaped literal. | | `createdAtFrom` | ISO-8601 | Inclusive lower bound. | | `createdAtTo` | ISO-8601 | Exclusive upper bound. | | `limit` | int 1..200 | Default **50**. | | `offset` | int ≥0 | Default **0**. | Bound through Nest's global `ValidationPipe` — unknown query keys are rejected by `forbidNonWhitelisted` (defends against query-string smuggling), `transform: true` coerces numeric strings into numbers. ### [`AdminAuditController`](apps/portal-bff/src/admin/admin-audit.controller.ts) `@Controller('admin/audit')` + `@RequireAdmin()` at the class level. The handler: 1. Calls `AuditReader.findEvents(filters)`. 2. Emits `admin.audit.query` with `{ filters, resultCount }` so a reviewer can see exactly what the admin searched for and how many rows came back. 3. Returns the page to the SPA. `@RequireMfa({ freshness: 600 })` is **intentionally not applied** in v1: the admin surface already sits behind a freshly-MFA'd session at sign-in (per ADR-0020), and the per-query audit row is the deterrent. Adding `@RequireMfa` later is a one-line change — that's why the decorator was designed-in by PR #128. ### [`AuditWriter.adminAuditQuery()`](apps/portal-bff/src/audit/audit.service.ts) New typed method using `outcome=success`. The read **happened** regardless of whether it matched rows; row count lives in `resultCount`. An `outcome=denied` from this surface is reserved for the day we add per-row authZ. ## Operational notes - **Dev pool, `SET LOCAL ROLE` pattern** (per [ADR-0018](docs/decisions/0018-environment-configuration-strategy.md)): the BFF talks to Postgres on the shared `DATABASE_URL` pool and switches role per-transaction. In production the audit-write pool is already split via `AUDIT_DATABASE_URL`; a dedicated `audit_reader`-only pool is a future follow-up if read-side isolation is desired (the role-locking on the shared pool already prevents privilege bleed at the Postgres level). - COUNT(\*) is fine at v1 audit volume; if the table grows past a few million rows we'll switch to keyset pagination and drop the total. ## Test plan - [x] `pnpm nx test portal-bff` — **308 specs pass** (was 278; +30: AuditReader 10, AdminAuditController 6, AuditWriter typed event 2, DTO validation 12). - [x] `pnpm exec nx affected -t format:check lint test build --base=origin/main` — clean (the pre-existing `_res` / `_next` warnings in `rate-limit.middleware.ts` are unrelated). - [x] SQL-injection probe via fixture (`'; DROP TABLE events; --` as `eventType`) — value lands in the params array, SQL stays templated. - [x] LIKE escaping verified: `%`, `_`, `\` in `subjectPrefix` are escaped to their literal form. - [ ] e2e — pending the admin SPA + at least one `admin` Entra role assignment. Once both exist: `curl --cookie 'portal_admin_session=…' /api/admin/audit?eventType=auth.sign_in&limit=10` returns the most recent sign-ins and `psql` shows the matching `admin.audit.query` row in `audit.events`. --------- Co-authored-by: Julien Gautier Reviewed-on: https://git.unespace.com/julien/apf_portal/pulls/132 --- .../src/admin/admin-audit.controller.spec.ts | 100 +++++++++ .../src/admin/admin-audit.controller.ts | 58 +++++ apps/portal-bff/src/admin/admin.module.ts | 6 +- .../src/admin/audit-query.dto.spec.ts | 77 +++++++ apps/portal-bff/src/admin/audit-query.dto.ts | 98 +++++++++ .../src/admin/audit-reader.service.spec.ts | 181 ++++++++++++++++ .../src/admin/audit-reader.service.ts | 198 ++++++++++++++++++ .../src/audit/audit.service.spec.ts | 38 ++++ apps/portal-bff/src/audit/audit.service.ts | 32 +++ apps/portal-bff/src/audit/audit.types.ts | 17 ++ 10 files changed, 803 insertions(+), 2 deletions(-) create mode 100644 apps/portal-bff/src/admin/admin-audit.controller.spec.ts create mode 100644 apps/portal-bff/src/admin/admin-audit.controller.ts create mode 100644 apps/portal-bff/src/admin/audit-query.dto.spec.ts create mode 100644 apps/portal-bff/src/admin/audit-query.dto.ts create mode 100644 apps/portal-bff/src/admin/audit-reader.service.spec.ts create mode 100644 apps/portal-bff/src/admin/audit-reader.service.ts diff --git a/apps/portal-bff/src/admin/admin-audit.controller.spec.ts b/apps/portal-bff/src/admin/admin-audit.controller.spec.ts new file mode 100644 index 0000000..c09cf23 --- /dev/null +++ b/apps/portal-bff/src/admin/admin-audit.controller.spec.ts @@ -0,0 +1,100 @@ +import type { Request } from 'express'; +import { AdminAuditController } from './admin-audit.controller'; +import type { AdminAuditQueryDto } from './audit-query.dto'; +import type { AuditReader, AdminAuditPage } from './audit-reader.service'; +import type { AuditWriter } from '../audit/audit.service'; + +const PAGE: AdminAuditPage = { + items: [ + { + id: '11111111-1111-1111-1111-111111111111', + createdAt: '2026-05-13T10:30:00.000Z', + eventType: 'auth.sign_in', + audience: 'workforce', + actorIdHash: 'hash(jane)', + traceId: 'trace-abc', + subject: 'session:sid-1', + outcome: 'success', + payload: { amr: ['pwd', 'mfa'] }, + }, + ], + total: 1, + limit: 50, + offset: 0, +}; + +function makeReq(user?: { oid: string }): Request { + return { session: user !== undefined ? { user } : {} } as unknown as Request; +} + +function makeFixture() { + const auditReader = { + findEvents: jest.fn().mockResolvedValue(PAGE), + }; + const auditWriter = { + adminAuditQuery: jest.fn().mockResolvedValue(undefined), + }; + const controller = new AdminAuditController( + auditReader as unknown as AuditReader, + auditWriter as unknown as AuditWriter, + ); + return { controller, auditReader, auditWriter }; +} + +describe('AdminAuditController.list', () => { + it('returns the page from AuditReader', async () => { + const { controller } = makeFixture(); + const page = await controller.list(makeReq({ oid: 'admin-oid' }), {} as AdminAuditQueryDto); + expect(page).toBe(PAGE); + }); + + it('forwards the validated DTO to AuditReader as-is', async () => { + const { controller, auditReader } = makeFixture(); + const filters: AdminAuditQueryDto = { + eventType: 'auth.sign_in', + audience: 'workforce', + createdAtFrom: '2026-05-01T00:00:00.000Z', + limit: 100, + offset: 0, + }; + await controller.list(makeReq({ oid: 'admin-oid' }), filters); + expect(auditReader.findEvents).toHaveBeenCalledWith(filters); + }); + + it('emits admin.audit.query with the filters + result count as deterrent', async () => { + const { controller, auditWriter } = makeFixture(); + const filters: AdminAuditQueryDto = { eventType: 'auth.sign_in', limit: 50 }; + await controller.list(makeReq({ oid: 'admin-oid' }), filters); + expect(auditWriter.adminAuditQuery).toHaveBeenCalledWith({ + actor: { oid: 'admin-oid' }, + filters: { eventType: 'auth.sign_in', limit: 50 }, + resultCount: PAGE.items.length, + }); + }); + + it('captures an empty filter object verbatim (the admin asked for everything)', async () => { + const { controller, auditWriter } = makeFixture(); + await controller.list(makeReq({ oid: 'admin-oid' }), {} as AdminAuditQueryDto); + expect(auditWriter.adminAuditQuery).toHaveBeenCalledWith( + expect.objectContaining({ filters: {} }), + ); + }); + + it('still returns the page when there is no session.user (defensive — guard should have caught it)', async () => { + const { controller, auditWriter } = makeFixture(); + const page = await controller.list(makeReq(), {} as AdminAuditQueryDto); + expect(page).toBe(PAGE); + // No actor → no audit row. The guard is the primary gate; this + // branch exists so a misconfiguration doesn't crash with a + // misleading error. + expect(auditWriter.adminAuditQuery).not.toHaveBeenCalled(); + }); + + it('propagates audit write failures (blocking per ADR-0013)', async () => { + const { controller, auditWriter } = makeFixture(); + auditWriter.adminAuditQuery.mockRejectedValueOnce(new Error('audit_writer denied')); + await expect( + controller.list(makeReq({ oid: 'admin-oid' }), {} as AdminAuditQueryDto), + ).rejects.toThrow('audit_writer denied'); + }); +}); diff --git a/apps/portal-bff/src/admin/admin-audit.controller.ts b/apps/portal-bff/src/admin/admin-audit.controller.ts new file mode 100644 index 0000000..2047ec1 --- /dev/null +++ b/apps/portal-bff/src/admin/admin-audit.controller.ts @@ -0,0 +1,58 @@ +import { Controller, Get, Query, Req } from '@nestjs/common'; +import type { Request } from 'express'; +import { AuditWriter } from '../audit/audit.service'; +import { AdminAuditQueryDto } from './audit-query.dto'; +import { AuditReader, type AdminAuditPage } from './audit-reader.service'; +import { RequireAdmin } from './require-admin.decorator'; + +/** + * `GET /api/admin/audit` — paginated audit-log viewer per + * ADR-0020's v1 admin module catalogue. Reads `audit.events` via + * the `audit_reader` Postgres role (locked inside the `AuditReader` + * service), shapes the result for SPA consumption, and emits an + * `admin.audit.query` row so the read itself is auditable per + * ADR-0020 §"Read actions are also captured … to deter fishing + * expeditions". + * + * The query DTO (`AdminAuditQueryDto`) carries every supported + * filter + pagination knob; Nest's global ValidationPipe rejects + * unknown keys before they reach this handler. + * + * `@RequireAdmin()` at the class level gates the route on the + * `admin` Entra app role per ADR-0020 §"Auth — same Entra ID". + * `@RequireMfa()` is not applied here in v1: the entire admin + * surface already sits behind a freshly-MFA'd session at the + * sign-in entry, and the audit row itself is the per-query + * deterrent. A future security review can layer `@RequireMfa` + * with a tighter freshness here without touching this code's + * shape — that's exactly why the decorator was designed-in. + */ +@Controller('admin/audit') +@RequireAdmin() +export class AdminAuditController { + constructor( + private readonly auditReader: AuditReader, + private readonly audit: AuditWriter, + ) {} + + @Get() + async list(@Req() req: Request, @Query() filters: AdminAuditQueryDto): Promise { + const page = await this.auditReader.findEvents(filters); + + // Guard guarantees `req.session.user`; we still defensively + // narrow rather than assert — the audit emission is what + // makes this read auditable, so if we somehow get here without + // an actor we should still surface the read (with no actor + // hash) rather than silently swallow it. + const actorOid = req.session.user?.oid; + if (actorOid !== undefined) { + await this.audit.adminAuditQuery({ + actor: { oid: actorOid }, + filters: { ...filters }, + resultCount: page.items.length, + }); + } + + return page; + } +} diff --git a/apps/portal-bff/src/admin/admin.module.ts b/apps/portal-bff/src/admin/admin.module.ts index b1fb3ab..a1f2e9e 100644 --- a/apps/portal-bff/src/admin/admin.module.ts +++ b/apps/portal-bff/src/admin/admin.module.ts @@ -1,8 +1,10 @@ import { Module } from '@nestjs/common'; import { AuthModule } from '../auth/auth.module'; +import { AdminAuditController } from './admin-audit.controller'; import { AdminAuthController } from './admin-auth.controller'; import { AdminController } from './admin.controller'; import { AdminRoleGuard } from './admin-role.guard'; +import { AuditReader } from './audit-reader.service'; /** * `AdminModule` — root of the `/api/admin/*` surface per ADR-0020. @@ -28,7 +30,7 @@ import { AdminRoleGuard } from './admin-role.guard'; */ @Module({ imports: [AuthModule], - controllers: [AdminController, AdminAuthController], - providers: [AdminRoleGuard], + controllers: [AdminController, AdminAuthController, AdminAuditController], + providers: [AdminRoleGuard, AuditReader], }) export class AdminModule {} diff --git a/apps/portal-bff/src/admin/audit-query.dto.spec.ts b/apps/portal-bff/src/admin/audit-query.dto.spec.ts new file mode 100644 index 0000000..402d0e5 --- /dev/null +++ b/apps/portal-bff/src/admin/audit-query.dto.spec.ts @@ -0,0 +1,77 @@ +import { ValidationPipe } from '@nestjs/common'; +import { AdminAuditQueryDto, MAX_LIMIT } from './audit-query.dto'; + +/** + * Spec exercises the DTO through Nest's `ValidationPipe` with the + * same options the BFF wires in `main.ts` (`whitelist`, + * `forbidNonWhitelisted`, `transform`) — the test is therefore a + * close stand-in for how the controller will see the parsed query + * at runtime. + */ +const pipe = new ValidationPipe({ + whitelist: true, + forbidNonWhitelisted: true, + transform: true, +}); + +async function transform(input: unknown): Promise { + return (await pipe.transform(input, { + type: 'query', + metatype: AdminAuditQueryDto, + })) as AdminAuditQueryDto; +} + +describe('AdminAuditQueryDto', () => { + it('accepts an empty query (every filter is optional)', async () => { + await expect(transform({})).resolves.toEqual({}); + }); + + it('coerces numeric strings (URL query format) into numbers', async () => { + const out = await transform({ limit: '25', offset: '100' }); + expect(out.limit).toBe(25); + expect(out.offset).toBe(100); + }); + + it('rejects a limit above MAX_LIMIT', async () => { + await expect(transform({ limit: String(MAX_LIMIT + 1) })).rejects.toThrow(); + }); + + it('rejects a negative offset', async () => { + await expect(transform({ offset: '-1' })).rejects.toThrow(); + }); + + it('rejects a non-integer limit', async () => { + await expect(transform({ limit: '12.5' })).rejects.toThrow(); + }); + + it('accepts each valid audience enum value', async () => { + await expect(transform({ audience: 'workforce' })).resolves.toMatchObject({ + audience: 'workforce', + }); + await expect(transform({ audience: 'customer' })).resolves.toMatchObject({ + audience: 'customer', + }); + }); + + it('rejects an unknown audience', async () => { + await expect(transform({ audience: 'visitor' })).rejects.toThrow(); + }); + + it('accepts each valid outcome enum value', async () => { + for (const outcome of ['success', 'failure', 'denied']) { + await expect(transform({ outcome })).resolves.toMatchObject({ outcome }); + } + }); + + it('rejects an ISO-8601 createdAtFrom that is not well-formed', async () => { + await expect(transform({ createdAtFrom: 'not-a-date' })).rejects.toThrow(); + }); + + it('rejects an unknown query key (forbidNonWhitelisted)', async () => { + await expect(transform({ password: 'leak' })).rejects.toThrow(); + }); + + it('rejects an over-long eventType (MaxLength guard)', async () => { + await expect(transform({ eventType: 'a'.repeat(129) })).rejects.toThrow(); + }); +}); diff --git a/apps/portal-bff/src/admin/audit-query.dto.ts b/apps/portal-bff/src/admin/audit-query.dto.ts new file mode 100644 index 0000000..f72f974 --- /dev/null +++ b/apps/portal-bff/src/admin/audit-query.dto.ts @@ -0,0 +1,98 @@ +import { Type } from 'class-transformer'; +import { + IsEnum, + IsInt, + IsISO8601, + IsOptional, + IsString, + Max, + MaxLength, + Min, +} from 'class-validator'; + +/** + * Query parameters accepted by `GET /api/admin/audit`. Bound from + * the URL via Nest's `@Query()` + the global `ValidationPipe` + * (whitelist + forbidNonWhitelisted + transform). Unknown keys are + * rejected at the pipe — saves the controller from having to vet + * them and shrinks the attack surface for query-string smuggling. + * + * Pagination is offset/limit, kept simple in v1: + * - `limit` defaults to 50 and is capped at {@link MAX_LIMIT} (200) + * so a single request can't pull tens of thousands of rows in + * one trip and stall the BFF's event loop on JSON serialisation. + * - `offset` defaults to 0. For deeper pagination the SPA should + * refine the date-range filter rather than walking thousands of + * offsets — Postgres' offset pagination performance degrades + * past a few thousand rows. + * + * Filters all OR-compose nothing — each is an additional AND clause. + * They are deliberately exact-match except `subjectPrefix` (which + * uses `LIKE 'prefix%'`) because the existing event catalogue uses + * structured prefixes like `session:` or `route:GET /api/...`. + */ +export const MAX_LIMIT = 200; +export const DEFAULT_LIMIT = 50; + +/** Subset of `AuditAudience` accepted by the filter. */ +const AUDIT_AUDIENCES = ['workforce', 'customer'] as const; +type AuditAudienceFilter = (typeof AUDIT_AUDIENCES)[number]; + +/** Subset of `AuditOutcome` accepted by the filter. */ +const AUDIT_OUTCOMES = ['success', 'failure', 'denied'] as const; +type AuditOutcomeFilter = (typeof AUDIT_OUTCOMES)[number]; + +export class AdminAuditQueryDto { + @IsOptional() + @IsString() + @MaxLength(128) + eventType?: string; + + @IsOptional() + @IsString() + @MaxLength(128) + actorIdHash?: string; + + @IsOptional() + @IsEnum(AUDIT_AUDIENCES) + audience?: AuditAudienceFilter; + + @IsOptional() + @IsEnum(AUDIT_OUTCOMES) + outcome?: AuditOutcomeFilter; + + /** + * Prefix match on the `subject` column. Matches the + * structured-prefix convention used by the event catalogue — + * e.g. `session:` to find every event tied to a particular + * session, or `GET /api/admin/` to find every denied admin + * access attempt. + */ + @IsOptional() + @IsString() + @MaxLength(128) + subjectPrefix?: string; + + /** ISO-8601 lower bound on `created_at` (inclusive). */ + @IsOptional() + @IsISO8601() + createdAtFrom?: string; + + /** ISO-8601 upper bound on `created_at` (exclusive). */ + @IsOptional() + @IsISO8601() + createdAtTo?: string; + + @IsOptional() + @Type(() => Number) + @IsInt() + @Min(1) + @Max(MAX_LIMIT) + limit?: number; + + @IsOptional() + @Type(() => Number) + @IsInt() + @Min(0) + offset?: number; +} diff --git a/apps/portal-bff/src/admin/audit-reader.service.spec.ts b/apps/portal-bff/src/admin/audit-reader.service.spec.ts new file mode 100644 index 0000000..2cf3250 --- /dev/null +++ b/apps/portal-bff/src/admin/audit-reader.service.spec.ts @@ -0,0 +1,181 @@ +import { Test } from '@nestjs/testing'; +import { PrismaService } from 'nestjs-prisma'; +import type { AdminAuditQueryDto } from './audit-query.dto'; +import { AuditReader, type AdminAuditEventDto } from './audit-reader.service'; + +interface MockTx { + $executeRawUnsafe: jest.Mock; + $queryRawUnsafe: jest.Mock; +} + +interface MockPrisma { + $transaction: jest.Mock; + tx: MockTx; +} + +function buildMocks(opts?: { + countRows?: Array<{ count: bigint }>; + selectRows?: unknown[]; +}): MockPrisma { + const tx: MockTx = { + $executeRawUnsafe: jest.fn().mockResolvedValue(0), + $queryRawUnsafe: jest + .fn() + // First $queryRawUnsafe call is the COUNT; second is the SELECT. + .mockResolvedValueOnce(opts?.countRows ?? [{ count: 0n }]) + .mockResolvedValueOnce(opts?.selectRows ?? []), + }; + return { + tx, + $transaction: jest.fn(async (fn: (tx: MockTx) => Promise) => fn(tx)), + }; +} + +async function createSubject(prisma: MockPrisma): Promise { + const moduleRef = await Test.createTestingModule({ + providers: [AuditReader, { provide: PrismaService, useValue: prisma }], + }).compile(); + return moduleRef.get(AuditReader); +} + +const ROW = { + id: '11111111-1111-1111-1111-111111111111', + created_at: new Date('2026-05-13T10:30:00.000Z'), + event_type: 'admin.audit.query', + audience: 'workforce', + actor_id_hash: 'hash(admin)', + trace_id: 'trace-abc', + subject: null, + outcome: 'success', + payload: { filters: {}, resultCount: 0 }, +}; + +describe('AuditReader.findEvents', () => { + it('locks the transaction to audit_reader before SELECT', async () => { + const prisma = buildMocks(); + const reader = await createSubject(prisma); + + await reader.findEvents({}); + + const setRoleCalls = prisma.tx.$executeRawUnsafe.mock.calls; + expect(setRoleCalls[0]?.[0]).toBe('SET LOCAL ROLE audit_reader'); + }); + + it('issues a COUNT(*) then a SELECT in the same transaction', async () => { + const prisma = buildMocks({ countRows: [{ count: 42n }], selectRows: [ROW] }); + const reader = await createSubject(prisma); + + const page = await reader.findEvents({}); + + expect(prisma.$transaction).toHaveBeenCalledTimes(1); + const queries = prisma.tx.$queryRawUnsafe.mock.calls; + expect(queries[0]?.[0]).toMatch(/SELECT COUNT\(\*\)::bigint AS count FROM "audit"\."events"/); + expect(queries[1]?.[0]).toMatch(/SELECT id, created_at, event_type, audience/); + expect(page.total).toBe(42); + expect(page.items).toHaveLength(1); + }); + + it('projects rows into the SPA-facing shape (ISO timestamp + null payload tolerance)', async () => { + const prisma = buildMocks({ + selectRows: [{ ...ROW, payload: null }, ROW], + }); + const reader = await createSubject(prisma); + const page = await reader.findEvents({}); + const first = page.items[0] as AdminAuditEventDto; + expect(first.createdAt).toBe('2026-05-13T10:30:00.000Z'); + expect(first.payload).toBeNull(); + const second = page.items[1] as AdminAuditEventDto; + expect(second.payload).toEqual({ filters: {}, resultCount: 0 }); + }); + + it('emits no WHERE clause when no filter is provided', async () => { + const prisma = buildMocks(); + const reader = await createSubject(prisma); + await reader.findEvents({}); + const countSql = prisma.tx.$queryRawUnsafe.mock.calls[0]?.[0] as string; + expect(countSql).not.toMatch(/WHERE/); + }); + + it('orders by created_at DESC, id DESC for deterministic pagination', async () => { + const prisma = buildMocks(); + const reader = await createSubject(prisma); + await reader.findEvents({}); + const selectSql = prisma.tx.$queryRawUnsafe.mock.calls[1]?.[0] as string; + expect(selectSql).toMatch(/ORDER BY created_at DESC, id DESC/); + }); + + it('passes filter values as positional parameters (never inlined into SQL)', async () => { + const prisma = buildMocks(); + const reader = await createSubject(prisma); + + await reader.findEvents({ + eventType: "admin.audit.query'; DROP TABLE events; --", + audience: 'workforce', + }); + + const countCall = prisma.tx.$queryRawUnsafe.mock.calls[0] ?? []; + const sql = countCall[0] as string; + const params = countCall.slice(1); + expect(sql).toContain('$1'); + expect(sql).toContain('$2'); + expect(sql).not.toContain('DROP TABLE'); + expect(params).toEqual(["admin.audit.query'; DROP TABLE events; --", 'workforce']); + }); + + it('casts enum filters through the proper enum types in SQL', async () => { + const prisma = buildMocks(); + const reader = await createSubject(prisma); + await reader.findEvents({ audience: 'workforce', outcome: 'denied' }); + const sql = prisma.tx.$queryRawUnsafe.mock.calls[0]?.[0] as string; + expect(sql).toMatch(/audience = \$\d+::"audit"\."AuditAudience"/); + expect(sql).toMatch(/outcome = \$\d+::"audit"\."AuditOutcome"/); + }); + + it('uses LIKE with ESCAPE for subjectPrefix and escapes %/_ in the literal', async () => { + const prisma = buildMocks(); + const reader = await createSubject(prisma); + await reader.findEvents({ subjectPrefix: 'session:abc%_' }); + const countCall = prisma.tx.$queryRawUnsafe.mock.calls[0] ?? []; + const sql = countCall[0] as string; + const params = countCall.slice(1) as string[]; + expect(sql).toMatch(/subject LIKE \$\d+ ESCAPE '\\'/); + expect(params[0]).toBe('session:abc\\%\\_%'); + }); + + it('translates createdAt ISO strings to Date instances in the params', async () => { + const prisma = buildMocks(); + const reader = await createSubject(prisma); + await reader.findEvents({ + createdAtFrom: '2026-05-01T00:00:00.000Z', + createdAtTo: '2026-05-13T23:59:59.999Z', + }); + const params = prisma.tx.$queryRawUnsafe.mock.calls[0]?.slice(1) as unknown[]; + expect(params[0]).toBeInstanceOf(Date); + expect((params[0] as Date).toISOString()).toBe('2026-05-01T00:00:00.000Z'); + expect((params[1] as Date).toISOString()).toBe('2026-05-13T23:59:59.999Z'); + }); + + it('applies the default limit (50) and offset (0) when not provided', async () => { + const prisma = buildMocks(); + const reader = await createSubject(prisma); + const page = await reader.findEvents({}); + const selectCall = prisma.tx.$queryRawUnsafe.mock.calls[1] ?? []; + // The last two params are LIMIT and OFFSET in that order. + const params = selectCall.slice(1) as unknown[]; + expect(params[params.length - 2]).toBe(50); + expect(params[params.length - 1]).toBe(0); + expect(page.limit).toBe(50); + expect(page.offset).toBe(0); + }); + + it('caps limit at MAX_LIMIT (200) even if a caller bypasses the DTO', async () => { + const prisma = buildMocks(); + const reader = await createSubject(prisma); + // Direct service call with limit above the ceiling — simulates + // a future caller (worker, batch job) that doesn't pass + // through the controller's ValidationPipe. + await reader.findEvents({ limit: 1000 } as AdminAuditQueryDto); + const params = prisma.tx.$queryRawUnsafe.mock.calls[1]?.slice(1) as unknown[]; + expect(params[params.length - 2]).toBe(200); + }); +}); diff --git a/apps/portal-bff/src/admin/audit-reader.service.ts b/apps/portal-bff/src/admin/audit-reader.service.ts new file mode 100644 index 0000000..7540402 --- /dev/null +++ b/apps/portal-bff/src/admin/audit-reader.service.ts @@ -0,0 +1,198 @@ +import { Injectable } from '@nestjs/common'; +import { PrismaService } from 'nestjs-prisma'; +import { DEFAULT_LIMIT, MAX_LIMIT, type AdminAuditQueryDto } from './audit-query.dto'; + +/** + * A single audit row projected for SPA consumption. Naming matches + * the `AuditEvent` Prisma model but exposes ISO-string timestamps + * (`created_at` → ISO 8601) and `Record` payloads — + * the SPA shouldn't have to know about Prisma `Json` runtime + * shapes. + */ +export interface AdminAuditEventDto { + readonly id: string; + readonly createdAt: string; + readonly eventType: string; + readonly audience: string; + readonly actorIdHash: string | null; + readonly traceId: string | null; + readonly subject: string | null; + readonly outcome: string; + readonly payload: Record | null; +} + +export interface AdminAuditPage { + readonly items: readonly AdminAuditEventDto[]; + /** + * Total rows matching the filter, ignoring `limit` / `offset`. + * COUNT(*) on the filtered set — fine at v1 audit volume; if the + * table grows past a few million rows we'll switch to keyset + * pagination and drop the total. + */ + readonly total: number; + readonly limit: number; + readonly offset: number; +} + +/** + * Raw shape returned by the underlying `$queryRawUnsafe`. Postgres + * returns timestamps as `Date` and json columns as already-parsed + * objects; the projection in {@link toDto} normalises both into + * SPA-friendly types. + */ +interface RawAuditRow { + id: string; + created_at: Date; + event_type: string; + audience: string; + actor_id_hash: string | null; + trace_id: string | null; + subject: string | null; + outcome: string; + payload: Record | null; +} + +/** + * `AuditReader` — single entry point for ADR-0013 audit-log reads. + * + * Contract + * -------- + * - **Always runs under `audit_reader`.** Every query is wrapped in + * a transaction whose first statement is `SET LOCAL ROLE + * audit_reader`. That role has SELECT-only on `audit.events` + * (per the migration that created the schema); UPDATE / INSERT / + * DELETE all fail at the Postgres level even when the BFF's + * connection is otherwise privileged. The role resets at + * transaction end. Mirrors the `AuditWriter`'s role-locking + * posture for symmetry. + * + * - **Parameterised SQL only.** Filter values flow into the + * `$queryRawUnsafe` parameter array, never concatenated into the + * SQL string — defends against admin-side SQL injection through + * the audit-viewer filter form. + * + * - **No PII redaction here.** The payload JSON is returned as-is + * to admin callers. Per ADR-0013, redaction is the writer's + * responsibility; what landed in the row is what comes out. The + * admin role is the trust boundary. + */ +@Injectable() +export class AuditReader { + constructor(private readonly prisma: PrismaService) {} + + async findEvents(filters: AdminAuditQueryDto): Promise { + const limit = filters.limit ?? DEFAULT_LIMIT; + const offset = filters.offset ?? 0; + // Hard cap at the DTO ceiling even if a future caller bypasses + // the ValidationPipe — defense in depth. + const safeLimit = Math.min(limit, MAX_LIMIT); + + const { whereSql, params } = buildWhere(filters); + + return this.prisma.$transaction(async (tx) => { + await tx.$executeRawUnsafe(`SET LOCAL ROLE audit_reader`); + + const totalRows = await tx.$queryRawUnsafe>( + `SELECT COUNT(*)::bigint AS count FROM "audit"."events" ${whereSql}`, + ...params, + ); + const total = Number(totalRows[0]?.count ?? 0n); + + // Ordering is `created_at DESC, id DESC` so the SPA gets the + // newest events first (typical audit-viewer expectation) and + // ties on identical timestamps break deterministically on the + // uuid — preventing pagination drift if two events share a + // microsecond. + // + // Postgres parameter numbering continues from the WHERE clause + // (`params.length`), so LIMIT lands at `$N+1` and OFFSET at + // `$N+2`. + const items = await tx.$queryRawUnsafe( + `SELECT id, created_at, event_type, audience, actor_id_hash, + trace_id, subject, outcome, payload + FROM "audit"."events" + ${whereSql} + ORDER BY created_at DESC, id DESC + LIMIT $${params.length + 1} + OFFSET $${params.length + 2}`, + ...params, + safeLimit, + offset, + ); + + return { + items: items.map(toDto), + total, + limit: safeLimit, + offset, + }; + }); + } +} + +/** + * Build the parameterised WHERE clause + the matching positional + * parameter array. Every filter is optional; missing filters drop + * out of the SQL so the planner can pick the cheapest index. + */ +function buildWhere(filters: AdminAuditQueryDto): { whereSql: string; params: unknown[] } { + const clauses: string[] = []; + const params: unknown[] = []; + + if (filters.eventType !== undefined) { + params.push(filters.eventType); + clauses.push(`event_type = $${params.length}`); + } + if (filters.actorIdHash !== undefined) { + params.push(filters.actorIdHash); + clauses.push(`actor_id_hash = $${params.length}`); + } + if (filters.audience !== undefined) { + params.push(filters.audience); + clauses.push(`audience = $${params.length}::"audit"."AuditAudience"`); + } + if (filters.outcome !== undefined) { + params.push(filters.outcome); + clauses.push(`outcome = $${params.length}::"audit"."AuditOutcome"`); + } + if (filters.subjectPrefix !== undefined) { + // `LIKE` with a literal escape so a `%` in the prefix doesn't + // act as a wildcard. The prefix bound is concatenated *to a + // parameter*, never to the SQL string — the parameter is the + // already-escaped literal followed by `%`. + params.push(`${escapeLikeLiteral(filters.subjectPrefix)}%`); + clauses.push(`subject LIKE $${params.length} ESCAPE '\\'`); + } + if (filters.createdAtFrom !== undefined) { + params.push(new Date(filters.createdAtFrom)); + clauses.push(`created_at >= $${params.length}`); + } + if (filters.createdAtTo !== undefined) { + params.push(new Date(filters.createdAtTo)); + clauses.push(`created_at < $${params.length}`); + } + + const whereSql = clauses.length > 0 ? `WHERE ${clauses.join(' AND ')}` : ''; + return { whereSql, params }; +} + +function escapeLikeLiteral(raw: string): string { + // `\` first, then the two LIKE meta-characters. Order matters — + // if `%`/`_` were escaped first their inserted `\` would be + // double-escaped on the second pass. + return raw.replace(/\\/g, '\\\\').replace(/%/g, '\\%').replace(/_/g, '\\_'); +} + +function toDto(row: RawAuditRow): AdminAuditEventDto { + return { + id: row.id, + createdAt: row.created_at.toISOString(), + eventType: row.event_type, + audience: row.audience, + actorIdHash: row.actor_id_hash, + traceId: row.trace_id, + subject: row.subject, + outcome: row.outcome, + payload: row.payload, + }; +} diff --git a/apps/portal-bff/src/audit/audit.service.spec.ts b/apps/portal-bff/src/audit/audit.service.spec.ts index a6e39ca..4ae700d 100644 --- a/apps/portal-bff/src/audit/audit.service.spec.ts +++ b/apps/portal-bff/src/audit/audit.service.spec.ts @@ -316,6 +316,44 @@ describe('AuditWriter — typed event methods', () => { }); }); + describe('adminAuditQuery()', () => { + it('records admin.audit.query (success) with filters + resultCount in payload', async () => { + const { writer, prisma, hashUserId } = await createSubject(); + await writer.adminAuditQuery({ + actor: { oid: 'admin-oid' }, + filters: { eventType: 'auth.sign_in', limit: 50 }, + resultCount: 12, + }); + expect(hashUserId.hash).toHaveBeenCalledWith('admin-oid'); + const row = extractInsertedRow(prisma); + expect(row.eventType).toBe('admin.audit.query'); + expect(row.audience).toBe('workforce'); + expect(row.outcome).toBe('success'); + expect(row.actorIdHash).toBe('hash(admin-oid)'); + // `subject` is left null — the event is system-wide on the + // audit log itself, not tied to a specific entity URI. + expect(row.subject).toBeNull(); + expect(row.payloadJson).toBe( + JSON.stringify({ + filters: { eventType: 'auth.sign_in', limit: 50 }, + resultCount: 12, + }), + ); + }); + + it('persists an empty filters object verbatim (the admin asked for everything)', async () => { + const { writer, prisma } = await createSubject(); + await writer.adminAuditQuery({ + actor: { oid: 'admin-oid' }, + filters: {}, + resultCount: 0, + }); + expect(extractInsertedRow(prisma).payloadJson).toBe( + JSON.stringify({ filters: {}, resultCount: 0 }), + ); + }); + }); + describe('mfaRequired()', () => { it('records auth.mfa_required (denied) with reason + freshnessSeconds in payload', async () => { const { writer, prisma, hashUserId } = await createSubject(); diff --git a/apps/portal-bff/src/audit/audit.service.ts b/apps/portal-bff/src/audit/audit.service.ts index f04550c..cdcc184 100644 --- a/apps/portal-bff/src/audit/audit.service.ts +++ b/apps/portal-bff/src/audit/audit.service.ts @@ -4,6 +4,7 @@ import { ClsService } from 'nestjs-cls'; import { PrismaService } from 'nestjs-prisma'; import type { AdminAccessDeniedInput, + AdminAuditQueryInput, AuditEventInput, MfaRequiredInput, SignInActor, @@ -141,6 +142,37 @@ export class AuditWriter { }); } + /** + * Typed event: an admin queried the audit-log viewer. Recorded at + * **coarser granularity** per ADR-0020 §"Audit log captures every + * admin write action … Read actions (audit log viewing, user list + * browsing) are also captured … to deter fishing expeditions". + * + * The filter object the admin used is serialised into the payload + * so a reviewer can see what was searched (event type, time range, + * actor hash, etc.). `resultCount` is the number of rows the SPA + * received in this trip — paged calls each emit one event; an + * auditor spotting a sequence of `admin.audit.query` with growing + * offsets is the v1 detection signal for a sweep. + * + * `outcome=success` regardless of whether the query returned any + * rows: the event captures "the admin ran a query", not "the + * query matched something". An `outcome=failure` audit row from + * this surface is reserved for the day we add per-row authZ. + */ + async adminAuditQuery(input: AdminAuditQueryInput): Promise { + await this.recordEvent({ + eventType: 'admin.audit.query', + audience: 'workforce', + outcome: 'success', + actorIdHash: this.hashUserId.hash(input.actor.oid), + payload: { + filters: input.filters, + resultCount: input.resultCount, + }, + }); + } + /** * Typed event: `/api/admin/*` request rejected by `AdminRoleGuard` * because the session's `roles` claim does not include `admin`. diff --git a/apps/portal-bff/src/audit/audit.types.ts b/apps/portal-bff/src/audit/audit.types.ts index 31d54ae..235df5c 100644 --- a/apps/portal-bff/src/audit/audit.types.ts +++ b/apps/portal-bff/src/audit/audit.types.ts @@ -101,6 +101,23 @@ export interface AdminAccessDeniedInput { rolesHeld: readonly string[]; } +export interface AdminAuditQueryInput { + actor: { oid: string }; + /** + * Filters the admin used on the audit-viewer query. Captured into + * the payload so a reviewer auditing audit-log reads can see what + * the admin was searching for — per ADR-0020 §"Read actions are + * also captured ... to deter fishing expeditions". Empty object + * is meaningful (the admin asked for everything). + */ + filters: Record; + /** + * Result-set size returned to the admin. Bounded by the DTO's + * `MAX_LIMIT`; persisted so an auditor can spot oversized scans. + */ + resultCount: number; +} + /** Reason `RequireMfaGuard` rejected a request — fed into the audit payload. */ export type MfaRequiredReason = 'no-mfa-in-amr' | 'no-mfa-verified-at' | 'mfa-stale';