fix(privacy): mask the owner BSN on the cross-owner case lists (RB-03)
Mappers.ToAdminSummaryDto set Owner to the raw BSN. Both consumers are
cross-owner lists read by someone who is not the subject — GET /admin/cases
and GET /werkvoorraad — while GET /beoordeling/{id}, the detail view of the
same data, already masked it. The detail screen showed ******782 and the list
one click earlier showed the whole thing.
Masked in the mapper rather than at each endpoint, so a third cross-owner
list cannot be added that forgets to.
MaskTail moves out of Program.cs into Domain/People/Pii.cs: it now has
callers in Contracts, Program.cs and (once RB-04 lands) Data, and a second
hand-rolled copy is how one of them drifts into leaking. Documented as
idempotent, which is what lets /beoordeling/{id} keep its own call —
IZaakSource has a second implementation whose Owner is mapped from the
OpenZaak zaak identificatie, so that endpoint should not depend on which
source answered.
No frontend change: all three consumers display the value, and the parse
boundaries only require a non-empty string.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -70,8 +70,12 @@ public static class Mappers
|
|||||||
a.CreatedAt.ToString("o"), a.UpdatedAt.ToString("o"), SubmittedAtOf(a));
|
a.CreatedAt.ToString("o"), a.UpdatedAt.ToString("o"), SubmittedAtOf(a));
|
||||||
|
|
||||||
/// Admin summary — same shape plus the owner (WP-36; the user-facing list leaves Owner null).
|
/// Admin summary — same shape plus the owner (WP-36; the user-facing list leaves Owner null).
|
||||||
|
/// The owner is a BSN, and both consumers of this mapper are cross-owner lists read by
|
||||||
|
/// someone who is not the subject (`/admin/cases`, `/werkvoorraad`), so it goes out masked
|
||||||
|
/// (RB-03/BIO-003). Masking here rather than at each endpoint means a third cross-owner
|
||||||
|
/// list cannot be added that forgets to.
|
||||||
public static ApplicationSummaryDto ToAdminSummaryDto(this Aanvraag a, DateTimeOffset now) =>
|
public static ApplicationSummaryDto ToAdminSummaryDto(this Aanvraag a, DateTimeOffset now) =>
|
||||||
a.ToSummaryDto(now) with { Owner = a.Owner };
|
a.ToSummaryDto(now) with { Owner = Pii.MaskTail(a.Owner, 3) };
|
||||||
|
|
||||||
public static ApplicationDetailDto ToDetailDto(this Aanvraag a, DateTimeOffset now) => new(
|
public static ApplicationDetailDto ToDetailDto(this Aanvraag a, DateTimeOffset now) => new(
|
||||||
a.Id, a.Type, a.ToStatusDto(now), DraftOf(a), a.DocumentIds,
|
a.Id, a.Type, a.ToStatusDto(now), DraftOf(a), a.DocumentIds,
|
||||||
|
|||||||
@@ -0,0 +1,18 @@
|
|||||||
|
namespace BigRegister.Domain.People;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// One redaction rule for identifiers that must not leave the server in full (BSN,
|
||||||
|
/// BIG-nummer). Lives in <c>Domain/</c> because three layers need it — the DTO mappers
|
||||||
|
/// (<c>Contracts/Mappers.cs</c>), the audit writes (<c>Data/DocumentStore.cs</c>) and the
|
||||||
|
/// endpoints themselves — and a second hand-rolled copy is exactly how one of them drifts
|
||||||
|
/// into leaking. Mirrors the FE <c>maskTail</c> (<c>libs/shared/src/ui/debug-state/mask.ts</c>)
|
||||||
|
/// so wire redaction and the dev panel agree on what a masked value looks like.
|
||||||
|
/// </summary>
|
||||||
|
public static class Pii
|
||||||
|
{
|
||||||
|
/// Keep the last <paramref name="keep"/> characters, mask the rest. Idempotent: masking an
|
||||||
|
/// already-masked value is a no-op, so a defence-in-depth second call is harmless.
|
||||||
|
public static string MaskTail(string value, int keep) =>
|
||||||
|
value.Length <= keep ? new string('*', value.Length)
|
||||||
|
: new string('*', value.Length - keep) + value[^keep..];
|
||||||
|
}
|
||||||
@@ -12,6 +12,7 @@ using BigRegister.Domain.Documents;
|
|||||||
using BigRegister.Domain.Features;
|
using BigRegister.Domain.Features;
|
||||||
using BigRegister.Domain.Intake;
|
using BigRegister.Domain.Intake;
|
||||||
using BigRegister.Domain.Letters;
|
using BigRegister.Domain.Letters;
|
||||||
|
using BigRegister.Domain.People;
|
||||||
using BigRegister.Domain.Registrations;
|
using BigRegister.Domain.Registrations;
|
||||||
using BigRegister.Domain.Submissions;
|
using BigRegister.Domain.Submissions;
|
||||||
using BigRegister.Api.Zgw;
|
using BigRegister.Api.Zgw;
|
||||||
@@ -460,7 +461,10 @@ api.MapGet("/beoordeling/{id}", (string id, HttpContext ctx, IZaakSource zaken)
|
|||||||
if (c is null || c.Status.Tag == "Concept") return Results.NotFound();
|
if (c is null || c.Status.Tag == "Concept") return Results.NotFound();
|
||||||
var docs = DocumentStore.ByIds(c.DocumentIds)
|
var docs = DocumentStore.ByIds(c.DocumentIds)
|
||||||
.Select(d => new BeoordelingDocumentDto(d.DocumentId, d.CategoryId, d.FileName)).ToList();
|
.Select(d => new BeoordelingDocumentDto(d.DocumentId, d.CategoryId, d.FileName)).ToList();
|
||||||
var masked = c with { Owner = MaskTail(c.Owner!, 3) };
|
// Belt and braces: ToAdminSummaryDto already masks the local source (RB-03) and
|
||||||
|
// MaskTail is idempotent, but IZaakSource has a second implementation whose Owner
|
||||||
|
// is mapped from OpenZaak, so this stays as the guarantee for this response.
|
||||||
|
var masked = c with { Owner = Pii.MaskTail(c.Owner!, 3) };
|
||||||
// WP-68 (F3): non-throwing — c.Status.Tag crosses the IZaakSource wire boundary, so an
|
// WP-68 (F3): non-throwing — c.Status.Tag crosses the IZaakSource wire boundary, so an
|
||||||
// unrecognised tag degrades to "cannot decide" instead of a 500.
|
// unrecognised tag degrades to "cannot decide" instead of a 500.
|
||||||
var canBesluiten = Enum.TryParse<AanvraagStatusTag>(c.Status.Tag, out var tag) && BeoordelingRules.CanDecide(tag);
|
var canBesluiten = Enum.TryParse<AanvraagStatusTag>(c.Status.Tag, out var tag) && BeoordelingRules.CanDecide(tag);
|
||||||
@@ -869,12 +873,6 @@ void RecordZgwDivergence(HttpContext ctx, string id, string referentie, Exceptio
|
|||||||
AuthzAuditStore.Record("zgw:divergence", referentie, allowed: false, Authz.ResolvePrincipal(ctx).Role.ToString(), cid);
|
AuthzAuditStore.Record("zgw:divergence", referentie, allowed: false, Authz.ResolvePrincipal(ctx).Role.ToString(), cid);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Keep the last `keep` characters, mask the rest — mirrors the FE maskTail
|
|
||||||
// (src/app/shared/ui/debug-state/mask.ts) so wire redaction and the dev panel agree.
|
|
||||||
static string MaskTail(string value, int keep) =>
|
|
||||||
value.Length <= keep ? new string('*', value.Length)
|
|
||||||
: new string('*', value.Length - keep) + value[^keep..];
|
|
||||||
|
|
||||||
static string Now() => DateTimeOffset.UtcNow.ToString("o");
|
static string Now() => DateTimeOffset.UtcNow.ToString("o");
|
||||||
|
|
||||||
BriefViewDto ToView(HttpContext ctx, BriefEntity e) => new(
|
BriefViewDto ToView(HttpContext ctx, BriefEntity e) => new(
|
||||||
@@ -888,7 +886,7 @@ BriefViewDto ToView(HttpContext ctx, BriefEntity e) => new(
|
|||||||
// behandel scherm can show whom/what it concerns without brief/ importing registratie.
|
// behandel scherm can show whom/what it concerns without brief/ importing registratie.
|
||||||
// The BIG-nummer ships MASKED by default (PRD-0002 §5c, field-level PII); the reveal
|
// The BIG-nummer ships MASKED by default (PRD-0002 §5c, field-level PII); the reveal
|
||||||
// endpoint returns the full value, gated + audited.
|
// endpoint returns the full value, gated + audited.
|
||||||
new CaseContextDto(SeedData.Registration.Naam, MaskTail(SeedData.Registration.BigNummer, 3), e.Beroep, BriefSeed.AanvraagReferentie));
|
new CaseContextDto(SeedData.Registration.Naam, Pii.MaskTail(SeedData.Registration.BigNummer, 3), e.Beroep, BriefSeed.AanvraagReferentie));
|
||||||
|
|
||||||
// Emit (decision flags, via ToView) and enforce (Forbidden/Conflict below) both run
|
// Emit (decision flags, via ToView) and enforce (Forbidden/Conflict below) both run
|
||||||
// through Authz — see BriefStore.Review and Authz.CanActOn — so they cannot drift.
|
// through Authz — see BriefStore.Review and Authz.CanActOn — so they cannot drift.
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
using System.Net;
|
using System.Net;
|
||||||
using System.Net.Http.Json;
|
using System.Net.Http.Json;
|
||||||
using BigRegister.Api.Contracts;
|
using BigRegister.Api.Contracts;
|
||||||
|
using BigRegister.Api.Data;
|
||||||
using Microsoft.AspNetCore.Mvc.Testing;
|
using Microsoft.AspNetCore.Mvc.Testing;
|
||||||
|
|
||||||
namespace BigRegister.Tests;
|
namespace BigRegister.Tests;
|
||||||
@@ -34,7 +35,10 @@ public class AdminCasesTests(TestWebApplicationFactory factory) : IClassFixture<
|
|||||||
list.EnsureSuccessStatusCode();
|
list.EnsureSuccessStatusCode();
|
||||||
var cases = (await list.Content.ReadFromJsonAsync<List<ApplicationSummaryDto>>())!;
|
var cases = (await list.Content.ReadFromJsonAsync<List<ApplicationSummaryDto>>())!;
|
||||||
var mine = cases.Single(x => x.Id == a.Id);
|
var mine = cases.Single(x => x.Id == a.Id);
|
||||||
Assert.False(string.IsNullOrEmpty(mine.Owner)); // admin list carries the owner
|
// RB-03/BIO-003: the owner is carried, but masked — it is a BSN, and this list is
|
||||||
|
// read by someone who is not the subject.
|
||||||
|
Assert.Equal("******782", mine.Owner);
|
||||||
|
Assert.DoesNotContain(DocumentStore.DemoOwner, mine.Owner);
|
||||||
}
|
}
|
||||||
finally
|
finally
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -38,7 +38,8 @@ public class WerkvoorraadTests(TestWebApplicationFactory factory) : IClassFixtur
|
|||||||
var queue = (await res.Content.ReadFromJsonAsync<List<ApplicationSummaryDto>>())!;
|
var queue = (await res.Content.ReadFromJsonAsync<List<ApplicationSummaryDto>>())!;
|
||||||
var mine = queue.Single(x => x.Id == a.Id);
|
var mine = queue.Single(x => x.Id == a.Id);
|
||||||
Assert.Equal("InBehandeling", mine.Status.Tag);
|
Assert.Equal("InBehandeling", mine.Status.Tag);
|
||||||
Assert.False(string.IsNullOrEmpty(mine.Owner)); // cross-owner, like /admin/cases
|
// RB-03/BIO-003: masked, like /admin/cases — both inherit ToAdminSummaryDto.
|
||||||
|
Assert.Equal("******782", mine.Owner);
|
||||||
}
|
}
|
||||||
finally
|
finally
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -0,0 +1,45 @@
|
|||||||
|
# RB-03 — mask the owner BSN on the cross-owner case lists
|
||||||
|
|
||||||
|
Status: **implemented** · 2026-08-27 · Source findings: `07-bio2-compliance.md` BIO-003 · `99-backlog.md` RB-03
|
||||||
|
|
||||||
|
## What was wrong
|
||||||
|
|
||||||
|
`Mappers.ToAdminSummaryDto` set `Owner = a.Owner` — the raw BSN. Two endpoints consume it,
|
||||||
|
both cross-owner lists read by someone who is **not** the subject:
|
||||||
|
|
||||||
|
- `GET /admin/cases` (`cases:manage`)
|
||||||
|
- `GET /werkvoorraad` (`aanvraag:beoordelen`)
|
||||||
|
|
||||||
|
`GET /beoordeling/{id}` — the *detail* view of the same data — already masked. So the
|
||||||
|
detail screen showed `******782` while the list one click earlier showed the whole BSN.
|
||||||
|
|
||||||
|
## What changed
|
||||||
|
|
||||||
|
| File | Change |
|
||||||
|
| --------------------------- | ----------------------------------------------------------------------------------- |
|
||||||
|
| `Domain/People/Pii.cs` | **new** — `Pii.MaskTail`, moved out of `Program.cs` |
|
||||||
|
| `Contracts/Mappers.cs` | `Owner = Pii.MaskTail(a.Owner, 3)` |
|
||||||
|
| `Program.cs` | local `MaskTail` deleted; two call sites point at `Pii.MaskTail` |
|
||||||
|
| `AdminCasesTests.cs` | asserts the masked value and that `DemoOwner` does not appear |
|
||||||
|
| `WerkvoorraadTests.cs` | same assertion, replacing the `IsNullOrEmpty` one |
|
||||||
|
|
||||||
|
**Masked in the mapper, not at the endpoints.** The point of the ticket is that both
|
||||||
|
lists *inherit* it, so a third cross-owner list cannot be added that forgets to mask.
|
||||||
|
|
||||||
|
**`MaskTail` moved to `Domain/People/Pii.cs`** because it now has three callers across
|
||||||
|
three folders (`Contracts`, `Program.cs`, and `Data` once **RB-04** lands), and a second
|
||||||
|
hand-rolled copy is how one of them drifts into leaking. It is documented as idempotent,
|
||||||
|
which is what lets `/beoordeling/{id}` keep its own call: `IZaakSource` has a second
|
||||||
|
implementation (`OpenZaakZaakSource` → `ZgwZaakMapper`, which maps `Owner` from the zaak
|
||||||
|
`identificatie`), so that endpoint's guarantee should not depend on which source answered.
|
||||||
|
|
||||||
|
## Blast radius on the frontend — none
|
||||||
|
|
||||||
|
Both consumers use the value for display only (`admin-cases.page.ts:101`,
|
||||||
|
`beoordeling-view.ts:40`, `werkvoorraad-item-view.ts:28`); the `parse*` boundaries require
|
||||||
|
a non-empty string, which a masked BSN still is. Nothing keys, filters or looks up by owner.
|
||||||
|
|
||||||
|
## Verification
|
||||||
|
|
||||||
|
`dotnet format --verify-no-changes` clean. `dotnet test`: **251 passed, 1 failed** — the
|
||||||
|
pre-existing `OpenZaakIntegrationTests.Admin_cases_…`, which needs a live container.
|
||||||
Reference in New Issue
Block a user