julien/apf_portal
1
0
Fork 0
Code Issues 1 Pull Requests Actions 1 Packages Projects Releases Wiki Activity
195 Commits 1 Branch 0 Tags
d9a14530422be7924d3deed803c4c22ef70a5ead
Commit Graph

3 Commits

Author SHA1 Message Date
julien a97be121e6 fix(portal-bff): audit writes use raw INSERT (audit_writer has no SELECT for RETURNING) (#121)
CI / check (push) Successful in 2m16s
Details
CI / commits (push) Has been skipped
Details
CI / scan (push) Successful in 1m29s
Details
CI / a11y (push) Successful in 1m20s
Details
CI / perf (push) Successful in 3m30s
Details 
## 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)
CI / check (push) Successful in 1m52s
Details
CI / commits (push) Has been skipped
Details
CI / scan (push) Successful in 1m7s
Details
CI / a11y (push) Successful in 52s
Details
CI / perf (push) Successful in 2m17s
Details 
## 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