diff --git a/actions/services/documentDirectService.js b/actions/services/documentDirectService.js index 77ae6479..4a5e986a 100644 --- a/actions/services/documentDirectService.js +++ b/actions/services/documentDirectService.js @@ -118,7 +118,7 @@ export const deleteAwaitingSubmissionsFromBlob = ( casefolderID }); - return getSignedFileJson(queryUrl).catch((error) => { + return postSignedFileJson(queryUrl).catch((error) => { consoleLogger(error); }); }; diff --git a/components/myportal/awaitingsubmissionfromblob.js b/components/myportal/awaitingsubmissionfromblob.js index ce22c602..93f8b5e2 100644 --- a/components/myportal/awaitingsubmissionfromblob.js +++ b/components/myportal/awaitingsubmissionfromblob.js @@ -70,7 +70,7 @@ const AwaitingSubmissionFromBlob = (props) => { const deleteCaseItem = (containerID, caseID) => { let cookies = parseCookies(); - console.log("sssss", containerID, caseID); + //console.log("sssss", containerID, caseID); deleteAwaitingSubmissionsFromBlob(containerID, caseID) .then((data) => { diff --git a/pages/api/file/deleteblobcase.js b/pages/api/file/deleteblobcase.js index 8fbf9408..e79cdb5a 100644 --- a/pages/api/file/deleteblobcase.js +++ b/pages/api/file/deleteblobcase.js @@ -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 checkHash = req.query.hash; diff --git a/tests/phase21/file-handler-contract.test.cjs b/tests/phase21/file-handler-contract.test.cjs index 66caa1f7..c9b0b057 100644 --- a/tests/phase21/file-handler-contract.test.cjs +++ b/tests/phase21/file-handler-contract.test.cjs @@ -1,6 +1,7 @@ const assert = require("assert"); const fs = require("fs"); const path = require("path"); +const vm = require("vm"); const { loadModule, createRes, @@ -12,6 +13,53 @@ const { const tests = []; const test = (name, fn) => tests.push({ name, fn }); +const TRUSTED_EN_ORIGIN = "https://portal.example.test"; +const TRUSTED_CY_ORIGIN = "https://porth.example.test"; +const deletionTestProcess = { + env: { + NEXTAUTH_URL: TRUSTED_EN_ORIGIN, + CY_API_ROOT: TRUSTED_CY_ORIGIN + } +}; + +const validCaseDeletionRequest = (overrides = {}) => ({ + method: "POST", + headers: { origin: TRUSTED_EN_ORIGIN }, + query: { + container: "c1", + casefolderID: "CASE-SUB", + hash: "expected" + }, + ...overrides +}); + +const loadDocumentDirectService = (injected = {}) => { + const sourcePath = path.resolve( + __dirname, + "..", + "..", + "actions/services/documentDirectService.js" + ); + let source = fs.readFileSync(sourcePath, "utf8"); + + source = source.replace(/import[\s\S]*?from\s+"[^"]+";\n?/g, ""); + source = source.replace(/export const\s+/g, "const "); + source += ` +module.exports = { + deleteAwaitingSubmissionsFromBlob, + deleteMyRepresentationsFromBlob +};`; + + const context = { + module: { exports: {} }, + exports: {}, + ...injected + }; + + vm.runInNewContext(source, context, { filename: sourcePath }); + return context.module.exports; +}; + test("upload handler returns HASH_REQUIRED for missing hash", async () => { const mod = loadModule("pages/api/file/upload.js", { nextConnect: createNextConnectMock(), @@ -351,19 +399,14 @@ test("deleteblobcase handler accepts a valid case folder", async () => { : "&hash=other", respondError: respondErrorMock, respondSuccess: respondSuccessMock, + process: deletionTestProcess, deleteBlobCase: async (_container, casefolder) => { deletedCaseFolder = casefolder; return { deleted: true }; } }); - const req = { - query: { - container: "c1", - casefolderID: "CASE-SUB", - hash: "expected" - } - }; + const req = validCaseDeletionRequest(); const res = createRes(); await mod.default.handler(req, res); @@ -382,6 +425,7 @@ test("deleteblobcase rejects unsafe normalized folder identifiers before deletio hashAPIPath: () => "&hash=expected", respondError: respondErrorMock, respondSuccess: respondSuccessMock, + process: deletionTestProcess, deleteBlobCase: async () => { deletionCalls += 1; } @@ -390,6 +434,8 @@ test("deleteblobcase rejects unsafe normalized folder identifiers before deletio await mod.default.handler( { + method: "POST", + headers: { origin: TRUSTED_EN_ORIGIN }, query: { container: "c1", casefolderID, @@ -414,12 +460,15 @@ test("deleteblobcase handler dependency failure returns DELETE_BLOB_CASE_FAILED" hashAPIPath: () => "&hash=expected", respondError: respondErrorMock, respondSuccess: respondSuccessMock, + process: deletionTestProcess, deleteBlobCase: async () => { throw new Error("delete case failed"); } }); const req = { + method: "POST", + headers: { origin: TRUSTED_EN_ORIGIN }, query: { container: "c1", casefolderID: "f1", @@ -436,6 +485,94 @@ test("deleteblobcase handler dependency failure returns DELETE_BLOB_CASE_FAILED" ); }); +const loadCaseDeletionSecurity = (overrides = {}) => { + let deletionCalls = 0; + let sessionCalls = 0; + const mod = loadModule("pages/api/file/deleteblobcase.js", { + nextConnect: createNextConnectMock(), + getServerSession: async () => { + sessionCalls += 1; + return { user: { id: "c1" } }; + }, + authOptions: () => ({}), + hashAPIPath: () => "&hash=expected", + respondError: respondErrorMock, + respondSuccess: respondSuccessMock, + process: deletionTestProcess, + deleteBlobCase: async () => { + deletionCalls += 1; + return { deleted: true }; + }, + ...overrides + }); + + return { + mod, + getDeletionCalls: () => deletionCalls, + getSessionCalls: () => sessionCalls + }; +}; + +for (const method of ["GET", "DELETE"]) { + test(`deleteblobcase rejects ${method} with 405 before deletion`, async () => { + const loaded = loadCaseDeletionSecurity(); + const res = createRes(); + + await loaded.mod.default.handler( + validCaseDeletionRequest({ 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(`deleteblobcase rejects ${name} before session or deletion`, async () => { + const loaded = loadCaseDeletionSecurity(); + const headers = origin === undefined ? {} : { origin }; + const res = createRes(); + + await loaded.mod.default.handler( + validCaseDeletionRequest({ 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("deleteblobcase accepts the configured Welsh portal Origin", async () => { + const loaded = loadCaseDeletionSecurity(); + const res = createRes(); + + await loaded.mod.default.handler( + validCaseDeletionRequest({ + headers: { origin: TRUSTED_CY_ORIGIN } + }), + res + ); + + assert.strictEqual(res.state.statusCode, 200); + assert.strictEqual(loaded.getSessionCalls(), 1); + assert.strictEqual(loaded.getDeletionCalls(), 1); +}); + test("deleteblobrep handler accepts encoded hash variant", async () => { let deletedRepPath = null; const mod = loadModule("pages/api/file/deleteblobrep.js", { @@ -543,6 +680,10 @@ const ordinaryDeletionCases = [ { name: "deleteblobcase", route: "pages/api/file/deleteblobcase.js", + request: { + method: "POST", + headers: { origin: TRUSTED_EN_ORIGIN } + }, query: { container: "user-container", casefolderID: "CASE-1", @@ -553,6 +694,7 @@ const ordinaryDeletionCases = [ { name: "deleteblobrep", route: "pages/api/file/deleteblobrep.js", + request: {}, query: { container: "user-container", casefolderID: "CASE-1", @@ -574,13 +716,17 @@ for (const deletionCase of ordinaryDeletionCases) { hashAPIPath: () => "&hash=expected", respondError: respondErrorMock, respondSuccess: respondSuccessMock, + process: deletionTestProcess, [deletionCase.dependency]: async () => { deletionCalls += 1; } }); const res = createRes(); - await mod.default.handler({ query: deletionCase.query }, res); + await mod.default.handler( + { ...deletionCase.request, query: deletionCase.query }, + res + ); assert.strictEqual(res.state.statusCode, 403); assert.strictEqual(res.state.jsonBody.error.code, "FORBIDDEN"); @@ -597,13 +743,17 @@ for (const deletionCase of ordinaryDeletionCases) { hashAPIPath: () => "&hash=expected", respondError: respondErrorMock, respondSuccess: respondSuccessMock, + process: deletionTestProcess, [deletionCase.dependency]: async () => { deletionCalls += 1; } }); const res = createRes(); - await mod.default.handler({ query: deletionCase.query }, res); + await mod.default.handler( + { ...deletionCase.request, query: deletionCase.query }, + res + ); assert.strictEqual(res.state.statusCode, 403); assert.strictEqual(deletionCalls, 0); @@ -623,13 +773,17 @@ for (const deletionCase of ordinaryDeletionCases) { }, respondError: respondErrorMock, respondSuccess: respondSuccessMock, + process: deletionTestProcess, [deletionCase.dependency]: async () => { deletionCalls += 1; } }); const res = createRes(); - await mod.default.handler({ query: deletionCase.query }, res); + await mod.default.handler( + { ...deletionCase.request, query: deletionCase.query }, + res + ); assert.strictEqual(res.state.statusCode, 403); assert.strictEqual(hashCalls, 0); @@ -654,9 +808,11 @@ for (const deletionCase of ordinaryDeletionCases) { hashAPIPath: () => "&hash=expected", respondError: respondErrorMock, respondSuccess: respondSuccessMock, + process: deletionTestProcess, [deletionCase.dependency]: async () => ({ deleted: true }) }); const req = { + ...deletionCase.request, query: deletionCase.query, body: { parsed: ["multipart"] } }; @@ -682,13 +838,17 @@ for (const deletionCase of ordinaryDeletionCases) { hashAPIPath: () => "&hash=different", respondError: respondErrorMock, respondSuccess: respondSuccessMock, + process: deletionTestProcess, [deletionCase.dependency]: async () => { deletionCalls += 1; } }); const res = createRes(); - await mod.default.handler({ query: deletionCase.query }, res); + await mod.default.handler( + { ...deletionCase.request, query: deletionCase.query }, + res + ); assert.strictEqual(res.state.statusCode, 400); assert.strictEqual(res.state.jsonBody.error.code, "INVALID_HASH"); @@ -705,6 +865,7 @@ for (const deletionCase of ordinaryDeletionCases) { hashAPIPath: () => "&hash=expected", respondError: respondErrorMock, respondSuccess: respondSuccessMock, + process: deletionTestProcess, [deletionCase.dependency]: async (...args) => { deletionCalls.push(args); return { deleted: true }; @@ -712,7 +873,10 @@ for (const deletionCase of ordinaryDeletionCases) { }); const res = createRes(); - await mod.default.handler({ query: deletionCase.query }, res); + await mod.default.handler( + { ...deletionCase.request, query: deletionCase.query }, + res + ); assert.strictEqual(res.state.statusCode, 200); assert.strictEqual(deletionCalls.length, 1); @@ -1045,28 +1209,39 @@ test("admin case deletion dependency failure returns DELETE_BLOB_CASE_FAILED", a assert.strictEqual(loaded.calls.deletion.length, 1); }); -test("admin storage case deletion uses dedicated POST route while ordinary callers remain signed GET", () => { - const rootDir = path.resolve(__dirname, "..", ".."); - const adminSource = fs.readFileSync( - path.join(rootDir, "components/admin/tabs/storage.js"), - "utf8" - ); - const ordinarySource = fs.readFileSync( - path.join(rootDir, "actions/services/documentDirectService.js"), - "utf8" - ); +test("case deletion uses signed POST while representation deletion remains signed GET", async () => { + const calls = []; + const mod = loadDocumentDirectService({ + buildFileQuery: (route, params) => { + const query = Object.entries(params) + .map(([key, value]) => `${key}=${value}`) + .join("&"); + return `${route}?${query}`; + }, + postSignedFileJson: async (...args) => { + calls.push(["post", ...args]); + return { deleted: true }; + }, + getSignedFileJson: async (...args) => { + calls.push(["get", ...args]); + return { deleted: true }; + }, + consoleLogger: () => {} + }); - assert.match(adminSource, /fetch\(\s*"\/api\/admin\/deleteblobcase"/); - assert.match( - adminSource, - /"\/api\/admin\/deleteblobcase"[\s\S]*method:\s*"POST"[\s\S]*container:\s*c\.container[\s\S]*casefolderID:\s*folder\.folderPath/ - ); - assert.match(adminSource, /if \(!response\.ok\)/); - assert.doesNotMatch(adminSource, /deleteAwaitingSubmissionsFromBlob/); - assert.match( - ordinarySource, - /buildFileQuery\("\/api\/file\/deleteblobcase"[\s\S]*getSignedFileJson\(queryUrl\)/ - ); + await mod.deleteAwaitingSubmissionsFromBlob("c1", "CASE-1"); + await mod.deleteMyRepresentationsFromBlob("c1", "CASE-1", "REP-1"); + + assert.deepStrictEqual(JSON.parse(JSON.stringify(calls)), [ + [ + "post", + "/api/file/deleteblobcase?container=c1&casefolderID=CASE-1" + ], + [ + "get", + "/api/file/deleteblobrep?container=c1&casefolderID=CASE-1&repfile=REP-1" + ] + ]); }); const loadAdminRepJsonEdit = (overrides = {}) => {