feat(audit): record the allow path, not just the denial (RB-07)
All five authorization gates audited only their deny branch, so /beheer/audit could answer "who was turned away" but never "who changed this" — for a register whose integrity is the product, the wrong half. Nothing recorded the flag toggle, either org-template write, the admin case or upload delete, the three brief transitions, or the besluit; the comment claiming endpoints log their own effect held for two of the eight. Each gate now computes the decision once, audits it, and then acts. The row is written by the gate rather than the endpoint, so a new admin endpoint cannot be added that forgets to audit itself. Same reasoning for the brief: every transition already funnelled through LogBrief for its log line, so the audit row goes there too — submit/approve/reject/send in one place, with the transition's own outcome as the decision, so a 403 or 409 is as visible as a success. FlagsAdmin gained a per-call resource, the one deviation from BIO-007's minimal remediation: the toggle endpoint writes no log line of its own, so a constant "feature-flags" row would say a flag changed without saying which. It now records feature-flags/<key>=<value>. OrgAdmin and CasesAdmin keep coarse refs because those endpoints do log the specific object. The besluit gets a second row: the gate records that a behandelaar was allowed to act, aanvraag:besluit records what they decided. Row volume goes up — StamdataAdmin gates read endpoints, so admin page loads now write rows. That is what auditing the allow path means; it is also what would make retention on AuthzAuditStore necessary later. Closes CQ-004's outstanding half and unblocks signing ADR-C-009. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -506,6 +506,10 @@ api.MapPost("/beoordeling/{id}/besluit", (string id, RecordBesluitRequest req, H
|
|||||||
statusCode: StatusCodes.Status409Conflict);
|
statusCode: StatusCodes.Status409Conflict);
|
||||||
|
|
||||||
app.Logger.LogInformation("aanvraag besluit id={Id} besluit={Besluit}", a.Id, besluit);
|
app.Logger.LogInformation("aanvraag besluit id={Id} besluit={Besluit}", a.Id, besluit);
|
||||||
|
// RB-07/BIO-007: the gate above records that a behandelaar was allowed to act; this
|
||||||
|
// records what they decided. Without it /beheer/audit cannot answer "who rejected this
|
||||||
|
// aanvraag", which is the question the trail exists for.
|
||||||
|
AuditAuthz(ctx, "aanvraag:besluit", $"aanvraag/{a.Id}/{besluit}", true, Authz.ResolvePrincipal(ctx));
|
||||||
|
|
||||||
// WP-60: the local decision above already committed — a ZGW failure here is caught and
|
// WP-60: the local decision above already committed — a ZGW failure here is caught and
|
||||||
// flagged rather than allowed to diverge silently, same handling as submit's create-zaak
|
// flagged rather than allowed to diverge silently, same handling as submit's create-zaak
|
||||||
@@ -598,7 +602,8 @@ api.MapGet("/flags", () =>
|
|||||||
Results.Ok(FeatureFlagStore.All().Select(f => new FeatureFlagDto(f.Key, f.Description, f.Enabled)).ToList()))
|
Results.Ok(FeatureFlagStore.All().Select(f => new FeatureFlagDto(f.Key, f.Description, f.Enabled)).ToList()))
|
||||||
.Produces<List<FeatureFlagDto>>();
|
.Produces<List<FeatureFlagDto>>();
|
||||||
|
|
||||||
api.MapPut("/admin/flags/{key}", (string key, SetFeatureFlagRequest req, HttpContext ctx) => FlagsAdmin(ctx, () =>
|
api.MapPut("/admin/flags/{key}", (string key, SetFeatureFlagRequest req, HttpContext ctx) =>
|
||||||
|
FlagsAdmin(ctx, $"feature-flags/{key}={req.Enabled}", () =>
|
||||||
FeatureFlagStore.Set(key, req.Enabled) ? Results.NoContent() : Results.NotFound()))
|
FeatureFlagStore.Set(key, req.Enabled) ? Results.NoContent() : Results.NotFound()))
|
||||||
.Produces(StatusCodes.Status204NoContent)
|
.Produces(StatusCodes.Status204NoContent)
|
||||||
.Produces(StatusCodes.Status404NotFound)
|
.Produces(StatusCodes.Status404NotFound)
|
||||||
@@ -629,7 +634,7 @@ api.MapPost("/brief/submit", (HttpContext ctx) =>
|
|||||||
{
|
{
|
||||||
var isDrafter = Authz.ResolvePrincipal(ctx).Role == PrincipalRole.Drafter;
|
var isDrafter = Authz.ResolvePrincipal(ctx).Role == PrincipalRole.Drafter;
|
||||||
var r = BriefStore.Submit(ctx.Zorgverlener().Bsn, isDrafter, Now());
|
var r = BriefStore.Submit(ctx.Zorgverlener().Bsn, isDrafter, Now());
|
||||||
LogBrief("submit", r);
|
LogBrief(ctx, "submit", r);
|
||||||
return BriefResult(ctx, r, "Alleen de opsteller mag indienen.");
|
return BriefResult(ctx, r, "Alleen de opsteller mag indienen.");
|
||||||
})
|
})
|
||||||
.WithName("briefSubmit") // distinct name so the generated client method isn't `submit2`
|
.WithName("briefSubmit") // distinct name so the generated client method isn't `submit2`
|
||||||
@@ -640,7 +645,7 @@ api.MapPost("/brief/submit", (HttpContext ctx) =>
|
|||||||
api.MapPost("/brief/approve", (HttpContext ctx) =>
|
api.MapPost("/brief/approve", (HttpContext ctx) =>
|
||||||
{
|
{
|
||||||
var r = BriefStore.Approve(ctx.Zorgverlener().Bsn, Authz.ResolvePrincipal(ctx), Now());
|
var r = BriefStore.Approve(ctx.Zorgverlener().Bsn, Authz.ResolvePrincipal(ctx), Now());
|
||||||
LogBrief("approve", r);
|
LogBrief(ctx, "approve", r);
|
||||||
return BriefResult(ctx, r, "De beoordelaar mag niet de opsteller zijn.");
|
return BriefResult(ctx, r, "De beoordelaar mag niet de opsteller zijn.");
|
||||||
})
|
})
|
||||||
.Produces<BriefViewDto>()
|
.Produces<BriefViewDto>()
|
||||||
@@ -650,7 +655,7 @@ api.MapPost("/brief/approve", (HttpContext ctx) =>
|
|||||||
api.MapPost("/brief/reject", (RejectBriefRequest req, HttpContext ctx) =>
|
api.MapPost("/brief/reject", (RejectBriefRequest req, HttpContext ctx) =>
|
||||||
{
|
{
|
||||||
var r = BriefStore.Reject(ctx.Zorgverlener().Bsn, Authz.ResolvePrincipal(ctx), req.Comments, Now());
|
var r = BriefStore.Reject(ctx.Zorgverlener().Bsn, Authz.ResolvePrincipal(ctx), req.Comments, Now());
|
||||||
LogBrief("reject", r);
|
LogBrief(ctx, "reject", r);
|
||||||
return BriefResult(ctx, r, "De beoordelaar mag niet de opsteller zijn.");
|
return BriefResult(ctx, r, "De beoordelaar mag niet de opsteller zijn.");
|
||||||
})
|
})
|
||||||
.Produces<BriefViewDto>()
|
.Produces<BriefViewDto>()
|
||||||
@@ -663,7 +668,7 @@ api.MapPost("/brief/send", (HttpContext ctx) =>
|
|||||||
// port); the backend only guards the approved→sent transition (not role-gated
|
// port); the backend only guards the approved→sent transition (not role-gated
|
||||||
// today — see Authz.CanActOn(Send, …), a mechanical dispatch step).
|
// today — see Authz.CanActOn(Send, …), a mechanical dispatch step).
|
||||||
var r = BriefStore.Send(ctx.Zorgverlener().Bsn, Now());
|
var r = BriefStore.Send(ctx.Zorgverlener().Bsn, Now());
|
||||||
LogBrief("send", r);
|
LogBrief(ctx, "send", r);
|
||||||
return BriefResult(ctx, r, "Versturen kan niet in deze status.");
|
return BriefResult(ctx, r, "Versturen kan niet in deze status.");
|
||||||
})
|
})
|
||||||
.Produces<BriefViewDto>()
|
.Produces<BriefViewDto>()
|
||||||
@@ -785,14 +790,19 @@ app.Run();
|
|||||||
static bool IsAdmin(HttpContext ctx) => ctx.Request.Headers["X-Admin"] == "true";
|
static bool IsAdmin(HttpContext ctx) => ctx.Request.Headers["X-Admin"] == "true";
|
||||||
|
|
||||||
// One gate for every org-template endpoint — the enforce twin of the
|
// One gate for every org-template endpoint — the enforce twin of the
|
||||||
// `orgtemplate:edit` capability RoleCapabilities emits (single Authz source). A denial
|
// `orgtemplate:edit` capability RoleCapabilities emits (single Authz source).
|
||||||
// is audited (PRD-0002 §8); the allow path is left un-logged (the endpoints log their
|
//
|
||||||
// own effect, e.g. publish).
|
// RB-07/BIO-007: every gate below audits the real decision, allow *and* deny. Auditing
|
||||||
|
// only denials left /beheer/audit able to answer "who was turned away" but not "who
|
||||||
|
// changed this", which for a register whose integrity is the product is the wrong half
|
||||||
|
// (PRD-0002 §8 lists approvals alongside denials). The allow row is written by the gate,
|
||||||
|
// not by the endpoint, so a new admin endpoint cannot be added that forgets it.
|
||||||
IResult OrgAdmin(HttpContext ctx, Func<IResult> action)
|
IResult OrgAdmin(HttpContext ctx, Func<IResult> action)
|
||||||
{
|
{
|
||||||
var principal = Authz.ResolvePrincipal(ctx);
|
var principal = Authz.ResolvePrincipal(ctx);
|
||||||
if (Authz.CanManageOrgTemplates(principal)) return action();
|
var ok = Authz.CanManageOrgTemplates(principal);
|
||||||
AuditAuthz(ctx, "orgtemplate:edit", "org-templates", false, principal);
|
AuditAuthz(ctx, "orgtemplate:edit", "org-templates", ok, principal);
|
||||||
|
if (ok) return action();
|
||||||
return Results.Problem(detail: "Alleen een beheerder mag organisatiesjablonen beheren.",
|
return Results.Problem(detail: "Alleen een beheerder mag organisatiesjablonen beheren.",
|
||||||
statusCode: StatusCodes.Status403Forbidden);
|
statusCode: StatusCodes.Status403Forbidden);
|
||||||
}
|
}
|
||||||
@@ -802,8 +812,9 @@ IResult OrgAdmin(HttpContext ctx, Func<IResult> action)
|
|||||||
IResult StamdataAdmin(HttpContext ctx, Func<IResult> action)
|
IResult StamdataAdmin(HttpContext ctx, Func<IResult> action)
|
||||||
{
|
{
|
||||||
var principal = Authz.ResolvePrincipal(ctx);
|
var principal = Authz.ResolvePrincipal(ctx);
|
||||||
if (Authz.CanEditStamdata(principal)) return action();
|
var ok = Authz.CanEditStamdata(principal);
|
||||||
AuditAuthz(ctx, "stamdata:edit", "stamdata", false, principal);
|
AuditAuthz(ctx, "stamdata:edit", "stamdata", ok, principal);
|
||||||
|
if (ok) return action();
|
||||||
return Results.Problem(detail: "Alleen een beheerder mag stamdata onderhouden.",
|
return Results.Problem(detail: "Alleen een beheerder mag stamdata onderhouden.",
|
||||||
statusCode: StatusCodes.Status403Forbidden);
|
statusCode: StatusCodes.Status403Forbidden);
|
||||||
}
|
}
|
||||||
@@ -813,8 +824,9 @@ IResult StamdataAdmin(HttpContext ctx, Func<IResult> action)
|
|||||||
IResult CasesAdmin(HttpContext ctx, Func<IResult> action)
|
IResult CasesAdmin(HttpContext ctx, Func<IResult> action)
|
||||||
{
|
{
|
||||||
var principal = Authz.ResolvePrincipal(ctx);
|
var principal = Authz.ResolvePrincipal(ctx);
|
||||||
if (Authz.CanManageCases(principal)) return action();
|
var ok = Authz.CanManageCases(principal);
|
||||||
AuditAuthz(ctx, "cases:manage", "cases", false, principal);
|
AuditAuthz(ctx, "cases:manage", "cases", ok, principal);
|
||||||
|
if (ok) return action();
|
||||||
return Results.Problem(detail: "Alleen een beheerder mag aanvragen beheren.",
|
return Results.Problem(detail: "Alleen een beheerder mag aanvragen beheren.",
|
||||||
statusCode: StatusCodes.Status403Forbidden);
|
statusCode: StatusCodes.Status403Forbidden);
|
||||||
}
|
}
|
||||||
@@ -825,18 +837,23 @@ IResult CasesAdmin(HttpContext ctx, Func<IResult> action)
|
|||||||
// zorgverlener with X-Role=admin still gets denied. `resource` feeds the denial's audit row.
|
// zorgverlener with X-Role=admin still gets denied. `resource` feeds the denial's audit row.
|
||||||
IResult Beoordelen(HttpContext ctx, string resource, Func<IResult> action)
|
IResult Beoordelen(HttpContext ctx, string resource, Func<IResult> action)
|
||||||
{
|
{
|
||||||
if (Authz.CanBeoordelen(ctx.Caller())) return action();
|
var ok = Authz.CanBeoordelen(ctx.Caller());
|
||||||
AuditAuthz(ctx, "aanvraag:beoordelen", resource, false, Authz.ResolvePrincipal(ctx));
|
AuditAuthz(ctx, "aanvraag:beoordelen", resource, ok, Authz.ResolvePrincipal(ctx));
|
||||||
|
if (ok) return action();
|
||||||
return Results.Problem(detail: "Alleen een behandelaar mag aanvragen beoordelen.",
|
return Results.Problem(detail: "Alleen een behandelaar mag aanvragen beoordelen.",
|
||||||
statusCode: StatusCodes.Status403Forbidden);
|
statusCode: StatusCodes.Status403Forbidden);
|
||||||
}
|
}
|
||||||
|
|
||||||
// One gate for the feature-flag toggle — the enforce twin of `flags:manage` (WP-47).
|
// One gate for the feature-flag toggle — the enforce twin of `flags:manage` (WP-47). Takes a
|
||||||
IResult FlagsAdmin(HttpContext ctx, Func<IResult> action)
|
// per-call `resource` like Beoordelen does, because the toggle endpoint writes no log line of
|
||||||
|
// its own (BIO-007): a bare "feature-flags" row would say a flag changed without saying which,
|
||||||
|
// and this is the surface CQ-004/ADR-C-009 hinge on.
|
||||||
|
IResult FlagsAdmin(HttpContext ctx, string resource, Func<IResult> action)
|
||||||
{
|
{
|
||||||
var principal = Authz.ResolvePrincipal(ctx);
|
var principal = Authz.ResolvePrincipal(ctx);
|
||||||
if (Authz.CanManageFeatureFlags(principal)) return action();
|
var ok = Authz.CanManageFeatureFlags(principal);
|
||||||
AuditAuthz(ctx, "flags:manage", "feature-flags", false, principal);
|
AuditAuthz(ctx, "flags:manage", resource, ok, principal);
|
||||||
|
if (ok) return action();
|
||||||
return Results.Problem(detail: "Alleen een beheerder mag functievlaggen beheren.",
|
return Results.Problem(detail: "Alleen een beheerder mag functievlaggen beheren.",
|
||||||
statusCode: StatusCodes.Status403Forbidden);
|
statusCode: StatusCodes.Status403Forbidden);
|
||||||
}
|
}
|
||||||
@@ -892,9 +909,16 @@ IResult BriefResult(HttpContext ctx, (BriefStore.Outcome outcome, BriefEntity? e
|
|||||||
_ => Results.Problem(detail: "Ongeldige overgang voor de huidige status van de brief.", statusCode: StatusCodes.Status409Conflict),
|
_ => Results.Problem(detail: "Ongeldige overgang voor de huidige status van de brief.", statusCode: StatusCodes.Status409Conflict),
|
||||||
};
|
};
|
||||||
|
|
||||||
void LogBrief(string action, (BriefStore.Outcome outcome, BriefEntity? entity) r) =>
|
// RB-07/BIO-007: every brief transition already funnelled through here for its log line,
|
||||||
|
// so the audit row goes here too — a fifth transition cannot be added that logs but leaves
|
||||||
|
// no trail. Resource is the bare "brief" (RB-02: never the owner's BSN); the decision is
|
||||||
|
// the transition's own outcome, so a 403 or a 409 is as visible as a success.
|
||||||
|
void LogBrief(HttpContext ctx, string action, (BriefStore.Outcome outcome, BriefEntity? entity) r)
|
||||||
|
{
|
||||||
app.Logger.LogInformation("brief {Action} outcome={Outcome} status={Status}",
|
app.Logger.LogInformation("brief {Action} outcome={Outcome} status={Status}",
|
||||||
action, r.outcome, r.entity?.Status.Tag ?? "-");
|
action, r.outcome, r.entity?.Status.Tag ?? "-");
|
||||||
|
AuditAuthz(ctx, "brief:" + action, "brief", r.outcome == BriefStore.Outcome.Ok, Authz.ResolvePrincipal(ctx));
|
||||||
|
}
|
||||||
|
|
||||||
// Audit + outcome for a submit, with NO personal data: only kind, outcome,
|
// Audit + outcome for a submit, with NO personal data: only kind, outcome,
|
||||||
// generated reference and the caller's correlation id (the observability seam — a
|
// generated reference and the caller's correlation id (the observability seam — a
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ using System.Net.Http.Json;
|
|||||||
using System.Text.RegularExpressions;
|
using System.Text.RegularExpressions;
|
||||||
using BigRegister.Api.Contracts;
|
using BigRegister.Api.Contracts;
|
||||||
using BigRegister.Api.Data;
|
using BigRegister.Api.Data;
|
||||||
|
using BigRegister.Domain.Features;
|
||||||
using Microsoft.AspNetCore.Mvc.Testing;
|
using Microsoft.AspNetCore.Mvc.Testing;
|
||||||
|
|
||||||
namespace BigRegister.Tests;
|
namespace BigRegister.Tests;
|
||||||
@@ -43,6 +44,43 @@ public class AuthzAuditTests(TestWebApplicationFactory factory) : IClassFixture<
|
|||||||
Assert.Contains(await AuditLog(), e => e.Action == "brief:reveal-bignummer");
|
Assert.Contains(await AuditLog(), e => e.Action == "brief:reveal-bignummer");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// RB-07/BIO-007: the trail used to record only denials, so `/beheer/audit` could answer
|
||||||
|
/// "who was turned away" but not "who changed this" — for a register whose integrity is the
|
||||||
|
/// product, the wrong half. Every gate now audits the real decision.
|
||||||
|
[Fact]
|
||||||
|
public async Task An_allowed_admin_action_is_recorded()
|
||||||
|
{
|
||||||
|
(await _client.SendAsync(Admin(HttpMethod.Get, "/api/v1/admin/cases"))).EnsureSuccessStatusCode();
|
||||||
|
Assert.Contains(await AuditLog(), e => e.Action == "cases:manage" && e.Decision == "allow" && e.Role == "Admin");
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The flag toggle writes no log line of its own, so the audit row is the only record that
|
||||||
|
/// it happened — a bare "feature-flags" resource would not say which flag.
|
||||||
|
[Fact]
|
||||||
|
public async Task A_feature_flag_toggle_records_which_flag_changed()
|
||||||
|
{
|
||||||
|
var toggle = Admin(HttpMethod.Put, $"/api/v1/admin/flags/{FeatureFlags.InschrijvingOpen}");
|
||||||
|
toggle.Content = JsonContent.Create(new { enabled = false });
|
||||||
|
(await _client.SendAsync(toggle)).EnsureSuccessStatusCode();
|
||||||
|
|
||||||
|
Assert.Contains(await AuditLog(), e =>
|
||||||
|
e.Action == "flags:manage" && e.Decision == "allow" &&
|
||||||
|
e.Resource == $"feature-flags/{FeatureFlags.InschrijvingOpen}=False");
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Every brief transition funnels through LogBrief, so all four are covered by the audit
|
||||||
|
/// call living there. The allow side is asserted in
|
||||||
|
/// <c>BriefEndpointTests.Submit_succeeds_when_required_sections_filled</c>, which already has
|
||||||
|
/// the fill-the-sections scaffolding; this is the refused side — a rejected transition must
|
||||||
|
/// leave a row rather than being dropped.
|
||||||
|
[Fact]
|
||||||
|
public async Task A_refused_brief_transition_is_recorded()
|
||||||
|
{
|
||||||
|
// No brief exists for this subject and nothing is filled in → illegal transition.
|
||||||
|
Assert.Equal(HttpStatusCode.Conflict, (await _client.PostAsync("/api/v1/brief/submit", null)).StatusCode);
|
||||||
|
Assert.Contains(await AuditLog(), e => e.Action == "brief:submit" && e.Decision == "deny");
|
||||||
|
}
|
||||||
|
|
||||||
/// RB-02/BIO-008: the schema test below asserts on **column names**, so a BSN inside a
|
/// 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
|
/// column called `Resource` was invisible to it — and one was there, concatenated as
|
||||||
/// `"brief/" + Bsn`. This asserts on the stored **values** instead. Four documents
|
/// `"brief/" + Bsn`. This asserts on the stored **values** instead. Four documents
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ using System.Net;
|
|||||||
using System.Net.Http.Headers;
|
using System.Net.Http.Headers;
|
||||||
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;
|
||||||
@@ -150,6 +151,12 @@ public class BeoordelingTests(TestWebApplicationFactory factory) : IClassFixture
|
|||||||
var view = (await detail.Content.ReadFromJsonAsync<BeoordelingViewDto>())!;
|
var view = (await detail.Content.ReadFromJsonAsync<BeoordelingViewDto>())!;
|
||||||
Assert.Equal("Goedgekeurd", view.Aanvraag.Status.Tag);
|
Assert.Equal("Goedgekeurd", view.Aanvraag.Status.Tag);
|
||||||
Assert.False(view.Decisions.CanBesluiten); // terminal — no further decision allowed
|
Assert.False(view.Decisions.CanBesluiten); // terminal — no further decision allowed
|
||||||
|
|
||||||
|
// RB-07/BIO-007: the gate records that a behandelaar was allowed to act; this records
|
||||||
|
// what they decided, which is the question /beheer/audit exists to answer.
|
||||||
|
Assert.Contains(AuthzAuditStore.List(), e =>
|
||||||
|
e.Action == "aanvraag:besluit" && e.Decision == "allow" &&
|
||||||
|
e.Resource == $"aanvraag/{a.Id}/Goedkeuren");
|
||||||
}
|
}
|
||||||
finally
|
finally
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -160,6 +160,11 @@ public class BriefEndpointTests(TestWebApplicationFactory factory) : IClassFixtu
|
|||||||
var submitted = await res.Content.ReadFromJsonAsync<BriefViewDto>();
|
var submitted = await res.Content.ReadFromJsonAsync<BriefViewDto>();
|
||||||
Assert.NotNull(submitted);
|
Assert.NotNull(submitted);
|
||||||
Assert.Equal("submitted", submitted.Brief.Status.Tag);
|
Assert.Equal("submitted", submitted.Brief.Status.Tag);
|
||||||
|
|
||||||
|
// RB-07/BIO-007: the allow side of the transition leaves a row, not just a log line.
|
||||||
|
// Resource is the bare "brief" — never the owner's BSN (RB-02).
|
||||||
|
Assert.Contains(AuthzAuditStore.List(),
|
||||||
|
e => e.Action == "brief:submit" && e.Decision == "allow" && e.Resource == "brief");
|
||||||
}
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
|
|||||||
@@ -0,0 +1,63 @@
|
|||||||
|
# RB-07 — audit the allow path, not just the denial
|
||||||
|
|
||||||
|
Status: **implemented** · 2026-08-27 · Source findings: `07-bio2-compliance.md` BIO-007 (+ the outstanding half of CQ-004) · `99-backlog.md` RB-07
|
||||||
|
|
||||||
|
## What was wrong
|
||||||
|
|
||||||
|
All five authorization gates called `AuditAuthz(..., allowed: false, ...)` only on the deny
|
||||||
|
branch; the allow branch called `action()` and returned. So `/beheer/audit` — the queryable
|
||||||
|
trail the product ships as its audit surface — could answer "who was turned away" but never
|
||||||
|
"who changed this".
|
||||||
|
|
||||||
|
Nothing recorded: `PUT /admin/flags/{key}`, `PUT /admin/org-template/{subOrgId}`,
|
||||||
|
`POST /admin/org-template/{subOrgId}/rollback/{version}`, `DELETE /admin/cases/{id}`,
|
||||||
|
`DELETE /admin/uploads/{documentId}`, `POST /brief/approve|reject|send`, and
|
||||||
|
`POST /beoordeling/{id}/besluit`. The comment above `OrgAdmin` claimed the endpoints logged
|
||||||
|
their own effect instead; publish and admin case delete do, the other six did not log at all.
|
||||||
|
|
||||||
|
## What changed
|
||||||
|
|
||||||
|
| File | Change |
|
||||||
|
| ------------------------- | ----------------------------------------------------------------------------- |
|
||||||
|
| `Program.cs` × 5 gates | `var ok = Authz.CanX(p); AuditAuthz(ctx, …, ok, p); if (ok) return action();` |
|
||||||
|
| `Program.cs` `FlagsAdmin` | takes a per-call `resource` (see below) |
|
||||||
|
| `Program.cs` `LogBrief` | takes `HttpContext`, writes the audit row alongside the log line |
|
||||||
|
| `Program.cs` besluit | one `aanvraag:besluit` row recording **what** was decided |
|
||||||
|
| `AuthzAuditTests.cs` | allow-path row; the flag key + value; a refused brief transition |
|
||||||
|
| `BriefEndpointTests.cs` | the allow side of `brief:submit` |
|
||||||
|
| `BeoordelingTests.cs` | the `aanvraag:besluit` row |
|
||||||
|
|
||||||
|
**The row is written by the gate, not the endpoint.** That is the point: a new admin
|
||||||
|
endpoint cannot be added that forgets to audit itself. Same reasoning for the brief — every
|
||||||
|
transition already funnelled through `LogBrief` for its log line, so the audit call went
|
||||||
|
there too, which covers `submit`/`approve`/`reject`/`send` in one place and any fifth
|
||||||
|
transition automatically. The decision recorded is the transition's own outcome, so a 403 or
|
||||||
|
a 409 is as visible as a success.
|
||||||
|
|
||||||
|
**`FlagsAdmin` gained a `resource` parameter** — the one deviation from BIO-007's minimal
|
||||||
|
remediation, and the reason is in the finding itself: the toggle endpoint writes no log line
|
||||||
|
of its own, so a constant `"feature-flags"` row would record that a flag changed without
|
||||||
|
recording _which_. It now writes `feature-flags/<key>=<value>`. One call site.
|
||||||
|
`OrgAdmin`/`CasesAdmin` keep their coarse refs because those endpoints do log the specific
|
||||||
|
object; **that asymmetry is deliberate, not an oversight.**
|
||||||
|
|
||||||
|
**The besluit gets a second row.** The `Beoordelen` gate records that a behandelaar was
|
||||||
|
_allowed to act_; `aanvraag:besluit` records _what they decided_
|
||||||
|
(`aanvraag/<id>/Goedkeuren`). Only the first would leave "who rejected this aanvraag"
|
||||||
|
unanswerable, which is the question the trail exists for.
|
||||||
|
|
||||||
|
## Consequences worth knowing
|
||||||
|
|
||||||
|
- **Row volume goes up.** `StamdataAdmin` gates read endpoints, so every admin page load now
|
||||||
|
writes rows. That is what "audit the allow path" means and BIO-007 asks for it explicitly;
|
||||||
|
if `AuthzAuditStore` ever needs retention or sampling, this is the change that made it
|
||||||
|
necessary.
|
||||||
|
- **This unblocks ADR-C-009.** Clause (4) of agent 06's four-part test is "writes are
|
||||||
|
admin-capability-gated **and** audited". Both surfaces now are, so the amendment can be
|
||||||
|
signed without ratifying a control the code does not implement.
|
||||||
|
- **CQ-004's outstanding half is closed.** `PUT /admin/flags/{key}` writes an audit row.
|
||||||
|
|
||||||
|
## Verification
|
||||||
|
|
||||||
|
`dotnet test`: **252 passed, 1 failed** — the pre-existing
|
||||||
|
`OpenZaakIntegrationTests.Admin_cases_…`, which needs a live container.
|
||||||
Reference in New Issue
Block a user