mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-14 06:40:50 +00:00
fix(labs): resolve biomarker delete through the record-scoped labs authority
DELETE /api/biomarkers/{id} used a bare requireAuth() while the PUT beside
it requires requireRecordAuth("manage", "labs"). The delete now uses the
same authority and, as a manage-level hard delete, files what it destroyed
(name, unit, reading count) through destroyedDetails.
This commit is contained in:
1 parent
b4dc0d5927
commit
5d6216a59f
5 files changed
+171
-14
No files matched your search
@@ -0,0 +1,101 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { NextRequest } from "next/server";
|
||||
|
||||
/**
|
||||
* `DELETE /api/biomarkers/{id}` takes a marker together with every reading
|
||||
* filed under it. It must resolve its record through the same record-scoped
|
||||
* labs authority as the PUT beside it — `requireRecordAuth("manage", "labs")`
|
||||
* — rather than the bare `requireAuth()` it carried before, so the delete is
|
||||
* held to the labs domain, the session fence, and the grant level like every
|
||||
* other labs write.
|
||||
*/
|
||||
|
||||
const requireAuth = vi.fn();
|
||||
const requireRecordAuth = vi.fn();
|
||||
|
||||
vi.mock("@/lib/api-handler", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("@/lib/api-handler")>();
|
||||
return {
|
||||
...actual,
|
||||
requireAuth: (...args: unknown[]) => requireAuth(...args),
|
||||
requireRecordAuth: (...args: unknown[]) => requireRecordAuth(...args),
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock("@/lib/db", () => ({
|
||||
prisma: {
|
||||
biomarker: { findFirst: vi.fn(), delete: vi.fn() },
|
||||
labResult: { deleteMany: vi.fn() },
|
||||
$transaction: vi.fn(async (ops: Promise<unknown>[]) => Promise.all(ops)),
|
||||
},
|
||||
}));
|
||||
vi.mock("@/lib/auth/audit", () => ({
|
||||
auditLog: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() }));
|
||||
vi.mock("@/lib/db-compat", () => ({
|
||||
ensureDbCompatibility: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
vi.mock("@/lib/cache/invalidate", () => ({
|
||||
invalidateUserHealthScore: vi.fn(),
|
||||
}));
|
||||
|
||||
import { DELETE } from "../route";
|
||||
import { HttpError } from "@/lib/api-handler";
|
||||
import { prisma } from "@/lib/db";
|
||||
|
||||
const RECORD_USER = { id: "record-1", username: "r", role: "USER" };
|
||||
|
||||
function delReq(): NextRequest {
|
||||
return new NextRequest("http://localhost/api/biomarkers/bm-1", {
|
||||
method: "DELETE",
|
||||
});
|
||||
}
|
||||
const params = { params: Promise.resolve({ id: "bm-1" }) };
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
requireAuth.mockRejectedValue(
|
||||
new HttpError(403, "Not available for this record"),
|
||||
);
|
||||
requireRecordAuth.mockResolvedValue({
|
||||
user: RECORD_USER,
|
||||
actor: { id: "actor-1" },
|
||||
grantId: "grant-1",
|
||||
authMethod: "cookie",
|
||||
});
|
||||
vi.mocked(prisma.biomarker.findFirst).mockResolvedValue({
|
||||
id: "bm-1",
|
||||
userId: "record-1",
|
||||
} as never);
|
||||
vi.mocked(prisma.labResult.deleteMany).mockResolvedValue({
|
||||
count: 2,
|
||||
} as never);
|
||||
vi.mocked(prisma.biomarker.delete).mockResolvedValue({} as never);
|
||||
});
|
||||
|
||||
describe("DELETE /api/biomarkers/[id] record authority", () => {
|
||||
it("resolves the record through requireRecordAuth('manage', 'labs')", async () => {
|
||||
const res = await DELETE(delReq(), params);
|
||||
expect(res.status).toBe(200);
|
||||
expect(requireRecordAuth).toHaveBeenCalledWith("manage", "labs");
|
||||
expect(requireAuth).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("narrows both delete legs to the resolved record", async () => {
|
||||
await DELETE(delReq(), params);
|
||||
expect(prisma.labResult.deleteMany).toHaveBeenCalledWith({
|
||||
where: { userId: "record-1", biomarkerId: "bm-1" },
|
||||
});
|
||||
});
|
||||
|
||||
it("refuses when the record authority refuses, deleting nothing", async () => {
|
||||
requireRecordAuth.mockRejectedValue(
|
||||
new HttpError(403, "Not available for this record"),
|
||||
);
|
||||
const res = await DELETE(delReq(), params);
|
||||
expect(res.status).toBe(403);
|
||||
expect(prisma.labResult.deleteMany).not.toHaveBeenCalled();
|
||||
expect(prisma.biomarker.delete).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -1,6 +1,6 @@
|
||||
import { NextRequest } from "next/server";
|
||||
|
||||
import { apiHandler, requireAuth, requireRecordAuth } from "@/lib/api-handler";
|
||||
import { apiHandler, requireRecordAuth } from "@/lib/api-handler";
|
||||
import {
|
||||
apiError,
|
||||
apiSuccess,
|
||||
@@ -10,7 +10,10 @@ import {
|
||||
sanitiseZodIssues,
|
||||
} from "@/lib/api-response";
|
||||
import { auditLog } from "@/lib/auth/audit";
|
||||
import { overwriteDetails } from "@/lib/sharing/audit-details";
|
||||
import {
|
||||
destroyedDetails,
|
||||
overwriteDetails,
|
||||
} from "@/lib/sharing/audit-details";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { invalidateUserHealthScore } from "@/lib/cache/invalidate";
|
||||
import {
|
||||
@@ -90,8 +93,8 @@ export const PUT = apiHandler(
|
||||
// v1.37.0 — MANAGE, and the closest call in this domain: a reference range
|
||||
// recolours every reading under it, so this is the one admitted edit whose
|
||||
// error is silent rather than visible. C4 on the audit row is what puts it
|
||||
// in the owner's feed. The DELETE below stays refused — it takes the
|
||||
// readings with it and files an id.
|
||||
// in the owner's feed. The DELETE below resolves through the same labs
|
||||
// authority; it takes the readings with it and files the id.
|
||||
const { user } = await requireRecordAuth("manage", "labs");
|
||||
const { id } = await params;
|
||||
|
||||
@@ -254,7 +257,10 @@ export const PUT = apiHandler(
|
||||
|
||||
export const DELETE = apiHandler(
|
||||
async (request: NextRequest, { params }: RouteParams) => {
|
||||
const { user } = await requireAuth();
|
||||
// The same record-scoped labs authority as the PUT above. A bare
|
||||
// `requireAuth()` here held the one destructive biomarker write to a
|
||||
// weaker gate than the edit beside it: no labs domain, no grant level.
|
||||
const { user } = await requireRecordAuth("manage", "labs");
|
||||
const { id } = await params;
|
||||
|
||||
const existing = await prisma.biomarker.findFirst({ where: { id } });
|
||||
@@ -267,7 +273,7 @@ export const DELETE = apiHandler(
|
||||
// readings go with it rather than lingering as orphaned, unlinked rows.
|
||||
// Both legs are userId-narrowed so a cross-user id can never reach another
|
||||
// account's data.
|
||||
await prisma.$transaction([
|
||||
const [removedReadings] = await prisma.$transaction([
|
||||
prisma.labResult.deleteMany({
|
||||
where: { userId: user.id, biomarkerId: id },
|
||||
}),
|
||||
@@ -278,7 +284,22 @@ export const DELETE = apiHandler(
|
||||
await auditLog("biomarker.delete", {
|
||||
userId: user.id,
|
||||
ipAddress: getClientIp(request),
|
||||
details: { biomarkerId: id },
|
||||
// C3: a hard delete files what it destroyed — the marker's name, unit,
|
||||
// and how many readings went with it — so the owner's feed can say what
|
||||
// a manager removed, not just an id that now points at nothing.
|
||||
details: {
|
||||
biomarkerId: id,
|
||||
...destroyedDetails({
|
||||
model: "Biomarker",
|
||||
id,
|
||||
label: existing.name,
|
||||
effectiveAt: existing.createdAt,
|
||||
extra: {
|
||||
unit: existing.unit,
|
||||
readingCount: removedReadings?.count ?? 0,
|
||||
},
|
||||
}),
|
||||
},
|
||||
});
|
||||
|
||||
annotate({
|
||||
|
||||
Reference in new issue
Block a user