8aec83f82e95457b913fdd2e4fb97f96f707157d
5 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Julien Gautier
|
8aec83f82e |
refactor(users): rename Prisma User model to UserDirectoryEntry
Mechanical refactor, prerequisite to ADR-0026 PR 1. Frees the `User` name for the upcoming ADR-0026 model (UUID PK + FK to Person + lastSignInAt), which has materially different semantics from the ADR-0020 user-directory cache. Zero behavioural change. Same columns, same indexes, same constraints, same call sites - just a different identifier on the model, the table, and the corresponding Prisma client field. Two renames in user-directory.service.ts: the Prisma model rename collided with an existing TS interface also called UserDirectoryEntry (which was actually the input shape of recordSignIn). The interface gets renamed to RecordSignInInput at the same time, which is its correct semantic name. Net effect: clearer on both sides. Postgres ALTER TABLE RENAME does NOT cascade to PK constraint / index names; the migration explicitly ALTER INDEXes the three named indexes (pkey + last_seen_at + username). The class name AdminUsersReader, the endpoint URL /api/admin/users, the DTO AdminUserDto and the local interface UserRow are unchanged - these are SPA/HTTP-facing identifiers where the "admin user list" URL semantics hold regardless of the backing table name. |
||
 julien
|
ff1713eb6d |
fix(structures): align Structure.delegation_code FK action with Prisma default (#229)
## Summary Single-line fix on the `add_org_hierarchy` migration shipped in [#228](#228) (ADR-0027 PR 1). The FK action on `Structure.delegation_code` was `ON DELETE RESTRICT` (Prisma's default for **required** relations); the field is **optional** (`Structure.delegation Delegation?`), so Prisma's default is `ON DELETE SET NULL`. Mismatch caused `prisma migrate dev` to detect drift and prompt for a corrective migration on every run. Caught locally on first VM-side validation (the workspace dev DB). No production deployment of #228 yet, so the fix is **edit-in-place** rather than a corrective sibling migration — keeps the repo's migration history clean. ## What lands | File | Change | | --- | --- | | `apps/portal-bff/prisma/migrations/20260526143000_add_org_hierarchy/migration.sql` | `ON DELETE RESTRICT` → `ON DELETE SET NULL` on the `structures_delegation_code_fkey` FK. Inline comment explains the rule (nullable Prisma relation → SET NULL is the matching default; not matching it generates drift on every `migrate dev`). | That's it. One line of SQL, one comment block. No schema.prisma change, no test change, no other file touched. ## Why edit-in-place vs new corrective migration Edit-in-place is safe here because: - The broken migration only exists in **dev local DBs** (Julien's WSL postgres). No staging, no preview env, no prod has applied it. - Every dev who pulls this PR's fix wipes their local DB (`./infra/local/dev.sh down -v`) and reapplies — clean migration history, no "modified after applied" warning. - The repo's migration list stays minimal — adding a corrective migration would carry the wart forever in `prisma/migrations/`. Once a non-dev environment has applied a migration, the rule reverses: corrective migration mandatory, never edit in place. ADR-0015's "trunk-based + squash-merge" and the absence of a deployed environment at this stage gives us this one-time window. ## Recovery procedure for anyone who applied #228 ```bash # From the workspace root ./infra/local/dev.sh down -v # wipe the local postgres volume ./infra/local/dev.sh up # Pull this fix git pull # or git switch fix/adr-0027-pr1-fk-action-on-delete depending on local state # Reapply migrations — no prompt this time cd apps/portal-bff && pnpm exec prisma migrate dev # Should report: "Applied migration `20260526143000_add_org_hierarchy`" and exit cleanly. ``` If anyone has a stray `drift_inspection/` folder under `prisma/migrations/` (artifact of the `--create-only` debugging step), delete it before reapplying — it's not in the repo, it was just diagnostic output. ## Test plan - [x] `node scripts/check-catalogue-drift.mjs` — clean (unchanged by this fix). - [x] `node --test scripts/check-catalogue-drift.spec.mjs` — 22 tests passing (unchanged). - [x] `pnpm exec prettier --check` clean (SQL file unaffected by prettier but verified). - [ ] **Locally on WSL** — wipe DB → pull fix → `prisma migrate dev` exits clean, no drift prompt. - [ ] **Review focus** — the comment block in the migration explaining the rule (future-proof against the same mistake when ADR-0026 PR 1 adds optional relations). ## Notes for the reviewer - **Why a comment block on the FK line, not on every nullable FK in future migrations?** This was caught the first time we hit it; a short note at the offending site is cheaper than amending `CLAUDE.md` with a "remember to match Prisma default actions" rule. If we hit the same mistake on ADR-0026 PR 1's `Person`/`User`/`UserScope` migration, we'll consider promoting it. For now: one inline comment at the place where the rule is non-obvious. - **The drift gate doesn't catch this kind of mistake.** Catalogue drift gate scans string literals against TypeScript catalogues — it doesn't look at SQL referential actions. Postgres CHECK constraints (introduced for `Structure.kind` in this same migration) are not the same surface as FK referential actions. Catching this required actually running `prisma migrate dev`. A possible future improvement: a CI gate that runs `prisma migrate diff --from-migrations --to-schema-datamodel --script --shadow-database-url …` and fails if the diff is non-empty (zero-drift gate). Worth an ADR amendment if it becomes a recurring class of bug. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #229 |
||
|
julien
|
ba4cdcee7a |
feat(structures): add Region/Delegation/Structure schema + seed (ADR-0027 PR 1) (#228)
## Summary
First implementation PR for [ADR-0027](docs/decisions/0027-portal-side-organisational-hierarchy.md) (`Region` / `Delegation` / `Structure` portal-side organisational hierarchy). **Schema + seed + drift-gate extension only** — no consumer code yet (the `PrismaScopeResolver` that dereferences `Structure.code` from `UserScope.value` lives in ADR-0026 PR 2, which depends on this PR landing first).
Independent of ADR-0026 PR 1 at the schema level — both can ship in parallel; ADR-0026 PR 2 depends on both.
## What lands
| File | Change |
| --- | --- |
| `apps/portal-bff/prisma/schema.prisma` | **+3 models**. `Region` (INSEE code PK + name + delegations[]). `Delegation` (dept code PK + name + regionCode FK + structures[]). `Structure` (portal code PK + name + kind discriminator + nullable unique finess/siret/codePaie + nullable delegationCode FK). All in the `public` schema; matches the ADR-0027 schema sketch verbatim. |
| `apps/portal-bff/prisma/migrations/20260526143000_add_org_hierarchy/migration.sql` | **New** hand-written migration. DDL for the 3 tables. `CHECK ("kind" IN (...))` constraint mirroring `STRUCTURE_KINDS`. Indexes (FK columns + kind + uniques). Inline INSERT seed: Région Nouvelle-Aquitaine (75), Délégation Gironde (33), structures `0330800013` + `0330800021` (médico-social, FINESS = code) + `siege` (APF national, no delegation/finess). |
| `apps/portal-bff/src/structures/structure-kind.ts` | **New**. Closed-set catalogue: `STRUCTURE_KINDS = [medico_social, antenne, dispositif, entreprise_adaptee, mouvement, administratif, siege] as const`. `type StructureKind` derived from the union, `isStructureKind` type guard. |
| `apps/portal-bff/src/structures/structure-kind.spec.ts` | **New**. Jest spec — catalogue content, no duplicates, type guard true for catalogue values + false for typos / cascade-only values / empty, type narrowing at call site. |
| `scripts/check-catalogue-drift.mjs` | **Extend**. New `extractStructureKinds(path)` + `findStructureKindViolationsInFile(path, validKinds, sourceText?)`. Property-literal scanner: detects `kind: 'X'` in object literals, restricted to files that import from `structure-kind.ts` (cheap text pre-filter — `kind` is a common property name on unrelated objects and we'd false-positive everywhere otherwise). Integrated into `scanWorkspace`. Error-message formatter switched on callee shape (`@Foo('x')` for decorators, `kind: 'x'` for property literals). Closing hint updated to reference both ADR-0025 and ADR-0027 catalogue locations. |
| `scripts/check-catalogue-drift.spec.mjs` | **Extend**. Fixture writes a synthesised `structure-kind.ts` alongside `authorization.types.ts`. New tests: extract STRUCTURE_KINDS, throw on missing constant, skip files without the import, no-violation on catalogue values, flag off-catalogue values, skip non-literal initialisers, line/column tracking, scanWorkspace aggregation (decorator + Structure.kind together), scanWorkspace exposes the structureKinds set. **22 tests total, all passing.** |
## Defense in depth
Three layers stack for `Structure.kind`, deliberately:
1. **TypeScript type union** `StructureKind` — compile-time check at every typed assignment.
2. **Postgres `CHECK` constraint** in the migration — runtime enforcement at INSERT / UPDATE, defends raw SQL / casts / untrusted API input.
3. **`scripts/check-catalogue-drift.mjs`** — CI gate asserting every `kind: 'X'` literal in structure-context files is in the catalogue.
The `Privilege` / `FunctionalRole` catalogues from ADR-0025 only have layer 1 + layer 3 (no DB enforcement — those values aren't persisted as schema-checked columns). ADR-0027's `Structure.kind` is persisted, so layer 2 was practical to add — bulletproof against any code path that bypasses the type system.
## Seed scope (and what's deliberately NOT in it)
Just what the 19 test-tenant personas reference per `notes/test-tenant-role-assignments.md`:
- `Region` 75 Nouvelle-Aquitaine (only region the personas exercise)
- `Delegation` 33 Gironde (only delegation)
- `Structure` 0330800013 (APF Bordeaux, medico-social, FINESS = code)
- `Structure` 0330800021 (Complexe Mérignac, medico-social)
- `Structure` `siege` (APF national, kind=siege, no FK to a delegation, no FINESS)
**No** placeholder `entreprise_adaptee`, `antenne`, or `dispositif` row — those kinds are valid per the catalogue but the test tenant doesn't exercise them, and adding gold-plate seed data would be misleading ("what is this row used for?"). The full APF inventory lands with [ADR-0029](#)'s cascade sync; this seed is **superseded** (not extended) by that sync.
## Notes for the reviewer
- **Naming conflict with the existing `User` model.** The current `schema.prisma` already has a `User` model — but it's the ADR-0020 user-directory cache (Entra `oid` as PK, written by `UserDirectoryService.recordSignIn`). ADR-0026 PR 1 introduces a different `User` (UUID PK, FK to `Person`, `lastSignInAt`). **Out of scope here** — ADR-0027 PR 1 doesn't touch `User`. Flagging now so ADR-0026 PR 1 can plan the migration path (likely: rename the existing `User` to `UserDirectoryEntry` or fold it into the new Person + User pair).
- **Migration is hand-written**, matching the style of the two existing migrations (`init_audit_schema`, `users_directory`). Timestamp `20260526143000` chosen so it sorts after the existing `20260514192014_users_directory`.
- **`@@schema("public")`** required on every new model because the audit log uses `multiSchema` (per ADR-0013) — the public/audit split is configured at the datasource.
- **Drift gate error-message formatter** now switches on callee shape — decorator violations still print as `@Foo('x')`, property-literal violations print as `kind: 'x'` to match the offending code shape.
- **No `pnpm ci:check` impact expected** at the bff level beyond the new spec; `pnpm ci:catalogue-drift` continues to report clean (`catalogues: 4 privileges, 24 roles, 7 structure kinds`).
## Test plan
- [x] `node scripts/check-catalogue-drift.mjs` — clean (4 / 24 / 7).
- [x] `node --test scripts/check-catalogue-drift.spec.mjs` — 22 tests passing.
- [x] `pnpm exec prettier --check` clean on the touched files.
- [ ] **On the dev VM**: `pnpm prisma migrate dev` applies the migration cleanly against a fresh `infra/local/dev.compose.yml` postgres. `pnpm prisma studio` shows the seeded Region / Delegation / Structure rows.
- [ ] **On the dev VM**: `pnpm exec nx test portal-bff` runs the new `structure-kind.spec.ts` green.
- [ ] **Review focus** — the `Structure` model shape (kind discriminator, nullable unique columns, FK to Delegation), the inline seed values vs `notes/test-tenant-role-assignments.md`, the drift-gate property-literal scanner's restriction to files importing from `structure-kind.ts`.
## What's next
Per [ADR-0027 §"Phasing"](docs/decisions/0027-portal-side-organisational-hierarchy.md):
1. **This PR** — Region / Delegation / Structure schema + seed + drift gate. ✅
2. **ADR-0026 PR 1** — `Person` / `User` / `UserScope` schema + `PersonAndUserProvisioner` + drift gate extension for `Person.source` + updated `PrincipalBuilder`. Independent of (1), can ship in parallel. **Needs to resolve the existing-`User`-name collision.**
3. **ADR-0026 PR 2** — `PrismaScopeResolver` replacing `StubScopeResolver` + `/admin/users/:id/scopes` admin screen + `prisma/seed.ts` populating the 19 personas' `user_scopes` rows pointing at this PR's `Structure.code` values. **Depends on both (1) and (2).**
4. **ADR-0029** (future) — Pléiades + Acteurs+ + cascade syncs + facet schemas + `Pole` / `Service` / per-source enrichment extensions to this PR's hierarchy.
---------
Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #228
|
||
|
julien
|
aca9e8d155 |
feat(portal-bff): user directory upserted at sign-in (#140)
## Summary
First PR of the **portal-admin User-list chantier** per [ADR-0020](docs/decisions/0020-portal-admin-app.md) §"v1 scope — User list (read-only)". Ships the **write side** only:
1. A new `public.users` table that holds the BFF's local cache of identities seen sign in to either portal-shell or portal-admin.
2. A `UserDirectoryService.recordSignIn(user)` upsert called from `SessionEstablisher.establish` after the blocking audit write.
The read side (`GET /api/admin/users` + the admin viewer SPA screen) lands in two follow-up PRs of the same chantier.
## Schema
[`prisma/schema.prisma`](apps/portal-bff/prisma/schema.prisma) gains a `User` model in the `public` schema:
| Column | Type | Notes |
| --- | --- | --- |
| `oid` | TEXT, PK | Entra's stable per-user identifier inside the tenant. Per-tenant uniqueness is sufficient for v1's single-workforce-tenant design (ADR-0008). |
| `tid` | TEXT | Tenant id. Updated on every upsert because a dual-audience future may legitimately change it. |
| `audience` | TEXT | `'workforce'` \| `'customer'`. Hardcoded to `workforce` in v1 per ADR-0008's simplification; will read from session/claims when External ID activates. |
| `username` | TEXT | Updated on every upsert (Entra-side rename possible). |
| `display_name` | TEXT | Same. |
| `first_seen_at` | TIMESTAMPTZ | Set once at first sign-in via DEFAULT NOW(); **never overwritten** thereafter. Enables "users since <date>" without joining anything. |
| `last_seen_at` | TIMESTAMPTZ | Updated on every upsert. Enables "most recently active" without scanning `audit.events`. |
Indexes:
- `last_seen_at DESC` — admin default sort.
- `username` — prefix filtering.
Migration in [`prisma/migrations/20260514192014_users_directory/`](apps/portal-bff/prisma/migrations/20260514192014_users_directory/migration.sql).
## [`UserDirectoryService`](apps/portal-bff/src/users/user-directory.service.ts)
```ts
async recordSignIn(entry): Promise<void> {
try {
await prisma.user.upsert({
where: { oid },
create: { oid, tid, audience, username, displayName },
update: { tid, audience, username, displayName, lastSeenAt: new Date() },
});
} catch (err) {
// logged, never propagated
}
}
```
**Best-effort write.** Catches its own errors, logs a Pino warn (`user_directory.record_sign_in_failed`), returns `undefined`. The directory is a convenience for admin browsing, not a security boundary — a Postgres hiccup must not lock a user out of sign-in. ADR-0013's "no audit ⇒ no action" applies to the audit module only.
## [`SessionEstablisher`](apps/portal-bff/src/auth/session-establisher.service.ts) wiring
The directory call lands right after the existing audit emission:
```ts
await this.audit.signIn({ actor: user, sessionId: req.sessionID }); // blocking per ADR-0013
await this.userDirectory.recordSignIn({ ...user, audience: 'workforce' }); // best-effort
this.logger.log(...);
```
Two invariants the tests pin:
1. **Audit-first**: when `audit.signIn` throws, `userDirectory.recordSignIn` is NOT called. The directory never holds a row for a sign-in the audit log doesn't carry.
2. **Awaited**: an admin who just signed in sees themselves on the user list immediately — no race between the upsert and the response.
## Module wiring
[`UsersModule`](apps/portal-bff/src/users/users.module.ts) is declared `@Global()` so `SessionEstablisher` (which lives in `AuthModule`) injects `UserDirectoryService` without forcing `AuthModule` to import `UsersModule`. The directory is a true cross-cutting concern: one writer (the auth callback) and one future reader (the admin endpoint).
Wired into [`AppModule`](apps/portal-bff/src/app/app.module.ts) alongside the other v1 modules. `auth.module.spec.ts` updated to also import `UsersModule` in its slice-of-graph compile (otherwise the test fails to resolve the new `SessionEstablisher` dep).
## Notes for the reviewer
- The directory write **awaits** (not fire-and-forget). The cost is one round-trip per sign-in on the response-critical path; the benefit is the no-race property called out above. If sign-in p95 becomes an issue we can revisit (e.g. background job) but the simpler shape is correct first.
- `firstSeenAt` is intentionally absent from the `update` payload. The Prisma upsert's `update` block is precisely what changes on conflict; omitting the field leaves it untouched at the column level (Postgres-side default doesn't refire on UPDATE).
- The model lives in `public`, not in a dedicated `identity` or `cms` schema. ADR-0020 enumerates `cms.*` for editorial data and `audit.*` for the audit ledger but doesn't require a separate schema for user-directory data. We can promote it to its own schema later if a role-isolation need emerges; the migration would be a `ALTER TABLE users SET SCHEMA …`.
- `audit.events.actor_id_hash` is **not** stored on `public.users`. A future admin endpoint that joins sign-in counts from `audit.events` can compute the hash on-the-fly via `HashUserIdService` — keeping the salted-hash invariant from ADR-0013 intact (the salt stays inside the audit module).
## Test plan
- [x] `pnpm nx test portal-bff` — **365 specs pass** (was 358; +7: UserDirectoryService 4, SessionEstablisher integration 3).
- [x] `pnpm exec nx affected -t format:check lint test build --base=origin/main` — clean (the pre-existing rate-limit warnings + one unused-eslint-disable from PR #137 are unrelated).
- [x] Prisma `migrate diff` confirms the model matches the migration SQL.
- [ ] e2e — after merge: sign in via portal-shell or portal-admin, expect a row in `public.users` with the right `oid` / `last_seen_at`; sign in again, expect the same row's `last_seen_at` to advance and `first_seen_at` to stay put.
## What's next
The chantier sequence:
1. **This PR** — write side: schema + service + sign-in upsert.
2. **PR 2** — BFF `GET /api/admin/users` (paginated + filterable, gated by `@RequireAdmin`, emits `admin.users.query` audit).
3. **PR 3** — portal-admin `/users` screen (table + filter form), promote the sidebar entry from "Soon" badge to live link.
---------
Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #140
|
||
|
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 |