diff --git a/backend/src/BigRegister.Api/Program.cs b/backend/src/BigRegister.Api/Program.cs index e390131..20e3ce4 100644 --- a/backend/src/BigRegister.Api/Program.cs +++ b/backend/src/BigRegister.Api/Program.cs @@ -681,7 +681,10 @@ api.MapPost("/brief/reveal-bignummer", (HttpContext ctx) => var canReveal = Authz.CanRevealBigNummer(principal); var steppedUp = ctx.Request.Headers["X-Step-Up"] == "true"; var allowed = canReveal && steppedUp; - AuditAuthz(ctx, "brief:reveal-bignummer", "brief/" + ctx.Zorgverlener().Bsn, allowed, principal); + // RB-02/BIO-008: the resource ref is the brief, not the subject — a BSN concatenated + // here lands in a persisted, admin-visible column the "no PII" guarantee covers. One + // brief exists per owner, so the id added nothing the acting principal did not imply. + AuditAuthz(ctx, "brief:reveal-bignummer", "brief", allowed, principal); if (!allowed) return Results.Problem( detail: canReveal diff --git a/backend/tests/BigRegister.Tests/AuthzAuditTests.cs b/backend/tests/BigRegister.Tests/AuthzAuditTests.cs index a670bd1..6a4848f 100644 --- a/backend/tests/BigRegister.Tests/AuthzAuditTests.cs +++ b/backend/tests/BigRegister.Tests/AuthzAuditTests.cs @@ -43,6 +43,24 @@ public class AuthzAuditTests(TestWebApplicationFactory factory) : IClassFixture< Assert.Contains(await AuditLog(), e => e.Action == "brief:reveal-bignummer"); } + /// RB-02/BIO-008: the schema test below asserts on **column names**, so a BSN inside a + /// column called `Resource` was invisible to it — and one was there, concatenated as + /// `"brief/" + Bsn`. This asserts on the stored **values** instead. Four documents + /// promise this trail holds no PII; this is the test that makes the promise checkable. + [Fact] + public async Task No_audit_row_carries_a_subjects_bsn() + { + const string subject = "999999990"; + var reveal = new HttpRequestMessage(HttpMethod.Post, "/api/v1/brief/reveal-bignummer"); + reveal.Headers.Add("X-Subject", subject); + Assert.Equal(HttpStatusCode.Forbidden, (await _client.SendAsync(reveal)).StatusCode); + + var bsns = new[] { subject, DocumentStore.DemoOwner }; + foreach (var e in await AuditLog()) + foreach (var field in new[] { e.Action, e.Resource, e.Decision, e.Role, e.At, e.CorrelationId }) + Assert.DoesNotContain(bsns, bsn => field.Contains(bsn, StringComparison.Ordinal)); + } + [Fact] public void The_audit_schema_carries_no_pii() { diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-02.md b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-02.md new file mode 100644 index 0000000..2d2ff8a --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-02.md @@ -0,0 +1,58 @@ +# RB-02 — stop concatenating the BSN into `AuthzAudit.Resource` + +Status: **implemented** · 2026-08-27 · Source findings: `07-bio2-compliance.md` BIO-008 · `99-backlog.md` RB-02 + +## What was wrong + +`Program.cs` (was `:674`) built the reveal attempt's audit resource ref as +`"brief/" + ctx.Zorgverlener().Bsn`. `AuditAuthz` persists that string to the +`AuthzAudit.Resource` column in SQLite, and `GET /admin/audit` renders it on the admin +audit page — so a BSN was written to durable storage and shown in a UI, on the one trail +four documents describe as data-minimised and PII-free. + +The endpoint is the *BIG-nummer reveal*, whose own comment says the audit carries +"NO PII. Never the value that was (or wasn't) revealed" — and it did not carry the +BIG-nummer. It carried the BSN instead, in the adjacent argument. + +## Why the existing test did not catch it + +`AuthzAuditTests.The_audit_schema_carries_no_pii` asserts on **column names**: + +```csharp +Assert.DoesNotContain(names, n => Regex.IsMatch(n, "naam|name|bsn|value|waarde", …)); +``` + +A BSN inside a column called `Resource` is invisible to a regex over the word `Resource`. +The test was structurally incapable of failing on this defect, which is why the +value-asserting test is part of this ticket's definition of done rather than a follow-up. + +## What changed + +| File | Change | +| ----------------------- | ---------------------------------------------------------------------------------------------------- | +| `Program.cs` | resource ref is `"brief"`; a comment records why the id added nothing | +| `AuthzAuditTests.cs` | **new** `No_audit_row_carries_a_subjects_bsn` — asserts on stored **values**, every string field | + +No identifier was lost. `BriefStore` keys one brief per owner, so `brief/` named the +same thing the row's acting principal already implies; there is no second brief the ref +could have disambiguated. + +The new test drives a denied reveal as a **non-default** subject (`X-Subject: 999999990`), +then scans every string field of every audit row for that BSN and for +`DocumentStore.DemoOwner`. Asserting against the two BSNs actually in play, rather than a +`\d{9}` shape, keeps it deterministic — a hex correlation id can hold nine consecutive +digits by chance. + +**Confirmed it fails without the fix**: reverting only the `Program.cs` line turns +`No_audit_row_carries_a_subjects_bsn` red, and restoring it turns it green. + +## Not in scope + +`AuditEntry.Actor` on document audit rows also holds a raw BSN. That is a different store +(`DocumentStore.Audit`) and is **RB-04**, which is where the masking decision for it lives. + +## Verification + +`dotnet format --verify-no-changes` clean. `dotnet test`: **251 passed, 1 failed** — the +failure is `OpenZaakIntegrationTests.Admin_cases_…`, which needs a live OpenZaak container +and fails identically on a stashed tree.