Files
apf_portal/apps/portal-bff/src/auth/principal-builder.spec.ts
T
julien 24071a63ec
CI / check (push) Successful in 7m16s
CI / commits (push) Has been skipped
CI / scan (push) Successful in 3m13s
CI / a11y (push) Successful in 2m22s
CI / perf (push) Successful in 5m37s
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
2026-05-26 13:57:36 +02:00

360 lines
12 KiB
TypeScript

import type { Logger } from 'nestjs-pino';
import {
EntraGroupToRoleResolver,
parseEntraGroupMap,
type FunctionalRole,
type Scope,
} from 'shared-auth';
import type { AuthenticatedUser } from './auth.service';
import { PrincipalBuilder } from './principal-builder';
import type { ScopeResolver } from './scope-resolver';
/**
* Mirrors the 24-entry `apf-role-*` catalogue: one synthetic GUID
* per slug, in catalogue order. The exact GUIDs do not matter to
* any assertion below — what matters is that the test resolver
* maps them deterministically so a persona's `groups` claim can
* be assembled by slug.
*/
const ROLE_GUID: Record<FunctionalRole, string> = {
collaborateur: '00000000-0000-0000-0000-000000000101',
'chef-equipe': '00000000-0000-0000-0000-000000000102',
'chef-service': '00000000-0000-0000-0000-000000000103',
'directeur-etablissement': '00000000-0000-0000-0000-000000000104',
'directeur-territorial': '00000000-0000-0000-0000-000000000105',
rh: '00000000-0000-0000-0000-000000000106',
'responsable-paie': '00000000-0000-0000-0000-000000000107',
comptable: '00000000-0000-0000-0000-000000000108',
juriste: '00000000-0000-0000-0000-000000000109',
dpo: '00000000-0000-0000-0000-00000000010a',
rssi: '00000000-0000-0000-0000-00000000010b',
it: '00000000-0000-0000-0000-00000000010c',
formation: '00000000-0000-0000-0000-00000000010d',
qualite: '00000000-0000-0000-0000-00000000010e',
communication: '00000000-0000-0000-0000-00000000010f',
'elu-ca': '00000000-0000-0000-0000-000000000110',
'elu-cd': '00000000-0000-0000-0000-000000000111',
'elu-cd-president': '00000000-0000-0000-0000-000000000112',
'elu-cd-tresorier': '00000000-0000-0000-0000-000000000113',
'elu-cd-secretaire': '00000000-0000-0000-0000-000000000114',
delegue: '00000000-0000-0000-0000-000000000115',
benevole: '00000000-0000-0000-0000-000000000116',
'benevole-responsable': '00000000-0000-0000-0000-000000000117',
partenaire: '00000000-0000-0000-0000-000000000118',
};
/**
* The 19 personas provisioned in the `apfrd.onmicrosoft.com` test
* tenant on 2026-05-20, transcribed from
* `notes/test-tenant-role-assignments.md` (reverse view). The
* builder's contract is that these inputs round-trip to the
* expected `(privileges, roles, scopes)` triple — when the spec
* passes, every persona we documented in the ADR is exercised
* end-to-end.
*
* Scopes are stubbed to `[{ kind: 'unrestricted' }]` for every
* persona in v1 per ADR-0025 §"Sources of truth — apf_portal-side
* `user_scopes` table". The intended per-persona scopes
* (`etablissement:…`, `delegation:33`, …) land with the Prisma
* `user_scopes` table — the next PR after this skeleton.
*/
interface Persona {
readonly label: string;
readonly username: string;
readonly privileges: readonly string[]; // raw `roles` claim values
readonly roles: readonly FunctionalRole[];
}
const PERSONAS: readonly Persona[] = [
{
label: 'admin',
username: 'admin@apfrd.onmicrosoft.com',
privileges: ['Portal.Admin'],
roles: ['collaborateur', 'rh'],
},
{
label: 'directeur-bordeaux',
username: 'directeur-bordeaux@apfrd.onmicrosoft.com',
privileges: [],
roles: ['collaborateur', 'directeur-etablissement'],
},
{
label: 'directeur-complexe',
username: 'directeur-complexe@apfrd.onmicrosoft.com',
privileges: [],
roles: ['collaborateur', 'directeur-etablissement'],
},
{
label: 'rh-aquitaine',
username: 'rh-aquitaine@apfrd.onmicrosoft.com',
privileges: [],
roles: ['collaborateur', 'rh', 'formation'],
},
{
label: 'rh-siege',
username: 'rh-siege@apfrd.onmicrosoft.com',
privileges: [],
roles: ['collaborateur', 'rh', 'responsable-paie', 'comptable'],
},
{
label: 'collaborateur-simple',
username: 'collaborateur-simple@apfrd.onmicrosoft.com',
privileges: [],
roles: ['collaborateur'],
},
{
label: 'tresorier-bordeaux',
username: 'tresorier-bordeaux@apfrd.onmicrosoft.com',
privileges: [],
roles: ['elu-cd', 'elu-cd-tresorier'],
},
{
label: 'dpo',
username: 'dpo@apfrd.onmicrosoft.com',
privileges: ['Portal.DPO', 'Portal.Auditor'],
roles: ['collaborateur', 'dpo', 'qualite'],
},
{
label: 'it',
username: 'it@apfrd.onmicrosoft.com',
privileges: [],
roles: ['collaborateur', 'it'],
},
{
label: 'benevole-aquitaine',
username: 'benevole-aquitaine@apfrd.onmicrosoft.com',
privileges: [],
// Catalogue order: delegue (governance) precedes benevole +
// benevole-responsable (volunteer). The resolver re-sorts on
// catalogue order regardless of the Entra claim's order.
roles: ['delegue', 'benevole', 'benevole-responsable'],
},
{
label: 'chef-equipe-bordeaux',
username: 'chef-equipe-bordeaux@apfrd.onmicrosoft.com',
privileges: [],
roles: ['collaborateur', 'chef-equipe'],
},
{
label: 'chef-service-bordeaux',
username: 'chef-service-bordeaux@apfrd.onmicrosoft.com',
privileges: [],
roles: ['collaborateur', 'chef-service'],
},
{
label: 'directeur-territorial-aquitaine',
username: 'directeur-territorial-aquitaine@apfrd.onmicrosoft.com',
privileges: [],
roles: ['collaborateur', 'directeur-territorial'],
},
{
label: 'juriste-siege',
username: 'juriste-siege@apfrd.onmicrosoft.com',
privileges: [],
roles: ['collaborateur', 'juriste'],
},
{
label: 'rssi',
username: 'rssi@apfrd.onmicrosoft.com',
privileges: ['Portal.SecurityOfficer'],
roles: ['collaborateur', 'rssi'],
},
{
label: 'communication-siege',
username: 'communication-siege@apfrd.onmicrosoft.com',
privileges: [],
roles: ['collaborateur', 'communication'],
},
{
label: 'elu-ca-national',
username: 'elu-ca-national@apfrd.onmicrosoft.com',
privileges: [],
roles: ['elu-ca'],
},
{
label: 'president-cd-aquitaine',
username: 'president-cd-aquitaine@apfrd.onmicrosoft.com',
privileges: [],
roles: ['elu-cd', 'elu-cd-president'],
},
{
label: 'secretaire-cd-aquitaine',
username: 'secretaire-cd-aquitaine@apfrd.onmicrosoft.com',
privileges: [],
roles: ['elu-cd', 'elu-cd-secretaire'],
},
];
function makeUser(p: Persona): AuthenticatedUser {
return {
oid: `oid-${p.label}`,
tid: 'tenant-1',
username: p.username,
displayName: p.label,
amr: ['pwd', 'mfa'],
roles: [...p.privileges],
groups: p.roles.map((slug) => ROLE_GUID[slug]),
};
}
/**
* Fixed identity used by every `build()` call in the spec. The
* provisioner is mocked at the `SessionEstablisher` test level; the
* `PrincipalBuilder` spec receives the UUIDs verbatim — what matters
* here is that they round-trip onto `Principal.user.{id, personId}`.
*/
const TEST_IDENTITY = {
userId: '00000000-0000-0000-0000-000000000fff',
personId: '00000000-0000-0000-0000-000000000eee',
};
function makeBuilder(): {
builder: PrincipalBuilder;
scopeResolver: { resolve: jest.Mock };
logger: { log: jest.Mock; warn: jest.Mock; error: jest.Mock };
} {
const rawMap: Record<string, FunctionalRole> = {};
for (const [slug, guid] of Object.entries(ROLE_GUID) as Array<[FunctionalRole, string]>) {
rawMap[guid] = slug;
}
const resolver = new EntraGroupToRoleResolver(parseEntraGroupMap(rawMap));
const scopeResolver = {
resolve: jest.fn().mockResolvedValue([{ kind: 'unrestricted' } as Scope]),
};
const logger = { log: jest.fn(), warn: jest.fn(), error: jest.fn() };
const builder = new PrincipalBuilder(
resolver,
scopeResolver as unknown as ScopeResolver,
logger as unknown as Logger,
);
return { builder, scopeResolver, logger };
}
describe('PrincipalBuilder — 19 test-tenant personas', () => {
for (const persona of PERSONAS) {
it(`builds the principal for ${persona.label}`, async () => {
const { builder } = makeBuilder();
const principal = await builder.build(makeUser(persona), TEST_IDENTITY);
expect(principal.privileges).toEqual(persona.privileges);
expect(principal.roles).toEqual(persona.roles);
expect(principal.scopes).toEqual([{ kind: 'unrestricted' }]);
expect(principal.user.entraOid).toBe(`oid-${persona.label}`);
expect(principal.user.tenantId).toBe('tenant-1');
// Real UUIDs from the provisioner, per ADR-0026 PR 1 — no
// longer the entraOid placeholder.
expect(principal.user.id).toBe(TEST_IDENTITY.userId);
expect(principal.user.personId).toBe(TEST_IDENTITY.personId);
// amr passes through verbatim — MFA freshness checks read it
// off the principal per ADR-0011.
expect(principal.amr).toEqual(['pwd', 'mfa']);
});
}
it('asserts the 19 personas cover all 4 privileges + 23 of 24 functional roles', () => {
// Coverage check baked into the spec so a regression that
// drops a role from a persona is caught here, not at PR review.
// The intentional gap is `partenaire` (placeholder per ADR-0025).
const privilegesUsed = new Set(PERSONAS.flatMap((p) => p.privileges));
expect(privilegesUsed).toEqual(
new Set(['Portal.Admin', 'Portal.Auditor', 'Portal.SecurityOfficer', 'Portal.DPO']),
);
const rolesUsed = new Set(PERSONAS.flatMap((p) => p.roles));
expect(rolesUsed.size).toBe(23);
expect(rolesUsed.has('partenaire')).toBe(false);
});
});
describe('PrincipalBuilder — edge cases', () => {
it('returns an empty privileges array when the user has no app role assignment', async () => {
const { builder } = makeBuilder();
const user = makeUser({
label: 'anon',
username: 'anon@example',
privileges: [],
roles: ['collaborateur'],
});
const principal = await builder.build(user, TEST_IDENTITY);
expect(principal.privileges).toEqual([]);
});
it('drops + warns on a claim value that is not a known privilege', async () => {
const { builder, logger } = makeBuilder();
const user = makeUser({
label: 'stale',
username: 'stale@example',
privileges: [],
roles: [],
});
// Force in a non-catalogue value the way an old assignment would.
const userWithDrift: AuthenticatedUser = {
...user,
roles: ['Portal.Admin', 'Portal.GhostRole'],
};
const principal = await builder.build(userWithDrift, TEST_IDENTITY);
expect(principal.privileges).toEqual(['Portal.Admin']);
expect(logger.warn).toHaveBeenCalledWith(
expect.objectContaining({
event: 'auth.unknown_privilege_claim',
value: 'Portal.GhostRole',
}),
'PrincipalBuilder',
);
});
it('drops + warns on an unknown group GUID (tenant misconfiguration)', async () => {
const { builder, logger } = makeBuilder();
const user = makeUser({
label: 'ghost-group',
username: 'ghost-group@example',
privileges: [],
roles: ['collaborateur'],
});
const userWithUnknownGroup: AuthenticatedUser = {
...user,
groups: [...user.groups, 'ffffffff-ffff-ffff-ffff-ffffffffffff'],
};
const principal = await builder.build(userWithUnknownGroup, TEST_IDENTITY);
expect(principal.roles).toEqual(['collaborateur']);
expect(logger.warn).toHaveBeenCalledWith(
expect.objectContaining({
event: 'auth.unknown_group_claim',
groupId: 'ffffffff-ffff-ffff-ffff-ffffffffffff',
}),
'PrincipalBuilder',
);
});
it('builds an empty-permissions principal when the user has no groups or roles', async () => {
const { builder } = makeBuilder();
const user = makeUser({
label: 'empty',
username: 'empty@example',
privileges: [],
roles: [],
});
const principal = await builder.build(user, TEST_IDENTITY);
expect(principal.privileges).toEqual([]);
expect(principal.roles).toEqual([]);
// Scope resolver stub returns unrestricted — the v1 behaviour
// per ADR-0025 §331 until the user_scopes table lands.
expect(principal.scopes).toEqual([{ kind: 'unrestricted' }]);
});
it('asks the scope resolver to resolve by userId (ADR-0026 PR 1 seam change)', async () => {
const { builder, scopeResolver } = makeBuilder();
await builder.build(
makeUser({
label: 'sr',
username: 'sr@example',
privileges: [],
roles: ['collaborateur'],
}),
TEST_IDENTITY,
);
// Pre-ADR-0026 the resolver was called with { entraOid }; PR 1
// switches the seam to { userId } so PR 2's PrismaScopeResolver
// can key queries on the real User.id.
expect(scopeResolver.resolve).toHaveBeenCalledWith({ userId: TEST_IDENTITY.userId });
});
});