Files
atomic-design-poc/docs/project/archive/backlog/WP-69-intake-scholing-threshold-enforcement.md
ehoandClaude Opus 5 12f17d9d73 docs: archive the finished backlogs (RD-30)
Two backlog trees are complete: `docs/project/backlog/` (75 files, every
WP done) and `docs/project/refactor-backlog-setup/` (the arc before it).
Move both under `docs/project/archive/` with `git mv`, so history stays
intact through `git log --follow`. `SHOWCASE-ROADMAP.md` moves with them,
because it points at the now-archived backlog README.

Add `docs/project/archive/README.md`. It states that these trees are
historical and names the two directories that are still live.

Repoint every inbound reference named in RD-30's Files table: CLAUDE.md,
the root README, both backend READMEs, `LetterHtml.cs`, `a11y.mdx`, the
`document-feature` and `new-ssp` skills, and the readable-codebase PLAN,
README, and RD-19 ticket. Fix two upward-relative links inside the moved
WP files (WP-68, WP-69) that gained a directory level and would otherwise
break. Repoint `.prettierignore`'s two agent-prompt exclusions to their
new path, so prettier keeps leaving those files' exact wording alone.

Mark RD-30 done and check off its acceptance criteria; flip its README
row to done.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-08 23:00:38 +02:00

14 KiB

WP-69 — Enforce the scholing threshold server-side

Status: done (5d73ca2) Phase: 12 — DDD hardening

Verification result (2026-08-18)

Backend 230 → 245 tests (+8 Domain/IntakeRuleTests, +7 Acceptance/IntakeSubmissionTests); frontend ssp 238 → 242. npm run ci green incl. check:seam (OK … (1000)), api-client and behaviour-spec drift.

The bypass was proven closed against a running backend, not just by green tests: POST /api/v1/intakes {"uren": 500} with no scholing fields → 400 with the Dutch detail; an intake-typed Concept submitted via POST /applications/{id}/submit {"uren": 500}400, and GET /applications/{id} afterwards still reports "tag":"Concept" (no state change, still retryable); the same request plus "aanvullendeScholing": false200 with a referentie and InBehandeling.

Deviation from the Files list: EndpointTests.Worked_hours_submission_succeeds had to be touched despite being on the do-not-modify list — it posts { uren: 40 } with no scholing answer, which is the crafted-POST bypass this WP closes, so the existing test was itself asserting the vulnerable behaviour. Fixed minimally by adding aanvullendeScholing = false (unknown to and ignored by HerregistratieRequest, so the paired /herregistraties row is unaffected). The zero-hours 422 rows — the ordering regression net — are unmodified as planned.

Why

WP-68 (F5) found that IntakePolicy's doc-comment claimed "the backend re-validates on submit as the authority" — it doesn't. GET /intake/policy only echoes ScholingThreshold; neither SubmitApplicationRequest (DiplomaHerkomst, Uren, Documents) nor IntakeRequest (Uren) carries a scholing answer at all, so there's nothing for the server to re-validate. Both submit paths apply only SubmissionRules.RejectZeroUren. A crafted POST — bypassing the wizard entirely — can skip the scholing requirement (scholingGevolgd/punten in intake.machine.ts) even though it's presented as mandatory in the UI. ADR-0001's canonical "config value" example (the FE applies the threshold for instant feedback, the backend re-validates as authority) is unenforced for the one rule it was written to illustrate.

