2ffbfc4034c59420933778df78f0a142f3b7514b
3 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 julien
|
24071a63ec |
feat(users): add Person + User + UserScope + lazy provisioner (ADR-0026 PR 1) (#232)
## Summary
First implementation PR for [ADR-0026](docs/decisions/0026-person-user-portal-data-model.md) (portal-side identity model). Ships:
- `Person` golden record + `User` portal-account overlay + `UserScope` Prisma models;
- `PersonAndUserProvisioner` service called from `SessionEstablisher.establish` (blocking — lazy-creates `Person` + `User` at first OIDC callback);
- `PERSON_SOURCES` closed-set catalogue + drift-gate extension;
- `PrincipalBuilder.build(user, identity)` signature update + `ScopeResolver.resolve({ userId })` seam change — `Principal.user.{id, personId}` now carry real portal UUIDs, no longer the `entraOid` placeholder.
No consumer for `UserScope` yet — `PrismaScopeResolver` lands in ADR-0026 PR 2. The `StubScopeResolver` continues to return `[{ kind: 'unrestricted' }]` for everyone.
## What lands
| File | Change |
| --- | --- |
| `apps/portal-bff/prisma/schema.prisma` | **+3 models** in the `public` schema: `Person` (UUID PK + PII + nullable email indexed-not-unique + source catalogue + nullable externalId + back-ref to User), `User` (UUID PK + 1-to-1 personId FK + unique entraOid + tenantId + lastSignInAt + scopes[]), `UserScope` (UUID PK + userId FK with `onDelete: Cascade` + kind + opaque value with `@default("")` + source + nullable expiresAt + `@@unique([userId, kind, value])`). All comments explain the cross-references to ADR-0025 / ADR-0027 / ADR-0029. |
| `apps/portal-bff/prisma/migrations/20260526210000_add_person_user_userscope/migration.sql` | **New** hand-written migration. DDL for the 3 tables, indexes (Person: source/externalId/email; User: unique personId + unique entraOid; UserScope: unique composite + plain userId), FKs with the correct `ON DELETE` actions (User.personId → RESTRICT; UserScope.userId → CASCADE — both Prisma defaults given the schema's `Delegation?` / explicit `onDelete: Cascade`). |
| `apps/portal-bff/src/users/person-source.ts` + `.spec.ts` | **New** catalogue. `PERSON_SOURCES = ['self-signin', 'admin-ui', 'seed'] as const`, `PersonSource` type union, `isPersonSource` type guard. Spec follows the structure-kind shape (catalogue content, no duplicates, type guard true/false, narrowing test via `String()` to widen). |
| `apps/portal-bff/src/users/person-and-user-provisioner.service.ts` + `.spec.ts` | **New** blocking provisioner. Fast path: `findUnique by entraOid` + `update lastSignInAt`. Cold path: nested `create` of Person + linked User in a single transaction. Race-condition handling: catches P2002 on `User.entraOid` and re-runs `ensureUser` (loser of a concurrent first-sign-in falls through to the now-warm fast path). Defense-in-depth `isPersonSource` check on the constant. Spec covers cold/warm/race/non-P2002-propagation paths + `splitDisplayName` cases (single token / trailing whitespace / empty / multi-word). |
| `scripts/check-catalogue-drift.mjs` | Property-literal scanner generalised. Adds `extractPersonSources` + `findPersonSourceViolationsInFile` (property `source` instead of `kind`, otherwise identical to the structure-kind scanner). The two extractors share `extractAsConstArray(path, constName)` and the two property scanners share `findPropertyLiteralViolations(path, validValues, sourceText, opts)`. Error-message formatter unchanged. Closing hint extended to reference all three catalogue locations. |
| `scripts/check-catalogue-drift.spec.mjs` | Fixture now writes a `person-source.ts` alongside `structure-kind.ts`. 7 new tests: extractPersonSources (2), findPersonSourceViolationsInFile (5: skip-no-import, no-violation, flag, non-literal, line/col). Aggregation test updated to assert decorator + Structure.kind + Person.source violations all surface together. **29 tests total, all passing.** |
| `apps/portal-bff/src/auth/scope-resolver.ts` | `resolve(input: { entraOid })` → `resolve(input: { userId })`. Stub still ignores its argument; PR 2's `PrismaScopeResolver` will key queries on `User.id`. Comment block updated to reflect that ADR-0026 PR 1 is now landed (no longer "proposed"). |
| `apps/portal-bff/src/auth/principal-builder.ts` | Signature: `build(user)` → `build(user, identity: { userId, personId })`. `Principal.user.id` / `Principal.user.personId` populated from `identity` instead of `user.oid`. Scope resolver called with `{ userId: identity.userId }`. Doc comment block updated to remove the "placeholder" caveat and document the new wiring. |
| `apps/portal-bff/src/auth/principal-builder.spec.ts` | New `TEST_IDENTITY` constant. Every `builder.build(...)` call gets the second arg. New assertions on `principal.user.id` / `principal.user.personId` carrying the test identity. Edge-case test "asks the scope resolver to resolve by entraOid" renamed + retargeted to `{ userId }`. **All 19 persona tests + 5 edge cases pass.** |
| `apps/portal-bff/src/auth/session-establisher.service.ts` | New constructor arg `personUserProvisioner: PersonAndUserProvisioner`. `establish()` calls `ensureUser({ oid, tenantId, displayName, email: user.username })` **before** `principalBuilder.build` so the identity is available. Comment block explains the Entra `preferred_username` → `Person.email` mapping and the blocking-vs-best-effort distinction with `UserDirectoryService.recordSignIn`. |
| `apps/portal-bff/src/auth/session-establisher.service.spec.ts` | Fixture extended with `provisioner` mock + `PROVISIONED_IDENTITY` constant. `STUB_PRINCIPAL` now carries those UUIDs instead of `user.oid`. New tests: (1) provisioner is called with the right input shape; (2) provisioner runs **before** `principalBuilder.build` (`mock.invocationCallOrder` assertion); (3) provisioner failure propagates and nothing downstream runs (build / audit / directory). |
| `apps/portal-bff/src/auth/auth.controller.spec.ts` | Provisioner mock added to the controller fixture (the controller's specs don't exercise provisioning themselves but `SessionEstablisher`'s constructor needs the arg). Principal stub's UUIDs aligned with the new provisioned shape. |
| `apps/portal-bff/src/users/users.module.ts` | `PersonAndUserProvisioner` registered + exported alongside `UserDirectoryService`. `@Global` so the auth module's `SessionEstablisher` can inject both without re-routing the module graph. Comment block updated to document the blocking/best-effort distinction between the two. |
## Key choices
- **Provisioner is blocking**, not best-effort. Distinct from `UserDirectoryService.recordSignIn` which is still best-effort (ADR-0020 admin-list cache, swallows its own errors). Both run from `SessionEstablisher.establish` per the new ordering: (1) provisioner — blocking, (2) build principal — uses identity, (3) save session, (4) cookie, (5) user-session index (best-effort), (6) audit (blocking ADR-0013), (7) UserDirectoryService.recordSignIn (best-effort), (8) log. A provisioner failure short-circuits the whole flow before any of (3)..(8) — the spec asserts this explicitly.
- **No email-based dedup in v1.** `Person.email` is indexed (not unique). The provisioner keys ONLY on `entraOid`. ADR-0026 §"Why no email-based merging in v1" — two distinct humans genuinely share emails; ADR-0029's sync flow handles operator-confirmed reconciliation.
- **Race-condition handling on first sign-in.** Two concurrent first-sign-ins for the same `entraOid` both fall through to `create`. The unique constraint on `User.entraOid` rejects the loser with P2002; the provisioner catches that specific code and re-runs `ensureUser` — fast path now warm. Pathological infinite-loop guarded by the warm-path behaviour on the second attempt. Spec covers the success retry + the non-P2002 propagation paths.
- **ScopeResolver seam moved from `{ entraOid }` to `{ userId }`.** The stub doesn't care about its argument either way; the change is the seam for ADR-0026 PR 2's `PrismaScopeResolver`, which keys `userScope` queries on `User.id` (UUID). Doing the rename now keeps PR 2 to "swap the implementation" only.
- **`PersonAndUserProvisioner.SELF_SIGNIN_SOURCE`** is a typed constant inside the class, not a magic string. Defense in depth: TS type union (compile-time) + drift gate (`source: 'self-signin'` literal is in a file importing `person-source.ts` — flagged if it ever drifts off-catalogue) + runtime `isPersonSource` check (error log + throw if the constant is mutated to an off-catalogue value).
- **UserDirectoryService stays.** ADR-0020's admin-list cache is functionally redundant with Person + User now, but folding it would extend the PR scope (DTO + reader + admin UI + migration of existing rows). Out of scope here — flagged as a follow-up.
## Local verification
- [x] `node scripts/check-catalogue-drift.mjs` — clean (`4 privileges, 24 roles, 7 structure kinds, 3 person sources`).
- [x] `node --test scripts/check-catalogue-drift.spec.mjs` — **29 tests passing** (was 22 — +7 for PERSON_SOURCES).
- [x] `pnpm exec nx lint portal-bff` — **0 errors**, 13 warnings (all pre-existing, none from this PR).
- [x] `pnpm exec nx test portal-bff` (filtered to the 6 affected spec files) — **766 tests passing**.
- [x] `pnpm exec prisma generate` — client regenerated with the 3 new models.
## Test plan — remaining (on a fresh dev DB)
- [ ] `./infra/local/dev.sh down -v && ./infra/local/dev.sh up` (wipe + reboot) then `cd apps/portal-bff && pnpm exec prisma migrate dev` — applies the new migration cleanly, no drift prompt.
- [ ] `pnpm exec prisma studio` — `persons` / `users` / `user_scopes` tables visible, all empty (no seed in this PR).
- [ ] Sign in via `apf-portal` against the test tenant — `users` table gets one row, `persons` table gets one row, `user_scopes` stays empty. Principal in the session carries the provisioned UUIDs (not the `entraOid`).
- [ ] **Review focus** — the provisioner's race-handling logic; the `Entra preferred_username → Person.email` mapping in `SessionEstablisher`; the ScopeResolver seam change rationale; the FK actions in the migration SQL (especially CASCADE on UserScope.userId vs RESTRICT on User.personId).
## What's next
- **ADR-0026 PR 2** — `PrismaScopeResolver` replacing `StubScopeResolver` + `/admin/users/:id/scopes` admin-app screen + `prisma/seed.ts` populating the 19 test personas' `user_scopes` per `notes/test-tenant-role-assignments.md` (referencing this PR's `User.id` and ADR-0027 PR 1's `Structure.code` values).
- **Future PR (out of this scope)** — fold `UserDirectoryEntry` into Person + User now that the latter exists. Touches AdminUsersReader, the DTO, the admin SPA, and a data migration for existing rows. Defer until ADR-0026 PR 2 stabilises.
---------
Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #232
|
||
|
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
|
2eb01f59b3 |
feat(ci): catalogue-drift gate for @RequirePrivilege/@RequireRole literals (ADR-0025) (#211)
## Summary
Phase 3 (and last) of [ADR-0025](docs/decisions/0025-authorization-model-privileges-roles-scopes.md)'s implementation phasing (§"More Information" / §347): a small CI gate that asserts every string literal passed to the authorization decorators belongs to the closed catalogue declared in `libs/shared/auth/src/lib/authorization.types.ts`.
The TypeScript decorator signatures (`(...privileges: [Privilege, ...Privilege[]])`) already enforce this at compile time. The gate is **defence-in-depth** against the escape hatches the type system cannot catch:
- explicit `as Privilege` / `as FunctionalRole` casts (`'Portal.Foo' as Privilege`),
- the rare case where a developer hand-edits the catalogue type union without updating the runtime constant.
Runs in `<1s`, so it rides the existing `check` CI job rather than spinning up its own.
## What lands
| File | Role |
| --- | --- |
| `scripts/check-catalogue-drift.mjs` | The gate. TypeScript compiler API. Parses `authorization.types.ts` to extract the catalogues, walks every `.ts` file under `apps/` and `libs/`, finds each `CallExpression` whose callee identifier matches `RequirePrivilege` / `RequireRole`, validates every string-literal argument. Non-literal args are skipped (the TypeScript signature catches them already). Reports grouped by file with `line:column` per violation, exits 1 on drift. |
| `scripts/check-catalogue-drift.spec.mjs` | 13 tests via `node --test` (built-in runner, no Vitest dep). Covers catalogue parsing, file-level violation detection, workspace-wide aggregation, skipped folders, gen-stub exclusion, line/column reporting. |
| `package.json` | `ci:catalogue-drift` + `ci:catalogue-drift:test` scripts. |
| `.gitea/workflows/ci.yml` | The existing `check` job now runs the gate's unit tests first (fail-fast if the gate itself is broken), then the gate against the live workspace. |
## Notes for the reviewer
- **Why not an ESLint custom rule.** ADR-0025 §347 floats either option. The ESLint route would give editor-time feedback (red squiggles) but means standing up a local ESLint plugin lib — net ~300 lines of plumbing for a gate the TypeScript signature already enforces in real time via the literal-union types. The pnpm-script route mirrors the existing `ci:gzip-budgets` pattern; ~250 lines including tests; runs in the same `check` CI job that already installs deps. Editor feedback is **already provided by `tsc`**, so the gate's job reduces to "catch escape hatches in CI", which the script does fine.
- **Why `node --test` rather than Vitest / Jest.** The script lives in `scripts/`, outside any Nx project. Wiring Vitest for one spec file would mean a vitest.config + tsconfig.spec + Nx project just to host it. Node's built-in test runner ships with the runtime we already pin (Node 24 in `.nvmrc`), no config, ~250ms wall clock for 13 tests.
- **Generated gRPC stubs are skipped.** `apps/portal-bff/src/grpc/gen/apf-ai/common.ts` defines a `roles: string[]` proto field used by the AI-bridge — entirely unrelated to the ADR-0025 functional-role catalogue. The skip list is explicit (`SKIPPED_SUBPATHS`); adding a new codegen output later is one line.
- **Spec files are NOT skipped.** `auth-guards.persona-matrix.spec.ts` references catalogue values via the decorators in its test fixtures. Those are deliberate references and must stay in sync — if a future ADR amendment removes a role, the spec catches it on the same run as the production code. Explicitly tested in `does NOT skip spec files`.
- **Non-literal arguments (variable indirection) are skipped.** A pattern like `const slug = getRole(); @RequireRole(slug)` would not be caught by string-literal inspection. This is intentional: the TypeScript decorator signature already requires `slug` to be typed `FunctionalRole`, so the type system handles it; the gate's value-add is on literal misspellings the type system cannot see past an `as Privilege` cast.
- **Self-test confirmed the gate works.** Injected `RequirePrivilege('Portal.RogueDrift')` and `RequireRole('rogue-role-x')` into an existing spec, ran `pnpm ci:catalogue-drift`, observed:
```
catalogue-drift: violations found
apps/portal-bff/src/auth/require-privilege.guard.spec.ts
135:18 @RequirePrivilege('Portal.RogueDrift') — not in PRIVILEGES
136:13 @RequireRole('rogue-role-x') — not in FUNCTIONAL_ROLES
```
Exit code 1. Reverted the injection; gate green again.
- **Adding a third decorator** (per ADR-0025's anticipated growth) is one line in `DECORATOR_TO_CATALOGUE` plus the matching test fixture. The script does not hardcode the decorator name list beyond that map.
- **Why no scope-kind check.** `@RequireScope` takes an `(req) => ScopableResource` extractor, not literals. The scope `kind` values are constrained by the `ScopeKind` discriminated union, which TypeScript enforces at every `{ kind: ... }` construction site. No literal escape hatch worth gating in v1.
## Test plan
- [x] `pnpm ci:catalogue-drift:test` — 13/13 green via `node --test`.
- [x] `pnpm ci:catalogue-drift` — clean against the live workspace: `catalogue-drift: clean (catalogues: 4 privileges, 24 roles).`
- [x] Self-test by injection — script catches `'Portal.RogueDrift'` and `'rogue-role-x'` with file:line:column, exits 1.
- [x] `pnpm exec prettier --check` on the new files — clean.
- [ ] CI run on this PR exercises the new step in the `check` job.
## What's next
ADR-0025's phasing closes with this PR. Remaining authorization work waits on [ADR-0026](docs/decisions/) (proposed) — the `Person` + `User` schema brings the Prisma-backed `user_scopes` table; that PR replaces `StubScopeResolver` with a `PrismaScopeResolver` and unlocks the first concrete `@RequireScope` consumer surfaces.
---------
Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #211
|