feat(portal-bff): admin audit-log read endpoint + audit_reader-locked path (#132)
## 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 <julien.gautier@apf.asso.fr>
Reviewed-on: #132
This commit was merged in pull request #132.
This commit is contained in:
@@ -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();
|
||||
|
||||
@@ -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<void> {
|
||||
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`.
|
||||
|
||||
@@ -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<string, unknown>;
|
||||
/**
|
||||
* 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';
|
||||
|
||||
|
||||
Reference in New Issue
Block a user