feat(backend): enforce the scholing threshold server-side (WP-69)

ADR-0001's own canonical "config value" example was unenforced: GET
/intake/policy echoed ScholingThreshold, but no request DTO carried a
scholing answer, so the server had nothing to re-validate. A crafted
POST could skip a requirement the wizard presents as mandatory.

IntakePolicy.RejectIncompleteScholing is the authority — three-valued
completeness (below threshold an answer is required; "nee" is legal and
still submits; punten only belong to a followed scholing), living in the
class that owns the constant so scripts/check-seam.sh keeps guarding the
FE/BE literal pair. Both submit paths call it; a violation 400s with
ProblemDetails and leaves the aanvraag a Concept. Gated on
Type == "intake" (the endpoint's switch lumps herregistratie with
intake, which has no scholing question), and guarded by `reject is null`
so a zero-uren submission is still decided on its merits.

Also fixes a live FE bug in the same rule: validateStep required punten
whenever scholingGevolgd was 'ja' regardless of lageUren, while the
template renders those fields only when lageUren — so answering 'ja'
then raising uren either blocked the user on an invisible field or
emitted aanvullendeScholing: undefined alongside punten. punten now
derives from aanvullendeScholing, so that combination is unrepresentable
in ValidIntake.

Note: EndpointTests' Worked_hours_submission_succeeds was itself
asserting the vulnerable payload ({ uren: 40 }, no answer) and needed a
complete answer added; the zero-hours rows are the ordering regression
net and are unmodified.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
eho
2026-08-18 22:42:14 +02:00
co-authored by Claude Sonnet 5
parent 9da385311d
commit 5d73ca21f6
16 changed files with 560 additions and 36 deletions
@@ -28,21 +28,179 @@ re-validates as authority) is unenforced for the one rule it was written to illu
- `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-sync` → `POST /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 herregistratie** —
`Program.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
Not yet made — this is a placeholder WP opened by WP-68, not a ready-to-implement one. Needs
a `planner` pass before work starts. Open questions to resolve then:
Made by a `planner` pass on 2026-08-18 — do not relitigate.
- The request DTOs need a scholing answer field (likely mirroring `intake.machine.ts`'s
`ValidIntake.aanvullendeScholing`/`punten`) — this is a wire change, so it touches
`contracts/`, the wizard's submit payload, and `npm run gen:api`.
- Whether to add the rule to `SubmissionRules` (alongside `RejectZeroUren`) or give
`IntakePolicy` its own `RejectMissingScholing(uren, scholing)`, matching the class that
already owns the threshold.
- Reading the answer out of the wizard's `Draft` JSON was rejected in WP-68 — the backend's
documented posture is that the draft is opaque (`AppDbContext`'s header comment) — so the
answer must arrive as an explicit request field, not be extracted from the opaque snapshot.
### 1. What the rule is (and deliberately is not)
## Out of scope (for now)
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:
Implementation — this WP exists to track the gap; do not implement without a Decisions block.
- `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 `Punten`** — `SubmitApplicationRequest` 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 `$localize`d):
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.