robbond/pedwfrontend
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(security): protect representation blob deletion against CSRF

Browse Source
This commit is contained in:
robbond
2026-11-09 09:31:41 +00:00
parent 4eba824ac2
commit fef9591b5d
3 changed files with 169 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files
actions/services/documentDirectService.js
+1 -1
View File
@@ -134,7 +134,7 @@ export const deleteMyRepresentationsFromBlob = (
repfile
});
return getSignedFileJson(queryUrl).catch((error) => {
return postSignedFileJson(queryUrl).catch((error) => {
consoleLogger(error);
});
};
pages/api/file/deleteblobrep.js
+48 -4
View File
@@ -5,10 +5,17 @@ import { getServerSession } from "next-auth/next";
import { authOptions } from "../auth/[...nextauth]";
import nextConnect from "next-connect";
import middleware from "../middleware/middleware";
const ApiProxy = nextConnect();
ApiProxy.use(middleware);
const methodNotAllowed = (req, res) => {
res.setHeader("Allow", "POST");
return respondError(res, {
status: 405,
code: "METHOD_NOT_ALLOWED",
message: "Method not allowed"
});
};
const ApiProxy = nextConnect({ onNoMatch: methodNotAllowed });
const forbidden = (res) =>
respondError(res, {
@@ -26,7 +33,44 @@ const isValidPathSegment = (value) =>
value !== ".." &&
!/[\\/\u0000-\u001f\u007f]/.test(value);
ApiProxy.get(async (req, res) => {
const hasTrustedOrigin = (req) => {
const requestOrigin = req?.headers?.origin;
if (typeof requestOrigin !== "string" || requestOrigin === "null") {
return false;
}
try {
const requestUrl = new URL(requestOrigin);
if (requestOrigin !== requestUrl.origin) {
return false;
}
const normalizedRequestOrigin = requestUrl.origin;
const trustedOrigins = [
process.env.NEXTAUTH_URL,
process.env.CY_API_ROOT
]
.filter((configuredUrl) =>
typeof configuredUrl === "string" && configuredUrl.length > 0
)
.map((configuredUrl) => new URL(configuredUrl).origin);
return trustedOrigins.includes(normalizedRequestOrigin);
} catch (_error) {
return false;
}
};
ApiProxy.post(async (req, res) => {
if (req.method !== "POST") {
return methodNotAllowed(req, res);
}
if (!hasTrustedOrigin(req)) {
return forbidden(res);
}
const containerName = req.query.container;
const casefolderID = req.query.casefolderID;
const repfile = req.query.repfile;
tests/phase21/file-handler-contract.test.cjs
+120 -7
View File
@@ -33,6 +33,18 @@ const validCaseDeletionRequest = (overrides = {}) => ({
...overrides
});
const validRepDeletionRequest = (overrides = {}) => ({
method: "POST",
headers: { origin: TRUSTED_EN_ORIGIN },
query: {
container: "c1",
casefolderID: "CASE-1",
repfile: "representation.json",
hash: "expected"
},
...overrides
});
const loadDocumentDirectService = (injected = {}) => {
const sourcePath = path.resolve(
__dirname,
@@ -586,6 +598,7 @@ test("deleteblobrep handler accepts encoded hash variant", async () => {
: "&hash=other",
respondError: respondErrorMock,
respondSuccess: respondSuccessMock,
process: deletionTestProcess,
deleteBlobRep: async (_container, repPath) => {
deletedRepPath = repPath;
return { deleted: true };
@@ -593,6 +606,8 @@ test("deleteblobrep handler accepts encoded hash variant", async () => {
});
const req = {
method: "POST",
headers: { origin: TRUSTED_EN_ORIGIN },
query: {
container: "c1",
casefolderID: "CASE-1",
@@ -624,6 +639,7 @@ test("deleteblobrep rejects missing or unsafe folder segments before deletion",
hashAPIPath: () => "&hash=expected",
respondError: respondErrorMock,
respondSuccess: respondSuccessMock,
process: deletionTestProcess,
deleteBlobRep: async () => {
deletionCalls += 1;
}
@@ -632,6 +648,8 @@ test("deleteblobrep rejects missing or unsafe folder segments before deletion",
await mod.default.handler(
{
method: "POST",
headers: { origin: TRUSTED_EN_ORIGIN },
query: {
container: "c1",
hash: "expected",
@@ -656,12 +674,15 @@ test("deleteblobrep handler dependency failure returns DELETE_BLOB_REP_FAILED",
hashAPIPath: () => "&hash=expected",
respondError: respondErrorMock,
respondSuccess: respondSuccessMock,
process: deletionTestProcess,
deleteBlobRep: async () => {
throw new Error("delete rep failed");
}
});
const req = {
method: "POST",
headers: { origin: TRUSTED_EN_ORIGIN },
query: {
container: "c1",
casefolderID: "f1",
@@ -676,6 +697,94 @@ test("deleteblobrep handler dependency failure returns DELETE_BLOB_REP_FAILED",
assert.strictEqual(res.state.jsonBody.error.code, "DELETE_BLOB_REP_FAILED");
});
const loadRepDeletionSecurity = (overrides = {}) => {
let deletionCalls = 0;
let sessionCalls = 0;
const mod = loadModule("pages/api/file/deleteblobrep.js", {
nextConnect: createNextConnectMock(),
getServerSession: async () => {
sessionCalls += 1;
return { user: { id: "c1" } };
},
authOptions: () => ({}),
hashAPIPath: () => "&hash=expected",
respondError: respondErrorMock,
respondSuccess: respondSuccessMock,
process: deletionTestProcess,
deleteBlobRep: async () => {
deletionCalls += 1;
return { deleted: true };
},
...overrides
});
return {
mod,
getDeletionCalls: () => deletionCalls,
getSessionCalls: () => sessionCalls
};
};
for (const method of ["GET", "DELETE"]) {
test(`deleteblobrep rejects ${method} with 405 before deletion`, async () => {
const loaded = loadRepDeletionSecurity();
const res = createRes();
await loaded.mod.default.handler(
validRepDeletionRequest({ method }),
res
);
assert.strictEqual(res.state.statusCode, 405);
assert.strictEqual(res.state.jsonBody.error.code, "METHOD_NOT_ALLOWED");
assert.strictEqual(res.state.headers.allow, "POST");
assert.strictEqual(loaded.getSessionCalls(), 0);
assert.strictEqual(loaded.getDeletionCalls(), 0);
});
}
for (const [name, origin] of [
["cross-origin", "https://attacker.example.test"],
["missing Origin", undefined],
["malformed Origin", "not a valid origin"],
["Origin containing a path", `${TRUSTED_EN_ORIGIN}/delete`],
["null Origin", "null"],
["subdomain spoof", "https://portal.example.test.attacker.test"],
["hostname suffix spoof", "https://attacker-portal.example.test"]
]) {
test(`deleteblobrep rejects ${name} before session or deletion`, async () => {
const loaded = loadRepDeletionSecurity();
const headers = origin === undefined ? {} : { origin };
const res = createRes();
await loaded.mod.default.handler(
validRepDeletionRequest({ headers }),
res
);
assert.strictEqual(res.state.statusCode, 403);
assert.strictEqual(res.state.jsonBody.error.code, "FORBIDDEN");
assert.strictEqual(loaded.getSessionCalls(), 0);
assert.strictEqual(loaded.getDeletionCalls(), 0);
});
}
test("deleteblobrep accepts the configured Welsh portal Origin", async () => {
const loaded = loadRepDeletionSecurity();
const res = createRes();
await loaded.mod.default.handler(
validRepDeletionRequest({
headers: { origin: TRUSTED_CY_ORIGIN }
}),
res
);
assert.strictEqual(res.state.statusCode, 200);
assert.strictEqual(loaded.getSessionCalls(), 1);
assert.strictEqual(loaded.getDeletionCalls(), 1);
});
const ordinaryDeletionCases = [
{
name: "deleteblobcase",
@@ -694,7 +803,10 @@ const ordinaryDeletionCases = [
{
name: "deleteblobrep",
route: "pages/api/file/deleteblobrep.js",
request: {},
request: {
method: "POST",
headers: { origin: TRUSTED_EN_ORIGIN }
},
query: {
container: "user-container",
casefolderID: "CASE-1",
@@ -1022,7 +1134,7 @@ test("admin storage caller uses dedicated POST deletion while ordinary caller re
assert.doesNotMatch(adminSource, /deleteMyRepresentationsFromBlob/);
assert.match(
ordinarySource,
/buildFileQuery\("\/api\/file\/deleteblobrep"[\s\S]*getSignedFileJson\(queryUrl\)/
/buildFileQuery\("\/api\/file\/deleteblobrep"[\s\S]*postSignedFileJson\(queryUrl\)/
);
});
@@ -1209,7 +1321,7 @@ test("admin case deletion dependency failure returns DELETE_BLOB_CASE_FAILED", a
assert.strictEqual(loaded.calls.deletion.length, 1);
});
test("case deletion uses signed POST while representation deletion remains signed GET", async () => {
test("case and representation deletion use signed POST", async () => {
const calls = [];
const mod = loadDocumentDirectService({
buildFileQuery: (route, params) => {
@@ -1233,12 +1345,9 @@ test("case deletion uses signed POST while representation deletion remains signe
await mod.deleteMyRepresentationsFromBlob("c1", "CASE-1", "REP-1");
assert.deepStrictEqual(JSON.parse(JSON.stringify(calls)), [
["post", "/api/file/deleteblobcase?container=c1&casefolderID=CASE-1"],
[
"post",
"/api/file/deleteblobcase?container=c1&casefolderID=CASE-1"
],
[
"get",
"/api/file/deleteblobrep?container=c1&casefolderID=CASE-1&repfile=REP-1"
]
]);
@@ -5933,6 +6042,10 @@ test("whole-representation deletion remains cross-user protected with zero Azure
await loaded.mod.default.handler(
{
method: "POST",
headers: {
origin: "http://localhost:3000"
},
query: {
container: "organisation-container",
casefolderID: "CASE-1",