Merge RB-13 — land Session -> Principal, add MedewerkerAdapter
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> # Conflicts: # libs/shared/docs/behaviour-spec.mdx
This commit is contained in:
@@ -0,0 +1,186 @@
|
||||
# RB-13 — land `Session → Principal`; `MedewerkerAdapter`; the backoffice login stops being a DigiD/BSN form
|
||||
|
||||
Status: **implemented** · 2026-08-27 · Source findings: `06-adr-conformance.md` ADR-C-004 · `00-baseline.md` BL-002 · `docs/reference/architecture/0002-user-groups-and-bounded-contexts.md` §3, "Known debt" · `99-backlog.md` RB-13
|
||||
|
||||
## What was wrong
|
||||
|
||||
ADR-0002 §3 ("Separate identity from authorization") specifies a discriminated
|
||||
`Principal` union — `{ kind: 'zorgverlener'; bsn; naam } | { kind: 'medewerker';
|
||||
medewerkerId; naam; rollen }` — as "the one concrete FE change when actor #2 lands."
|
||||
Actor #2 (`apps/behandelportal`) landed in WP-61/67; the union did not follow.
|
||||
|
||||
Verified before this ticket:
|
||||
|
||||
- `grep -rn "Principal" apps libs` returned exactly one hit — a comment in
|
||||
`libs/shared/src/infrastructure/role.ts:8`. No such type existed.
|
||||
- `apps/ssp/src/app/auth/domain/session.ts` and
|
||||
`apps/behandelportal/src/app/auth/domain/session.ts` were byte-identical:
|
||||
`interface Session { readonly bsn: string; readonly naam: string }` — a Behandelaar
|
||||
carrying a `bsn`, which §3 names as precisely the state the union exists to make
|
||||
unrepresentable.
|
||||
- `apps/behandelportal/src/app/auth/ui/login.page.ts` rendered `intro="Log in op uw
|
||||
persoonlijke BIG-register omgeving."` and called `SessionStore.login(bsn)` →
|
||||
`DigidAdapter.authenticate(bsn)`, resolving `{ bsn: r.value, naam: 'Dr. A. (Anna) de
|
||||
Vries' }` — a backoffice employee logging into the backoffice as a citizen, by DigiD,
|
||||
under a citizen's name.
|
||||
- `apps/behandelportal/src/app/auth/infrastructure/medewerker.interceptor.ts` already
|
||||
stamps every backend request with `X-Medewerker`/`X-Rollen`, independently of
|
||||
`SessionStore` — the divergence ADR-0002 predicted took this orthogonal side door
|
||||
instead of the `Principal` union, which is why the two `auth` contexts still measured
|
||||
as identical.
|
||||
- `tools/baseline-scan.mjs --dup`, measured immediately before this ticket (after
|
||||
ADR-C-006 shared the route guards): `ssp/auth` 168/168 dup lines (100.0%),
|
||||
`bhp/auth` 168/200 (84.0%) — down from the original 211/211, but the WP-67 amendment's
|
||||
"auth stays duplicated because it's expected to diverge" claim had never actually been
|
||||
tested, only asserted.
|
||||
|
||||
RB-09 (a prerequisite, landed the day before) made the backend's `IIdentityProvider`
|
||||
able to say "no identity" and fail closed; this ticket is its stated FE half — without
|
||||
it, a production behandelportal falls through to the seeded zorgverlener by default,
|
||||
open on every citizen-scoped endpoint and holding `CanRevealBigNummer`. This ticket
|
||||
does not touch that backend behaviour — it makes the FE identity model honest about
|
||||
who is actually authenticating.
|
||||
|
||||
## What changed
|
||||
|
||||
| File | Change |
|
||||
| -------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||
| `apps/ssp/src/app/auth/domain/session.ts` → `principal.ts` | `Session` → `Principal`, `{ kind: 'zorgverlener'; bsn; naam }`; `parseStoredSession` → `parseStoredPrincipal` (G1/G2 unchanged) |
|
||||
| `apps/ssp/src/app/auth/domain/session.spec.ts` → `principal.spec.ts` | renamed, updated to the `Principal`/`kind` shape |
|
||||
| `apps/ssp/src/app/auth/application/session.store.ts` | `Session` → `Principal`; header doc rewritten to state _why_ G1 applies here and not in behandelportal (cross-reference, not shared prose) |
|
||||
| `apps/ssp/src/app/auth/infrastructure/digid.adapter.ts` | resolves `{ kind: 'zorgverlener', bsn, naam }` |
|
||||
| `apps/ssp/src/app/shell/debug-state/debug-state.component.ts` | `Session` → `Principal` (the one other consumer of the domain type) |
|
||||
| `apps/behandelportal/src/app/auth/domain/session.ts` → `principal.ts` | new `medewerker` variant: `{ kind: 'medewerker'; medewerkerId; naam; rollen: readonly Rol[] }`; `parseStoredPrincipal` validates the full shape (no BSN to strip — G2 only); new `parseRollen(raw): Rol[]`, mirroring the backend's `StubIdentityProvider.ParseRollen` (comma-separated, case-insensitive, unrecognized tokens dropped) |
|
||||
| `apps/behandelportal/src/app/auth/domain/session.spec.ts` → `principal.spec.ts` | rewritten: `isAuthenticated`, `parseStoredPrincipal` (5 cases including "kind is not medewerker" and "unrecognized rol"), `parseRollen` (4 cases) |
|
||||
| `apps/behandelportal/src/app/auth/infrastructure/digid.adapter.ts` → `medewerker.adapter.ts` | **new `MedewerkerAdapter`** — resolves `MEDEWERKER_ID` + `currentRollen()` (`medewerker.ts`, unchanged) into a `Principal`; no input, returns the `Principal` directly (no `Result` — there is nothing for this stand-in to fail on) |
|
||||
| `apps/behandelportal/src/app/auth/application/session.store.ts` | `MedewerkerAdapter` replaces `DigidAdapter`; `login()` takes no argument; the whole principal round-trips through `localStorage` (no G1 field to strip); header doc rewritten, cross-referencing the SSP's instead of repeating it |
|
||||
| `apps/behandelportal/src/app/auth/ui/login-form/login-form.component.ts` | rewritten: no BSN/wachtwoord fields — one explainer line + one "Inloggen met SSO" button, `submitted = output<void>()` |
|
||||
| `apps/behandelportal/src/app/auth/ui/login.page.ts` | new heading/intro copy ("Inloggen bij het behandelportal" / "Voor medewerkers die aanvragen beoordelen."); `login()` takes no argument; the error-alert branch is gone (nothing can fail) |
|
||||
| `apps/behandelportal/src/locale/messages.en.xlf` | new id `login.ssoExplainer`; `login.submit`/`login.heading`/`login.intro` updated to the new source text + English target; `login.bsnLabel`/`bsnDescription`/`wachtwoordLabel`/`form.verplichteVelden` removed (no longer reachable from this app — confirmed by grep and by a trial `extract-i18n:behandelportal` run) |
|
||||
| `libs/shared/src/infrastructure/subject.ts`, `subject.interceptor.ts` | doc comments: `` `Session.bsn` `` → `` `Principal.bsn` `` (the type these comments cite renamed; the design they describe — `libs/shared` can't reach an app-local `auth` context, so `?subject=` exists instead — is unchanged) |
|
||||
| `docs/reference/architecture/0002-user-groups-and-bounded-contexts.md` | new "Amendment (RB-13, 2026-08-27)" replacing the "Known debt" section it closes out; records what landed and the re-measured duplication figure |
|
||||
| `libs/shared/docs/behaviour-spec.mdx` | regenerated (`npm run gen:behaviour-spec`) — reflects the renamed spec titles and the new `parseRollen`/medewerker `parseStoredPrincipal` cases |
|
||||
|
||||
## Judgement calls
|
||||
|
||||
- **Each app's `Principal` holds only the one variant it has an actor for**, not the
|
||||
full two-member union ADR-0002 §3 writes as a single illustrative type. The ADR's own
|
||||
proposed resolution under ADR-C-004 says this explicitly ("In `apps/behandelportal`:
|
||||
replace `Session` with the `medewerker` variant … In `apps/ssp`: the `zorgverlener`
|
||||
variant"), and it matches how the codebase already splits `auth` per app. `kind` stays
|
||||
on both single-member types anyway — it is what makes the two types genuinely
|
||||
different rather than a same-shaped coincidence, and it is where a third actor (§4 —
|
||||
admin/auditor/institution-rep) would add a member.
|
||||
- **`MedewerkerAdapter.authenticate()` returns `Promise<Principal>`, not
|
||||
`Promise<Result<string, Principal>>`.** The first draft mirrored `DigidAdapter`'s
|
||||
`Result`-returning shape for symmetry, but that `Result`'s error variant could never
|
||||
actually be produced — there is no credential to check, so wrapping the return in a
|
||||
type that claims to have a failure mode was itself a small instance of the thing
|
||||
CLAUDE.md §3 warns against (representing a state that can't happen). Reverted to a
|
||||
direct `Promise<Principal>` and dropped the now-dead error-handling branch from
|
||||
`login.page.ts` (`error` signal, the `<app-alert type="error">`, the `AlertComponent`
|
||||
import) — a real SSO integration is where that branch would come back, not before.
|
||||
This was also the change that did the most to bring the duplication figure down (see
|
||||
below): `login.page.ts`'s 7-window overlap with the SSP's disappeared once the two
|
||||
pages' control flow, not just their copy, actually differed.
|
||||
- **`rollen` is typed `readonly Rol[]` with `Rol = 'behandelaar'`, and `parseRollen`
|
||||
lives in `domain/`, not the adapter.** The raw `currentRollen()` stand-in returns an
|
||||
unvalidated string (`medewerker.ts`, untouched by this ticket); turning it into typed
|
||||
`Rol[]` is pure string logic with no Angular dependency, so it belongs in
|
||||
`domain/principal.ts` per CLAUDE.md §1's layer table — the adapter (`infrastructure/`)
|
||||
stays a thin wire-up that only reaches for `MEDEWERKER_ID`/`currentRollen()` and
|
||||
hands them to a pure function. `parseRollen` deliberately mirrors the backend's own
|
||||
`StubIdentityProvider.ParseRollen` (comma-separated, unrecognized tokens dropped, so
|
||||
`?rollen=geen` yields `[]`) — this is not the FE recomputing a business rule
|
||||
(ADR-0001's boundary is about _authorization decisions_, which still come only from
|
||||
`GET /me`/`AccessStore`); it is the FE's own dev-only identity stand-in echoing the
|
||||
same header value it is about to send, for display, the same way `DigidAdapter`
|
||||
already fabricates its own fake identity.
|
||||
- **`SessionStore` (bhp) persists the whole `Principal` to `localStorage`, not a
|
||||
stripped-down `{ naam }` copy.** The SSP's G1 guarantee ("never persist the BSN")
|
||||
doesn't apply here — a `medewerker` principal has no national identifier — so there is
|
||||
nothing to strip. `parseStoredPrincipal` validates the full shape (G2 only) and
|
||||
restores it as-is. This was a deliberate choice against an alternative: reconstructing
|
||||
`medewerkerId`/`rollen` from the live `MEDEWERKER_ID`/`currentRollen()` on every
|
||||
restore, which would have made `domain/principal.ts` depend on
|
||||
`infrastructure/medewerker.ts` — backwards per CLAUDE.md §1's inward-only dependency
|
||||
rule, and it would have made `parseStoredPrincipal` impure. Consequence: changing
|
||||
`?rollen=` mid-session does not retroactively change an already-restored `Principal`
|
||||
until the next `login()`/`logout()` — the same way changing the DigiD demo BSN
|
||||
requires a fresh login in the SSP. The backend's own authorization is unaffected
|
||||
either way, since `medewerkerInterceptor` reads `currentRollen()` fresh on every HTTP
|
||||
request regardless of what `SessionStore` holds.
|
||||
- **Session/store class names (`SessionStore`, `SESSION_PORT`, `SessionPort`) were left
|
||||
unchanged.** ADR-0002 §3's own Consequences section names `SessionStore` — alongside
|
||||
`auth.guard.ts` — as one of the _seams that localise_ the `Session → Principal` change,
|
||||
not as something the change renames. `libs/shared/src/application/session.port.ts`'s
|
||||
`SessionPort` (ADR-C-006) is unaffected: it only ever exposed `{ naam }` and
|
||||
`isAuthenticated`, neither of which is `kind`-dependent.
|
||||
- **`libs/shared/src/infrastructure/subject.ts`/`subject.interceptor.ts` doc comments
|
||||
updated, code untouched.** Both cite `` `Session.bsn` `` by name to explain why
|
||||
`?subject=` exists instead of reading the store directly; renaming the type these
|
||||
comments describe without updating the comment would have left them citing a type
|
||||
that no longer exists.
|
||||
- **`auth.guard.ts`'s verbatim re-export in both apps was left alone.** ADR-C-006 is
|
||||
explicit that a route guard is actor-agnostic and out of ADR-0002 §3's scope — it
|
||||
reads only `SESSION_PORT`/`AccessStore`, never `Principal`, so there was nothing for
|
||||
this ticket to change there.
|
||||
- **No backend change.** RB-09 already made `IIdentityProvider` nullable and
|
||||
Production-fail-fast; this ticket is purely the frontend counterpart it named. The
|
||||
residual RB-09 flagged (`GET /uploads/{documentId}/content`'s plain-navigation
|
||||
callers carrying no identity header once a real, non-stub `IIdentityProvider` exists)
|
||||
is unaffected by anything here — it is about a _future_ real provider replacing the
|
||||
Development-only stub, which this ticket does not touch.
|
||||
|
||||
## Duplication, measured (`tools/baseline-scan.mjs --dup`)
|
||||
|
||||
| When | `ssp/auth` dup lines | `bhp/auth` dup lines |
|
||||
| ----------------------------------- | -------------------: | -------------------: |
|
||||
| Before ADR-C-006 (baseline, BL-002) | 211/211 (100%) | — |
|
||||
| After ADR-C-006, before this ticket | 168/168 (100.0%) | 168/200 (84.0%) |
|
||||
| **After this ticket** | **32/179 (17.9%)** | **32/259 (12.4%)** |
|
||||
|
||||
Expected by the backlog: "<40 after this." Measured: **32 lines each side** — under
|
||||
target. The full clone-pair listing (the script's own output truncates to the top 15
|
||||
pairs repo-wide; re-run with the pair filter widened to confirm nothing auth-related was
|
||||
hiding below that cut) resolves to exactly four remaining pairs:
|
||||
|
||||
- `principal.spec.ts` (6 windows) — both files test the same G2 "validate before
|
||||
trusting a stored shape" concept with a parallel `describe`/`it` structure (including
|
||||
the shared `import { describe, it, expect } from 'vitest';` line); the assertions
|
||||
themselves differ (BSN-stripping vs. kind/rollen validation).
|
||||
- `login-form.stories.ts` (3 windows) — the generic Storybook `Meta`/`StoryObj`/`Default`
|
||||
scaffold, unavoidable for any two co-located `.stories.ts` files regardless of subject.
|
||||
- `auth.guard.ts` (2 windows) — the intentional verbatim re-export (ADR-C-006); this is
|
||||
meant to stay identical.
|
||||
- `session.store.ts` (1 window) — down from 33 windows before this ticket to one small
|
||||
shared fragment (the `@Injectable`/signal/`asReadonly`/`computed` wiring any root
|
||||
singleton store in this codebase shares).
|
||||
|
||||
None of what remains is re-converged identity or login-flow logic — the domain type,
|
||||
the adapter, and the login UI all now differ in kind, not just in copy. §3's prediction
|
||||
("the two groups authenticate differently") has been tested for the first time by this
|
||||
ticket, not just asserted, and it held.
|
||||
|
||||
## Verification
|
||||
|
||||
Confirmed each non-trivial change is red without its fix (edited in place, verified red,
|
||||
edited back — never `git checkout`):
|
||||
|
||||
- **ssp `parseStoredPrincipal` (G1):** changed `bsn: ''` to `bsn: parsed.bsn ?? ''` →
|
||||
`G1: a stored bsn is never restored…` failed with `expected { bsn: '19012345601', …}
|
||||
to deeply equal { bsn: '', … }`. Reverted; all other tests unaffected.
|
||||
- **bhp `parseStoredPrincipal` (kind guard):** dropped the `parsed?.kind === 'medewerker'`
|
||||
clause → `returns null when kind is not medewerker` failed, returning the parsed
|
||||
zorgverlener-shaped object instead of `null`. Reverted.
|
||||
- **bhp `parseRollen`:** dropped `.filter(isRol)` → `drops unrecognized tokens` and
|
||||
`returns an empty list for an empty string` both failed (`['geen']`/`['']` returned
|
||||
instead of `[]`). Reverted.
|
||||
|
||||
`npm test` (both apps + both libraries): all green, 258 (ssp) + 37 (behandelportal) +
|
||||
133 (shared) + 23 (beheer) tests passing, including the new/renamed auth specs.
|
||||
`npm run lint`: clean. `npm run dep:check`: 0 violations, both apps. `ng build ssp
|
||||
--localize` and `ng build behandelportal --localize`: both succeed (the new
|
||||
`login.ssoExplainer` id and the updated `login.submit`/`login.heading`/`login.intro`
|
||||
sources all resolve to an English `<target>`). `npm run ci`: green (see the commit this
|
||||
doc ships with).
|
||||
Reference in New Issue
Block a user