eho/atomic-design-poc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(auth): land Session -> Principal, add MedewerkerAdapter (RB-13)

Browse Source 
ADR-0002 SS3 models Zorgverlener/Medewerker as different Principal
variants with different login flows. Actor #2 (apps/behandelportal)
landed in WP-61/67 and the union never followed: grep -rn "Principal"
returned one hit, a comment. Both apps' auth/domain/session.ts stayed
byte-identical (`{ bsn, naam }`), so the backoffice's Behandelaar
carried a BSN and logged into the backoffice as a citizen, by DigiD,
under a fabricated citizen's name (login.page.ts). The divergence
ADR-0002 predicted took an orthogonal side door instead
(medewerker.interceptor.ts's X-Medewerker/X-Rollen stamp, which never
touches SessionStore) -- which is why ssp/auth and bhp/auth still
measured as 100%/84% duplicated after ADR-C-006 shared the route
guards. RB-09 (landed the day before) made the backend's
IIdentityProvider able to say "no identity" and fail closed; this
ticket is its named FE half.

Each app's auth/domain/session.ts becomes principal.ts, holding the
one Principal variant that app actually has an actor for: ssp keeps
`{ kind: 'zorgverlener', bsn, naam }` (G1 still strips the BSN before
persisting); behandelportal gets `{ kind: 'medewerker', medewerkerId,
naam, rollen }` (no BSN to strip -- G2 shape validation only). A new
MedewerkerAdapter replaces DigidAdapter in behandelportal, resolving
the existing MEDEWERKER_ID/currentRollen() dev stand-in into a
Principal; because there is no credential to check, it returns
Principal directly rather than a Result whose error variant could
never occur. login.page.ts stops being a BSN/wachtwoord form -- one
explainer line and an "Inloggen met SSO" button -- and its dead
error-handling branch goes with the Result wrapper that justified it.

Measured with tools/baseline-scan.mjs --dup: auth duplication drops
from 168/168 (ssp) and 168/200 (bhp) to 32/179 and 32/259 -- under the
backlog's <40 target. What remains is the ADR-C-006 route-guard
re-export (deliberately identical), generic test/story-file
boilerplate, and one shared fragment of the root-singleton-store
idiom -- not re-converged identity or login-flow logic. SS3's
prediction that the two actors would authenticate differently enough
to justify not sharing auth has now actually been tested, not just
asserted, and held.

Also: renamed Session.bsn to Principal.bsn in two doc comments
(libs/shared/src/infrastructure/subject.ts, subject.interceptor.ts)
that cited the old type name; regenerated
libs/shared/docs/behaviour-spec.mdx (generated file, per its own
banner); recorded the resolution in ADR-0002 as a new amendment,
replacing its "Known debt" section.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
eho
2026-08-27 16:54:26 +02:00
co-authored by Claude Opus 5
parent 988612cd7e
commit f19185ed81
22 changed files with 588 additions and 319 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-13.md
+186
View File
@@ -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).
docs/reference/architecture/0002-user-groups-and-bounded-contexts.md
+29 -23
View File
@@ -1,6 +1,6 @@
# ADR 0002 — User groups as actors, not bounded contexts
Status: Accepted · Date: 2026-07-01 · Amended 2026-08-01 (WP-67)
Status: Accepted · Date: 2026-07-01 · Amended 2026-08-01 (WP-67), 2026-08-27 (RB-13)
## Problem
@@ -167,28 +167,34 @@ status lifecycle + authorization endpoints/DTOs — **shipped** (WP-61…WP-67):
`AanvraagStatusTag` (`Domain/Applications/AanvraagStatus.cs`), `GET /me` (`Program.cs:578`),
`Domain/Authorization/Authz.cs`.
## Known debt: `Session → Principal` was never built
A third bullet stood here too — `Session → Principal` — from 2026-08-26 until it was paid
off by RB-13 the next day. See the amendment below for the historical record and what
landed.
§3's `Principal` union is the one decision here that has **not** been executed, and it is now
debt rather than a deferral. Actor #2 arrived — `apps/behandelportal` shipped — and the union
did not follow. `grep -rn "Principal" apps libs` returns a single hit: a comment in
`libs/shared/src/infrastructure/role.ts`. There is no such type.
## Amendment (RB-13, 2026-08-27): `Session → Principal` landed
What that omission actually costs, measured 2026-08-26:
§3's `Principal` union was accepted on 2026-07-01 and not executed until now — see the
"Known debt" record this replaces, added 2026-08-26 by the refactor-backlog audit
(`ADR-C-004`) that found it. `apps/ssp/src/app/auth/domain/principal.ts` now exports the
`zorgverlener` variant (`{ kind: 'zorgverlener'; bsn; naam }`);
`apps/behandelportal/src/app/auth/domain/principal.ts` exports the `medewerker` variant
(`{ kind: 'medewerker'; medewerkerId; naam; rollen }`) — each app holds only the one
member of the union it actually has an actor for, per this ADR's own proposed resolution.
`apps/behandelportal`'s `DigidAdapter` is gone; a `MedewerkerAdapter` resolves the
dev-stand-in medewerker identity (`medewerker.ts`'s `MEDEWERKER_ID`/`currentRollen()` —
unchanged, still the mechanism `medewerkerInterceptor` uses for the backend headers) into
a `Principal` instead, and `login.page.ts` is an SSO-stand-in entry (one button, no BSN
field) rather than the citizen DigiD form it used to share with the SSP verbatim.
- `apps/ssp/src/app/auth` and `apps/behandelportal/src/app/auth` are byte-identical —
`diff -rq` reports **zero** content differences across 9 of 11 files, the only delta being
two extra files in behandelportal.
- `behandelportal`'s Behandelaar still carries a `bsn` and logs in through `DigidAdapter`.
A backoffice user authenticates as a citizen, which is precisely what §3 was written to prevent.
- The divergence that _did_ occur took an orthogonal side door — `medewerker.interceptor.ts`,
a dev-only `X-Medewerker` header stamp that never touches `Session`.
The WP-67 amendment above justifies keeping `auth` duplicated on the grounds that it is
"expected to diverge". That reasoning still holds — but it has never been **tested**, because
the change that would test it is this one. Read the two identical copies as evidence that
§3 is unexecuted, not as evidence that §3 was wrong.
ponytail: this ADR draws the boundaries so nothing has to be undone later. The original
"YAGNI until the backoffice work starts" call was right when written and has now expired —
the backoffice started. `Principal` is owed.
The two `auth` contexts, measured 2026-08-27 after the change
(`tools/baseline-scan.mjs --dup`): **32 duplicated lines each** (from 168 at the
2026-08-26 measurement above; from 211 before ADR-C-006 shared the route guards). What
remains is not re-converged identity/login-flow code — it is `auth.guard.ts`'s intentional
verbatim re-export (ADR-C-006: a route guard is actor-agnostic, not in this ADR's scope)
plus ordinary test/story-file boilerplate (`describe`/`it` shape, a `Meta`/`StoryObj`
scaffold) that any two spec or story files share regardless of subject. The prediction in
§3 — that Zorgverlener and Medewerker, modelled as distinct `Principal` variants, would
turn out to authenticate differently enough that sharing `auth` would have been the wrong
call — has now actually been tested, not just asserted, and held: the two contexts diverge
in domain type, adapter, and login UI as soon as the union exists to make that
divergence possible.