Files
atomic-design-poc/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-09.md
T
ehoandClaude Opus 5 8b8b522052 fix(auth): make no-identity representable; stub dev-only (RB-09)
IIdentityProvider.Resolve returned a non-nullable CallerIdentity, so
the interface could not express "no identity" - StubIdentityProvider
was forced to invent one for any request carrying no credential at
all. Consequence: a production behandelportal build sends no
X-Medewerker header (medewerkerInterceptor is dev-only), so it used
to authenticate as the seeded citizen, role drafter - failing closed
on backoffice capabilities but open on every citizen-scoped endpoint,
including CanRevealBigNummer.

Resolve now returns CallerIdentity?. StubIdentityProvider keeps a
non-nullable return type (a valid narrower override) since it never
itself has "no identity" to report - it is registered only under
IsDevelopment() now. Production registers nothing and throws an
InvalidOperationException immediately during startup instead: there
is no real DigiD/employee-SSO provider in this POC yet, so a
misconfigured Production deploy must fail before serving a single
request, not resolve one per request. The identity-resolution
middleware turns a null resolution into a 401 rather than passing it
downstream.

Added StubIdentityProviderTests.Never_returns_null_even_with_no_headers_at_all
and ProductionIdentityProviderTests, which builds its own
WebApplicationFactory<Program> with UseEnvironment("Production") and
asserts startup throws. Verified both new tests fail red against the
pre-fix code.

RB-01's residual (GET /uploads/{id}/content reached via plain browser
navigation, no identity header) is confirmed unchanged in Development
and its Production consequence is written up in
implementation/rb-09.md for whoever lands the real identity provider -
no signed-URL/cookie scheme was designed here, per scope.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-27 14:06:49 +02:00

10 KiB

RB-09 — make "no identity" representable; stub Development-only; fail fast in Production

Status: implemented · 2026-08-27 · Source findings: 07-bio2-compliance.md BIO-002 (folds in BIO-001(a)/(b)) · 99-backlog.md RB-09

What was wrong

IIdentityProvider.Resolve returned a non-nullable CallerIdentity (IIdentityProvider.cs:12, pre-change), so the interface could not express "no identity" — any implementation, stub or real, was forced to invent one for an unauthenticated request. StubIdentityProvider was registered unconditionally, for every environment.

Consequence, traced end to end: apps/behandelportal/src/app/app.config.ts:57-63 puts medewerkerInterceptor inside the isDevMode() provider array, so a production behandelportal build sends no X-Medewerker/X-Rollen header. With neither header, StubIdentityProvider.Resolve fell through to new ZorgverlenerCaller(DocumentStore.DemoOwner, ..., PrincipalRole.Drafter) — the single seeded citizen, role drafter. That:

  • Fails closed, correctly, on backoffice capabilitiesAuthz.CanBeoordelen is caller is MedewerkerCaller, so a zorgverlener caller is always false regardless of role. This part of the design was right and is untouched.
  • Fails open on every citizen-scoped endpointGET/PUT/DELETE /applications*, POST /applications/{id}/submit, DELETE /uploads/{id}, GET|PUT /brief, POST /brief/submit|send|reset all resolve ctx.Zorgverlener().Bsn to the seeded citizen's BSN. An employee with no employee identity was granted a citizen's own read/write rights.
  • Holds CanRevealBigNummer — that capability is Role == PrincipalRole.Drafter, and drafter is exactly the no-header default.

CallerIdentityHttpContextExtensions.Caller() (CallerIdentity.cs:44-50) already throws loudly when the identity middleware didn't run — the codebase reached for fail-loud one layer up and then defaulted one layer down, which is the shape of the bug.

What changed

File Change
Domain/Authorization/IIdentityProvider.cs Resolve returns CallerIdentity?; doc comment states what null means and where it's turned into a response
Domain/Authorization/StubIdentityProvider.cs implementation signature unchanged (CallerIdentity, non-nullable) — a valid, narrower override of the nullable interface method (return-type covariance; the compiler accepts it with zero warnings); doc comment records it is Development-only and never itself returns null
Program.cs — registration StubIdentityProvider registered only under builder.Environment.IsDevelopment(); an else if (builder.Environment.IsProduction()) branch throws InvalidOperationException immediately, before builder.Build() — the earliest possible failure point
Program.cs — identity middleware resolves the identity once; if null, sets 401 and returns without calling next(), instead of passing a null (or invented) caller downstream
tests/BigRegister.Tests/StubIdentityProviderTests.cs new Never_returns_null_even_with_no_headers_at_all; new ProductionIdentityProviderTests.Production_environment_with_no_real_identity_provider_fails_at_startup

