fix(audit): stop writing a BSN into the authz audit Resource (RB-02)
Program.cs built the BIG-nummer reveal's audit resource ref as
"brief/" + ctx.Zorgverlener().Bsn. AuditAuthz persists that to the
AuthzAudit.Resource column in SQLite and /admin/audit renders it, so a BSN
reached durable storage and a UI on the one trail four documents describe as
data-minimised and PII-free — on the endpoint whose own comment promises the
audit carries no PII.
The ref is now "brief". Nothing is lost: BriefStore keys one brief per owner,
so the id named what the row's acting principal already implies.
The existing guard, The_audit_schema_carries_no_pii, asserts on column names,
so a BSN inside a column called Resource could never fail it. Added
No_audit_row_carries_a_subjects_bsn, which drives a denied reveal as a
non-default subject and scans every string field of every row for that BSN
and for DemoOwner — asserting on the two BSNs actually in play rather than a
\d{9} shape, since a hex correlation id can hold nine digits by chance.
Verified it goes red when only the Program.cs line is reverted.
AuditEntry.Actor on document audit rows holds a raw BSN too; that is a
different store and stays with RB-04.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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()
|
||||
{
|
||||
|
||||
@@ -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/<bsn>` 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.
|
||||
Reference in New Issue
Block a user