diff --git a/apps/behandelportal/src/app/auth/application/session.store.ts b/apps/behandelportal/src/app/auth/application/session.store.ts index 47374d3..88ed651 100644 --- a/apps/behandelportal/src/app/auth/application/session.store.ts +++ b/apps/behandelportal/src/app/auth/application/session.store.ts @@ -1,23 +1,16 @@ import { Injectable, computed, effect, inject, signal } from '@angular/core'; import { Result } from '@shared/kernel/fp'; -import { Session } from '../domain/session'; +import { Session, parseStoredSession } from '../domain/session'; import { DigidAdapter } from '../infrastructure/digid.adapter'; const STORAGE_KEY = 'session-v1'; /** Restore a persisted session (best-effort; corrupt entry → logged out). - G2: validate the shape before trusting it. G1: the BSN is never persisted - (see the effect below), so a restored session carries an empty one — it is - unused after login; only `naam` is shown in the chrome. */ + The parse + shape validation (G1/G2) lives in `parseStoredSession` + (`../domain/session`) — pure, spec'd, and testable without stubbing + `localStorage`; this just supplies the raw value. */ function restore(): Session | null { - try { - const raw = localStorage.getItem(STORAGE_KEY); - if (!raw) return null; - const parsed = JSON.parse(raw) as Partial; - return typeof parsed?.naam === 'string' ? { bsn: '', naam: parsed.naam } : null; - } catch { - return null; - } + return parseStoredSession(localStorage.getItem(STORAGE_KEY)); } /** diff --git a/apps/behandelportal/src/app/auth/domain/session.spec.ts b/apps/behandelportal/src/app/auth/domain/session.spec.ts index c46343d..af90034 100644 --- a/apps/behandelportal/src/app/auth/domain/session.spec.ts +++ b/apps/behandelportal/src/app/auth/domain/session.spec.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { isAuthenticated, Session } from './session'; +import { isAuthenticated, parseStoredSession, Session } from './session'; const session: Session = { bsn: '19012345601', naam: 'Test' }; @@ -12,3 +12,22 @@ describe('isAuthenticated', () => { expect(isAuthenticated(null)).toBe(false); }); }); + +describe('parseStoredSession', () => { + it('returns null when nothing is stored', () => { + expect(parseStoredSession(null)).toBeNull(); + }); + + it('returns null for a non-JSON string', () => { + expect(parseStoredSession('not json')).toBeNull(); + }); + + it('returns null when the stored shape is wrong (no naam)', () => { + expect(parseStoredSession(JSON.stringify({ bsn: '19012345601' }))).toBeNull(); + }); + + it('G1: a stored bsn is never restored, even if present in the raw value', () => { + const restored = parseStoredSession(JSON.stringify({ bsn: '19012345601', naam: 'Test' })); + expect(restored).toEqual({ bsn: '', naam: 'Test' }); + }); +}); diff --git a/apps/behandelportal/src/app/auth/domain/session.ts b/apps/behandelportal/src/app/auth/domain/session.ts index 486dc85..abbbbf6 100644 --- a/apps/behandelportal/src/app/auth/domain/session.ts +++ b/apps/behandelportal/src/app/auth/domain/session.ts @@ -7,3 +7,21 @@ export interface Session { export function isAuthenticated(s: Session | null): s is Session { return s !== null; } + +/** + * Parse a persisted session out of a raw `localStorage` string (best-effort; + * anything that isn't a well-shaped record → logged out). G2: validate the + * shape before trusting it. G1: even if a stored entry carries a `bsn`, the + * restored session's `bsn` is always `''` — the BSN is never persisted (see + * the `SessionStore` effect that writes it), so a legacy or tampered entry + * cannot resurrect one. + */ +export function parseStoredSession(raw: string | null): Session | null { + try { + if (!raw) return null; + const parsed = JSON.parse(raw) as Partial; + return typeof parsed?.naam === 'string' ? { bsn: '', naam: parsed.naam } : null; + } catch { + return null; + } +} diff --git a/apps/ssp/src/app/auth/application/session.store.ts b/apps/ssp/src/app/auth/application/session.store.ts index 47374d3..88ed651 100644 --- a/apps/ssp/src/app/auth/application/session.store.ts +++ b/apps/ssp/src/app/auth/application/session.store.ts @@ -1,23 +1,16 @@ import { Injectable, computed, effect, inject, signal } from '@angular/core'; import { Result } from '@shared/kernel/fp'; -import { Session } from '../domain/session'; +import { Session, parseStoredSession } from '../domain/session'; import { DigidAdapter } from '../infrastructure/digid.adapter'; const STORAGE_KEY = 'session-v1'; /** Restore a persisted session (best-effort; corrupt entry → logged out). - G2: validate the shape before trusting it. G1: the BSN is never persisted - (see the effect below), so a restored session carries an empty one — it is - unused after login; only `naam` is shown in the chrome. */ + The parse + shape validation (G1/G2) lives in `parseStoredSession` + (`../domain/session`) — pure, spec'd, and testable without stubbing + `localStorage`; this just supplies the raw value. */ function restore(): Session | null { - try { - const raw = localStorage.getItem(STORAGE_KEY); - if (!raw) return null; - const parsed = JSON.parse(raw) as Partial; - return typeof parsed?.naam === 'string' ? { bsn: '', naam: parsed.naam } : null; - } catch { - return null; - } + return parseStoredSession(localStorage.getItem(STORAGE_KEY)); } /** diff --git a/apps/ssp/src/app/auth/domain/session.spec.ts b/apps/ssp/src/app/auth/domain/session.spec.ts index c46343d..af90034 100644 --- a/apps/ssp/src/app/auth/domain/session.spec.ts +++ b/apps/ssp/src/app/auth/domain/session.spec.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { isAuthenticated, Session } from './session'; +import { isAuthenticated, parseStoredSession, Session } from './session'; const session: Session = { bsn: '19012345601', naam: 'Test' }; @@ -12,3 +12,22 @@ describe('isAuthenticated', () => { expect(isAuthenticated(null)).toBe(false); }); }); + +describe('parseStoredSession', () => { + it('returns null when nothing is stored', () => { + expect(parseStoredSession(null)).toBeNull(); + }); + + it('returns null for a non-JSON string', () => { + expect(parseStoredSession('not json')).toBeNull(); + }); + + it('returns null when the stored shape is wrong (no naam)', () => { + expect(parseStoredSession(JSON.stringify({ bsn: '19012345601' }))).toBeNull(); + }); + + it('G1: a stored bsn is never restored, even if present in the raw value', () => { + const restored = parseStoredSession(JSON.stringify({ bsn: '19012345601', naam: 'Test' })); + expect(restored).toEqual({ bsn: '', naam: 'Test' }); + }); +}); diff --git a/apps/ssp/src/app/auth/domain/session.ts b/apps/ssp/src/app/auth/domain/session.ts index 486dc85..abbbbf6 100644 --- a/apps/ssp/src/app/auth/domain/session.ts +++ b/apps/ssp/src/app/auth/domain/session.ts @@ -7,3 +7,21 @@ export interface Session { export function isAuthenticated(s: Session | null): s is Session { return s !== null; } + +/** + * Parse a persisted session out of a raw `localStorage` string (best-effort; + * anything that isn't a well-shaped record → logged out). G2: validate the + * shape before trusting it. G1: even if a stored entry carries a `bsn`, the + * restored session's `bsn` is always `''` — the BSN is never persisted (see + * the `SessionStore` effect that writes it), so a legacy or tampered entry + * cannot resurrect one. + */ +export function parseStoredSession(raw: string | null): Session | null { + try { + if (!raw) return null; + const parsed = JSON.parse(raw) as Partial; + return typeof parsed?.naam === 'string' ? { bsn: '', naam: parsed.naam } : null; + } catch { + return null; + } +} diff --git a/apps/ssp/src/app/shell/debug-state/mask.spec.ts b/apps/ssp/src/app/shell/debug-state/mask.spec.ts new file mode 100644 index 0000000..92915a4 --- /dev/null +++ b/apps/ssp/src/app/shell/debug-state/mask.spec.ts @@ -0,0 +1,58 @@ +import { describe, it, expect } from 'vitest'; +import { BigProfile } from '@registratie/domain/big-profile'; +import { REDACTED } from '@shared/kernel/pii'; +import { redactProfile } from './mask'; + +const profile: BigProfile = { + registration: { + bigNummer: '12345678901', + naam: 'J. Jansen', + beroep: 'arts', + registratiedatum: '2015-03-01', + geboortedatum: '1980-06-12', + status: { tag: 'Geregistreerd', herregistratieDatum: '2027-03-01' }, + }, + person: { + naam: 'J. Jansen', + geboortedatum: '1980-06-12', + adres: { straat: 'Hoofdstraat 1', postcode: '1234AB', woonplaats: 'Utrecht' }, + }, +}; + +describe('redactProfile', () => { + const redacted = redactProfile(profile) as { + registration: { + bigNummer: string; + naam: string; + beroep: string; + registratiedatum: string; + geboortedatum: string; + status: unknown; + }; + person: { naam: string; geboortedatum: string; adres: string }; + }; + + it('masks the BIG-nummer to its last 3 digits', () => { + expect(redacted.registration.bigNummer).toBe('********901'); + }); + + it('redacts the name on both the registration and the person', () => { + expect(redacted.registration.naam).toBe(REDACTED); + expect(redacted.person.naam).toBe(REDACTED); + }); + + it('redacts every date of birth', () => { + expect(redacted.registration.geboortedatum).toBe(REDACTED); + expect(redacted.person.geboortedatum).toBe(REDACTED); + }); + + it('redacts the address', () => { + expect(redacted.person.adres).toBe(REDACTED); + }); + + it('keeps structural/decision-relevant fields untouched', () => { + expect(redacted.registration.beroep).toBe('arts'); + expect(redacted.registration.registratiedatum).toBe('2015-03-01'); + expect(redacted.registration.status).toEqual(profile.registration.status); + }); +}); diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-10.md b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-10.md new file mode 100644 index 0000000..00ce89c --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-10.md @@ -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(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). diff --git a/libs/shared/docs/behaviour-spec.mdx b/libs/shared/docs/behaviour-spec.mdx index f951757..6a55c75 100644 --- a/libs/shared/docs/behaviour-spec.mdx +++ b/libs/shared/docs/behaviour-spec.mdx @@ -20,8 +20,8 @@ tested where._ Every bullet below is a real test name from the suite — an `it()` title (frontend) or a test method name (backend), read as a sentence. Nothing here is hand-written prose: this page -**is** the suite, reshaped for a business reader. 402 frontend behaviours across -8 contexts; 228 backend behaviours across 38 test +**is** the suite, reshaped for a business reader. 415 frontend behaviours across +9 contexts; 228 backend behaviours across 38 test classes. ## Frontend (by context) @@ -35,6 +35,17 @@ classes. - narrows a present session to Session - reports no session as not authenticated +#### parseStoredSession + +- returns null when nothing is stored +- returns null for a non-JSON string +- returns null when the stored shape is wrong (no naam) +- G1: a stored bsn is never restored, even if present in the raw value +- returns null when nothing is stored +- returns null for a non-JSON string +- returns null when the stored shape is wrong (no naam) +- G1: a stored bsn is never restored, even if present in the raw value + ### behandeling #### besluit reduce @@ -781,6 +792,16 @@ classes. - clears the key once the wrapped fn settles - falls back to a generated uuid-shaped key when none is pending +### shell + +#### redactProfile + +- masks the BIG-nummer to its last 3 digits +- redacts the name on both the registration and the person +- redacts every date of birth +- redacts the address +- keeps structural/decision-relevant fields untouched + ### showcase #### highlightTs