No consumer beyond the middleware itself calls IIdentityProvider.Resolve (grep -rn "IIdentityProvider\|identityProvider\." over backend/src confirms exactly one call site) — Authz.ResolvePrincipal, ZgwTokenProvider.Mint, and every endpoint read ctx.Caller() / ctx.Zorgverlener(), which already throw on a missing identity and are untouched. The 401 now happens before those are ever reached for a request the middleware rejects.

Judgement calls

  • StubIdentityProvider's own method signature stays CallerIdentity, not CallerIdentity?. The interface needed the nullable shape to make "no identity" representable in general; the stub itself never has that case (it is a developer convenience that always invents a caller by design) and returning a narrower, non-nullable type from an override of a nullable-returning interface method is valid C# nullable-reference-type covariance — verified with a clean dotnet build (0 warnings). This kept every existing StubIdentityProviderTests call site (private static CallerIdentity Resolve(...)) compiling with zero changes, rather than sprinkling null-forgiving operators through a file whose entire point is "the stub always resolves."
  • The Production-only check is IsProduction(), not !IsDevelopment(). The ticket and BIO-002 both say "Production must fail at startup" specifically. A third environment (e.g. a hypothetical Staging) falls through neither branch, registers no IIdentityProvider at all, and would still fail — one line later, when app.Services.GetRequiredService< IIdentityProvider>() throws .NET's own "no service for type" exception — just with a less specific message than the one this ticket adds for Production. That fallback is a safety net, not the intended fail-fast message; if a real non-Production, non-Development environment is added later, giving it the same explicit message is a one-line follow-up, not a design gap today.
  • The throw sits before builder.Build(), not after (where GetRequiredService already runs today). Both satisfy "throw during service registration / app build so a misconfigured deploy never serves a request" — throwing earlier was free and gives a message naming the actual cause (no real identity provider) rather than a generic DI resolution failure.
  • The 401 short-circuits before ctx.SetCaller, not after. next(ctx) is never called, so no downstream middleware or endpoint runs for a request with no identity — a citizen or behandelaar endpoint reached this way now gets a clean 401 instead of ever executing.
  • TestWebApplicationFactory needed no change. WebApplicationFactory<T> defaults its test host to the Development environment when nothing overrides it (confirmed empirically: dotnet test — every non-Production test, all 253 of them pre-existing plus 2 new, passed unchanged), so the entire existing test suite continues to exercise the Development path exactly as before. The Production test builds its own WebApplicationFactory<Program>().WithWebHostBuilder(b => b.UseEnvironment("Production")) rather than touching the shared fixture.

Known residual — explicitly out of scope, confirmed and written up per the ticket

RB-01's residual is this ticket's territory but is explicitly out of scope for this ticket, per the task: GET /uploads/{documentId}/content is reached by a plain browser navigation (<a href> in beoordeling-documenten.component.ts, previewUrl in libs/shared/src/upload/upload.adapter.ts) that sends no identity header and never passes through an Angular interceptor.

  • In Development, this is unchanged — verified by reading the endpoint (Program.cs:253-266) and confirming StubIdentityProvider is still registered and still resolves the same non-null seeded-citizen default it always did when no headers are present. dotnet test's full pass (255/255, excluding the pre-existing OpenZaak failure) including UploadAccessTests — which exercises exactly this endpoint — confirms it byte-for-byte.
  • The Production consequence, for the next ticket: today Production cannot start at all (this ticket's fail-fast), so the question is moot until a real IIdentityProvider exists. Once one does, this endpoint's plain-navigation callers carry no credential a real provider could resolve — the identity middleware would treat that as "no identity" and return 401 before the endpoint ever runs, breaking both preview links outright. Making the stub Development-only does not itself break anything (nothing in Production exists yet to break), but it does mean whoever builds the real provider must also solve this endpoint's credential-carrying problem in the same change, or ship it broken. This is not a signed-URL or cookie scheme, and no such scheme was designed here, per the ticket's explicit instruction — it is recorded so the next ticket (RB-13, or whichever lands the real provider) picks it up deliberately rather than discovering it in a production incident.

Verification

  • Reverted the registration change only (kept AddSingleton<IIdentityProvider, StubIdentityProvider>() unconditional, left the new tests in place) and ran ProductionIdentityProviderTests: it failed red — Assert.ThrowsAny() Failure: No exception was thrown (the stub gets registered in every environment, so the host builds fine and factory.CreateClient() never throws). Restored the fix and re-ran: green.
  • dotnet build: clean, 0 warnings (confirms the nullable-covariance judgement call above compiles cleanly).
  • dotnet format BigRegister.slnx --verify-no-changes: clean.
  • dotnet test (full suite): 255 passed, 1 failed — the pre-existing OpenZaakIntegrationTests.Admin_cases_returns_the_seeded_zaak_mapped_through_real_HTTP_and_JWT (needs a live OpenZaak container; fails identically on a clean tree). CI's actual filter, dotnet test BigRegister.slnx --filter "Category!=Integration": 255 passed, 0 failed.