Read first

  • backend/src/BigRegister.Api/Domain/Intake/IntakePolicy.cs (the corrected doc-comment, WP-68)
  • ADR-0001 — BFF-lite + decision DTOs §"config value"
  • apps/ssp/src/app/herregistratie/domain/intake.machine.ts (lageUren, scholingGevolgd, punten — the wizard's existing FE-side rule and its answers)
  • backend/src/BigRegister.Api/Contracts/Dtos.cs (SubmitApplicationRequest, IntakeRequest, DocumentRefDto)
  • backend/src/BigRegister.Api/Program.cs — the intakes and applications/{id}/submit endpoints

Stale premises in the original placeholder (verified 2026-08-18)

  • "Both submit paths" is half-stale. POST /api/v1/intakes is dead from the UI — the wizard submits via draft-syncPOST /applications/{id}/submit; no code in apps/ or libs/ calls the generated intakes()/herregistraties() methods. It is still a live crafted-POST surface, so fix both; do not delete it here (see Risks).
  • The submit endpoint does not distinguish intake from herregistratieProgram.cs lumps them: _ /* herregistratie | intake */ => (RejectZeroUren(...), true). herregistratie.machine.ts has no scholing question, so the new check must be gated on existing.Type == "intake" or the herregistratie wizard starts 400-ing for every low-uren user.
  • RejectMissingScholing(uren, scholing) is under-specified. The rule is three-valued (answer present / true needs punten / punten without true is illegal); two parameters cannot express it.
  • A live FE bug shares this rule and must be fixed here. intake.machine.ts:116 requires punten whenever scholingGevolgd === 'ja' regardless of lageUren, while the template renders both fields only inside @if (scholingZichtbaar()) (= lageUren). Answer scholing 'ja', then raise uren above the threshold: either the user is blocked by an error on an invisible field, or validateAll emits aanvullendeScholing: undefined together with punten: 150 — exactly the payload the new server rule rejects. Both branches reachable today.

Decisions

Made by a planner pass on 2026-08-18 — do not relitigate.

1. What the rule is (and deliberately is not)

The FE rule is completeness, not merit: below the threshold the scholing question must be answered; 'nee' is a legal answer that still submits. So the server authority is:

  • uren < IntakePolicy.ScholingThreshold ⇒ an answer must be present;
  • answer true ⇒ punten present and >= 0 (mirrors parseUren);
  • answer not true ⇒ punten must be absent.

Out of scope, deliberately: turning "few uren + no scholing" into an Afgewezen decision. The wizard accepts that today; inventing a substantive rejection would create a new FE/BE divergence in the WP that closes one. Boundary is <, not <= — mirrors lageUren.

2. Wire shape

Two nullable fields appended (positionally last, defaulted) to both request records in Contracts/Dtos.cs: bool? AanvullendeScholing = null, int? ScholingPunten = null.

  • ScholingPunten, not PuntenSubmitApplicationRequest is shared by all three wizard types and the herregistratie wizard has its own unrelated punten.
  • Illegal states are representable on the wire, unrepresentable past the boundary. A JSON DTO consumed by NSwag can't carry a union without hand-written polymorphism, and both fields must be optional for the other wizards anyway. Closure happens at the rule boundary — the same posture WP-68 took for AanvraagStatus. Rejected: a nested ScholingDto (removes one of three illegal combinations, adds a DTO); a closed ScholingAnswer type (one call site, not persisted — ceremony).
  • Not persisted. Submit-time rule input, not aggregate state: no Aanvraag column, no EF migration. The draft JSON stays opaque (WP-68) — the answer arrives as an explicit field.

3. Rule home — IntakePolicy, not SubmissionRules

public static string? RejectIncompleteScholing(int uren, bool? aanvullendeScholing, int? scholingPunten) — same "reason or null" idiom as SubmissionRules, so endpoints compose both identically.

The rule is the threshold's enforcement and the class already owns the constant. Putting it in SubmissionRules would either re-declare 1000 there (silent drift — exactly what WP-71's check:seam exists to catch, and which it would not catch outside IntakePolicy.cs) or make the generic cross-wizard class depend on one wizard's policy. SubmissionRules.cs and SubmissionRuleTests.cs are not modified.

check:seam constraint (load-bearing): scripts/check-seam.sh greps all ScholingThreshold\s*=\s*[0-9]+ matches in IntakePolicy.cs. The new code must reference the const (uren < ScholingThreshold, $"…{ScholingThreshold}…") and must never introduce a second literal (e.g. a default parameter int scholingThreshold = 1000) — a second match makes backend_value two lines and fails with a misleading "drift" message.

4. HTTP shape: 400 ProblemDetails, matching WP-68 F1

A missing/contradictory conditionally-required field is a contract violation, not a business outcome → Results.Problem(detail: …, statusCode: 400). Deliberately unlike RejectZeroUren, which is a merit rejection (422 legacy / Afgewezen + 200 on the aanvraag path).

Ordering: the zero-uren rejection wins. Guard with reject is null && so { uren: 0 } is decided on merit and completeness is moot — this keeps EndpointTests' 422 rows passing unmodified. Place the check before the document-ownership check and ApplicationStore.Submit, so a rejected submit leaves the aanvraag a Concept (retryable).

Gated on existing.Type == "intake". /applications/{id}/submit already declares .ProducesProblem(400) (WP-68 F1) — no metadata change; /intakes needs one added, with the check outside the Submit(...) helper so the 400 is not cached in IdempotencyStore.

Detail copy (Dutch, like all backend ProblemDetails — backend copy is not $localized): missing answer → $"Beantwoord de vraag over aanvullende scholing: bij minder dan {ScholingThreshold} gewerkte uren is dit verplicht."; true without punten → "Vul het aantal behaalde nascholingspunten in."; punten without true"Nascholingspunten horen alleen bij een gevolgde aanvullende scholing."

5. Backwards compatibility

Fields optional on the wire, conditionally required by the rule (the same DTO serves registratie and herregistratie, which never send them). In-flight Concept drafts (WP-22) are unaffected — the draft JSON already holds scholingGevolgd/punten, its format doesn't change, and the new FE derives the request fields at submit time. The one real incompatibility is a stale FE bundle submitting a below-threshold intake: it gets a 400 with an actionable Dutch detail via problemDetail(). Accepted — the POC has no API versioning, and the alternatives (grace period, inferring from the draft) are what WP-68 forbade. Rejected: a feature flag whose only purpose is to leave a security gap open.

