From 056848e50274ce59b6610a19aa9e638e578c28b8 Mon Sep 17 00:00:00 2001 From: Julien Gautier Date: Tue, 26 May 2026 13:54:41 +0200 Subject: [PATCH] feat(users): add Person + User + UserScope + lazy provisioner (ADR-0026 PR 1) Schema: Person golden record + User portal-account overlay (1-to-0-or-1) + UserScope (the table behind the ADR-0025 scope axis). All in the public schema. UserScope.value is opaque (no FK to ADR-0027 tables - historical scope rows survive structure decommissioning). Migration: hand-written, follows the existing convention. FKs with correct ON DELETE actions (RESTRICT on User.personId for the required relation, CASCADE on UserScope.userId via explicit onDelete: Cascade in the schema). Catalogue: PERSON_SOURCES = [self-signin, admin-ui, seed] as const under apps/portal-bff/src/users/person-source.ts. Defense in depth = TS type union + drift gate scanner extension (property literal "source: 'X'" in files importing person-source.ts). PersonAndUserProvisioner: blocking lazy-create at first OIDC callback. Fast path = findUnique by entraOid + update lastSignInAt. Cold path = nested create of Person + User in one transaction. Race-condition handling: catches P2002 on User.entraOid unique constraint and re-runs ensureUser. Defense-in-depth isPersonSource check on the constant. PrincipalBuilder signature: build(user, identity: { userId, personId }). Principal.user.{id, personId} populated from real UUIDs. ScopeResolver seam moved from { entraOid } to { userId } so PR 2's PrismaScopeResolver can key queries on User.id. SessionEstablisher: new constructor arg personUserProvisioner. establish() calls ensureUser BEFORE principalBuilder.build so the identity is available. Entra preferred_username (= AuthenticatedUser. username) maps to Person.email per ADR-0026 lifecycle. Provisioner failure short-circuits the whole flow before save / cookie / audit / directory. UserDirectoryService stays as the ADR-0020 admin-list cache - folding it into Person+User is a follow-up after ADR-0026 PR 2 stabilises. Local verification: drift gate clean (4 / 24 / 7 / 3), 29 drift-gate spec tests passing, portal-bff lint 0 errors, 766 portal-bff specs passing. --- .../migration.sql | 84 ++++++++ apps/portal-bff/prisma/schema.prisma | 112 ++++++++++ .../src/auth/auth.controller.spec.ts | 15 +- .../src/auth/principal-builder.spec.ts | 33 ++- apps/portal-bff/src/auth/principal-builder.ts | 26 +-- apps/portal-bff/src/auth/scope-resolver.ts | 38 ++-- .../auth/session-establisher.service.spec.ts | 59 +++++- .../src/auth/session-establisher.service.ts | 18 +- ...erson-and-user-provisioner.service.spec.ts | 198 ++++++++++++++++++ .../person-and-user-provisioner.service.ts | 184 ++++++++++++++++ .../src/users/person-source.spec.ts | 38 ++++ apps/portal-bff/src/users/person-source.ts | 34 +++ apps/portal-bff/src/users/users.module.ts | 33 +-- scripts/check-catalogue-drift.mjs | 176 ++++++++++------ scripts/check-catalogue-drift.spec.mjs | 111 +++++++++- 15 files changed, 1033 insertions(+), 126 deletions(-) create mode 100644 apps/portal-bff/prisma/migrations/20260526210000_add_person_user_userscope/migration.sql create mode 100644 apps/portal-bff/src/users/person-and-user-provisioner.service.spec.ts create mode 100644 apps/portal-bff/src/users/person-and-user-provisioner.service.ts create mode 100644 apps/portal-bff/src/users/person-source.spec.ts create mode 100644 apps/portal-bff/src/users/person-source.ts diff --git a/apps/portal-bff/prisma/migrations/20260526210000_add_person_user_userscope/migration.sql b/apps/portal-bff/prisma/migrations/20260526210000_add_person_user_userscope/migration.sql new file mode 100644 index 0000000..c04e0c3 --- /dev/null +++ b/apps/portal-bff/prisma/migrations/20260526210000_add_person_user_userscope/migration.sql @@ -0,0 +1,84 @@ +-- Identity model (per ADR-0026 PR 1). +-- +-- Person golden record + User portal-account overlay (1-to-0-or-1) +-- + UserScope (the table behind the ADR-0025 scope axis). Distinct +-- from `user_directory_entries` (ADR-0020 sign-in cache, renamed in +-- the previous migration) — those keep their role; these are new. +-- +-- No seed data — the test-tenant scope rows ship in ADR-0026 PR 2 +-- via `prisma/seed.ts`, after this schema is in place. + +-- CreateTable +CREATE TABLE "persons" ( + "id" UUID NOT NULL, + "first_name" TEXT NOT NULL, + "last_name" TEXT NOT NULL, + "email" TEXT, + "source" TEXT NOT NULL, + "external_id" TEXT, + "created_at" TIMESTAMPTZ(6) NOT NULL DEFAULT CURRENT_TIMESTAMP, + "updated_at" TIMESTAMPTZ(6) NOT NULL, + + CONSTRAINT "persons_pkey" PRIMARY KEY ("id") +); + +-- CreateTable +CREATE TABLE "users" ( + "id" UUID NOT NULL, + "person_id" UUID NOT NULL, + "entra_oid" TEXT NOT NULL, + "tenant_id" TEXT NOT NULL, + "last_sign_in_at" TIMESTAMPTZ(6), + "created_at" TIMESTAMPTZ(6) NOT NULL DEFAULT CURRENT_TIMESTAMP, + "updated_at" TIMESTAMPTZ(6) NOT NULL, + + CONSTRAINT "users_pkey" PRIMARY KEY ("id") +); + +-- CreateTable +CREATE TABLE "user_scopes" ( + "id" UUID NOT NULL, + "user_id" UUID NOT NULL, + "kind" TEXT NOT NULL, + "value" TEXT NOT NULL DEFAULT '', + "source" TEXT NOT NULL, + "created_at" TIMESTAMPTZ(6) NOT NULL DEFAULT CURRENT_TIMESTAMP, + "expires_at" TIMESTAMPTZ(6), + + CONSTRAINT "user_scopes_pkey" PRIMARY KEY ("id") +); + +-- Person indexes +CREATE INDEX "persons_source_idx" ON "persons"("source"); + +CREATE INDEX "persons_external_id_idx" ON "persons"("external_id"); + +CREATE INDEX "persons_email_idx" ON "persons"("email"); + +-- User indexes — both unique constraints back btree indexes. +CREATE UNIQUE INDEX "users_person_id_key" ON "users"("person_id"); + +CREATE UNIQUE INDEX "users_entra_oid_key" ON "users"("entra_oid"); + +-- UserScope indexes — the unique constraint backs the composite, +-- the plain index supports the read-by-userId hot path on sign-in. +CREATE UNIQUE INDEX "user_scopes_user_id_kind_value_key" ON "user_scopes"("user_id", "kind", "value"); + +CREATE INDEX "user_scopes_user_id_idx" ON "user_scopes"("user_id"); + +-- Foreign keys. +-- +-- User.person_id is REQUIRED → ON DELETE RESTRICT (Prisma default for +-- required relations). Removing a Person with a portal User must be +-- an explicit two-step in the admin path; never an accidental cascade. +ALTER TABLE "users" ADD CONSTRAINT "users_person_id_fkey" + FOREIGN KEY ("person_id") REFERENCES "persons"("id") + ON DELETE RESTRICT ON UPDATE CASCADE; + +-- UserScope.user_id has explicit `onDelete: Cascade` in the Prisma +-- schema — revoking a User wipes their scope rows in one delete, +-- which is the desired admin-UI semantic (deactivating an account +-- should not leave dangling authorisation rows). +ALTER TABLE "user_scopes" ADD CONSTRAINT "user_scopes_user_id_fkey" + FOREIGN KEY ("user_id") REFERENCES "users"("id") + ON DELETE CASCADE ON UPDATE CASCADE; diff --git a/apps/portal-bff/prisma/schema.prisma b/apps/portal-bff/prisma/schema.prisma index 24b3279..d3a0f7a 100644 --- a/apps/portal-bff/prisma/schema.prisma +++ b/apps/portal-bff/prisma/schema.prisma @@ -106,6 +106,118 @@ model UserDirectoryEntry { @@index([username]) } +// ============================================================ +// Identity model (per ADR-0026) +// ============================================================ +// +// `Person` golden record + `User` portal-account overlay (one-to-zero- +// or-one). Distinct from `UserDirectoryEntry` above — that one is the +// ADR-0020 sign-in cache keyed on Entra `oid`; these are the portal's +// stable identity model keyed on UUIDs and form the basis for the +// `@RequireScope` Prisma resolver landing in ADR-0026 PR 2. +// +// `Person` exists whether or not the human ever signs in (workforce +// pre-provisioning, dossier bénéficiaires, alumni). `User` is the +// portal-access overlay, lazy-created at first OIDC callback by +// `PersonAndUserProvisioner.ensureUser`. `UserScope` backs the ADR-0025 +// scope axis with opaque `value` strings referencing ADR-0027's +// `Structure.code` / `Delegation.code` / `Region.code` — no FK at the +// DB level so historical scope rows survive structure decommissioning. + +model Person { + id String @id @default(uuid()) @db.Uuid + // PII — firstName + lastName are subject to ADR-0013 redaction rules + // when emitted to logs. + firstName String @map("first_name") + lastName String @map("last_name") + // Primary contact email. Indexed for operator lookup but NOT unique + // — two distinct humans genuinely can share emails (shared family + // alias, generic info@ at a small partner organisation, error in an + // upstream feed). ADR-0029's reconciliation flow surfaces candidate + // duplicates for operator confirmation rather than auto-merging. + email String? + // Closed-set catalogue, drift-gated. See + // `apps/portal-bff/src/users/person-source.ts` for the legal values. + // v1: 'self-signin' / 'admin-ui' / 'seed'. ADR-0029 adds 'pleiades' + // and 'acteurs-plus'. + source String + // Upstream-system identifier (Pléiades matricule, Acteurs+ id). + // Nullable in v1 — populated by ADR-0029's syncs. + externalId String? @map("external_id") + createdAt DateTime @default(now()) @map("created_at") @db.Timestamptz(6) + updatedAt DateTime @updatedAt @map("updated_at") @db.Timestamptz(6) + + // Back-ref. NULL when the Person has no portal account (workforce + // pre-provisioning, dossier bénéficiaire, alumni). + user User? + + @@map("persons") + @@schema("public") + @@index([source]) + @@index([externalId]) + @@index([email]) +} + +model User { + id String @id @default(uuid()) @db.Uuid + // 1-to-1 with Person. The unique constraint on personId enforces + // "at most one User per Person"; back-ref `Person.user` is the + // other half. + personId String @unique @map("person_id") @db.Uuid + person Person @relation(fields: [personId], references: [id]) + // Entra object id — stable per-user identifier inside the tenant. + // Unique because two separate Person+User pairs cannot share an + // Entra identity. + entraOid String @unique @map("entra_oid") + // Entra tenant id. Stored alongside the oid so a future dual- + // audience activation (ADR-0008) can disambiguate. + tenantId String @map("tenant_id") + // Refreshed by `PersonAndUserProvisioner.ensureUser` on every + // sign-in. Surfaced in the future /admin/users/:id screen. + lastSignInAt DateTime? @map("last_sign_in_at") @db.Timestamptz(6) + createdAt DateTime @default(now()) @map("created_at") @db.Timestamptz(6) + updatedAt DateTime @updatedAt @map("updated_at") @db.Timestamptz(6) + + scopes UserScope[] + + @@map("users") + @@schema("public") +} + +model UserScope { + id String @id @default(uuid()) @db.Uuid + userId String @map("user_id") @db.Uuid + // CASCADE so revoking a User wipes their scope rows in one delete. + user User @relation(fields: [userId], references: [id], onDelete: Cascade) + // One of the six ADR-0025 scope kinds: 'self' / 'etablissement' / + // 'delegation' / 'region' / 'siege' / 'unrestricted'. Not pulled + // from a Prisma enum because the value's semantics belong to the + // `shared-auth` catalogue, not the database; the drift gate + // already enforces the closed set at the call sites. + kind String + // Per-kind payload — semantics owned by ADR-0027. For 'etablissement' + // the value is a Structure.code; for 'delegation' it is a + // Delegation.code; for 'region' it is a Region.code. Empty string + // for 'self' / 'siege' / 'unrestricted'. NO FK to ADR-0027 tables — + // historical scope rows survive structure decommissioning; the + // admin-UI write path (ADR-0026 PR 2) validates at insert time. + value String @default("") + // Provenance, same catalogue posture as Person.source. v1: + // 'admin-ui' / 'seed'. ADR-0029 adds upstream-sync values and a + // reconciliation policy. + source String + createdAt DateTime @default(now()) @map("created_at") @db.Timestamptz(6) + // When non-null and past, the row is ignored at sign-in. Supports + // interim-director-for-two-months patterns and admin UI scope + // revocations. + expiresAt DateTime? @map("expires_at") @db.Timestamptz(6) + + @@map("user_scopes") + @@schema("public") + @@unique([userId, kind, value]) + @@index([userId]) +} + // ============================================================ // Organisational hierarchy (per ADR-0027) // ============================================================ diff --git a/apps/portal-bff/src/auth/auth.controller.spec.ts b/apps/portal-bff/src/auth/auth.controller.spec.ts index 7fdce9c..8837f37 100644 --- a/apps/portal-bff/src/auth/auth.controller.spec.ts +++ b/apps/portal-bff/src/auth/auth.controller.spec.ts @@ -147,6 +147,16 @@ function makeController(opts?: { completeAuthCodeFlow?: jest.Mock }): Controller // The directory service is a best-effort dependency — a noop mock // is sufficient for the controller's behavioural assertions. const userDirectory = { recordSignIn: jest.fn().mockResolvedValue(undefined) }; + // The provisioner is blocking per ADR-0026 — the controller specs + // don't exercise the provisioning behaviour itself, so a noop mock + // returning a fixed UUID pair is enough to let SessionEstablisher + // proceed. + const personUserProvisioner = { + ensureUser: jest.fn().mockResolvedValue({ + userId: '00000000-0000-0000-0000-000000000fff', + personId: '00000000-0000-0000-0000-000000000eee', + }), + }; // Real SessionEstablisher with the same mocks the legacy tests // already wire — keeps the behavioural assertions on session // fields / audit calls untouched after the controller refactor. @@ -156,8 +166,8 @@ function makeController(opts?: { completeAuthCodeFlow?: jest.Mock }): Controller // so SessionEstablisher persists it without throwing. build: jest.fn().mockResolvedValue({ user: { - id: 'oid', - personId: 'oid', + id: '00000000-0000-0000-0000-000000000fff', + personId: '00000000-0000-0000-0000-000000000eee', entraOid: 'oid', tenantId: 'tid', displayName: 'Jane', @@ -173,6 +183,7 @@ function makeController(opts?: { completeAuthCodeFlow?: jest.Mock }): Controller userSessionIndex as unknown as UserSessionIndexService, audit as unknown as AuditWriter, userDirectory as unknown as UserDirectoryService, + personUserProvisioner as unknown as import('../users/person-and-user-provisioner.service').PersonAndUserProvisioner, principalBuilder as unknown as import('./principal-builder').PrincipalBuilder, ); return { diff --git a/apps/portal-bff/src/auth/principal-builder.spec.ts b/apps/portal-bff/src/auth/principal-builder.spec.ts index 3a7676e..9538de9 100644 --- a/apps/portal-bff/src/auth/principal-builder.spec.ts +++ b/apps/portal-bff/src/auth/principal-builder.spec.ts @@ -197,6 +197,17 @@ function makeUser(p: Persona): AuthenticatedUser { }; } +/** + * 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 }; @@ -223,12 +234,16 @@ 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)); + 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']); @@ -258,7 +273,7 @@ describe('PrincipalBuilder — edge cases', () => { privileges: [], roles: ['collaborateur'], }); - const principal = await builder.build(user); + const principal = await builder.build(user, TEST_IDENTITY); expect(principal.privileges).toEqual([]); }); @@ -275,7 +290,7 @@ describe('PrincipalBuilder — edge cases', () => { ...user, roles: ['Portal.Admin', 'Portal.GhostRole'], }; - const principal = await builder.build(userWithDrift); + const principal = await builder.build(userWithDrift, TEST_IDENTITY); expect(principal.privileges).toEqual(['Portal.Admin']); expect(logger.warn).toHaveBeenCalledWith( expect.objectContaining({ @@ -298,7 +313,7 @@ describe('PrincipalBuilder — edge cases', () => { ...user, groups: [...user.groups, 'ffffffff-ffff-ffff-ffff-ffffffffffff'], }; - const principal = await builder.build(userWithUnknownGroup); + const principal = await builder.build(userWithUnknownGroup, TEST_IDENTITY); expect(principal.roles).toEqual(['collaborateur']); expect(logger.warn).toHaveBeenCalledWith( expect.objectContaining({ @@ -317,7 +332,7 @@ describe('PrincipalBuilder — edge cases', () => { privileges: [], roles: [], }); - const principal = await builder.build(user); + const principal = await builder.build(user, TEST_IDENTITY); expect(principal.privileges).toEqual([]); expect(principal.roles).toEqual([]); // Scope resolver stub returns unrestricted — the v1 behaviour @@ -325,7 +340,7 @@ describe('PrincipalBuilder — edge cases', () => { expect(principal.scopes).toEqual([{ kind: 'unrestricted' }]); }); - it('asks the scope resolver to resolve by entraOid', async () => { + it('asks the scope resolver to resolve by userId (ADR-0026 PR 1 seam change)', async () => { const { builder, scopeResolver } = makeBuilder(); await builder.build( makeUser({ @@ -334,7 +349,11 @@ describe('PrincipalBuilder — edge cases', () => { privileges: [], roles: ['collaborateur'], }), + TEST_IDENTITY, ); - expect(scopeResolver.resolve).toHaveBeenCalledWith({ entraOid: 'oid-sr' }); + // 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 }); }); }); diff --git a/apps/portal-bff/src/auth/principal-builder.ts b/apps/portal-bff/src/auth/principal-builder.ts index 666d998..14e71d1 100644 --- a/apps/portal-bff/src/auth/principal-builder.ts +++ b/apps/portal-bff/src/auth/principal-builder.ts @@ -21,17 +21,18 @@ import type { AuthenticatedUser } from './auth.service'; * WARN (per ADR-0025 §"Sources of truth — Entra-side * configuration") and ignored. * - **Scopes** — resolved by `ScopeResolver` from the portal-side - * `user_scopes` table once ADR-0026 lands. v1 stubs to + * `user_scopes` table (schema landed in ADR-0026 PR 1; the + * `PrismaScopeResolver` consumer lands in PR 2). v1 stubs to * `[{ kind: 'unrestricted' }]`. * * Built once per sign-in, not per request: the session payload * carries the resolved `Principal` so guards can read it without * re-doing the GUID lookup on every API call. * - * `user.id` and `user.personId` are placeholders (= `entraOid`) - * until the `User` + `Person` schema (proposed ADR-0026) lands. - * The seam is here so the next PR populates the real UUIDs in one - * place, not in every guard. + * The `identity` parameter (UUIDs from `PersonAndUserProvisioner`) + * populates `Principal.user.{id, personId}` with the real portal- + * side identifiers introduced in ADR-0026 PR 1 — pre-PR-1 the BFF + * reused the Entra `oid` as a placeholder. */ @Injectable() export class PrincipalBuilder { @@ -42,7 +43,10 @@ export class PrincipalBuilder { private readonly logger: Logger, ) {} - async build(user: AuthenticatedUser): Promise { + async build( + user: AuthenticatedUser, + identity: { userId: string; personId: string }, + ): Promise { const privileges = filterPrivileges(user.roles, user.oid, this.logger); const roles = this.groupToRole.resolve(user.groups, (groupId: string) => { @@ -56,16 +60,12 @@ export class PrincipalBuilder { ); }); - const scopes = await this.scopes.resolve({ entraOid: user.oid }); + const scopes = await this.scopes.resolve({ userId: identity.userId }); return { user: { - // Real UUIDs land with ADR-0026's User + Person schema. - // Until then `entraOid` is the closest thing to a stable - // portal identity, so callers that key on `principal.user.id` - // get a usable value today and a real one later. - id: user.oid, - personId: user.oid, + id: identity.userId, + personId: identity.personId, entraOid: user.oid, tenantId: user.tid, displayName: user.displayName, diff --git a/apps/portal-bff/src/auth/scope-resolver.ts b/apps/portal-bff/src/auth/scope-resolver.ts index 9924f65..1fc69f9 100644 --- a/apps/portal-bff/src/auth/scope-resolver.ts +++ b/apps/portal-bff/src/auth/scope-resolver.ts @@ -6,38 +6,40 @@ import type { Scope } from 'shared-auth'; * * Per [ADR-0025 §"Sources of truth — apf_portal-side `user_scopes` * table"](../../../../docs/decisions/0025-authorization-model-privileges-roles-scopes.md), - * scopes are *not* carried by Entra: they will live in a portal - * `user_scopes` Prisma table (proposed ADR-0026), queried by Entra - * `oid` at sign-in. Each row materialises one `Scope` entry on the - * session principal. + * scopes are *not* carried by Entra: they live in the portal-side + * `user_scopes` Prisma table (per ADR-0026 PR 1's schema), keyed on + * `User.id` (UUID). Each row materialises one `Scope` entry on the + * session-resident principal. * - * The seam is here in v1 so the next PR replaces only the - * implementation, leaving the call-site in `PrincipalBuilder` - * untouched. The v1 implementation (`StubScopeResolver`) always - * returns `[{ kind: 'unrestricted' }]` per ADR-0025 §331 — until - * the Prisma table lands the test tenant runs against a single - * "see everything" scope; the absence of fine-grained scoping is - * acceptable because guards consuming `@RequireScope` are not in - * the codebase yet. + * The seam is here in v1 so ADR-0026 PR 2 (the `PrismaScopeResolver` + * landing the seed + the `/admin/users/:id/scopes` screen) replaces + * only the implementation, leaving the call-site in + * `PrincipalBuilder` untouched. The v1 stub (`StubScopeResolver`) + * always returns `[{ kind: 'unrestricted' }]` per ADR-0025 §331. + * + * Note: the `input` shape switched from `{ entraOid }` to `{ userId }` + * with ADR-0026 PR 1 (Person + User + UserScope schema landed). The + * stub ignores its argument either way; the change is the seam for + * PR 2's Prisma resolver, which keys queries on `User.id`. */ export abstract class ScopeResolver { - abstract resolve(input: { entraOid: string }): Promise>; + abstract resolve(input: { userId: string }): Promise>; } /** * v1 stub — returns `unrestricted` for every user. Per ADR-0025 * §"Sources of truth — apf_portal-side `user_scopes` table" the * real implementation queries `user_scopes WHERE userId = ?`; that - * lands with the `Person` + `User` schema (proposed ADR-0026) and - * the seed PR that follows. + * lands as `PrismaScopeResolver` in ADR-0026 PR 2 alongside the + * test-tenant seed. * * Deliberately not an in-memory hard-coded persona-keyed map: the * 19 test personas have *intended* scopes (see * `notes/test-tenant-role-assignments.md`) but those scopes have * no guard consuming them yet, so per-persona stub data would be - * write-only documentation. When ADR-0026 lands, the seed PR - * populates `user_scopes`; this class is replaced by a - * Prisma-backed implementation in the same change. + * write-only documentation. ADR-0026 PR 2 populates the table; this + * class is replaced by a Prisma-backed implementation in the same + * change. */ @Injectable() export class StubScopeResolver extends ScopeResolver { diff --git a/apps/portal-bff/src/auth/session-establisher.service.spec.ts b/apps/portal-bff/src/auth/session-establisher.service.spec.ts index f241011..b166585 100644 --- a/apps/portal-bff/src/auth/session-establisher.service.spec.ts +++ b/apps/portal-bff/src/auth/session-establisher.service.spec.ts @@ -3,6 +3,7 @@ import type { Logger } from 'nestjs-pino'; import type { Principal } from 'shared-auth'; import type { AuditWriter } from '../audit/audit.service'; import type { UserSessionIndexService } from '../session/user-session-index.service'; +import type { PersonAndUserProvisioner } from '../users/person-and-user-provisioner.service'; import type { UserDirectoryService } from '../users/user-directory.service'; import type { AuthenticatedUser } from './auth.service'; import type { PrincipalBuilder } from './principal-builder'; @@ -55,14 +56,20 @@ interface Fixture { index: { add: jest.Mock; remove: jest.Mock; list: jest.Mock }; audit: { signIn: jest.Mock; signOut: jest.Mock }; directory: { recordSignIn: jest.Mock }; + provisioner: { ensureUser: jest.Mock }; logger: ReturnType; principalBuilder: { build: jest.Mock }; } +const PROVISIONED_IDENTITY = { + userId: 'user-uuid-1', + personId: 'person-uuid-1', +}; + const STUB_PRINCIPAL: Principal = { user: { - id: USER.oid, - personId: USER.oid, + id: PROVISIONED_IDENTITY.userId, + personId: PROVISIONED_IDENTITY.personId, entraOid: USER.oid, tenantId: USER.tid, displayName: USER.displayName, @@ -86,6 +93,9 @@ function makeFixture(): Fixture { const directory = { recordSignIn: jest.fn().mockResolvedValue(undefined), }; + const provisioner = { + ensureUser: jest.fn().mockResolvedValue(PROVISIONED_IDENTITY), + }; const logger = makeLoggerStub(); const principalBuilder = { build: jest.fn().mockResolvedValue(STUB_PRINCIPAL), @@ -95,9 +105,10 @@ function makeFixture(): Fixture { index as unknown as UserSessionIndexService, audit as unknown as AuditWriter, directory as unknown as UserDirectoryService, + provisioner as unknown as PersonAndUserProvisioner, principalBuilder as unknown as PrincipalBuilder, ); - return { est, index, audit, directory, logger, principalBuilder }; + return { est, index, audit, directory, provisioner, logger, principalBuilder }; } describe('SessionEstablisher.establish', () => { @@ -119,20 +130,56 @@ describe('SessionEstablisher.establish', () => { expect((sess['csrfToken'] as string).length).toBeGreaterThan(20); }); - it('builds the authorization principal and stamps mfaVerifiedAt on it (ADR-0025)', async () => { - const { est, principalBuilder } = makeFixture(); + it('provisions Person + User before building the Principal (ADR-0026 §Lifecycle)', async () => { + const { est, provisioner } = makeFixture(); const req = makeReqStub(); const res = makeResStub(); await est.establish({ user: USER, req, res, surface: 'user' }); - expect(principalBuilder.build).toHaveBeenCalledWith(USER); + expect(provisioner.ensureUser).toHaveBeenCalledWith({ + oid: USER.oid, + tenantId: USER.tid, + displayName: USER.displayName, + // Entra preferred_username (= AuthenticatedUser.username) maps + // to Person.email per ADR-0026 §"Lifecycle". + email: USER.username, + }); + }); + + it('builds the authorization principal with the provisioned UUIDs and stamps mfaVerifiedAt (ADR-0025 + ADR-0026)', async () => { + const { est, provisioner, principalBuilder } = makeFixture(); + const req = makeReqStub(); + const res = makeResStub(); + await est.establish({ user: USER, req, res, surface: 'user' }); + expect(principalBuilder.build).toHaveBeenCalledWith(USER, PROVISIONED_IDENTITY); + // Order: provisioner MUST run before principalBuilder so the + // build call has real UUIDs to populate Principal.user.{id,personId}. + const provisionerOrder = provisioner.ensureUser.mock.invocationCallOrder[0] ?? Infinity; + const buildOrder = principalBuilder.build.mock.invocationCallOrder[0] ?? 0; + expect(provisionerOrder).toBeLessThan(buildOrder); const sess = (req as unknown as { session: Record }).session; const principal = sess['principal'] as Principal; expect(principal).toBeDefined(); expect(principal.user.entraOid).toBe(USER.oid); + expect(principal.user.id).toBe(PROVISIONED_IDENTITY.userId); + expect(principal.user.personId).toBe(PROVISIONED_IDENTITY.personId); expect(principal.scopes).toEqual([{ kind: 'unrestricted' }]); expect(principal.mfaVerifiedAt).toBe(sess['createdAt']); }); + it('propagates provisioner failures (blocking per ADR-0026)', async () => { + const { est, provisioner, audit, directory, principalBuilder } = makeFixture(); + provisioner.ensureUser.mockRejectedValueOnce(new Error('postgres unreachable')); + const req = makeReqStub(); + const res = makeResStub(); + await expect(est.establish({ user: USER, req, res, surface: 'user' })).rejects.toThrow( + 'postgres unreachable', + ); + // Nothing downstream of the provisioner runs when it fails. + expect(principalBuilder.build).not.toHaveBeenCalled(); + expect(audit.signIn).not.toHaveBeenCalled(); + expect(directory.recordSignIn).not.toHaveBeenCalled(); + }); + it('saves the session before returning (no race with the controller redirect)', async () => { const { est } = makeFixture(); const req = makeReqStub(); diff --git a/apps/portal-bff/src/auth/session-establisher.service.ts b/apps/portal-bff/src/auth/session-establisher.service.ts index 3d68f69..fd03e4a 100644 --- a/apps/portal-bff/src/auth/session-establisher.service.ts +++ b/apps/portal-bff/src/auth/session-establisher.service.ts @@ -6,6 +6,7 @@ import { AuditWriter } from '../audit/audit.service'; import { csrfCookieName, csrfCookieOptions } from '../security/csrf-cookie'; import { readSessionTimeouts } from '../session/session-cookie'; import { UserSessionIndexService } from '../session/user-session-index.service'; +import { PersonAndUserProvisioner } from '../users/person-and-user-provisioner.service'; import { UserDirectoryService } from '../users/user-directory.service'; import type { AuthenticatedUser } from './auth.service'; import { PrincipalBuilder } from './principal-builder'; @@ -48,6 +49,7 @@ export class SessionEstablisher { private readonly userSessionIndex: UserSessionIndexService, private readonly audit: AuditWriter, private readonly userDirectory: UserDirectoryService, + private readonly personUserProvisioner: PersonAndUserProvisioner, private readonly principalBuilder: PrincipalBuilder, ) {} @@ -81,13 +83,27 @@ export class SessionEstablisher { // does not re-validate factors. Refreshed by future step-up // re-auth flows. req.session.mfaVerifiedAt = now; + // Provision Person + User (blocking per ADR-0026 §"Lifecycle"): + // the BFF cannot build a Principal without User.id / Person.id, + // so a Postgres outage here fails the sign-in. Idempotent — + // returns the existing pair's UUIDs on subsequent sign-ins. + // Note: the Entra `preferred_username` claim maps to Person.email + // (it's typically the user's UPN / email); `AuthenticatedUser.username` + // is the BFF's name for the same field. + const identity = await this.personUserProvisioner.ensureUser({ + oid: user.oid, + tenantId: user.tid, + displayName: user.displayName, + email: user.username, + }); + // Authorization principal per ADR-0025: composes the three // axes (privileges / functional roles / scopes) once at // sign-in so guards read a single coherent shape instead of // re-parsing claims on every request. Built before the // session is persisted so a Redis hiccup either persists // everything or nothing. - const principal = await this.principalBuilder.build(user); + const principal = await this.principalBuilder.build(user, identity); req.session.principal = { ...principal, mfaVerifiedAt: now }; await saveSession(req); diff --git a/apps/portal-bff/src/users/person-and-user-provisioner.service.spec.ts b/apps/portal-bff/src/users/person-and-user-provisioner.service.spec.ts new file mode 100644 index 0000000..823b23d --- /dev/null +++ b/apps/portal-bff/src/users/person-and-user-provisioner.service.spec.ts @@ -0,0 +1,198 @@ +import { Prisma } from '@prisma/client'; +import type { Logger } from 'nestjs-pino'; +import type { PrismaService } from 'nestjs-prisma'; +import { + PersonAndUserProvisioner, + splitDisplayName, + type EnsureUserInput, +} from './person-and-user-provisioner.service'; + +interface PrismaStub { + user: { + findUnique: jest.Mock; + update: jest.Mock; + create: jest.Mock; + }; +} + +function makeLogger() { + return { log: jest.fn(), warn: jest.fn(), error: jest.fn() } as unknown as Logger & { + log: jest.Mock; + warn: jest.Mock; + error: jest.Mock; + }; +} + +function makeSubject(opts?: { findUnique?: jest.Mock; update?: jest.Mock; create?: jest.Mock }): { + service: PersonAndUserProvisioner; + prisma: PrismaStub; + logger: ReturnType; +} { + const prisma: PrismaStub = { + user: { + findUnique: opts?.findUnique ?? jest.fn().mockResolvedValue(null), + update: opts?.update ?? jest.fn().mockResolvedValue(undefined), + create: + opts?.create ?? jest.fn().mockResolvedValue({ id: 'user-uuid', personId: 'person-uuid' }), + }, + }; + const logger = makeLogger(); + const service = new PersonAndUserProvisioner(prisma as unknown as PrismaService, logger); + return { service, prisma, logger }; +} + +const INPUT: EnsureUserInput = { + oid: 'entra-oid-1', + tenantId: 'tenant-1', + displayName: 'Jane Doe', + email: 'jane.doe@apf.example', +}; + +describe('PersonAndUserProvisioner.ensureUser — first sign-in (cold path)', () => { + it('creates Person + User in a single nested-create call', async () => { + const { service, prisma } = makeSubject(); + await service.ensureUser(INPUT); + expect(prisma.user.findUnique).toHaveBeenCalledWith({ + where: { entraOid: 'entra-oid-1' }, + select: { id: true, personId: true }, + }); + expect(prisma.user.create).toHaveBeenCalledTimes(1); + const createArgs = prisma.user.create.mock.calls[0]?.[0] as { + data: { + entraOid: string; + tenantId: string; + lastSignInAt: Date; + person: { + create: { firstName: string; lastName: string; email: string | null; source: string }; + }; + }; + }; + expect(createArgs.data.entraOid).toBe('entra-oid-1'); + expect(createArgs.data.tenantId).toBe('tenant-1'); + expect(createArgs.data.lastSignInAt).toBeInstanceOf(Date); + expect(createArgs.data.person.create.firstName).toBe('Jane'); + expect(createArgs.data.person.create.lastName).toBe('Doe'); + expect(createArgs.data.person.create.email).toBe('jane.doe@apf.example'); + expect(createArgs.data.person.create.source).toBe('self-signin'); + }); + + it('returns the UUIDs the caller needs to populate the Principal', async () => { + const { service } = makeSubject(); + const result = await service.ensureUser(INPUT); + expect(result).toEqual({ userId: 'user-uuid', personId: 'person-uuid' }); + }); + + it('writes Person.email = null when no email is provided', async () => { + const { service, prisma } = makeSubject(); + await service.ensureUser({ ...INPUT, email: null }); + const args = prisma.user.create.mock.calls[0]?.[0] as { + data: { person: { create: { email: string | null } } }; + }; + expect(args.data.person.create.email).toBeNull(); + }); + + it('logs the create with the resulting UUIDs', async () => { + const { service, logger } = makeSubject(); + await service.ensureUser(INPUT); + expect(logger.log).toHaveBeenCalledWith( + expect.objectContaining({ + event: 'provisioner.person_and_user_created', + oid: 'entra-oid-1', + userId: 'user-uuid', + personId: 'person-uuid', + }), + 'PersonAndUserProvisioner', + ); + }); +}); + +describe('PersonAndUserProvisioner.ensureUser — subsequent sign-in (warm path)', () => { + it('refreshes lastSignInAt and returns the existing UUIDs without re-creating', async () => { + const findUnique = jest.fn().mockResolvedValue({ id: 'user-uuid', personId: 'person-uuid' }); + const update = jest.fn().mockResolvedValue(undefined); + const create = jest.fn(); + const { service } = makeSubject({ findUnique, update, create }); + const result = await service.ensureUser(INPUT); + expect(create).not.toHaveBeenCalled(); + expect(update).toHaveBeenCalledTimes(1); + const updateArgs = update.mock.calls[0]?.[0] as { + where: { id: string }; + data: { lastSignInAt: Date }; + }; + expect(updateArgs.where).toEqual({ id: 'user-uuid' }); + expect(updateArgs.data.lastSignInAt).toBeInstanceOf(Date); + expect(result).toEqual({ userId: 'user-uuid', personId: 'person-uuid' }); + }); +}); + +describe('PersonAndUserProvisioner.ensureUser — race condition on first sign-in', () => { + it('catches the P2002 unique violation and re-runs', async () => { + // First call: findUnique misses, create raises P2002 (race + // loser). Second call: findUnique now finds the row written by + // the winner; update refreshes lastSignInAt, return. + const findUnique = jest + .fn() + .mockResolvedValueOnce(null) + .mockResolvedValueOnce({ id: 'user-uuid', personId: 'person-uuid' }); + const create = jest.fn().mockRejectedValueOnce( + new Prisma.PrismaClientKnownRequestError('Unique constraint failed', { + code: 'P2002', + clientVersion: '6.19.3', + }), + ); + const update = jest.fn().mockResolvedValue(undefined); + const { service, logger } = makeSubject({ findUnique, update, create }); + const result = await service.ensureUser(INPUT); + expect(create).toHaveBeenCalledTimes(1); + expect(findUnique).toHaveBeenCalledTimes(2); + expect(update).toHaveBeenCalledTimes(1); // refresh on the retry's warm path + expect(result).toEqual({ userId: 'user-uuid', personId: 'person-uuid' }); + expect(logger.warn).toHaveBeenCalledWith( + expect.objectContaining({ event: 'provisioner.race_retry', oid: 'entra-oid-1' }), + 'PersonAndUserProvisioner', + ); + }); + + it('propagates non-P2002 Prisma errors unchanged (no retry)', async () => { + const create = jest.fn().mockRejectedValueOnce( + new Prisma.PrismaClientKnownRequestError('Connection error', { + code: 'P1001', + clientVersion: '6.19.3', + }), + ); + const { service } = makeSubject({ create }); + await expect(service.ensureUser(INPUT)).rejects.toMatchObject({ code: 'P1001' }); + }); + + it('propagates non-Prisma errors unchanged', async () => { + const create = jest.fn().mockRejectedValueOnce(new Error('boom')); + const { service } = makeSubject({ create }); + await expect(service.ensureUser(INPUT)).rejects.toThrow('boom'); + }); +}); + +describe('splitDisplayName', () => { + it('splits on the first space', () => { + expect(splitDisplayName('Jane Doe')).toEqual({ firstName: 'Jane', lastName: 'Doe' }); + }); + + it('absorbs trailing whitespace in the last-name half', () => { + expect(splitDisplayName('Jean-Pierre Dupont De La Mer')).toEqual({ + firstName: 'Jean-Pierre', + lastName: 'Dupont De La Mer', + }); + }); + + it('returns lastName = "" for single-token names', () => { + expect(splitDisplayName('Madonna')).toEqual({ firstName: 'Madonna', lastName: '' }); + }); + + it('trims surrounding whitespace before splitting', () => { + expect(splitDisplayName(' Jane Doe ')).toEqual({ firstName: 'Jane', lastName: 'Doe' }); + }); + + it('returns both empty for empty input', () => { + expect(splitDisplayName('')).toEqual({ firstName: '', lastName: '' }); + expect(splitDisplayName(' ')).toEqual({ firstName: '', lastName: '' }); + }); +}); diff --git a/apps/portal-bff/src/users/person-and-user-provisioner.service.ts b/apps/portal-bff/src/users/person-and-user-provisioner.service.ts new file mode 100644 index 0000000..6b38fbf --- /dev/null +++ b/apps/portal-bff/src/users/person-and-user-provisioner.service.ts @@ -0,0 +1,184 @@ +import { Injectable } from '@nestjs/common'; +import { Prisma } from '@prisma/client'; +import { Logger } from 'nestjs-pino'; +import { PrismaService } from 'nestjs-prisma'; +import { isPersonSource, type PersonSource } from './person-source'; + +/** + * Minimal input slice from `AuthenticatedUser`. Defined here so the + * provisioner stays decoupled from the auth module's internal shape + * — the spec hand-rolls inputs without going through the OIDC flow. + */ +export interface EnsureUserInput { + readonly oid: string; + readonly tenantId: string; + /** + * Entra `displayName` — split on the first space into `firstName` + * + `lastName` on Person. Best effort: single-word names produce + * a Person with `lastName = ''`. + */ + readonly displayName: string; + /** + * Entra `preferred_username` (typically the user's email / UPN). + * Stored verbatim on Person.email and NOT used for dedup in v1 — + * ADR-0026 §"Lifecycle" deliberately treats email as an attribute, + * not a natural key. + */ + readonly email: string | null; +} + +/** + * Identifier pair the BFF carries on the session-resident `Principal` + * once `User` + `Person` are real rows (per ADR-0026 PR 1). Pre-PR-1 + * the BFF reused the Entra `oid` as a placeholder; the new shape + * makes the two concerns explicit: + * - `userId` — `User.id` (UUID), the portal-account identifier. + * - `personId` — `Person.id` (UUID), the golden-record identifier. + */ +export interface ProvisionedIdentity { + readonly userId: string; + readonly personId: string; +} + +/** + * `PersonAndUserProvisioner` — lazy-create of the `Person` + `User` + * pair at first OIDC callback, per + * [ADR-0026 §"Lifecycle"](../../../../docs/decisions/0026-person-user-portal-data-model.md). + * + * **Blocking** per the ADR: a `Principal` cannot be built without + * `User.id` / `Person.id`, so a Postgres outage on this path MUST + * fail the sign-in. Distinct from `UserDirectoryService.recordSignIn` + * (ADR-0020 admin-list cache) which is best-effort — both run from + * `SessionEstablisher.establish`, but with different invariants. + * + * v1 keys ONLY on `entraOid`. Email-based dedup is deliberately not + * implemented (ADR-0026 §"Why no email-based merging in v1" — two + * distinct humans genuinely share emails); ADR-0029's sync flow adds + * an operator-confirmed reconciliation path. + */ +@Injectable() +export class PersonAndUserProvisioner { + /** + * Source value written to `Person.source` for self-signin lazy + * creation. Hardcoded as the constant value, not a parameter — + * the ADR-0026 lifecycle ONLY produces 'self-signin' rows; admin- + * UI and seed paths construct Person rows through different code. + * Kept as a `PersonSource`-typed local so the drift gate and the + * TS compiler both catch a future divergence. + */ + private readonly SELF_SIGNIN_SOURCE: PersonSource = 'self-signin'; + + constructor( + private readonly prisma: PrismaService, + private readonly logger: Logger, + ) {} + + async ensureUser(input: EnsureUserInput): Promise { + // Fast path: User exists. Refresh `lastSignInAt`, return the + // UUIDs the caller needs to build the Principal. + const existing = await this.prisma.user.findUnique({ + where: { entraOid: input.oid }, + select: { id: true, personId: true }, + }); + if (existing) { + await this.prisma.user.update({ + where: { id: existing.id }, + data: { lastSignInAt: new Date() }, + }); + return { userId: existing.id, personId: existing.personId }; + } + + // First sign-in: create Person + linked User in one transaction. + // The nested `create` on `person` is Prisma's idiom for a 1-to-1 + // create that emits a single SQL transaction internally — no + // explicit `$transaction` needed. + // + // Race: two concurrent first sign-ins for the same `entraOid` — + // both `findUnique` calls miss, both reach the `create`. The + // unique constraint on `User.entraOid` rejects the loser with + // P2002; we treat that as "the winner has now created the row" + // and re-run `ensureUser`. Pathological infinite-loop guarded by + // the fast path returning on the second attempt. + const { firstName, lastName } = splitDisplayName(input.displayName); + // Defense in depth — flagged by the drift gate (`source: 'X'` + // literal in a file importing person-source) AND by the TS type + // union, but we re-check the constant value here so a future + // refactor of `SELF_SIGNIN_SOURCE` that drops the type annotation + // doesn't silently write a non-catalogue value. + if (!isPersonSource(this.SELF_SIGNIN_SOURCE)) { + this.logger.error( + { + event: 'provisioner.invalid_person_source', + source: this.SELF_SIGNIN_SOURCE, + }, + 'PersonAndUserProvisioner', + ); + throw new Error(`invalid Person.source constant: ${this.SELF_SIGNIN_SOURCE}`); + } + try { + const created = await this.prisma.user.create({ + data: { + entraOid: input.oid, + tenantId: input.tenantId, + lastSignInAt: new Date(), + person: { + create: { + firstName, + lastName, + email: input.email, + source: this.SELF_SIGNIN_SOURCE, + }, + }, + }, + select: { id: true, personId: true }, + }); + this.logger.log( + { + event: 'provisioner.person_and_user_created', + oid: input.oid, + userId: created.id, + personId: created.personId, + }, + 'PersonAndUserProvisioner', + ); + return { userId: created.id, personId: created.personId }; + } catch (err) { + if (err instanceof Prisma.PrismaClientKnownRequestError && err.code === 'P2002') { + // Loser of the race against a concurrent first sign-in for + // the same `entraOid`. Re-run — fast path will pick up the + // winning row. + this.logger.warn( + { event: 'provisioner.race_retry', oid: input.oid }, + 'PersonAndUserProvisioner', + ); + return this.ensureUser(input); + } + throw err; + } + } +} + +/** + * Split a single-string display name into first / last on the first + * whitespace boundary. Best effort — Entra `displayName` is + * tenant-formatted (some "First Last", some "Last, First", some + * single-name). v1 documents the limitation; the admin-UI scope- + * seeding screen (ADR-0026 PR 2) will surface mis-split rows for + * manual correction. + * + * Exported for the spec. + */ +export function splitDisplayName(displayName: string): { + firstName: string; + lastName: string; +} { + const trimmed = displayName.trim(); + const space = trimmed.indexOf(' '); + if (space < 0) { + return { firstName: trimmed, lastName: '' }; + } + return { + firstName: trimmed.slice(0, space), + lastName: trimmed.slice(space + 1).trim(), + }; +} diff --git a/apps/portal-bff/src/users/person-source.spec.ts b/apps/portal-bff/src/users/person-source.spec.ts new file mode 100644 index 0000000..c09a883 --- /dev/null +++ b/apps/portal-bff/src/users/person-source.spec.ts @@ -0,0 +1,38 @@ +import { isPersonSource, PERSON_SOURCES, type PersonSource } from './person-source'; + +describe('PERSON_SOURCES', () => { + it('contains the three v1 sources from ADR-0026', () => { + expect(PERSON_SOURCES).toEqual(['self-signin', 'admin-ui', 'seed']); + }); + + it('has no duplicate entries', () => { + expect(new Set(PERSON_SOURCES).size).toBe(PERSON_SOURCES.length); + }); +}); + +describe('isPersonSource', () => { + it('returns true for every catalogue value', () => { + for (const source of PERSON_SOURCES) { + expect(isPersonSource(source)).toBe(true); + } + }); + + it('returns false for values outside the catalogue', () => { + // future ADR-0029 values, not shipped in v1 + expect(isPersonSource('pleiades')).toBe(false); + expect(isPersonSource('acteurs-plus')).toBe(false); + // common typos / casing variations + expect(isPersonSource('self_signin')).toBe(false); + expect(isPersonSource('Self-Signin')).toBe(false); + expect(isPersonSource('')).toBe(false); + }); + + it('narrows the type at the call site', () => { + const candidate = String('self-signin'); + if (!isPersonSource(candidate)) { + throw new Error('unreachable — candidate is a known source'); + } + const narrowed: PersonSource = candidate; + expect(narrowed).toBe('self-signin'); + }); +}); diff --git a/apps/portal-bff/src/users/person-source.ts b/apps/portal-bff/src/users/person-source.ts new file mode 100644 index 0000000..ae4a171 --- /dev/null +++ b/apps/portal-bff/src/users/person-source.ts @@ -0,0 +1,34 @@ +/** + * Closed-set catalogue for `Person.source` per + * [ADR-0026 §"Confirmation"](../../../../../docs/decisions/0026-person-user-portal-data-model.md). + * + * Defense in depth, same posture as STRUCTURE_KINDS (ADR-0027): + * + * 1. TypeScript type union `PersonSource` — compile-time check at + * every typed assignment. + * 2. `scripts/check-catalogue-drift.mjs` — CI gate asserting every + * `source: 'X'` literal in files importing this module is in + * the catalogue. + * + * Person.source is NOT enforced at the DB level (no CHECK constraint + * in the migration) — the field is documented as free-form string in + * the ADR with promotion to enum-on-DB a follow-up decision once the + * sync sources (ADR-0029) are concrete. The drift gate + type union + * are enough for v1 since the only writer is + * `PersonAndUserProvisioner.ensureUser` and any future writer will + * import this module to get the type. + * + * v1 values: + * - 'self-signin' — lazy creation at first OIDC callback. + * - 'admin-ui' — manually entered by an admin before first sign-in. + * - 'seed' — test-tenant provisioning. + * + * ADR-0029 will add 'pleiades' and 'acteurs-plus'. + */ +export const PERSON_SOURCES = ['self-signin', 'admin-ui', 'seed'] as const; + +export type PersonSource = (typeof PERSON_SOURCES)[number]; + +export function isPersonSource(value: string): value is PersonSource { + return (PERSON_SOURCES as readonly string[]).includes(value); +} diff --git a/apps/portal-bff/src/users/users.module.ts b/apps/portal-bff/src/users/users.module.ts index c01e72e..4d2265c 100644 --- a/apps/portal-bff/src/users/users.module.ts +++ b/apps/portal-bff/src/users/users.module.ts @@ -1,26 +1,29 @@ import { Global, Module } from '@nestjs/common'; +import { PersonAndUserProvisioner } from './person-and-user-provisioner.service'; import { UserDirectoryService } from './user-directory.service'; /** - * `UsersModule` — owns the persistent user directory per ADR-0020 - * §"v1 scope — User list". + * `UsersModule` — owns the portal-side identity surface. * - * v1 ships `UserDirectoryService` only (the write path: upsert at - * every sign-in via `SessionEstablisher`). The future - * `GET /api/admin/users` read endpoint lands in a sibling PR - * alongside the SPA viewer screen — those add a controller and - * possibly a read-side service, but the storage layer is here. + * v1 ships two services: + * - `UserDirectoryService` (ADR-0020 §"User list") — the persistent + * sign-in cache. Upserts a `UserDirectoryEntry` per sign-in; + * read by the `/api/admin/users` admin endpoint. Best-effort. + * - `PersonAndUserProvisioner` (ADR-0026 §"Lifecycle") — lazy-creates + * the `Person` + `User` pair at first sign-in and returns the + * UUIDs the `PrincipalBuilder` needs. Blocking. * - * Declared `@Global()` so `SessionEstablisher` (which lives in the - * auth module) can inject `UserDirectoryService` without re-routing - * the module graph through an import. The directory is a true - * cross-cutting concern: it has one writer (the auth callback) and - * one reader (admin), neither of which "owns" identity in a - * domain-module sense. + * Both run from `SessionEstablisher.establish` with different + * invariants (best-effort vs blocking). A future PR may fold the + * UserDirectoryEntry cache into Person+User now that the latter + * exists — out of scope here. + * + * Declared `@Global()` so `SessionEstablisher` (auth module) can + * inject both providers without re-routing the module graph. */ @Global() @Module({ - providers: [UserDirectoryService], - exports: [UserDirectoryService], + providers: [UserDirectoryService, PersonAndUserProvisioner], + exports: [UserDirectoryService, PersonAndUserProvisioner], }) export class UsersModule {} diff --git a/scripts/check-catalogue-drift.mjs b/scripts/check-catalogue-drift.mjs index 2fa8d47..8fb3844 100644 --- a/scripts/check-catalogue-drift.mjs +++ b/scripts/check-catalogue-drift.mjs @@ -48,15 +48,18 @@ const WORKSPACE_ROOT = resolve(__dirname, '..'); const CATALOGUE_PATH = join(WORKSPACE_ROOT, 'libs/shared/auth/src/lib/authorization.types.ts'); const STRUCTURE_KIND_PATH = 'apps/portal-bff/src/structures/structure-kind.ts'; +const PERSON_SOURCE_PATH = 'apps/portal-bff/src/users/person-source.ts'; /** - * A file is only scanned for `Structure.kind` violations if it - * imports from the structure-kind module. Outside that context - * `kind` is a common property name on unrelated objects and the - * scanner would false-positive. The check is text-level (cheap) - * and runs before any AST work — files that fail it short-circuit. + * A file is only scanned for property-literal catalogue violations + * if it imports from the relevant catalogue module. Outside that + * context `kind` / `source` are common property names on unrelated + * objects and the scanner would false-positive. The check is + * text-level (cheap) and runs before any AST work — files that fail + * it short-circuit. */ const STRUCTURE_KIND_IMPORT_PATTERN = /\bfrom\s+['"][^'"]*structure-kind['"]/; +const PERSON_SOURCE_IMPORT_PATTERN = /\bfrom\s+['"][^'"]*person-source['"]/; /** * Decorator name → catalogue array name. Adding a third decorator @@ -148,13 +151,107 @@ export function extractCatalogues(sourceFilePath = CATALOGUE_PATH) { * literal pattern, single declaration per file. Exported for the spec. */ export function extractStructureKinds(sourceFilePath = join(WORKSPACE_ROOT, STRUCTURE_KIND_PATH)) { + return extractAsConstArray(sourceFilePath, 'STRUCTURE_KINDS'); +} + +/** + * Find every `kind: 'X'` property literal whose X is not in the + * STRUCTURE_KINDS catalogue. Restricted to files that import from + * the structure-kind module (heuristic — `kind` is a common + * property name on unrelated objects; without the restriction the + * scanner would false-positive everywhere). Exported for the spec. + */ +export function findStructureKindViolationsInFile(filePath, validKinds, sourceText) { + return findPropertyLiteralViolations(filePath, validKinds, sourceText, { + importPattern: STRUCTURE_KIND_IMPORT_PATTERN, + propertyName: 'kind', + callee: 'Structure.kind', + catalogueName: 'STRUCTURE_KINDS', + }); +} + +/** + * Parse `person-source.ts` and extract the `PERSON_SOURCES` constant + * as `Set`. Mirror of `extractStructureKinds` — same `as const` + * array literal pattern, single declaration per file. Exported for + * the spec. + */ +export function extractPersonSources(sourceFilePath = join(WORKSPACE_ROOT, PERSON_SOURCE_PATH)) { + return extractAsConstArray(sourceFilePath, 'PERSON_SOURCES'); +} + +/** + * Find every `source: 'X'` property literal whose X is not in the + * PERSON_SOURCES catalogue. Mirror of `findStructureKindViolationsInFile` + * with a different property name + catalogue. Exported for the spec. + */ +export function findPersonSourceViolationsInFile(filePath, validSources, sourceText) { + return findPropertyLiteralViolations(filePath, validSources, sourceText, { + importPattern: PERSON_SOURCE_IMPORT_PATTERN, + propertyName: 'source', + callee: 'Person.source', + catalogueName: 'PERSON_SOURCES', + }); +} + +/** + * Helper for both property-literal scanners. Walks every + * PropertyAssignment whose key matches `propertyName`; for each + * string-literal initialiser, checks that the value is in + * `validValues`. Skips files that don't import the catalogue module + * (text-level pre-filter, cheap). + */ +function findPropertyLiteralViolations(filePath, validValues, sourceText, opts) { + const text = sourceText ?? readFileSync(filePath, 'utf8'); + if (!opts.importPattern.test(text)) return []; + + const sourceFile = ts.createSourceFile(filePath, text, ts.ScriptTarget.Latest, true); + const violations = []; + + function visit(node) { + if (ts.isPropertyAssignment(node)) { + const name = node.name; + const matchesProperty = + (ts.isIdentifier(name) && name.text === opts.propertyName) || + (ts.isStringLiteral(name) && name.text === opts.propertyName); + if (matchesProperty) { + const init = node.initializer; + if (ts.isStringLiteral(init) || ts.isNoSubstitutionTemplateLiteral(init)) { + if (!validValues.has(init.text)) { + const { line, character } = sourceFile.getLineAndCharacterOfPosition(init.getStart()); + violations.push({ + file: filePath, + line: line + 1, + column: character + 1, + callee: opts.callee, + value: init.text, + catalogue: opts.catalogueName, + }); + } + } + } + } + ts.forEachChild(node, visit); + } + + visit(sourceFile); + return violations; +} + +/** + * Helper for extractStructureKinds / extractPersonSources — both + * read a top-level `export const = [...] as const` from a + * single-file catalogue module. Generalised once instead of + * duplicated per catalogue. + */ +function extractAsConstArray(sourceFilePath, constName) { const text = readFileSync(sourceFilePath, 'utf8'); const sourceFile = ts.createSourceFile(sourceFilePath, text, ts.ScriptTarget.Latest, true); for (const stmt of sourceFile.statements) { if (!ts.isVariableStatement(stmt)) continue; for (const decl of stmt.declarationList.declarations) { - if (!ts.isIdentifier(decl.name) || decl.name.text !== 'STRUCTURE_KINDS') continue; + if (!ts.isIdentifier(decl.name) || decl.name.text !== constName) continue; let init = decl.initializer; if (init && ts.isAsExpression(init)) init = init.expression; if (!init || !ts.isArrayLiteralExpression(init)) continue; @@ -170,55 +267,11 @@ export function extractStructureKinds(sourceFilePath = join(WORKSPACE_ROOT, STRU } throw new Error( - `Failed to extract STRUCTURE_KINDS from ${sourceFilePath} — ` + - `the script expected a top-level "export const STRUCTURE_KINDS = [...] as const" declaration.`, + `Failed to extract ${constName} from ${sourceFilePath} — ` + + `the script expected a top-level "export const ${constName} = [...] as const" declaration.`, ); } -/** - * Find every `kind: 'X'` property literal whose X is not in the - * STRUCTURE_KINDS catalogue. Restricted to files that import from - * the structure-kind module (heuristic — `kind` is a common - * property name on unrelated objects; without the restriction the - * scanner would false-positive everywhere). Exported for the spec. - */ -export function findStructureKindViolationsInFile(filePath, validKinds, sourceText) { - const text = sourceText ?? readFileSync(filePath, 'utf8'); - if (!STRUCTURE_KIND_IMPORT_PATTERN.test(text)) return []; - - const sourceFile = ts.createSourceFile(filePath, text, ts.ScriptTarget.Latest, true); - const violations = []; - - function visit(node) { - if (ts.isPropertyAssignment(node)) { - const name = node.name; - const isKindKey = - (ts.isIdentifier(name) && name.text === 'kind') || - (ts.isStringLiteral(name) && name.text === 'kind'); - if (isKindKey) { - const init = node.initializer; - if (ts.isStringLiteral(init) || ts.isNoSubstitutionTemplateLiteral(init)) { - if (!validKinds.has(init.text)) { - const { line, character } = sourceFile.getLineAndCharacterOfPosition(init.getStart()); - violations.push({ - file: filePath, - line: line + 1, - column: character + 1, - callee: 'Structure.kind', - value: init.text, - catalogue: 'STRUCTURE_KINDS', - }); - } - } - } - } - ts.forEachChild(node, visit); - } - - visit(sourceFile); - return violations; -} - /** * Walk `dir` recursively and yield every `.ts` file path * (excluding declaration files and the dirs listed in @@ -302,26 +355,29 @@ export function scanWorkspace(workspaceRoot = WORKSPACE_ROOT) { join(workspaceRoot, 'libs/shared/auth/src/lib/authorization.types.ts'), ); const structureKinds = extractStructureKinds(join(workspaceRoot, STRUCTURE_KIND_PATH)); + const personSources = extractPersonSources(join(workspaceRoot, PERSON_SOURCE_PATH)); const out = []; for (const dirName of ['apps', 'libs']) { const dir = join(workspaceRoot, dirName); for (const file of walkTsFiles(dir, workspaceRoot)) { out.push(...findViolationsInFile(file, catalogues)); out.push(...findStructureKindViolationsInFile(file, structureKinds)); + out.push(...findPersonSourceViolationsInFile(file, personSources)); } } - return { violations: out, catalogues, structureKinds }; + return { violations: out, catalogues, structureKinds, personSources }; } function main() { - const { violations, catalogues, structureKinds } = scanWorkspace(); + const { violations, catalogues, structureKinds, personSources } = scanWorkspace(); if (violations.length === 0) { const privCount = catalogues.PRIVILEGES.size; const roleCount = catalogues.FUNCTIONAL_ROLES.size; const kindCount = structureKinds.size; + const sourceCount = personSources.size; console.log( - `catalogue-drift: clean (catalogues: ${privCount} privileges, ${roleCount} roles, ${kindCount} structure kinds).`, + `catalogue-drift: clean (catalogues: ${privCount} privileges, ${roleCount} roles, ${kindCount} structure kinds, ${sourceCount} person sources).`, ); process.exit(0); } @@ -351,10 +407,12 @@ function main() { } console.error(''); console.error( - `Catalogues are closed in v1 per ADR-0025 (PRIVILEGES / FUNCTIONAL_ROLES) ` + - `and ADR-0027 (STRUCTURE_KINDS). To add a value, amend the ADR and the ` + - `corresponding constant in libs/shared/auth/src/lib/authorization.types.ts ` + - `or apps/portal-bff/src/structures/structure-kind.ts.`, + `Catalogues are closed in v1 per ADR-0025 (PRIVILEGES / FUNCTIONAL_ROLES), ` + + `ADR-0027 (STRUCTURE_KINDS), and ADR-0026 (PERSON_SOURCES). To add a value, ` + + `amend the ADR and the corresponding constant in ` + + `libs/shared/auth/src/lib/authorization.types.ts, ` + + `apps/portal-bff/src/structures/structure-kind.ts, or ` + + `apps/portal-bff/src/users/person-source.ts.`, ); process.exit(1); } diff --git a/scripts/check-catalogue-drift.spec.mjs b/scripts/check-catalogue-drift.spec.mjs index 55062eb..15ea185 100644 --- a/scripts/check-catalogue-drift.spec.mjs +++ b/scripts/check-catalogue-drift.spec.mjs @@ -15,7 +15,9 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { extractCatalogues, + extractPersonSources, extractStructureKinds, + findPersonSourceViolationsInFile, findStructureKindViolationsInFile, findViolationsInFile, scanWorkspace, @@ -59,6 +61,17 @@ function makeFixture({ files }) { export type StructureKind = (typeof STRUCTURE_KINDS)[number]; `, ); + mkdirSync(join(root, 'apps/portal-bff/src/users'), { recursive: true }); + writeFileSync( + join(root, 'apps/portal-bff/src/users/person-source.ts'), + `export const PERSON_SOURCES = [ + 'self-signin', + 'admin-ui', + 'seed', + ] as const; + export type PersonSource = (typeof PERSON_SOURCES)[number]; + `, + ); for (const [relativePath, contents] of Object.entries(files)) { const full = join(root, relativePath); mkdirSync(join(full, '..'), { recursive: true }); @@ -262,6 +275,91 @@ describe('findStructureKindViolationsInFile', () => { }); }); +describe('extractPersonSources', () => { + it('reads the PERSON_SOURCES catalogue from person-source.ts', () => { + const root = makeFixture({ files: {} }); + const sources = extractPersonSources(join(root, 'apps/portal-bff/src/users/person-source.ts')); + assert.deepEqual([...sources].sort(), ['admin-ui', 'seed', 'self-signin']); + }); + + it('throws when the constant is missing', () => { + const root = mkdtempSync(join(tmpdir(), 'drift-fixture-')); + mkdirSync(join(root, 'apps/portal-bff/src/users'), { recursive: true }); + writeFileSync( + join(root, 'apps/portal-bff/src/users/person-source.ts'), + `export const NOT_PERSON_SOURCES = ['foo'] as const;\n`, + ); + assert.throws( + () => extractPersonSources(join(root, 'apps/portal-bff/src/users/person-source.ts')), + /PERSON_SOURCES/, + ); + }); +}); + +describe('findPersonSourceViolationsInFile', () => { + const validSources = new Set(['self-signin', 'admin-ui', 'seed']); + + it('skips files that do not import person-source (would false-positive)', () => { + // `source: 'X'` is an extremely common pattern (event sources, log + // sources, etc.). Without the import filter, every such literal + // would be flagged. + const text = ` + const evt = { source: 'logger', message: 'hello' }; + `; + const violations = findPersonSourceViolationsInFile('virtual.ts', validSources, text); + assert.equal(violations.length, 0); + }); + + it('returns no violations when every source literal is in the catalogue', () => { + const text = ` + import type { PersonSource } from '../users/person-source'; + const a = { source: 'self-signin' as PersonSource }; + const b = { source: 'seed' as PersonSource }; + `; + const violations = findPersonSourceViolationsInFile('virtual.ts', validSources, text); + assert.equal(violations.length, 0); + }); + + it('flags every source literal not in the catalogue', () => { + const text = ` + import { PERSON_SOURCES } from '../users/person-source'; + const a = { source: 'self-signin' }; + const b = { source: 'pleiades' }; + const c = { source: 'rogue-source' }; + `; + const violations = findPersonSourceViolationsInFile('virtual.ts', validSources, text); + assert.equal(violations.length, 2); + const values = violations.map((v) => v.value).sort(); + assert.deepEqual(values, ['pleiades', 'rogue-source']); + assert.equal(violations[0].callee, 'Person.source'); + assert.equal(violations[0].catalogue, 'PERSON_SOURCES'); + }); + + it('skips non-literal initialisers (variable indirection)', () => { + const text = ` + import { PERSON_SOURCES } from '../users/person-source'; + const s = 'something'; + const a = { source: s }; + `; + const violations = findPersonSourceViolationsInFile('virtual.ts', validSources, text); + assert.equal(violations.length, 0); + }); + + it('reports line and column of the offending literal', () => { + const text = [ + `import { PERSON_SOURCES } from '../users/person-source';`, + `const x = {`, + ` source: 'rogue-source',`, + `};`, + ].join('\n'); + const violations = findPersonSourceViolationsInFile('virtual.ts', validSources, text); + assert.equal(violations.length, 1); + assert.equal(violations[0].line, 3); + // 'rogue-source' literal starts at column 11 (0-indexed 10, +1 in the report). + assert.equal(violations[0].column, 11); + }); +}); + describe('scanWorkspace', () => { it('returns 0 violations on a clean fixture', () => { const root = makeFixture({ @@ -333,25 +431,28 @@ describe('scanWorkspace', () => { assert.equal(violations.length, 1); }); - it('aggregates decorator + Structure.kind violations together', () => { + it('aggregates decorator + Structure.kind + Person.source violations together', () => { const root = makeFixture({ files: { 'apps/portal-bff/src/foo.ts': ` import { STRUCTURE_KINDS } from './structures/structure-kind'; + import { PERSON_SOURCES } from './users/person-source'; @RequirePrivilege('Portal.Ghost') class Bad {} const s = { kind: 'phantom_kind' }; + const p = { source: 'rogue-source' }; `, }, }); const { violations } = scanWorkspace(root); - assert.equal(violations.length, 2); + assert.equal(violations.length, 3); const values = violations.map((v) => v.value).sort(); - assert.deepEqual(values, ['Portal.Ghost', 'phantom_kind']); + assert.deepEqual(values, ['Portal.Ghost', 'phantom_kind', 'rogue-source']); }); - it('returns the structureKinds set alongside catalogues', () => { + it('returns the structureKinds and personSources sets alongside catalogues', () => { const root = makeFixture({ files: {} }); - const { structureKinds } = scanWorkspace(root); + const { structureKinds, personSources } = scanWorkspace(root); assert.deepEqual([...structureKinds].sort(), ['antenne', 'medico_social', 'siege']); + assert.deepEqual([...personSources].sort(), ['admin-ui', 'seed', 'self-signin']); }); }); -- 2.30.2