From 9c2a80451f2b217b41c2965d87f47790c2860286 Mon Sep 17 00:00:00 2001 From: Edwin van den Houdt Date: Sat, 27 Jun 2026 14:07:10 +0200 Subject: [PATCH] Step 3 (production-readiness): PII storage, validated reads, seams MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implement-now: - G1: keep PII out of persistent storage — never persist BSN (only `naam`); move both wizard drafts (address/email, work data) localStorage → sessionStorage so they clear on tab close. - G2: validate storage reads before trusting the cast — shape/tag guard in every restore() (mirrors the parse* HTTP boundary); corrupt/foreign shape → start fresh. - G3: already satisfied (debug-state redacts via mask.ts). Show-the-seam (hook + doc, not fully built): - G4: problemFieldErrors() maps a server validation envelope (ASP.NET ValidationProblemDetails `errors`) to the field-keyed map the wizards already render; returns {} until the backend sends it. +spec. - G5: documented the retry/backoff seam at the adapter GET loader; reads may retry, mutating submits never do. Out of scope (named): unsaved-changes warning (persistence prevents data loss), real auth/tokens, axe-core in CI. Gate green: lint, check:tokens, build, test 79/79. Co-Authored-By: Claude Opus 4.8 --- src/app/auth/application/session.store.ts | 12 +++++++--- .../intake-wizard/intake-wizard.component.ts | 16 ++++++++------ .../infrastructure/dashboard-view.adapter.ts | 6 +++++ .../registratie-wizard.component.ts | 16 ++++++++------ .../shared/infrastructure/api-error.spec.ts | 15 ++++++++++++- src/app/shared/infrastructure/api-error.ts | 22 +++++++++++++++++++ 6 files changed, 69 insertions(+), 18 deletions(-) diff --git a/src/app/auth/application/session.store.ts b/src/app/auth/application/session.store.ts index f5a540f..26080d0 100644 --- a/src/app/auth/application/session.store.ts +++ b/src/app/auth/application/session.store.ts @@ -5,11 +5,16 @@ import { DigidAdapter } from '../infrastructure/digid.adapter'; 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 + (see the effect below), so a restored session carries an empty one — it is + unused after login; only `naam` is shown in the chrome. */ function restore(): Session | null { try { const raw = sessionStorage.getItem(STORAGE_KEY); - return raw ? (JSON.parse(raw) as Session) : null; + if (!raw) return null; + const parsed = JSON.parse(raw) as Partial; + return typeof parsed?.naam === 'string' ? { bsn: '', naam: parsed.naam } : null; } catch { return null; } @@ -35,7 +40,8 @@ export class SessionStore { constructor() { effect(() => { const s = this._session(); - if (s) sessionStorage.setItem(STORAGE_KEY, JSON.stringify(s)); + // G1: persist only `naam` — never write the BSN (national ID) to storage. + if (s) sessionStorage.setItem(STORAGE_KEY, JSON.stringify({ naam: s.naam })); else sessionStorage.removeItem(STORAGE_KEY); }); } diff --git a/src/app/herregistratie/ui/intake-wizard/intake-wizard.component.ts b/src/app/herregistratie/ui/intake-wizard/intake-wizard.component.ts index 6eb8086..551be97 100644 --- a/src/app/herregistratie/ui/intake-wizard/intake-wizard.component.ts +++ b/src/app/herregistratie/ui/intake-wizard/intake-wizard.component.ts @@ -31,7 +31,7 @@ const STORAGE_KEY = 'intake-v3'; // ponytail: bump the suffix if the persisted s driven by the pure `reduce` (intake.machine.ts). Which step renders is derived from the answers via `visibleSteps`, never stored — so editing an earlier answer immediately changes the remaining steps. Answers are persisted to - localStorage so a page reload keeps the user's progress. */ + sessionStorage so a page reload keeps the user's progress (cleared on tab close). */ @Component({ selector: 'app-intake-wizard', imports: [FormsModule, FormFieldComponent, TextInputComponent, RadioGroupComponent, ButtonComponent, AlertComponent, DataRowComponent, WizardShellComponent], @@ -169,14 +169,15 @@ export class IntakeWizardComponent { protected set = (key: keyof Answers, value: string) => this.dispatch({ tag: 'SetAnswer', key, value }); constructor() { - // An explicit seed (stories) wins; otherwise resume from localStorage. + // An explicit seed (stories) wins; otherwise resume from sessionStorage. const seeded = this.seed(); queueMicrotask(() => this.dispatch({ tag: 'Seed', state: seeded !== initial ? seeded : (this.restore() ?? initial) })); - // Persist only while answering; clear once the flow is done. + // Persist only while answering; clear once the flow is done. G1: sessionStorage + // (not localStorage) — the draft holds personal data and must not outlive the tab. effect(() => { const s = this.state(); - if (s.tag === 'Answering') localStorage.setItem(STORAGE_KEY, JSON.stringify(s)); - else localStorage.removeItem(STORAGE_KEY); + if (s.tag === 'Answering') sessionStorage.setItem(STORAGE_KEY, JSON.stringify(s)); + else sessionStorage.removeItem(STORAGE_KEY); }); // Apply the server-owned threshold into machine state as it arrives. Track // only the policy value; untrack the dispatch (it reads the state signal @@ -188,10 +189,11 @@ export class IntakeWizardComponent { } private restore(): IntakeState | null { - const raw = localStorage.getItem(STORAGE_KEY); + const raw = sessionStorage.getItem(STORAGE_KEY); if (!raw) return null; try { - return JSON.parse(raw) as IntakeState; + const parsed = JSON.parse(raw) as IntakeState; + return parsed?.tag === 'Answering' ? parsed : null; // G2: only resume a known shape. } catch { return null; // ponytail: corrupt entry -> start fresh, no migration. } diff --git a/src/app/registratie/infrastructure/dashboard-view.adapter.ts b/src/app/registratie/infrastructure/dashboard-view.adapter.ts index aec6497..e85eb17 100644 --- a/src/app/registratie/infrastructure/dashboard-view.adapter.ts +++ b/src/app/registratie/infrastructure/dashboard-view.adapter.ts @@ -15,6 +15,12 @@ export class DashboardViewAdapter { // The value is still untrusted JSON — parseDashboardView validates it at the // boundary and maps DTO → domain before the app uses it. + // + // SEAM (G5): retry-with-backoff for non-mutating reads wraps the loader here — + // e.g. `loader: () => withBackoff(() => this.client.dashboardView())` — since the + // adapter is the single place HTTP lives. Reads are safe to retry; MUTATING calls + // (the submit-* commands) must NEVER auto-retry — and don't. Manual retry + // (resource.reload via ) covers the UX today, so backoff stays unbuilt. dashboardViewResource() { return resource({ loader: () => this.client.dashboardView() }); } diff --git a/src/app/registratie/ui/registratie-wizard/registratie-wizard.component.ts b/src/app/registratie/ui/registratie-wizard/registratie-wizard.component.ts index adf3b79..9d0a65c 100644 --- a/src/app/registratie/ui/registratie-wizard/registratie-wizard.component.ts +++ b/src/app/registratie/ui/registratie-wizard/registratie-wizard.component.ts @@ -41,7 +41,7 @@ const HANDMATIG = '__handmatig__'; // sentinel option: "my diploma isn't listed" the pure `reduce` (registratie-wizard.machine.ts). The BRP address prefills the draft via an effect; the DUO diploma list renders through ; choosing a diploma reveals its server-derived beroep. The draft is persisted to - localStorage so a reload keeps the user's progress. Built entirely from existing + sessionStorage so a reload keeps the user's progress (cleared on tab close). Built from existing atoms/molecules. */ @Component({ selector: 'app-registratie-wizard', @@ -294,14 +294,15 @@ export class RegistratieWizardComponent { } constructor() { - // An explicit seed (stories) wins; otherwise resume from localStorage. + // An explicit seed (stories) wins; otherwise resume from sessionStorage. const seeded = this.seed(); queueMicrotask(() => this.dispatch({ tag: 'Seed', state: seeded !== initial ? seeded : (this.restore() ?? initial) })); - // Persist only while filling in; clear once the flow is done. + // Persist only while filling in; clear once the flow is done. G1: sessionStorage + // (not localStorage) — the draft holds address/email and must not outlive the tab. effect(() => { const s = this.state(); - if (s.tag === 'Invullen') localStorage.setItem(STORAGE_KEY, JSON.stringify(s)); - else localStorage.removeItem(STORAGE_KEY); + if (s.tag === 'Invullen') sessionStorage.setItem(STORAGE_KEY, JSON.stringify(s)); + else sessionStorage.removeItem(STORAGE_KEY); }); // Prefill the address from the BRP lookup as it arrives. Track only the resource // value; untrack the dispatch (it reads the state signal, which would otherwise @@ -325,10 +326,11 @@ export class RegistratieWizardComponent { } private restore(): RegistratieState | null { - const raw = localStorage.getItem(STORAGE_KEY); + const raw = sessionStorage.getItem(STORAGE_KEY); if (!raw) return null; try { - return JSON.parse(raw) as RegistratieState; + const parsed = JSON.parse(raw) as RegistratieState; + return parsed?.tag === 'Invullen' ? parsed : null; // G2: only resume a known shape. } catch { return null; // ponytail: corrupt entry -> start fresh, no migration. } diff --git a/src/app/shared/infrastructure/api-error.spec.ts b/src/app/shared/infrastructure/api-error.spec.ts index 8572810..4193360 100644 --- a/src/app/shared/infrastructure/api-error.spec.ts +++ b/src/app/shared/infrastructure/api-error.spec.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { problemDetail } from './api-error'; +import { problemDetail, problemFieldErrors } from './api-error'; describe('problemDetail', () => { it('extracts the detail from an RFC-7807 ProblemDetails', () => { @@ -12,3 +12,16 @@ describe('problemDetail', () => { expect(problemDetail(undefined, 'fallback')).toBe('fallback'); }); }); + +describe('problemFieldErrors (G4 seam)', () => { + it('maps a ValidationProblemDetails errors dict to first-message-per-field', () => { + expect(problemFieldErrors({ errors: { straat: ['Verplicht.'], postcode: ['Ongeldig.', 'x'] } })) + .toEqual({ straat: 'Verplicht.', postcode: 'Ongeldig.' }); + }); + + it('returns {} when there is no errors envelope (the current backend shape)', () => { + expect(problemFieldErrors({ detail: 'one banner' })).toEqual({}); + expect(problemFieldErrors(new Error('boom'))).toEqual({}); + expect(problemFieldErrors(undefined)).toEqual({}); + }); +}); diff --git a/src/app/shared/infrastructure/api-error.ts b/src/app/shared/infrastructure/api-error.ts index f65dbb4..e8cf95b 100644 --- a/src/app/shared/infrastructure/api-error.ts +++ b/src/app/shared/infrastructure/api-error.ts @@ -12,3 +12,25 @@ export function problemDetail(e: unknown, fallback: string): string { } return fallback; } + +/** + * SEAM (G4): map a server validation envelope to field-level errors. + * + * ASP.NET's ValidationProblemDetails carries `errors: { field: string[] }`. The + * backend today returns only `detail` (one banner message), so this returns `{}`. + * When the backend starts sending `errors`, a machine's `SubmitFailed` handler can + * merge this into its own `errors` map — the field-keyed shape the wizards already + * render — so a rejection shows inline per field, not just as a banner. The + * consumer hook is the only thing left to wire; the contract boundary lives here. + */ +export function problemFieldErrors(e: unknown): Record { + if (!e || typeof e !== 'object' || !('errors' in e)) return {}; + const errors = (e as { errors?: unknown }).errors; + if (!errors || typeof errors !== 'object') return {}; + const out: Record = {}; + for (const [field, msgs] of Object.entries(errors as Record)) { + const first = Array.isArray(msgs) ? msgs[0] : msgs; + if (typeof first === 'string') out[field] = first; + } + return out; +}