fix(uploads): route the admin delete through CasesAdmin (RB-08)
DELETE /admin/uploads/{documentId} was gated by a standalone
`X-Admin: true` header check (`IsAdmin`), outside the `Authz` module
entirely and outside the `CasesAdmin`/`StamdataAdmin`/`OrgAdmin`/
`FlagsAdmin` wrappers the four sibling admin surfaces use. It wrote no
AuthzAuditStore row, so a destructive cross-owner document delete never
appeared on /beheer/audit. A repo-wide grep confirmed the only sender of
X-Admin was the backend test itself — no frontend or e2e path depends on
it — so the gate was safe to delete outright.
Routed the endpoint through CasesAdmin (Authz.CanManageCases), the same
wrapper the other admin-cases endpoints use. RB-07 already moved
AuditAuthz onto every *Admin wrapper's allow path, so this gets the
missing audit row for free with no second AuditAuthz call. Deleted the
now-unused IsAdmin function and updated the two comments that referenced
the old X-Admin seam.
Updated EndpointTests.cs's Admin_delete_requires_admin_role to send
X-Role: admin instead of X-Admin: true, and added
AuthzAuditTests.An_admin_upload_delete_is_recorded, which asserts the
cases:manage/allow row count increases by exactly one (a plain
Contains would already be satisfied by this test class's other
cases:manage calls). Verified both tests fail red against the
pre-fix gate.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -269,13 +269,14 @@ api.MapDelete("/uploads/{documentId}", (string documentId, HttpContext ctx) =>
|
||||
.ProducesProblem(StatusCodes.Status409Conflict)
|
||||
.Produces(StatusCodes.Status404NotFound);
|
||||
|
||||
// Admin delete (seam): a real system requires an admin role; here an X-Admin header
|
||||
// stands in. Bypasses ownership, unlinks, and flags the submission for review.
|
||||
api.MapDelete("/admin/uploads/{documentId}", (string documentId, HttpContext ctx) =>
|
||||
!IsAdmin(ctx) ? Results.StatusCode(StatusCodes.Status403Forbidden)
|
||||
: DocumentStore.AdminDelete(documentId, "admin") ? Results.NoContent() : Results.NotFound())
|
||||
// Admin delete: bypasses ownership, unlinks, and flags the submission for review. Gated
|
||||
// by the same CasesAdmin wrapper (cases:manage) the other admin-cases endpoints use
|
||||
// (RB-08/BIO-003) — it used to be gated by a standalone X-Admin header, outside Authz and
|
||||
// unaudited; CasesAdmin gives it the missing AuthzAuditStore row for free (RB-07).
|
||||
api.MapDelete("/admin/uploads/{documentId}", (string documentId, HttpContext ctx) => CasesAdmin(ctx, () =>
|
||||
DocumentStore.AdminDelete(documentId, "admin") ? Results.NoContent() : Results.NotFound()))
|
||||
.Produces(StatusCodes.Status204NoContent)
|
||||
.Produces(StatusCodes.Status403Forbidden)
|
||||
.ProducesProblem(StatusCodes.Status403Forbidden)
|
||||
.Produces(StatusCodes.Status404NotFound);
|
||||
|
||||
// --- Applications (aanvragen): the system of record the dashboard reads. ---
|
||||
@@ -611,8 +612,8 @@ api.MapPut("/admin/flags/{key}", (string key, SetFeatureFlagRequest req, HttpCon
|
||||
|
||||
// --- Brief (letter composition). One demo brief per owner; the server owns the
|
||||
// status machine + authorization (Authz, PRD-0002 phase P1). Principal is a
|
||||
// dev-only stand-in via X-Role (mirrors the X-Admin seam and the FE ?role=
|
||||
// toggle) — no real identities in this POC. ---
|
||||
// dev-only stand-in via X-Role (mirrors the FE ?role= toggle) — no real
|
||||
// identities in this POC. ---
|
||||
|
||||
api.MapGet("/brief", (HttpContext ctx) =>
|
||||
{
|
||||
@@ -787,8 +788,6 @@ api.MapPost("/admin/org-template/{subOrgId}/rollback/{version:int}", (string sub
|
||||
|
||||
app.Run();
|
||||
|
||||
static bool IsAdmin(HttpContext ctx) => ctx.Request.Headers["X-Admin"] == "true";
|
||||
|
||||
// One gate for every org-template endpoint — the enforce twin of the
|
||||
// `orgtemplate:edit` capability RoleCapabilities emits (single Authz source).
|
||||
//
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
using System.Net;
|
||||
using System.Net.Http.Headers;
|
||||
using System.Net.Http.Json;
|
||||
using System.Text.RegularExpressions;
|
||||
using BigRegister.Api.Contracts;
|
||||
@@ -27,6 +28,20 @@ public class AuthzAuditTests(TestWebApplicationFactory factory) : IClassFixture<
|
||||
return (await res.Content.ReadFromJsonAsync<List<AuthzAuditDto>>())!;
|
||||
}
|
||||
|
||||
private async Task<string> UploadAsOwner()
|
||||
{
|
||||
var form = new MultipartFormDataContent();
|
||||
var file = new ByteArrayContent(new byte[] { 1, 2, 3 });
|
||||
file.Headers.ContentType = new MediaTypeHeaderValue("application/pdf");
|
||||
form.Add(file, "file", "diploma.pdf");
|
||||
form.Add(new StringContent("diploma"), "categoryId");
|
||||
form.Add(new StringContent("local-rb08"), "localId");
|
||||
form.Add(new StringContent("registratie"), "wizardId");
|
||||
var res = await _client.PostAsync("/api/v1/uploads", form);
|
||||
res.EnsureSuccessStatusCode();
|
||||
return (await res.Content.ReadFromJsonAsync<UploadResponse>())!.DocumentId;
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task A_denied_admin_action_is_recorded()
|
||||
{
|
||||
@@ -54,6 +69,30 @@ public class AuthzAuditTests(TestWebApplicationFactory factory) : IClassFixture<
|
||||
Assert.Contains(await AuditLog(), e => e.Action == "cases:manage" && e.Decision == "allow" && e.Role == "Admin");
|
||||
}
|
||||
|
||||
/// RB-08/BIO-003: the admin upload delete used to be gated by a standalone X-Admin
|
||||
/// header, outside Authz and writing no AuthzAuditStore row at all. Routing it through
|
||||
/// CasesAdmin (cases:manage) gives it the same allow-path row every other admin-cases
|
||||
/// endpoint gets, for free, per RB-07. `CasesAdmin` audits under a fixed "cases"
|
||||
/// resource shared with the other admin-cases endpoints, so this asserts a **count**
|
||||
/// increase — reading the store directly (not via `GET /admin/audit`, itself a
|
||||
/// `CasesAdmin` endpoint that would write its own row and confound the count) —
|
||||
/// rather than mere presence, which this class's other cases:manage calls would
|
||||
/// already satisfy even without the fix.
|
||||
[Fact]
|
||||
public async Task An_admin_upload_delete_is_recorded()
|
||||
{
|
||||
bool IsCasesManageAllow(AuthzAuditEntry e) =>
|
||||
e.Action == "cases:manage" && e.Decision == "allow" && e.Role == "Admin";
|
||||
|
||||
var documentId = await UploadAsOwner();
|
||||
var before = AuthzAuditStore.List().Count(IsCasesManageAllow);
|
||||
|
||||
(await _client.SendAsync(Admin(HttpMethod.Delete, $"/api/v1/admin/uploads/{documentId}")))
|
||||
.EnsureSuccessStatusCode();
|
||||
|
||||
Assert.Equal(before + 1, AuthzAuditStore.List().Count(IsCasesManageAllow));
|
||||
}
|
||||
|
||||
/// 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]
|
||||
|
||||
@@ -213,11 +213,13 @@ public class EndpointTests(TestWebApplicationFactory factory) : IClassFixture<Te
|
||||
[Fact]
|
||||
public async Task Admin_delete_requires_admin_role()
|
||||
{
|
||||
// RB-08: routed through CasesAdmin (cases:manage), like the other admin-cases
|
||||
// endpoints, not the standalone X-Admin header this used to accept.
|
||||
var doc = await Upload(Guid.NewGuid().ToString());
|
||||
Assert.Equal(HttpStatusCode.Forbidden, (await _client.DeleteAsync($"/api/v1/admin/uploads/{doc.DocumentId}")).StatusCode);
|
||||
|
||||
var req = new HttpRequestMessage(HttpMethod.Delete, $"/api/v1/admin/uploads/{doc.DocumentId}");
|
||||
req.Headers.Add("X-Admin", "true");
|
||||
req.Headers.Add("X-Role", "admin");
|
||||
Assert.Equal(HttpStatusCode.NoContent, (await _client.SendAsync(req)).StatusCode);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user