Merge RB-10 — extract and spec the stored-session parse boundary
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> # Conflicts: # libs/shared/docs/behaviour-spec.mdx
This commit is contained in:
@@ -1,23 +1,16 @@
|
|||||||
import { Injectable, computed, effect, inject, signal } from '@angular/core';
|
import { Injectable, computed, effect, inject, signal } from '@angular/core';
|
||||||
import { Result } from '@shared/kernel/fp';
|
import { Result } from '@shared/kernel/fp';
|
||||||
import { Session } from '../domain/session';
|
import { Session, parseStoredSession } from '../domain/session';
|
||||||
import { DigidAdapter } from '../infrastructure/digid.adapter';
|
import { DigidAdapter } from '../infrastructure/digid.adapter';
|
||||||
|
|
||||||
const STORAGE_KEY = 'session-v1';
|
const STORAGE_KEY = 'session-v1';
|
||||||
|
|
||||||
/** Restore a persisted session (best-effort; corrupt entry → logged out).
|
/** Restore a persisted session (best-effort; corrupt entry → logged out).
|
||||||
G2: validate the shape before trusting it. G1: the BSN is never persisted
|
The parse + shape validation (G1/G2) lives in `parseStoredSession`
|
||||||
(see the effect below), so a restored session carries an empty one — it is
|
(`../domain/session`) — pure, spec'd, and testable without stubbing
|
||||||
unused after login; only `naam` is shown in the chrome. */
|
`localStorage`; this just supplies the raw value. */
|
||||||
function restore(): Session | null {
|
function restore(): Session | null {
|
||||||
try {
|
return parseStoredSession(localStorage.getItem(STORAGE_KEY));
|
||||||
const raw = localStorage.getItem(STORAGE_KEY);
|
|
||||||
if (!raw) return null;
|
|
||||||
const parsed = JSON.parse(raw) as Partial<Session>;
|
|
||||||
return typeof parsed?.naam === 'string' ? { bsn: '', naam: parsed.naam } : null;
|
|
||||||
} catch {
|
|
||||||
return null;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
import { describe, it, expect } from 'vitest';
|
import { describe, it, expect } from 'vitest';
|
||||||
import { isAuthenticated, Session } from './session';
|
import { isAuthenticated, parseStoredSession, Session } from './session';
|
||||||
|
|
||||||
const session: Session = { bsn: '19012345601', naam: 'Test' };
|
const session: Session = { bsn: '19012345601', naam: 'Test' };
|
||||||
|
|
||||||
@@ -12,3 +12,22 @@ describe('isAuthenticated', () => {
|
|||||||
expect(isAuthenticated(null)).toBe(false);
|
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' });
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -7,3 +7,21 @@ export interface Session {
|
|||||||
export function isAuthenticated(s: Session | null): s is Session {
|
export function isAuthenticated(s: Session | null): s is Session {
|
||||||
return s !== null;
|
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<Session>;
|
||||||
|
return typeof parsed?.naam === 'string' ? { bsn: '', naam: parsed.naam } : null;
|
||||||
|
} catch {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -1,23 +1,16 @@
|
|||||||
import { Injectable, computed, effect, inject, signal } from '@angular/core';
|
import { Injectable, computed, effect, inject, signal } from '@angular/core';
|
||||||
import { Result } from '@shared/kernel/fp';
|
import { Result } from '@shared/kernel/fp';
|
||||||
import { Session } from '../domain/session';
|
import { Session, parseStoredSession } from '../domain/session';
|
||||||
import { DigidAdapter } from '../infrastructure/digid.adapter';
|
import { DigidAdapter } from '../infrastructure/digid.adapter';
|
||||||
|
|
||||||
const STORAGE_KEY = 'session-v1';
|
const STORAGE_KEY = 'session-v1';
|
||||||
|
|
||||||
/** Restore a persisted session (best-effort; corrupt entry → logged out).
|
/** Restore a persisted session (best-effort; corrupt entry → logged out).
|
||||||
G2: validate the shape before trusting it. G1: the BSN is never persisted
|
The parse + shape validation (G1/G2) lives in `parseStoredSession`
|
||||||
(see the effect below), so a restored session carries an empty one — it is
|
(`../domain/session`) — pure, spec'd, and testable without stubbing
|
||||||
unused after login; only `naam` is shown in the chrome. */
|
`localStorage`; this just supplies the raw value. */
|
||||||
function restore(): Session | null {
|
function restore(): Session | null {
|
||||||
try {
|
return parseStoredSession(localStorage.getItem(STORAGE_KEY));
|
||||||
const raw = localStorage.getItem(STORAGE_KEY);
|
|
||||||
if (!raw) return null;
|
|
||||||
const parsed = JSON.parse(raw) as Partial<Session>;
|
|
||||||
return typeof parsed?.naam === 'string' ? { bsn: '', naam: parsed.naam } : null;
|
|
||||||
} catch {
|
|
||||||
return null;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
import { describe, it, expect } from 'vitest';
|
import { describe, it, expect } from 'vitest';
|
||||||
import { isAuthenticated, Session } from './session';
|
import { isAuthenticated, parseStoredSession, Session } from './session';
|
||||||
|
|
||||||
const session: Session = { bsn: '19012345601', naam: 'Test' };
|
const session: Session = { bsn: '19012345601', naam: 'Test' };
|
||||||
|
|
||||||
@@ -12,3 +12,22 @@ describe('isAuthenticated', () => {
|
|||||||
expect(isAuthenticated(null)).toBe(false);
|
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' });
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -7,3 +7,21 @@ export interface Session {
|
|||||||
export function isAuthenticated(s: Session | null): s is Session {
|
export function isAuthenticated(s: Session | null): s is Session {
|
||||||
return s !== null;
|
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<Session>;
|
||||||
|
return typeof parsed?.naam === 'string' ? { bsn: '', naam: parsed.naam } : null;
|
||||||
|
} catch {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -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);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -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).
|
||||||
@@ -20,8 +20,8 @@ tested where._
|
|||||||
|
|
||||||
Every bullet below is a real test name from the suite — an `it()` title (frontend) or a test
|
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
|
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
|
**is** the suite, reshaped for a business reader. 415 frontend behaviours across
|
||||||
8 contexts; 228 backend behaviours across 38 test
|
9 contexts; 228 backend behaviours across 38 test
|
||||||
classes.
|
classes.
|
||||||
|
|
||||||
## Frontend (by context)
|
## Frontend (by context)
|
||||||
@@ -35,6 +35,17 @@ classes.
|
|||||||
- narrows a present session to Session
|
- narrows a present session to Session
|
||||||
- reports no session as not authenticated
|
- 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
|
### behandeling
|
||||||
|
|
||||||
#### besluit reduce
|
#### besluit reduce
|
||||||
@@ -781,6 +792,16 @@ classes.
|
|||||||
- clears the key once the wrapped fn settles
|
- clears the key once the wrapped fn settles
|
||||||
- falls back to a generated uuid-shaped key when none is pending
|
- 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
|
### showcase
|
||||||
|
|
||||||
#### highlightTs
|
#### highlightTs
|
||||||
|
|||||||
Reference in New Issue
Block a user