4 Commits

Author	SHA1	Message	Date
Julien Gautier	875d61d858	feat(portal-bff): audit-stats endpoint — server-side aggregations with redis cache ... PR 1 of the tabs + full-result charts chantier. New BFF endpoint that powers the upcoming "Charts" tab on portal-admin's /audit page with aggregations computed over the FULL filtered set rather than the paginated slice that currently feeds the charts. New route: GET /api/admin/audit/stats * Same query DTO shape as /audit (AdminAuditStatsQueryDto) minus pagination — stats endpoint always aggregates the whole filtered set. * Returns three projections: - dailyVolume: [{day: 'YYYY-MM-DD', count}] - outcomeBreakdown: [{outcome, count}] - eventTypeByDay: [{day, eventType, count}] + total (sum of dailyVolume.count) for the donut centre label. * Time bound: respects filters strictly (per chantier brief). No filter → aggregates across the full audit retention (365 days per ADR-0013). The Redis cache below absorbs repeated heavy queries. New service: AuditStatsReader * Same role-locking posture as AuditReader — every query inside a transaction that opens with SET LOCAL ROLE audit_reader. SELECT-only on audit.events even if the BFF connection is otherwise privileged. * Parameterised SQL only. Filter values flow through positional parameters, never concatenated into the SQL string. * Three GROUP BY queries scoped by the same WHERE clause as the list endpoint's buildWhere (kept structurally in sync by convention, not extracted yet because the two readers' pagination shapes differ). Redis cache: 5-min TTL per filters-hash * Cache key = `audit:stats:` + sha256(canonical-JSON of filters) truncated to 16 hex chars. Sorted-keys canonicalisation so the same filters in different argument orders map to the same key. * Cache writes are best-effort — Redis-write failure does not fail the response. The DB read already happened. * Spec covers: cache-hit path skips DB, cache-key stability across key ordering, write happens with EX 300, write failure tolerance. Audit event: admin.audit.stats.query * New typed AuditWriter method (mirrors adminAuditQuery but captures `total` instead of `resultCount` — stats responses don't paginate, the value carries more "size of scan" signal). * Emitted on every stats call per ADR-0020's read-deterrence posture. Distinct event_type from admin.audit.query so an auditor can spot "scanned aggregations" vs "paged through rows" — different observation signals. ADR-0013 amended (light): new "Reader endpoints" subsection documents the two-endpoint reader surface + Redis cache caveat. The events table grows four rows it was previously missing (admin.access_denied, admin.audit.query, admin.audit.stats.query, admin.users.query). Verification: * 414 portal-bff specs pass (was 401; +13: 8 service-level + 5 controller-level). * `pnpm nx lint test build --projects=portal-bff` — green. PR 2 (SPA) lands next: Tabs UX (Table / Charts) + replaces the client-side per-page computeds with consumption of this endpoint.	2026-05-16 23:07:51 +02:00
julien	a97be121e6	fix(portal-bff): audit writes use raw INSERT (audit_writer has no SELECT for RETURNING) (#121) ... ## Summary #120 shipped the audit pipeline but the end-to-end path was never smoke-tested against a running Postgres. First click on `/auth/logout` returned 500 with the Pino log: ``` PostgresError code 42501 — permission denied for table events ``` Despite: - ACL on `audit.events` showing `audit_writer=a/audit_owner` (INSERT granted). - `has_table_privilege('audit_writer', 'audit.events', 'INSERT')` returning `t`. - `has_schema_privilege` / `has_type_privilege` all `t`. - A direct psql `INSERT INTO audit.events ...` after `SET LOCAL ROLE audit_writer` **succeeding**. - A psql `INSERT ... RETURNING id` after the same `SET LOCAL ROLE` **failing** with the exact same error. Root cause: Prisma's ORM `tx.auditEvent.create(...)` issues `INSERT ... RETURNING *` to hydrate the returned entity. Postgres requires **SELECT** on every column listed in `RETURNING`. `audit_writer` has INSERT only by ADR-0013 design — RETURNING fails with `code 42501` and the error message reads "permission denied for table events" (no mention of SELECT or RETURNING, which is what made it deeply non-obvious to diagnose). ## Fix `AuditWriter.recordEvent` now issues a parameterised raw INSERT via `tx.$executeRawUnsafe` instead of the ORM `create()`: ```ts await tx.$executeRawUnsafe( `INSERT INTO "audit"."events" (id, event_type, audience, outcome, subject, actor_id_hash, trace_id, payload) VALUES (gen_random_uuid(), $1, $2::"audit"."AuditAudience", $3::"audit"."AuditOutcome", $4, $5, $6, $7::jsonb)`, input.eventType, input.audience, input.outcome, input.subject ?? null, actorIdHash, traceId, payloadJson, ); ``` The role contract per ADR-0013 stays strict: `audit_writer` keeps INSERT only, no SELECT/UPDATE/DELETE/TRUNCATE. The other natural fix (`GRANT SELECT` to `audit_writer`) would have weakened the writer/reader role separation, so we deliberately went the other way. ## Notable choices **`gen_random_uuid()` server-side instead of Prisma's `@default(uuid())` client-side.** The model still declares `@default(uuid())` for any future ORM read or `audit_reader`-side query, but the write path uses the built-in Postgres function. No extension required (Postgres 13+). **Explicit enum and jsonb casts.** Parameters travel as TEXT over the wire; the SQL casts (`$2::"audit"."AuditAudience"`, `$7::jsonb`) ensure Postgres parses them as the right type. Without the casts, the type system rejects the INSERT before privilege check even fires. **Parameterised, not interpolated.** `$executeRawUnsafe` accepts a SQL template with `$1, $2, …` placeholders and a vararg of values — same wire-level parameter binding as a prepared statement, so SQL injection isn't possible even on caller-controlled inputs like `eventType`. The spec pins this with a malicious-input test. **Also fixes an env-sensitivity bug in `auth.controller.spec.ts`.** The test that asserts `session.absoluteExpiresAt == createdAt + 43200000` was reading the default via `readSessionTimeouts()` but didn't override `SESSION_ABSOLUTE_TIMEOUT_SECONDS`. If `apps/portal-bff/.env` has a custom value (as it did during the manual audit debugging), the test failed non-deterministically. Now the test deletes the env var before running and restores it after — same pattern as the other env-touching tests in this file. ## ADR amendment [ADR-0013](docs/decisions/0013-audit-trail-separated-postgres-append-only.md) §"Writer" now carries an **Implementation trap** callout explaining why Prisma's ORM `create()` cannot be used for audit writes (RETURNING requires SELECT, audit_writer has INSERT only). The corresponding Confirmation entry cross-references the callout. Two-commit shape on this PR (code + docs) — the squash-merge will fold them. ## Test plan - [x] `pnpm nx test portal-bff` (clean env) → **144/144 pass**. - [x] `pnpm nx lint portal-bff` → clean. - [x] `pnpm nx build portal-bff` → clean. - [x] Prettier-clean. - [ ] Manual smoke against running BFF + Postgres: - [ ] Sign in → row in `audit.events` with `event_type = 'auth.sign_in'`. - [ ] Sign out → row with `event_type = 'auth.sign_out'`. **The 500 from before is gone.** - [ ] Verify the role contract is still strict : ```sql SET ROLE audit_writer; SELECT * FROM audit.events LIMIT 1; -- should fail "permission denied" UPDATE audit.events SET event_type = 'x'; -- should fail DELETE FROM audit.events; -- should fail ``` --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #121	2026-05-13 19:48:32 +02:00
julien	02ac44e498	feat(portal-bff): audit log foundation per ADR-0013 (#76) ... ## Summary Lays down the append-only audit log per ADR-0013: schema declaration, first migration with role grants, NestJS `AuditWriter` service. Typed event-family methods, the separate `AUDIT_DATABASE_URL` pool, the retention job, and the live-DB integration tests are explicitly listed as "wired as features land" in the ADR's confirmation block — they ship when the matching feature ADRs do. ## What lands **Prisma schema** ([`apps/portal-bff/prisma/schema.prisma`](apps/portal-bff/prisma/schema.prisma)): - `multiSchema` preview enabled; datasource declares `public` + `audit` schemas. - `AuditEvent` model: `id` (uuid), `createdAt`, `eventType` (free-form in v1), `audience` enum (`workforce | customer`), `actorIdHash`, `traceId`, `subject`, `outcome` enum (`success | failure | denied`), `payload` (jsonb). - Indexes on `createdAt`, `eventType`, `traceId` — covering the three obvious query shapes. **Migration** ([`prisma/migrations/*_init_audit_schema/migration.sql`](apps/portal-bff/prisma/migrations/20260510011453_init_audit_schema/migration.sql)): - Standard Prisma `CREATE TABLE` / enums output, then the **append-only contract** re-applied explicitly: - `ALTER TABLE/TYPE OWNER TO audit_owner`. - `GRANT INSERT` to `audit_writer`, `SELECT` to `audit_reader`, **`SELECT, DELETE`** to `audit_archiver` (SELECT is needed to evaluate the `created_at` predicate of "delete older than retention" — Postgres requires SELECT on every column referenced in DELETE's WHERE). - `GRANT USAGE` on the enum types to all three roles (without it `audit_writer.INSERT` fails with "permission denied for type"). - **No** GRANT for `UPDATE` / `TRUNCATE` to anyone — including `audit_owner` at runtime; only fresh schema migrations amend the table. **Service** ([`apps/portal-bff/src/audit/`](apps/portal-bff/src/audit/)): - `AuditWriter.recordEvent(input)` — single entry point. Wraps every INSERT in a transaction whose first statement is `SET LOCAL ROLE audit_writer`, so the role contract holds at runtime even from the otherwise-privileged BFF connection. - `traceId` auto-resolved from the active OTel span (so audit row joins with traces and Pino logs on the same `trace_id`). - `actorIdHash` auto-resolved from CLS (key `actorIdHash`) with explicit input-side override; `null` when neither is set (placeholder until ADR-0009 / ADR-0010 guards populate CLS). - Errors propagate (no catch-and-swallow), per ADR-0013's "blocking writes: no audit ⇒ no action". **Tests** — 8 unit tests on `AuditWriter` (mocked Prisma + CLS): role-locking ordering, input pass-through, `Prisma.JsonNull` for missing payload, CLS-vs-input precedence on `actorIdHash`, OTel trace capture, error propagation. ## End-to-end verification (manual, against local-dev Postgres) ``` INSERT under audit_writer: ok UPDATE under audit_writer: permission denied for table events DELETE under audit_writer: permission denied for table events DELETE under audit_archiver: ok, row removed (after the SELECT-grant fix) ``` ## ADR-0013 §Confirmation rewritten Two-block split: "wired in foundation PR" lists what landed here; "wired as features land" lists the typed event-family methods, AUDIT_DATABASE_URL connection split, startup self-test probe, retention purge job, salt-shared cross-correlation test, and live-DB role-contract integration tests — each anchored to the feature ADR that triggers it. ## Recovery for anyone with a pre-existing local-dev DB If your local-dev Postgres already had the audit migration applied **before** the SELECT-grant fix, the archiver's DELETE will fail. Two options: 1. Apply the missing grant directly: ```bash psql "$DATABASE_URL" -c "GRANT SELECT ON audit.events TO audit_archiver;" ``` 2. Or wipe the volume and re-migrate cleanly: ```bash ./infra/local/dev.sh down -v ./infra/local/dev.sh up pnpm --filter @apf-portal/source exec prisma migrate deploy # or `cd apps/portal-bff && pnpm exec prisma migrate deploy` ``` Fresh DBs land with the corrected migration directly. ## Out of scope (separate PRs) - Typed event-family methods (`signIn`, `signInFailed`, …) — added per matching feature ADR. - `AUDIT_DATABASE_URL` separate connection pool — defense-in-depth, when production needs it. - Startup self-test probe (deliberate failing UPDATE asserting rejection) — lands with the connection split. - Retention purge job (`audit_archiver` daily cron) — phase-3b infra. - Live-DB integration tests asserting the role contract — Testcontainers-style harness, separate PR. ## Test plan - [ ] CI green on this PR. - [ ] `prisma migrate deploy` succeeds on a fresh DB (the recovery instructions cover the SELECT-grant gap for already-migrated dev DBs). - [ ] `psql -c "\dp audit.events"` shows the expected privilege matrix: `audit_owner=arwdDxtm/audit_owner`, `audit_writer=a/audit_owner`, `audit_reader=r/audit_owner`, `audit_archiver=rd/audit_owner`. - [ ] BFF boots; calling `AuditWriter.recordEvent` from a controller (manual smoke once a real flow lands) writes to `audit.events` with the expected `trace_id` matching the request's Jaeger span. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #76	2026-05-10 03:44:01 +02:00
Julien Gautier	0e58e32d29	chore: relocate ADRs from decisions/ to docs/decisions/ to consolidate documentation ... Move the ADR folder under docs/ alongside the rest of the project documentation. Convention (flat folder, globally-sequential 4-digit numbering, tags-based categorization, MADR 4.0.0 format) is unchanged - only the path moved. - git mv decisions docs/decisions preserves history for all 18 ADRs + README + template (19 files renamed in this commit). - ADR-0001 amended in-place with a dated note documenting the relocation. Status remains 'accepted' - the location detail changed, the decision did not. - All cross-references updated: - CLAUDE.md (~17 ADR links + 3 mentions of decisions/ in the Project rules section) - docs/README.md (now references decisions/ as a sibling under docs/) - docs/setup/03-angular-nx-monorepo.md (paths shortened from ../../decisions/ to ../decisions/, since setup/ and decisions/ are now both inside docs/) - docs/decisions/0003 ../CLAUDE.md adjusted to ../../CLAUDE.md (one extra level of nesting) - docs/decisions/template.md mention of the README path - notes/asvs-level-decision-briefing-rssi.md mention of the index Sanity verified: every ADR link in CLAUDE.md, docs/setup/03, and docs/decisions/0001 resolves to an existing file. pnpm nx run-many -t lint passes on 8 projects.	2026-04-30 18:57:59 +02:00