test(auth): extract and spec the stored-session parse boundary (RB-10)
SessionStore.restore() — identical in both apps — read localStorage itself
and did the parse plus shape validation in the same module-private function,
invoked from a field initializer, so the storage read happened the instant
the singleton was constructed and no spec could feed it a raw string. The
logic it guards is a trust boundary, not incidental validation: the comment
above it names G1 (never persist the BSN) and G2 (validate the shape before
trusting it), and CLAUDE.md mandates a spec for boundary parse* adapters.
ssp/auth and bhp/auth were jointly the worst-covered frontend modules.
parseStoredSession(raw) moves into each app's auth/domain/session.ts, which
is pure TS and already had a spec, so no new scaffolding was needed;
restore() collapses to one line. Four cases: absent, non-JSON, wrong shape,
and — BIO-017's addition — a stored {"bsn":…,"naam":…} restoring with bsn
'', which makes the G1 guarantee executable rather than merely commented.
Verified red without the fix.
Landed twice, once per app, deliberately. TE-001 and BL-002 both say an
extract-to-shared here would contradict ADR-0002, which models the two
actors as different Principal variants and expects the two auth contexts to
diverge; RB-13 is what differentiates them.
Also specs redactProfile (BIO-017's second half) — a pure exported
PII-redaction function that had none.
behaviour-spec.mdx is regenerated, which also picks up the test names RB-07
added; that commit should have carried them and did not.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
@@ -0,0 +1,87 @@
|
||||
# RB-10 — extract `parseStoredSession` (both apps) and spec `redactProfile`
|
||||
|
||||
Status: **implemented** · 2026-08-27 · Source findings: `02-testability.md` TE-001 (ssp/auth and bhp/auth) · `07-bio2-compliance.md` BIO-017 · `99-backlog.md` RB-10
|
||||
|
||||
## What was wrong
|
||||
|
||||
`SessionStore.restore()` — identical in `apps/ssp/src/app/auth/application/session.store.ts`
|
||||
and `apps/behandelportal/src/app/auth/application/session.store.ts` — called
|
||||
`localStorage.getItem(STORAGE_KEY)` itself and did the parse + shape validation in the same
|
||||
module-private function. It was invoked from a field initializer
|
||||
(`private _session = signal<Session | null>(restore())`), so the storage read happened the
|
||||
instant the singleton was constructed; a spec could not feed it a raw string without
|
||||
stubbing the `localStorage` global before the injector built the store.
|
||||
|
||||
The logic behind that guard is a trust boundary, not incidental validation — the comment
|
||||
above it names two guarantees: **G1** (never persist the BSN) and **G2** (validate the shape
|
||||
before trusting it). CLAUDE.md §5 mandates a spec for boundary `parse*` adapters, and none
|
||||
existed. Baseline evidence: `02-testability.md` §3a cites `ssp/auth` and `bhp/auth` at
|
||||
42.9% line / 46.2% branch — jointly the worst line coverage in the frontend table — with
|
||||
this file's own lcov at LH 2/LF 20 (10.0% line), BRH 3/BRF 13 (23.1% branch).
|
||||
|
||||
BIO-017 read the same code and confirmed G1 holds on every path by inspection (`restore()`
|
||||
returns `{ bsn: '', naam }`, the persistence `effect()` writes only `naam`, `login()`/
|
||||
`logout()` never touch storage with a BSN) — but "correct, unverified by a test" is exactly
|
||||
the gap TE-001 already targeted, so BIO-017 folds into it and adds one required assertion:
|
||||
a stored `{"bsn":"…","naam":"…"}` must yield a session whose `bsn` is `''`.
|
||||
|
||||
Separately, `apps/ssp/src/app/shell/debug-state/mask.ts` — confirmed at the path the finding
|
||||
cites — has `redactProfile`, a pure, exported, directly callable PII-redaction function with
|
||||
no spec. It redacts name, birthdate and address and masks the BIG-nummer; BIO-017 verified it
|
||||
correct by reading, same "no regression net" gap.
|
||||
|
||||
## What changed
|
||||
|
||||
| File | Change |
|
||||
| --------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||
| `apps/ssp/src/app/auth/domain/session.ts` | added `export function parseStoredSession(raw: string \| null): Session \| null` — the exact parse+validate body `restore()` used to hold |
|
||||
| `apps/ssp/src/app/auth/application/session.store.ts` | `restore()` collapses to `parseStoredSession(localStorage.getItem(STORAGE_KEY))` |
|
||||
| `apps/ssp/src/app/auth/domain/session.spec.ts` | 4 new cases: absent, non-JSON, wrong shape, and the G1 assertion |
|
||||
| `apps/behandelportal/src/app/auth/domain/session.ts` | identical extraction, second app |
|
||||
| `apps/behandelportal/src/app/auth/application/session.store.ts` | identical collapse, second app |
|
||||
| `apps/behandelportal/src/app/auth/domain/session.spec.ts` | identical 4 cases, second app |
|
||||
| `apps/ssp/src/app/shell/debug-state/mask.spec.ts` | new file — spec for `redactProfile`: masks the BIG-nummer, redacts name/geboortedatum/adres on both `registration` and `person`, leaves `beroep`/`registratiedatum`/`status` untouched |
|
||||
|
||||
The extracted function's body is a byte-for-byte move — same `try`/`catch`, same
|
||||
`JSON.parse` cast, same `typeof parsed?.naam === 'string'` guard, same `{ bsn: '', naam }`
|
||||
construction. Only its location and the doc comment (rewritten to explain the _why_ of G1/G2
|
||||
for a function now read on its own, rather than inline next to the `effect()` it used to sit
|
||||
beside) changed.
|
||||
|
||||
## The seam lands twice, on purpose
|
||||
|
||||
`auth` is deliberately unshared per ADR-0002 / CLAUDE.md §1: Zorgverlener and Medewerker are
|
||||
different `Principal` variants with different login flows, and the two `session.ts` files are
|
||||
expected to diverge. TE-001 says this outright, and BL-002 flags any extract-to-`libs/shared`
|
||||
here as contradicting an accepted ADR. `parseStoredSession` was therefore written twice, once
|
||||
per app's own `domain/session.ts` — not factored into a shared helper, and not resisted only
|
||||
in this note; the two functions are word-for-word identical today and that is expected to
|
||||
change the moment `RB-13` (`Session → Principal`) lands.
|
||||
|
||||
## Judgement calls
|
||||
|
||||
- **`redactProfile`'s spec asserts on the concrete shape**, not just "not equal to the input" —
|
||||
it pins `bigNummer` to `'********901'`, checks `REDACTED` on each PII field individually, and
|
||||
separately asserts the non-PII fields (`beroep`, `registratiedatum`, `status`) survive
|
||||
unchanged. A looser "no PII substring appears" assertion would have been weaker at catching
|
||||
the regression this ticket exists to prevent (e.g. a future field added to `redactProfile`'s
|
||||
output that is left unmasked by accident).
|
||||
- **The G1 spec case uses `toEqual`, not `toBe`**, since the parser constructs a new object;
|
||||
this matches the existing `isAuthenticated` spec's style in the same file.
|
||||
- No production code beyond the `restore()` one-liner in each `session.store.ts` changed —
|
||||
`login()`, `logout()`, and the persistence `effect()` were already correct and are
|
||||
unaffected.
|
||||
|
||||
## Verification
|
||||
|
||||
Confirmed both new specs are red without the fix:
|
||||
|
||||
- Temporarily changed `parseStoredSession` to keep a stored `bsn` (`bsn: parsed.bsn ?? ''`
|
||||
instead of `bsn: ''`) — the new G1 test failed with
|
||||
`expected { bsn: '19012345601', naam: 'Test' } to deeply equal { bsn: '', naam: 'Test' }`,
|
||||
all 243 other tests stayed green. Reverted; `git diff` on the file is empty afterward.
|
||||
- Temporarily changed `redactProfile` to pass `naam` through unmasked — the new "redacts the
|
||||
name" test failed with `expected 'J. Jansen' to be '‹redacted›'`. Reverted; `git diff` on the
|
||||
file is empty afterward.
|
||||
|
||||
`npm run ci`: **green** (see PR/commit for the run this doc ships with).
|
||||
Reference in New Issue
Block a user