Files
atomic-design-poc/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-02.md
T
ehoandClaude Opus 5 0298ecc506 fix(uploads): delete the dead POST /registrations (RB-06)
POST /registrations passed its Documents list straight to Submit, which calls
DocumentStore.Link on every digital documentId in it — and linking a document
blocks its owner from ever deleting it (DeleteOwned returns 409 Linked). That
path had no ForeignIds ownership check, so any authenticated citizen could
post another citizen's document id and permanently block them from deleting
their own diploma scan. POST /applications/{id}/submit, the endpoint actually
in use, has had that guard since it was written.

Deleted rather than guarded: the endpoint is dead. No frontend caller, and
the whole registratie flow goes through /applications/{id}/submit.
RegistratieRequest went with it, and so did SubmissionRules.RejectRegistratie
— reachable only from here, and contradicted by the live path, which treats a
handmatig diploma as "does not auto-approve" rather than a 422 rejection. Its
own message said as much while being returned as a rejection. That last part
is a judgement call beyond the ticket's wording; reverting the two
SubmissionRules hunks restores it in isolation.

Coverage moved rather than vanished: the problem+json shape assertion is now
on /change-requests (the other endpoint on the same Submit helper), and the
linked-delete 409 test goes through the real submit path.

swagger.json, the generated client and the behaviour spec regenerated.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-27 11:04:03 +02:00

2.9 KiB

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:

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.