julien
261203ec5a
## 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
78 lines
2.5 KiB
TypeScript
78 lines
2.5 KiB
TypeScript
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<AdminAuditQueryDto> {
|
|
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();
|
|
});
|
|
});
|