6. Frontend changes

  • Wizard payload gains aanvullendeScholing + scholingPunten (undefined members are dropped by JSON.stringify and bind to null server-side).
  • intake.machine.ts needs two narrowing edits (see Stale premises — this is a live bug): validateStep('werk') requires punten only when lageUren(…) && scholingGevolgd === 'ja' (matching the template's @if), and validateAll computes punten from aanvullendeScholing === true rather than scholingGevolgd === 'ja', so a stale answer left by raising uren can't leak into ValidIntake. Answers (the raw record) is unchanged — stale raw answers are fine; ValidIntake, the parsed type, must be honest.
  • No change to SCHOLING_THRESHOLD_DEFAULT, lageUren, SetPolicy, the policy adapter/store, or the template. No new $localize id ⇒ no messages.en.xlf change.

7. Test plan (WP-71 conventions)

G/W/T bodies, Domain/<Aggregate>RuleTests.cs, Acceptance/, fixtures via the Given builder — never hand-built initializers.

New Domain/IntakeRuleTests.cs (pure rule, no HTTP; the arguments are the Given, so these degenerate to When/Then per bdd.mdx): answer required below threshold; not required at the threshold (pins < vs <=); niet gevolgd is a complete answer (pins §1's scope); gevolgd requires punten; zero punten valid; negative refused; [Theory] — punten without gevolgd refused (two rows, incl. the stale-punten shape §6 removes).

New Acceptance/IntakeSubmissionTests.cs (HTTP, both paths, Given.Concept(type: "intake")

  • a local Persist mirroring BesluitLifecycleTests; the builder's default owner is StubIdentityProvider's default caller, so no header juggling): below threshold without an answer → 400 and still a Concept; answered → 200; above threshold → 200; punten without gevolgd → 400; herregistratie unaffected (guards the Type gate); { uren: 0 } still Afgewezen + 200, not 400 (pins the ordering); legacy /intakes enforces it too.

Not modified: EndpointTests.cs (its 422 rows are the ordering regression net), SubmissionRuleTests.cs, ApplicationTests.cs, Builders/AanvraagBuilder.cs.

Frontend: intake.machine.spec.ts — drops punten when raising uren hides the question; does not require punten for a hidden question. intake.acceptance.spec.ts — one journey: low uren → 'ja' + punten → back → raise uren → submit → both fields undefined.

8. Sequencing

  1. IntakePolicy.RejectIncompleteScholing + Domain/IntakeRuleTests.cs (red→green, no wire change).
  2. Contracts/Dtos.cs + both endpoints + /intakes' .ProducesProblem(400).
  3. Acceptance/IntakeSubmissionTests.cs; dotnet test.
  4. npm run gen:api — after step 2, before the FE payload change. Commit backend/swagger.json
    • libs/shared/src/infrastructure/api-client.ts. CI's drift job fails if skipped/hand-edited.
  5. FE: intake.machine.ts narrowing + specs, then the wizard payload.
  6. npm run gen:snippets (expect no diff) and npm run gen:behaviour-spec (will diff — new test names; commit it or CI's drift step fails).
  7. Docs in the same diff: rewrite IntakePolicy's doc-comment from "gap deferred to WP-69" to what it now guarantees; one line in ADR-0001 §"config value"/worked example B; backend/README.md's /api/intakes row (add the 400) + its IntakePolicy.cs bullet.
  8. npm run ci.

Out of scope

  • Turning "few uren + no scholing" into an Afgewezen decision (§1) — a decision flag, an FE change, and a separate WP.
  • Deleting the dead /intakes + /herregistraties endpoints (with EndpointTests, backend/README.md, gen:api) — real cleanup, but not this WP's security fix.
  • The herregistratie wizard's jaren/punten, equally un-re-validated server-side.
  • docs/reference/fp-tea-atomic-design.md:587 / ARCHITECTURE.md:464 still teach a visibleSteps-with-a-'scholing'-step intake the fixed-3-step wizard no longer matches.

Risks

  • Ordering regression (highest). Running completeness before RejectZeroUren silently turns { uren: 0 } from 422/Afgewezen into 400 and breaks two existing endpoint tests. The reject is null && guard is load-bearing — keep the comment saying why.
  • check:seam false failure if a second ScholingThreshold = <digits> literal lands in IntakePolicy.cs (§3). The message will say "FE/BE seam drift" and mislead.
  • Missing the Type == "intake" gate breaks the herregistratie wizard for every low-uren user; the herregistratie is unaffected test is the only net.
  • Stale-bundle 400 loop: the wizard's Retry re-sends the identical payload, so a pre-deploy tab loops until reloaded. Acceptable for a POC.