From ee0d449510dd49d1e511e644ef8b1a83112dd042 Mon Sep 17 00:00:00 2001 From: Edwin van den Houdt Date: Thu, 27 Aug 2026 16:36:30 +0200 Subject: [PATCH 1/4] test(backend): assert every route is authz-gated (RB-12) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BL-006: the backend has zero automated architecture enforcement. BIO-016 names the concrete consequence for authorization — nothing asserted the *set* of gated endpoints, so BIO-003's X-Admin gate (outside Authz) and BIO-004's two ungated endpoints were caught only by a human reading Program.cs, not by CI. Adds RouteInventoryTests: walks the real app's EndpointDataSource and asserts every mapped route either carries a .Gate("XAdmin") metadata marker (added at the 16 call sites that already call one of the five admin wrappers — OrgAdmin/StamdataAdmin/CasesAdmin/Beoordelen/ FlagsAdmin) or appears in a written-down, reasoned allow-list. Proved it's hard to fool by adding a throwaway unguarded route, watching the test go red, and reverting. The allow-list is not "public routes" as the ticket's shorthand put it — 19 of its 31 entries are ownership-scoped inline (ctx.Zorgverlener()/ ctx.Caller()) endpoints, not public ones, and labelling them public would misrepresent the exact property BIO-004 was about. Each entry instead carries its own reason. Implementation note has the full route-by-route breakdown and judgement calls. Co-Authored-By: Claude Opus 5 --- backend/src/BigRegister.Api/Program.cs | 36 +++++ .../BigRegister.Tests/RouteInventoryTests.cs | 150 ++++++++++++++++++ .../refactor-backlog/implementation/rb-12.md | 115 ++++++++++++++ 3 files changed, 301 insertions(+) create mode 100644 backend/tests/BigRegister.Tests/RouteInventoryTests.cs create mode 100644 docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-12.md diff --git a/backend/src/BigRegister.Api/Program.cs b/backend/src/BigRegister.Api/Program.cs index e630dd6..2d605d1 100644 --- a/backend/src/BigRegister.Api/Program.cs +++ b/backend/src/BigRegister.Api/Program.cs @@ -188,6 +188,7 @@ api.MapGet("/intake/policy", () => new IntakePolicyDto(IntakePolicy.ScholingThre api.MapGet("/stamdata", (HttpContext ctx) => StamdataAdmin(ctx, () => Results.Ok(StamdataCatalog.All.Select(t => new StamdataTableSummaryDto(t.Id, t.Label, t.Columns.Select(ToColumnDto).ToList(), t.Temporal)).ToList()))) +.Gate("StamdataAdmin") .WithName("stamdataTables") .Produces>() .ProducesProblem(StatusCodes.Status403Forbidden); @@ -201,6 +202,7 @@ api.MapGet("/stamdata/{table}", (string table, string? peildatum, HttpContext ct var rows = peildatum is { Length: > 0 } p ? t.RowsOn(DateOnly.Parse(p)) : t.Rows(); return Results.Ok(new StamdataTableDto(t.Id, t.Label, t.Columns.Select(ToColumnDto).ToList(), t.Temporal, rows)); })) +.Gate("StamdataAdmin") .WithName("stamdataTable") .Produces() .ProducesProblem(StatusCodes.Status403Forbidden) @@ -298,6 +300,7 @@ api.MapDelete("/uploads/{documentId}", (string documentId, HttpContext ctx) => // unaudited; CasesAdmin gives it the missing AuthzAuditStore row for free (RB-07). api.MapDelete("/admin/uploads/{documentId}", (string documentId, HttpContext ctx) => CasesAdmin(ctx, () => DocumentStore.AdminDelete(documentId, "admin") ? Results.NoContent() : Results.NotFound())) +.Gate("CasesAdmin") .Produces(StatusCodes.Status204NoContent) .ProducesProblem(StatusCodes.Status403Forbidden) .Produces(StatusCodes.Status404NotFound); @@ -454,6 +457,7 @@ api.MapPost("/applications/{id}/submit", (string id, SubmitApplicationRequest re // --- Admin cases (WP-36): cross-owner list + admin delete, gated by `cases:manage`. --- api.MapGet("/admin/cases", (HttpContext ctx, IZaakSource zaken) => CasesAdmin(ctx, () => Results.Ok(zaken.ListCases(DateTimeOffset.UtcNow)))) +.Gate("CasesAdmin") .Produces>() .ProducesProblem(StatusCodes.Status403Forbidden); @@ -465,6 +469,7 @@ api.MapGet("/werkvoorraad", (HttpContext ctx, IZaakSource zaken) => Beoordelen(c Results.Ok(zaken.ListCases(DateTimeOffset.UtcNow) .Where(c => c.Status.Tag is "Ingediend" or "InBehandeling") .ToList()))) +.Gate("Beoordelen") .Produces>() .ProducesProblem(StatusCodes.Status403Forbidden); @@ -490,6 +495,7 @@ api.MapGet("/beoordeling/{id}", (string id, HttpContext ctx, IZaakSource zaken) var decisions = new BeoordelingDecisionsDto(canBesluiten); return Results.Ok(new BeoordelingViewDto(masked, docs, decisions)); })) +.Gate("Beoordelen") .Produces() .ProducesProblem(StatusCodes.Status403Forbidden) .Produces(StatusCodes.Status404NotFound); @@ -550,6 +556,7 @@ api.MapPost("/beoordeling/{id}/besluit", (string id, RecordBesluitRequest req, H return Results.Ok(new RecordBesluitResponse(updated!.ToStatusDto(now))); })) +.Gate("Beoordelen") .Produces() .ProducesProblem(StatusCodes.Status400BadRequest) .ProducesProblem(StatusCodes.Status403Forbidden) @@ -594,6 +601,7 @@ api.MapDelete("/admin/cases/{id}", (string id, HttpContext ctx) => CasesAdmin(ct app.Logger.LogInformation("admin case delete id={Id}", id); return Results.NoContent(); })) +.Gate("CasesAdmin") .Produces(StatusCodes.Status204NoContent) .Produces(StatusCodes.Status404NotFound) .ProducesProblem(StatusCodes.Status403Forbidden); @@ -604,6 +612,7 @@ api.MapGet("/admin/audit", (HttpContext ctx) => CasesAdmin(ctx, () => Results.Ok(AuthzAuditStore.List() .Select(a => new AuthzAuditDto(a.At.ToString("o"), a.Action, a.Resource, a.Decision, a.Role, a.CorrelationId)) .ToList()))) +.Gate("CasesAdmin") .Produces>() .ProducesProblem(StatusCodes.Status403Forbidden); @@ -629,6 +638,7 @@ api.MapGet("/flags", () => api.MapPut("/admin/flags/{key}", (string key, SetFeatureFlagRequest req, HttpContext ctx) => FlagsAdmin(ctx, $"feature-flags/{key}={req.Enabled}", () => FeatureFlagStore.Set(key, req.Enabled) ? Results.NoContent() : Results.NotFound())) +.Gate("FlagsAdmin") .Produces(StatusCodes.Status204NoContent) .Produces(StatusCodes.Status404NotFound) .ProducesProblem(StatusCodes.Status403Forbidden); @@ -748,6 +758,7 @@ api.MapGet("/admin/org-template/{subOrgId}/preview", (string subOrgId, HttpConte var fixture = BriefSeed.NewBrief("proefbrief"); return Results.Content(LetterHtml.Render(fixture, view.Draft, Now(), watermark: true), "text/html"); })) +.Gate("OrgAdmin") .ExcludeFromDescription(); api.MapPost("/brief/reset", (HttpContext ctx) => @@ -766,12 +777,14 @@ api.MapPost("/brief/reset", (HttpContext ctx) => api.MapGet("/admin/org-templates", (HttpContext ctx) => OrgAdmin(ctx, () => Results.Ok(OrgTemplateStore.List()))) +.Gate("OrgAdmin") .WithName("orgTemplates") .Produces>() .ProducesProblem(StatusCodes.Status403Forbidden); api.MapGet("/admin/org-template/{subOrgId}", (string subOrgId, HttpContext ctx) => OrgAdmin(ctx, () => OrgTemplateStore.AdminView(subOrgId) is { } view ? Results.Ok(view) : Results.NotFound())) +.Gate("OrgAdmin") .WithName("orgTemplateGET") .Produces() .ProducesProblem(StatusCodes.Status403Forbidden) @@ -783,6 +796,7 @@ api.MapPut("/admin/org-template/{subOrgId}", (string subOrgId, SaveOrgTemplateRe if (reject is not null) return Results.Problem(detail: reject, statusCode: StatusCodes.Status400BadRequest); return OrgTemplateStore.SaveDraft(subOrgId, req.Draft) is { } view ? Results.Ok(view) : Results.NotFound(); })) +.Gate("OrgAdmin") .WithName("orgTemplatePUT") .Produces() .ProducesProblem(StatusCodes.Status400BadRequest) @@ -797,6 +811,7 @@ api.MapPost("/admin/org-template/{subOrgId}/publish", (string subOrgId, HttpCont subOrgId, r.Version, r.AffectedUnsentBriefs); return r is not null ? Results.Ok(r) : Results.NotFound(); })) +.Gate("OrgAdmin") .WithName("orgTemplatePublish") .Produces() .ProducesProblem(StatusCodes.Status403Forbidden) @@ -804,6 +819,7 @@ api.MapPost("/admin/org-template/{subOrgId}/publish", (string subOrgId, HttpCont api.MapPost("/admin/org-template/{subOrgId}/rollback/{version:int}", (string subOrgId, int version, HttpContext ctx) => OrgAdmin(ctx, () => OrgTemplateStore.Rollback(subOrgId, version) is { } view ? Results.Ok(view) : Results.NotFound())) +.Gate("OrgAdmin") .WithName("orgTemplateRollback") .Produces() .ProducesProblem(StatusCodes.Status403Forbidden) @@ -988,5 +1004,25 @@ IResult Submit(HttpContext ctx, string kind, string? reject, IReadOnlyList(this TBuilder builder, string wrapper) + where TBuilder : IEndpointConventionBuilder + { + builder.WithMetadata(new AuthzGateMetadata(wrapper)); + return builder; + } +} + // Exposed so the integration tests can spin up the app with WebApplicationFactory. public partial class Program { } diff --git a/backend/tests/BigRegister.Tests/RouteInventoryTests.cs b/backend/tests/BigRegister.Tests/RouteInventoryTests.cs new file mode 100644 index 0000000..8c284cf --- /dev/null +++ b/backend/tests/BigRegister.Tests/RouteInventoryTests.cs @@ -0,0 +1,150 @@ +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Routing; +using Microsoft.Extensions.DependencyInjection; + +namespace BigRegister.Tests; + +/// RB-12/BIO-016 (BL-006 — "the backend has zero automated architecture enforcement"): the +/// only thing that used to keep an admin-shaped endpoint behind `Authz` was a human noticing +/// in review. BIO-003 (`X-Admin`, a second gate outside `Authz`) and BIO-004 (two endpoints +/// with no gate at all) are exactly the failure mode this test is a safety net for — and it is +/// the safety net RB-19 (a 900-line `Program.cs` reorder) leans on, so its value is entirely in +/// being hard to fool. +/// +/// Every mapped route must be accounted for exactly one of two ways: +/// - it carries an marker (.Gate("XAdmin"), added at the +/// call site in Program.cs) naming one of the five admin authz wrappers, or +/// - it is named, with a reason, in below. +/// +/// The allow-list is deliberately not "public routes" — most of its entries are NOT public. +/// `GET /applications/{id}` requires a caller identity and is scoped to that caller's own BSN +/// inline (`ctx.Zorgverlener()`), not through one of the five wrappers, which only gate the +/// coarse admin/behandelaar surfaces. Recording that here, with the actual reason, is the point +/// of BIO-016's remediation ("makes 'this endpoint is public' a decision someone wrote down") +/// generalised to every route that isn't wrapper-gated: the reviewer reads a name and a reason, +/// not silence. +public class RouteInventoryTests(TestWebApplicationFactory factory) : IClassFixture +{ + // Not TestWebApplicationFactory's HttpClient — this never issues a request, only reads the + // route table off the host's DI container. Uses the shared per-class isolated db file (see + // TestWebApplicationFactory's own doc comment) rather than a bare `new + // WebApplicationFactory()`, which would share the mutable static Db.ConnectionString + // with whatever other test class last set it and race "table already exists" against it. + private TestWebApplicationFactory Factory { get; } = factory; + + private sealed record AllowListEntry(string Method, string Pattern, string Reason); + + private static readonly AllowListEntry[] AllowList = + [ + // --- Orchestrator probes: no data, no PII, run before any identity concern applies. --- + new("GET", "/health", "Liveness probe for orchestrators."), + new("GET", "/health/ready", "Readiness probe for orchestrators."), + + // --- Static/reference demo data (SeedData & friends): identical for every caller in + // this POC (one seeded citizen), nothing to scope by. --- + new("GET", "/api/v1/dashboard-view", "Static reference data (SeedData) — same for every caller in this POC."), + new("GET", "/api/v1/notes", "Static reference data (SeedData.Notes) — same for every caller in this POC."), + new("GET", "/api/v1/brp/address", "Static BRP reference fixture — same for every caller in this POC."), + new("GET", "/api/v1/duo/diplomas", "Static DUO reference fixture + manual-diploma policy — same for every caller."), + new("GET", "/api/v1/intake/policy", "Config VALUE shipped for instant FE feedback (ADR-0001); the server re-validates as authority."), + new("GET", "/api/v1/uploads/categories", "Static per-wizard category config, no PII, no per-caller distinction."), + new("GET", "/api/v1/flags", "Feature-flag catalog + state, readable by any principal by design (WP-47) — only the PUT toggle is admin-gated."), + new("GET", "/api/v1/me", "Reflects only the ACTING caller's own role-derived capabilities — no other caller's data to leak."), + + // --- Citizen-submitted writes / ownership-scoped inline (ctx.Zorgverlener()/ctx.Caller()), + // not a role-only admin wrapper because the boundary is resource ownership, not a role. --- + new("POST", "/api/v1/change-requests", "Citizen submission; Submit() records outcome + idempotency, attributed to the acting caller."), + new("POST", "/api/v1/uploads", "Upload is attributed to ctx.Zorgverlener() as owner — there is no pre-existing resource to own yet."), + new("GET", "/api/v1/uploads/{documentId}/content", "Ownership-scoped inline (RB-01/BIO-004): owning citizen, or a behandelaar via Authz.CanBeoordelen."), + new("GET", "/api/v1/uploads/status", "Ownership-scoped inline: DocumentStore.ByLocalIds filtered to ctx.Zorgverlener().Bsn."), + new("DELETE", "/api/v1/uploads/{documentId}", "Ownership-scoped inline: DocumentStore.DeleteOwned keyed by ctx.Zorgverlener().Bsn."), + new("GET", "/api/v1/applications", "Ownership-scoped inline: IZaakSource.ListMyCases(ctx.Zorgverlener(), ...)."), + new("GET", "/api/v1/applications/{id}", "Ownership-scoped inline: ApplicationStore.Get(id, ctx.Zorgverlener().Bsn)."), + new("POST", "/api/v1/applications", "Ownership-scoped inline: created under ctx.Zorgverlener().Bsn."), + new("PUT", "/api/v1/applications/{id}", "Ownership-scoped inline: ApplicationStore.SyncDraft keyed by ctx.Zorgverlener().Bsn."), + new("DELETE", "/api/v1/applications/{id}", "Ownership-scoped inline: ApplicationStore.Get/.Delete keyed by ctx.Zorgverlener().Bsn."), + new("POST", "/api/v1/applications/{id}/submit", "Ownership-scoped inline: ApplicationStore.Submit keyed by ctx.Zorgverlener().Bsn."), + + // --- External caller, not a Principal at all. --- + new("POST", "/api/v1/zgw/notificaties", "OpenZaak's NRC, not a user: gated by a fixed-time shared-secret comparison, audited directly."), + + // --- Brief (letter composition): PRD-0002's own status-machine enforcement is the + // enforce/emit twin for this whole surface (Authz.CanActOn via BriefStore, ToView's + // Decisions dto) — a different single-source-of-truth than the five Program.cs wrappers, + // not a missing one. --- + new("GET", "/api/v1/brief", "Ownership-scoped inline: BriefStore.GetOrCreate(ctx.Zorgverlener().Bsn)."), + new("PUT", "/api/v1/brief", "Brief status-machine enforcement: BriefStore.Save + Authz.CanActOn (drafter-only)."), + new("POST", "/api/v1/brief/submit", "Brief status-machine enforcement: BriefStore.Submit + Authz.CanActOn."), + new("POST", "/api/v1/brief/approve", "Brief status-machine enforcement: BriefStore.Approve + Authz.CanActOn (approver != drafter)."), + new("POST", "/api/v1/brief/reject", "Brief status-machine enforcement: BriefStore.Reject + Authz.CanActOn."), + new("POST", "/api/v1/brief/send", "Brief status-machine enforcement: BriefStore.Send; not role-gated today, per the endpoint's own comment."), + new("POST", "/api/v1/brief/reveal-bignummer", "Own inline capability + step-up check (Authz.CanRevealBigNummer + X-Step-Up), audited directly."), + new("GET", "/api/v1/brief/preview", "Ownership-scoped inline: BriefStore.GetOrCreate(ctx.Zorgverlener().Bsn); hand-written FE fetch."), + new("POST", "/api/v1/brief/reset", "Deliberately unguarded demo affordance — the endpoint's own comment says so: 'showcase affordance only'."), + ]; + + private static readonly HashSet KnownWrappers = + ["OrgAdmin", "StamdataAdmin", "CasesAdmin", "Beoordelen", "FlagsAdmin"]; + + private static IEnumerable RealRoutes(EndpointDataSource source) => + source.Endpoints.OfType() + // MapGroup's own catch-all/description endpoints carry no HTTP method — not a route + // an HTTP client can actually hit distinctly, so not this test's concern. + .Where(e => e.Metadata.GetMetadata() is not null); + + private static string Key(string method, string pattern) => $"{method} {pattern}"; + + [Fact] + public void Every_mapped_route_is_authz_gated_or_on_the_named_allow_list() + { + var source = Factory.Services.GetRequiredService(); + + var allowed = AllowList.ToDictionary(e => Key(e.Method, e.Pattern)); + var seenAllowListKeys = new HashSet(); + var unaccounted = new List(); + + foreach (var route in RealRoutes(source)) + { + var pattern = route.RoutePattern.RawText!; + foreach (var method in route.Metadata.GetMetadata()!.HttpMethods) + { + var key = Key(method, pattern); + var gated = route.Metadata.GetMetadata() is { } gate && KnownWrappers.Contains(gate.Wrapper); + var listed = allowed.ContainsKey(key); + if (listed) seenAllowListKeys.Add(key); + if (!gated && !listed) unaccounted.Add(key); + } + } + + Assert.True(unaccounted.Count == 0, + "Route(s) with no authz gate and no allow-list entry — either add `.Gate(\"XAdmin\")` " + + "at the mapping site, or add a named, reasoned entry to RouteInventoryTests.AllowList:\n" + + string.Join("\n", unaccounted)); + + // The allow-list is a decision log, not a wishlist — an entry for a route that no longer + // exists (renamed, removed) is exactly the kind of drift this test exists to catch. + var stale = allowed.Keys.Except(seenAllowListKeys).ToList(); + Assert.True(stale.Count == 0, + "Allow-list entry with no matching live route (stale — the route was renamed or " + + "removed):\n" + string.Join("\n", stale)); + } + + /// Every `.Gate(...)` call must name one of the five known wrappers — a typo here would + /// silently fall back to "unaccounted for" above, but pinning it down explicitly gives a + /// clearer failure than the generic route-mismatch message. + [Fact] + public void Every_gate_marker_names_a_known_admin_wrapper() + { + var source = Factory.Services.GetRequiredService(); + + var unknown = RealRoutes(source) + .Select(r => r.Metadata.GetMetadata()) + .Where(g => g is not null) + .Select(g => g!.Wrapper) + .Where(w => !KnownWrappers.Contains(w)) + .Distinct() + .ToList(); + + Assert.True(unknown.Count == 0, "Unknown wrapper name(s) in a .Gate(...) call: " + string.Join(", ", unknown)); + } +} diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-12.md b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-12.md new file mode 100644 index 0000000..7b4d06b --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-12.md @@ -0,0 +1,115 @@ +# RB-12 — a route-table test: every route hits an authz wrapper or an explicit allow-list + +Status: **implemented** · 2026-08-27 · Source findings: `07-bio2-compliance.md` BIO-016, `00-baseline.md` BL-006 · `99-backlog.md` RB-12 + +## What was wrong + +BL-006, verbatim: "the backend has zero automated architecture enforcement … `Domain/` +purity currently holds by convention." BIO-016 names the specific consequence for +authorization: nothing asserted the **set** of gated endpoints, so an endpoint added +without a gate (BIO-003's `X-Admin` gate outside `Authz`, BIO-004's two endpoints with +no gate at all) failed no test. Both were caught by a human reading `Program.cs`, not by +CI. + +## What changed + +| File | Change | +| ------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `Program.cs` — 16 endpoint mappings | each chains a new `.Gate("XAdmin")` call, naming the admin wrapper (`OrgAdmin`, `StamdataAdmin`, `CasesAdmin`, `Beoordelen`, `FlagsAdmin`) already used inside its handler | +| `Program.cs` — new types, end of file | `public sealed record AuthzGateMetadata(string Wrapper)` + a `Gate(...)` extension method on `IEndpointConventionBuilder` that attaches it via `.WithMetadata(...)` | +| `tests/BigRegister.Tests/RouteInventoryTests.cs` | **new** — walks the real app's `EndpointDataSource`, asserts every route carries either an `AuthzGateMetadata` naming a known wrapper, or an entry in a written-down allow-list; a second test asserts every `.Gate(...)` name is one of the five known wrappers | + +## Design: metadata at mapping time, not reflection over the compiled lambda + +The ticket left the detection mechanism open, noting the wrappers are local functions +in `Program.cs`. Reflecting over a compiled minimal-API lambda to determine which local +function its closure calls is fragile-to-impossible (the call is inside IL a test would +have to disassemble, and a local function's identity isn't easily recoverable from the +delegate's `MethodInfo`). Endpoint **metadata**, attached at the same call site where the +route is mapped, is exactly what `EndpointDataSource` hands back to a test host and +doesn't depend on inspecting compiled code at all — so a `.Gate("XAdmin")` extension +method was added and chained onto each of the 16 mappings that call one of the five +wrappers. + +This is a **declaration**, not a **derivation**: the test does not verify that +`.Gate("CasesAdmin")` and an actual `CasesAdmin(ctx, …)` call inside the handler agree — +it only verifies that a marker is present. A handler that swapped its `CasesAdmin(ctx, +…)` call for a no-op without updating `.Gate(...)` would go undetected here. What _is_ +caught, reliably, is the actual BIO-003/BIO-004 failure mode: a new endpoint mapped with +**no** marker and **no** allow-list entry — verified below by adding one and watching the +test go red. + +## Judgement call: the allow-list is not "public routes" + +The ticket's literal framing — every route "goes through one of the authz wrappers … +or appears in an explicit, named allow-list of deliberately-public routes" — doesn't fit +this codebase as read. Only 16 of the app's 47 routes go through one of the five admin +wrappers. The other 31 are not uniformly public: + +- **10 are genuinely public** — orchestrator health probes and static/reference demo + data (`SeedData`, the DUO/BRP fixtures, the scholing-threshold config value, the + feature-flag catalog, `/me`'s reflection of the caller's own capabilities) that reads + the same for every caller in this one-seeded-citizen POC. +- **19 are ownership-scoped inline**, not public and not wrapper-gated: `GET +/applications/{id}`, the upload endpoints, every brief transition, etc. all key off + `ctx.Zorgverlener().Bsn` / `ctx.Caller()` — an authenticated citizen (or, for the + uploads-content endpoint, a behandelaar) reading or writing only their own resource. + Calling these "public" in an allow-list would misrepresent exactly the property + BIO-004 was about — object-level authorization existing at all. +- **1 (`POST /zgw/notificaties`) uses a different mechanism entirely** — a fixed-time + shared-secret comparison for a non-Principal external caller (OpenZaak's + notifications), audited the same way but never going through `Authz`. +- **1 (`POST /brief/reset`) is deliberately, literally unguarded** — the endpoint's own + pre-existing comment says so ("No guards — showcase affordance only"). + +The allow-list (`RouteInventoryTests.AllowList`) keeps all 31 as one array for the test's +sake, but every entry carries its own reason string rather than a blanket "public" label — +preserving BIO-016's actual intent ("makes 'this endpoint is public' a decision someone +wrote down rather than an omission") generalised to "this endpoint's access boundary is +_X_, deliberately," which is true of all 31 and false of "public" for 20 of them. This is +recorded here rather than silently reinterpreted, per this task's brief: implementing the +literal "public" framing would have been actively misleading about which endpoints have no +access control at all. + +## Other judgement calls + +- **`AuthzGateMetadata` and its extension method are `public`, not `internal`.** The test + project has no `InternalsVisibleTo` wired up for `BigRegister.Api` (checked — none + exists anywhere in `backend/`), and adding one for a single marker type was more + machinery than the alternative. Both types carry a comment stating why. +- **A second test (`Every_gate_marker_names_a_known_admin_wrapper`) guards against a typo + in a `.Gate(...)` call.** Without it, a call like `.Gate("CasesAdmn")` would just fall + through to "unaccounted for" in the main test with a less specific failure message — + fine, but a dedicated assertion names the actual mistake. +- **The main test also asserts the reverse direction: no stale allow-list entries.** An + allow-list entry for a route that was renamed or removed is exactly the kind of drift + a "decision someone wrote down" ledger needs to catch, not just silently keep. Verified + this fires: temporarily added one extra `AllowList` entry for a route that doesn't + exist (via Edit, not committed) — every real route was still covered, so only the + stale-entry assertion tripped, naming exactly that bogus entry. Reverted the same way. +- **`RouteInventoryTests` uses the house `TestWebApplicationFactory` + `IClassFixture` + idiom**, not a bare `new WebApplicationFactory()` per test. The first draft did + the latter and immediately hit `SQLite Error 1: 'table "Applications" already exists'` + — `Db.ConnectionString` (`Data/Db.cs`) is a shared mutable **static** field, and a bare + factory doesn't override `ConnectionStrings:AppDb`, so two such factories in the same + class end up pointed at the same file, and the second one's `Migrate()` collides with + the first's already-created tables (the first factory's default `Dispose()` doesn't + delete that file — only `TestWebApplicationFactory`'s override does, to its own + per-instance temp path). This is exactly the hazard `TestWebApplicationFactory`'s own + doc comment describes; switching to it (as every other endpoint-test class in this + suite already does) fixed it outright — no product code involved, purely a test-fixture + choice. + +## Verification + +- **Proved the test is hard to fool**, per the ticket's explicit ask: added a throwaway + `api.MapDelete("/rb12-throwaway-unguarded/{id}", …)` with no `.Gate(...)` and no + allow-list entry (via Edit, not `git checkout`) — `Every_mapped_route_is_authz_gated_or_ +on_the_named_allow_list` failed red, naming exactly that route. Reverted the same way; + re-ran green. Repeated once more after switching to `TestWebApplicationFactory` to + confirm the fixture change didn't weaken the check — same red, same green. +- `dotnet build` (both `src/BigRegister.Api` and `tests/BigRegister.Tests`): clean, 0 + warnings. +- `dotnet format BigRegister.slnx --verify-no-changes`: clean. +- `dotnet test --filter "Category!=Integration"`: **257 passed, 0 failed** (255 + pre-existing + 2 new). From a93218e8ac1a2c5893c0a07d7a31b4833a0df395 Mon Sep 17 00:00:00 2001 From: Edwin van den Houdt Date: Thu, 27 Aug 2026 16:40:23 +0200 Subject: [PATCH 2/4] fix(backend): gate Swagger + the OpenAPI doc behind IsDevelopment (RB-15) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BIO-015: app.UseSwagger()/app.UseSwaggerUI() ran unconditionally, so the full OpenAPI document (every route + request/response shape) and SwaggerUI's interactive "Try it out" were reachable in every environment, including a real deployment. Both now run only inside `if (app.Environment.IsDevelopment())`. AddSwaggerGen/AddEndpointsApiExplorer stay unconditional — DI registration only, no HTTP surface by itself. RB-09 already made a non-Development environment throw at startup, which broke `npm run gen:api` until that script pinned ASPNETCORE_ENVIRONMENT=Development for its one CLI invocation. This change sits in the same pipeline, so it was verified rather than assumed: `dotnet swagger tofile` resolves ISwaggerProvider straight out of DI and never sends an HTTP request through this middleware, so gating it can't affect that tool by construction. Ran the real `npm run gen:api` to confirm — exit 0, regenerated files byte-identical to what's committed. New tests exercise the gate on a third ("Staging") environment name, not Production — Production already can't boot at all post-RB-09, so a Production-environment test would only re-prove that unrelated startup throw, not this gate. Co-Authored-By: Claude Opus 5 --- backend/src/BigRegister.Api/Program.cs | 17 +++- .../BigRegister.Tests/SwaggerGateTests.cs | 48 ++++++++++ .../refactor-backlog/implementation/rb-15.md | 93 +++++++++++++++++++ 3 files changed, 156 insertions(+), 2 deletions(-) create mode 100644 backend/tests/BigRegister.Tests/SwaggerGateTests.cs create mode 100644 docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-15.md diff --git a/backend/src/BigRegister.Api/Program.cs b/backend/src/BigRegister.Api/Program.cs index 2d605d1..208be87 100644 --- a/backend/src/BigRegister.Api/Program.cs +++ b/backend/src/BigRegister.Api/Program.cs @@ -142,8 +142,21 @@ app.Use(async (ctx, next) => await next(ctx); }); -app.UseSwagger(); -app.UseSwaggerUI(); +// RB-15/BIO-015: the OpenAPI document + its UI are a genuine attack-surface reduction to +// gate — they enumerate every route, request/response shape and (via SwaggerUI's "Try it +// out") let a caller fire requests straight from the browser. Development-only, like the +// dev-role/scenario-toggle hatches this POC already keeps out of production builds +// (docker-compose.prod.yml runs Production; only docker-compose.yml's dev image runs +// Development). `dotnet swagger tofile` (npm run gen:api) is unaffected: Swashbuckle's CLI +// resolves ISwaggerProvider straight out of the DI container to build swagger.json — it +// never sends an HTTP request through this pipeline, so it never touches this middleware at +// all, gated or not. Verified empirically (see rb-15.md) rather than assumed, per RB-09's +// note that this exact file has already broken that tool once. +if (app.Environment.IsDevelopment()) +{ + app.UseSwagger(); + app.UseSwaggerUI(); +} app.UseCors(SpaCors); // Liveness/readiness for orchestrators (k8s probes, load balancers). No data, no PII. diff --git a/backend/tests/BigRegister.Tests/SwaggerGateTests.cs b/backend/tests/BigRegister.Tests/SwaggerGateTests.cs new file mode 100644 index 0000000..90fb36a --- /dev/null +++ b/backend/tests/BigRegister.Tests/SwaggerGateTests.cs @@ -0,0 +1,48 @@ +using System.Net; +using BigRegister.Domain.Authorization; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.DependencyInjection; + +namespace BigRegister.Tests; + +/// RB-15/BIO-015: `app.UseSwagger()`/`app.UseSwaggerUI()` used to run unconditionally — the +/// OpenAPI document (every route + request/response shape) and SwaggerUI's "Try it out" were +/// reachable in every environment, including a real deployment. Both are now gated behind +/// `app.Environment.IsDevelopment()`. +public class SwaggerGateTests(TestWebApplicationFactory factory) : IClassFixture +{ + [Fact] + public async Task Swagger_document_is_served_in_development() + { + // The default test environment (WebApplicationFactory defaults to "Development" when + // nothing overrides it — same fact RB-09's implementation note relies on) — this is the + // regression guard that the gate didn't also break the documented `npm run gen:api` / + // local-dev-Swagger-UI experience. + var res = await factory.CreateClient().GetAsync("/swagger/v1/swagger.json"); + Assert.Equal(HttpStatusCode.OK, res.StatusCode); + } + + /// Production cannot boot at all today (RB-09: no real IIdentityProvider exists yet), which + /// is a *stronger* guarantee than "no Swagger in Production" — but it also means a plain + /// `UseEnvironment("Production")` host never reaches this middleware to prove the gate + /// itself works, only that the whole app refuses to start. This uses a third environment + /// name (neither "Development" nor "Production") with a test-supplied `IIdentityProvider` — + /// the one thing Program.cs doesn't register outside those two branches — so the host + /// actually boots and this test exercises the real gate, not RB-09's unrelated startup throw. + [Fact] + public async Task Swagger_document_is_not_served_outside_development() + { + // Built on top of the shared `factory` fixture (via WithWebHostBuilder), not a bare `new + // WebApplicationFactory()` — that keeps this host on the fixture's own per-class + // isolated AppDb temp path (see TestWebApplicationFactory's doc comment; RB-12's + // implementation note records the "table already exists" collision a bare factory hits + // by sharing the mutable static Db.ConnectionString instead). + using var staging = factory.WithWebHostBuilder(builder => builder + .UseEnvironment("Staging") + .ConfigureTestServices(services => services.AddSingleton())); + + var res = await staging.CreateClient().GetAsync("/swagger/v1/swagger.json"); + Assert.Equal(HttpStatusCode.NotFound, res.StatusCode); + } +} diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-15.md b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-15.md new file mode 100644 index 0000000..a555e25 --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-15.md @@ -0,0 +1,93 @@ +# RB-15 — Swagger and the OpenAPI document behind `IsDevelopment()` + +Status: **implemented** · 2026-08-27 · Source findings: `07-bio2-compliance.md` BIO-015 · `99-backlog.md` RB-15 + +## What was wrong + +`Program.cs:145-146` (pre-change) ran `app.UseSwagger(); app.UseSwaggerUI();` +unconditionally — the full OpenAPI document (every route, every request/response shape) +and SwaggerUI's interactive "Try it out" were reachable in every environment, including a +real deployment, with no `app.Environment.IsDevelopment()` guard. BIO-015's own evidence +notes this is one line of genuine attack-surface reduction with no POC cost. + +## What changed + +| File | Change | +| --------------------------------------------- | -------------------------------------------------------------------------------------------------------- | +| `Program.cs` | `app.UseSwagger(); app.UseSwaggerUI();` now run only inside `if (app.Environment.IsDevelopment()) { … }` | +| `tests/BigRegister.Tests/SwaggerGateTests.cs` | **new** — asserts `/swagger/v1/swagger.json` is served in Development and 404s outside it | + +`builder.Services.AddSwaggerGen(...)` and `AddEndpointsApiExplorer()` were left +unconditional — they only register DI services (the swagger-generation machinery), +expose nothing over HTTP by themselves, and (see below) are exactly what `npm run +gen:api` depends on staying registered in every environment it might run against. + +## The hazard, checked rather than assumed + +RB-09 made a non-Development environment throw during `builder.Build()` (no +`IIdentityProvider` registered for a bare/unset environment, which defaults to +Production), which crashed `dotnet swagger tofile` until `package.json`'s `gen:api` +script was pinned to `ASPNETCORE_ENVIRONMENT=Development` for that one invocation +(`docs/.../implementation/rb-09.md`). This ticket's change sits in exactly the same +pipeline, so it needed the same empirical check, not an assumption. + +**Mechanism, confirmed by reading Swashbuckle's CLI behaviour and then proving it:** +`dotnet swagger tofile` (`Swashbuckle.AspNetCore.Cli`) loads the built DLL through +.NET's design-time `HostFactoryResolver`, builds the host, and then resolves +`ISwaggerProvider` **directly out of the DI container** to produce `swagger.json` — it +never issues an HTTP request through the ASP.NET Core middleware pipeline this ticket's +`if (app.Environment.IsDevelopment())` guard lives in. Gating `UseSwagger()`/ +`UseSwaggerUI()` therefore cannot affect it, in any environment, by construction — those +are pipeline middleware; the CLI tool bypasses the pipeline entirely. + +**Verified, not assumed:** ran `npm run gen:api` for real. It exited 0, printed "Swagger +JSON/YAML successfully written to …/backend/swagger.json", and regenerated the NSwag +client. `git status`/`git diff` on both `backend/swagger.json` and +`libs/shared/src/infrastructure/api-client.ts` showed **zero changes** — the regenerated +files are byte-identical to what's already committed, confirming the gate has no effect +on the generated contract at all. + +## Judgement calls + +- **The guard wraps both `UseSwagger()` and `UseSwaggerUI()` together**, not just one — + the ticket's own wording lists both, and gating only the document while leaving the UI + reachable (or vice versa) would be a strange half-measure: SwaggerUI without the + document 404s on load anyway, and the document without the UI still leaks the same + route/shape enumeration BIO-015 is about. +- **`AddSwaggerGen`/`AddEndpointsApiExplorer` were left unconditional.** They're + DI-registration-time calls with no HTTP surface, and — now confirmed rather than + assumed — `dotnet swagger tofile` needs `ISwaggerProvider` registered in whatever + environment it runs the host under (pinned to Development by `gen:api`'s own script, + but nothing stops a future non-Development invocation), so conditioning those + registrations on `IsDevelopment()` would risk breaking the CLI tool for no + attack-surface benefit — nobody can reach a DI-registered-but-never-routed service + over HTTP. +- **New tests build the "non-Development" case on a third environment name + ("Staging"), not `"Production"`.** RB-09 already made Production fail at startup + entirely (no real `IIdentityProvider` exists yet) — a stronger guarantee than "no + Swagger in Production," but one that means a `UseEnvironment("Production")` host + never reaches this middleware to prove the gate itself works; it only proves RB-09's + unrelated startup throw, which already has its own test. A `"Staging"` environment + satisfies neither `IsDevelopment()` nor `IsProduction()`, so `Program.cs` registers no + `IIdentityProvider` for it — the test supplies one via `ConfigureTestServices` + (`StubIdentityProvider`, the same one Development uses) so the host actually boots, + and the test exercises this ticket's real gate rather than a different ticket's. +- **The Staging host is built via `factory.WithWebHostBuilder(...)`** (layering on the + shared `TestWebApplicationFactory` fixture), not a bare `new +WebApplicationFactory()` — RB-12's implementation note already records the + "table already exists" collision a bare factory hits by sharing the mutable static + `Db.ConnectionString` instead of the fixture's own per-class isolated temp path; + layering avoids repeating that mistake here. + +## Verification + +- **Reverted the guard only** (unwrapped `UseSwagger()`/`UseSwaggerUI()` back to + unconditional, via Edit, tests left in place) and ran `SwaggerGateTests`: + `Swagger_document_is_not_served_outside_development` failed red (`Expected: NotFound, +Actual: OK`). Restored the fix (via Edit) and re-ran: both green. +- **`npm run gen:api`, run for real**: exit 0; `backend/swagger.json` and + `libs/shared/src/infrastructure/api-client.ts` both unchanged (`git status` clean on + both) — see "The hazard" above. +- `dotnet build` (both projects): clean, 0 warnings. +- `dotnet format BigRegister.slnx --verify-no-changes`: clean. +- `dotnet test --filter "Category!=Integration"`: **259 passed, 0 failed** (257 + 2 new). From 2627799284a83cf254a31201351c6227c25f90d0 Mon Sep 17 00:00:00 2001 From: Edwin van den Houdt Date: Thu, 27 Aug 2026 16:53:29 +0200 Subject: [PATCH 3/4] fix(backend): 400 instead of 500 on an unparseable peildatum (RB-16) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BIO-019: GET /stamdata/{table}?peildatum= called DateOnly.Parse directly, which throws FormatException on anything unparseable — an unhandled 500 (leaking exception detail in Development) instead of the 400-with-problem-details every other bad-input check in this file returns. §3c named backend/Stamdata's 71.7% branch coverage (BL-005) as the weak spot this bug lived in. Switched to DateOnly.TryParse; an unparseable value now returns Results.Problem(detail: ..., statusCode: 400), matching the shape the upload/change-request endpoints already use. Endpoint doc gained .ProducesProblem(400), so the OpenAPI doc + generated client were regenerated and committed in this same diff (RB-09's note records a prior incident where a response-shape change shipped without this and the drift went unnoticed). No FE change needed: libs/beheer's stamdata adapter already funnels every call through runSubmit, which folds any thrown ApiException (now including this 400) into a generic Result error — ADR-0001's "the FE renders the decision" already covers "the server rejected this input". Co-Authored-By: Claude Opus 5 --- backend/src/BigRegister.Api/Program.cs | 14 +++- backend/swagger.json | 10 +++ .../StamdataEndpointTests.cs | 10 +++ .../refactor-backlog/implementation/rb-16.md | 82 +++++++++++++++++++ libs/shared/src/infrastructure/api-client.ts | 6 ++ 5 files changed, 121 insertions(+), 1 deletion(-) create mode 100644 docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-16.md diff --git a/backend/src/BigRegister.Api/Program.cs b/backend/src/BigRegister.Api/Program.cs index 208be87..c80d2fe 100644 --- a/backend/src/BigRegister.Api/Program.cs +++ b/backend/src/BigRegister.Api/Program.cs @@ -212,12 +212,24 @@ api.MapGet("/stamdata/{table}", (string table, string? peildatum, HttpContext ct { var t = StamdataCatalog.Find(table); if (t is null) return Results.NotFound(); - var rows = peildatum is { Length: > 0 } p ? t.RowsOn(DateOnly.Parse(p)) : t.Rows(); + DateOnly? peildatumWaarde = null; + // RB-16/BIO-019: DateOnly.Parse threw FormatException on unparseable input, surfacing as + // an unhandled 500 (and, in Development, an exception detail leaked to the caller) — an + // admin-gated but still user-supplied string needs the same 400 path every other bad-input + // check in this file uses, not a crash. + if (peildatum is { Length: > 0 } p) + { + if (!DateOnly.TryParse(p, out var parsed)) + return Results.Problem(detail: $"Ongeldige peildatum '{p}'.", statusCode: StatusCodes.Status400BadRequest); + peildatumWaarde = parsed; + } + var rows = peildatumWaarde is { } d ? t.RowsOn(d) : t.Rows(); return Results.Ok(new StamdataTableDto(t.Id, t.Label, t.Columns.Select(ToColumnDto).ToList(), t.Temporal, rows)); })) .Gate("StamdataAdmin") .WithName("stamdataTable") .Produces() +.ProducesProblem(StatusCodes.Status400BadRequest) .ProducesProblem(StatusCodes.Status403Forbidden) .Produces(StatusCodes.Status404NotFound); diff --git a/backend/swagger.json b/backend/swagger.json index dbad12d..1a0dad1 100644 --- a/backend/swagger.json +++ b/backend/swagger.json @@ -194,6 +194,16 @@ } } }, + "400": { + "description": "Bad Request", + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "403": { "description": "Forbidden", "content": { diff --git a/backend/tests/BigRegister.Tests/StamdataEndpointTests.cs b/backend/tests/BigRegister.Tests/StamdataEndpointTests.cs index ac197ae..a676cc1 100644 --- a/backend/tests/BigRegister.Tests/StamdataEndpointTests.cs +++ b/backend/tests/BigRegister.Tests/StamdataEndpointTests.cs @@ -59,6 +59,16 @@ public class StamdataEndpointTests(TestWebApplicationFactory factory) : IClassFi Assert.Empty(table.Rows); } + /// RB-16/BIO-019: DateOnly.Parse used to throw FormatException on unparseable input, + /// surfacing as an unhandled 500 instead of the 400-with-problem-details every other + /// bad-input check in this endpoint file returns. + [Fact] + public async Task Unparseable_peildatum_is_400_not_500() + { + var res = await _client.SendAsync(Req(HttpMethod.Get, "/api/v1/stamdata/professions?peildatum=not-a-date", role: "admin")); + Assert.Equal(HttpStatusCode.BadRequest, res.StatusCode); + } + [Fact] public async Task Unknown_table_is_404() { diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-16.md b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-16.md new file mode 100644 index 0000000..557c4ff --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-16.md @@ -0,0 +1,82 @@ +# RB-16 — `DateOnly.TryParse` on `?peildatum=` + +Status: **implemented** · 2026-08-27 · Source findings: `07-bio2-compliance.md` BIO-019 · `99-backlog.md` RB-16 + +## What was wrong + +`Program.cs:215` (pre-change) — +`var rows = peildatum is { Length: > 0 } p ? t.RowsOn(DateOnly.Parse(p)) : t.Rows();`. +`DateOnly.Parse` throws `FormatException` on anything unparseable; there was no +`TryParse`, no 400 path, and `.Produces` on the endpoint declared only 200/403/404 — so +an unparseable `?peildatum=` value 500'd, and in Development the exception detail was +returned to the caller. §3c's baseline named `backend/Stamdata` 96.8% line but **71.7% +branch** (BL-005) — this was one of the unentered branches. + +## What changed + +| File | Change | +| ---------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `Program.cs` — `GET /stamdata/{table}` | `DateOnly.Parse` replaced with `DateOnly.TryParse`; an unparseable value now returns `Results.Problem(detail: …, statusCode: 400)` instead of throwing; endpoint doc gained `.ProducesProblem(StatusCodes.Status400BadRequest)` | +| `tests/BigRegister.Tests/StamdataEndpointTests.cs` | **new** `Unparseable_peildatum_is_400_not_500` | +| `backend/swagger.json`, `libs/shared/src/infrastructure/api-client.ts` | regenerated (`npm run gen:api`) — the new 400 response is now part of the documented contract | + +## What the fix looks like + +```csharp +DateOnly? peildatumWaarde = null; +if (peildatum is { Length: > 0 } p) +{ + if (!DateOnly.TryParse(p, out var parsed)) + return Results.Problem(detail: $"Ongeldige peildatum '{p}'.", statusCode: StatusCodes.Status400BadRequest); + peildatumWaarde = parsed; +} +var rows = peildatumWaarde is { } d ? t.RowsOn(d) : t.Rows(); +``` + +Matches the shape every other bad-input check in this file already uses (e.g. the +upload endpoint's `Results.Problem(detail: …, statusCode: 400)` for a malformed +multipart request) — a `Results.Problem` with a Dutch detail message, not a bespoke +response shape. + +## Judgement calls + +- **No FE change needed, and none made.** `libs/beheer/src/infrastructure/ +stamdata.adapter.ts`'s `load()` already routes every call through `runSubmit` + (`libs/shared/src/application/submit.ts`), which try/catches any thrown + `ApiException` — including the client's new 400 branch — into a generic `Result` + error string via `problemDetail`. There is no status-code-specific branching to + extend; ADR-0001's "the FE renders the decision, it does not recompute the rule" + already covers "the server rejected this input" as a case the generic error path + handles, same as the existing 403. +- **Regenerated the API client and committed it in this ticket's diff**, rather than + leaving it to drift. RB-09's implementation note records a real prior incident where + a response-shape change (RB-08's 403 → `ProducesProblem`) landed without a + regeneration and the drift went unnoticed until the next ticket's `gen:api` run. This + ticket's `.ProducesProblem(400)` is exactly that same category of change, so + `npm run gen:api` was run immediately as part of implementing it, not deferred. +- **The Dutch detail message follows the file's own convention** (`$"Ongeldige +peildatum '{p}'."`) rather than English — every other `Results.Problem(detail: …)` + call in `Program.cs` (change-request rejection, upload validation, submit rejection) + is Dutch; this is server-internal wire text, not `$localize`-wrapped UI copy (the FE + never renders it verbatim — CLAUDE.md's `$localize` rule is about user-facing copy + the FE owns, not backend `ProblemDetails.detail` strings), so no locale entry was + needed. + +## Verification + +- **Reverted the fix only** (`DateOnly.Parse` restored, ternary un-nested, via Edit — + test left in place) and ran `StamdataEndpointTests`: + `Unparseable_peildatum_is_400_not_500` failed red (`Expected: BadRequest, Actual: +InternalServerError` — confirming the endpoint really did 500, not some other status). + Restored the fix (via Edit) and re-ran: all 6 tests in the class green. +- `dotnet build`: clean, 0 warnings. +- `dotnet format BigRegister.slnx --verify-no-changes`: clean. +- `dotnet test --filter "Category!=Integration"`: **260 passed, 0 failed** (259 + 1 + new). +- `npm run gen:api`: exit 0; `backend/swagger.json` gained the 400 response shape for + this one endpoint; `libs/shared/src/infrastructure/api-client.ts` gained the + matching `status === 400` branch. Both regenerated files committed alongside the + code change. +- `npm test` (all four Vitest projects — ssp/behandelportal/shared/beheer): **445 + passed, 0 failed**, confirming the regenerated client doesn't break any existing FE + consumer of `stamdataTable(...)`. diff --git a/libs/shared/src/infrastructure/api-client.ts b/libs/shared/src/infrastructure/api-client.ts index 3d4da57..04e7639 100644 --- a/libs/shared/src/infrastructure/api-client.ts +++ b/libs/shared/src/infrastructure/api-client.ts @@ -341,6 +341,12 @@ export class ApiClient { result200 = _responseText === "" ? null : JSON.parse(_responseText, this.jsonParseReviver) as StamdataTableDto; return result200; }); + } else if (status === 400) { + return response.text().then((_responseText) => { + let result400: any = null; + result400 = _responseText === "" ? null : JSON.parse(_responseText, this.jsonParseReviver) as ProblemDetails; + return throwException("Bad Request", status, _responseText, _headers, result400); + }); } else if (status === 403) { return response.text().then((_responseText) => { let result403: any = null; From b617d2f09ad057d6496dc87873db2a64d9a3f121 Mon Sep 17 00:00:00 2001 From: Edwin van den Houdt Date: Thu, 27 Aug 2026 16:53:52 +0200 Subject: [PATCH 4/4] docs: regenerate behaviour-spec for RB-12/RB-15/RB-16 New backend test classes (RouteInventoryTests, SwaggerGateTests) plus one added case to StamdataEndpointTests. Co-Authored-By: Claude Opus 5 --- libs/shared/docs/behaviour-spec.mdx | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/libs/shared/docs/behaviour-spec.mdx b/libs/shared/docs/behaviour-spec.mdx index 37de06e..6fb2678 100644 --- a/libs/shared/docs/behaviour-spec.mdx +++ b/libs/shared/docs/behaviour-spec.mdx @@ -21,7 +21,7 @@ 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. 440 frontend behaviours across -9 contexts; 231 backend behaviours across 39 test +9 contexts; 236 backend behaviours across 41 test classes. ## Frontend (by context) @@ -1136,12 +1136,18 @@ classes. - A closed mapping is absent from its geldigTot onwards - ByProgram is evaluated per call not captured at type load +### RouteInventoryTests + +- Every mapped route is authz gated or on the named allow list +- Every gate marker names a known admin wrapper + ### StamdataEndpointTests - Stamdata reads are admin only - Table list exposes the reflected schema - Table returns all rows without a peildatum - Peildatum before the seed windows hides every row +- Unparseable peildatum is 400 not 500 - Unknown table is 404 ### StamdataValidationTests @@ -1172,6 +1178,11 @@ classes. - Worked hours are accepted - Phone change is validated +### SwaggerGateTests + +- Swagger document is served in development +- Swagger document is not served outside development + ### UploadAccessTests - The owner can read the bytes