From 65616f630731cc6fd3904bc4ae7182b97637315a Mon Sep 17 00:00:00 2001 From: robbond Date: Tue, 27 Oct 2026 06:05:47 +0000 Subject: [PATCH] ITHC: enforce container ownership on uploads --- docs/administrative-authentication.md | 6 + pages/api/file/upload.js | 49 ++- pages/api/file/uploadsinglefile.js | 37 ++- tests/phase21/file-handler-contract.test.cjs | 301 +++++++++++++++++++ 4 files changed, 381 insertions(+), 12 deletions(-) diff --git a/docs/administrative-authentication.md b/docs/administrative-authentication.md index ca380f12..788ed170 100644 --- a/docs/administrative-authentication.md +++ b/docs/administrative-authentication.md @@ -89,6 +89,12 @@ ID matches the requested Azure container. The existing signed-query/HMAC check also remains required. Requests that fail ownership or HMAC validation are rejected before the Azure storage deletion operation is called. +Ordinary JSON and multipart uploads through `/api/file/upload` and +`/api/file/uploadsinglefile` independently require a valid NextAuth session whose +user ID exactly matches the submitted `containerID`. The existing upload HMAC +remains mandatory. Missing or unusable sessions, cross-container requests, and +invalid HMACs are rejected before file processing or Azure storage writes. + Administrative cross-container representation deletion uses the dedicated `POST /api/admin/deleteblobrep` route. That endpoint independently validates the administrative session and requires a trusted same-origin request. It also diff --git a/pages/api/file/upload.js b/pages/api/file/upload.js index c4423a08..da97f5c9 100644 --- a/pages/api/file/upload.js +++ b/pages/api/file/upload.js @@ -1,6 +1,7 @@ import { createBlob, createRepBlob } from "../../../actions/azurestorage"; import { hashAPIPath } from "../../../actions/core/hash"; import { respondError, respondSuccess } from "../middleware/apiResponse"; +import { getSession } from "next-auth/react"; import nextConnect from "next-connect"; import middleware from "../middleware/middleware"; @@ -8,9 +9,48 @@ import middleware from "../middleware/middleware"; const ApiProxy = nextConnect(); ApiProxy.use(middleware); +const unauthenticated = (res) => + respondError(res, { + status: 401, + code: "UNAUTHENTICATED", + message: "Authentication required" + }); + +const forbidden = (res) => + respondError(res, { + status: 403, + code: "FORBIDDEN", + message: "Request not permitted" + }); + ApiProxy.post(async (req, res) => { const getFieldValue = (field) => (Array.isArray(field) ? field[0] : field); + const session = await getSession({ req }); + if ( + typeof session?.user?.id !== "string" || + session.user.id.length === 0 + ) { + return unauthenticated(res); + } + + const appealData = getFieldValue(req.body?.appealData); + const containerID = getFieldValue(req.body?.containerID); + const casefolderID = getFieldValue(req.body?.casefolderID); + const repOrAppealRaw = getFieldValue(req.body?.repOrAppeal); + const repOrAppeal = + repOrAppealRaw === true || + repOrAppealRaw === "true" || + repOrAppealRaw === "1"; + + if ( + typeof containerID !== "undefined" && + containerID.length > 0 && + session.user.id !== containerID + ) { + return forbidden(res); + } + const checkHash = req.query.hash; if (typeof checkHash === "undefined" || checkHash.length === 0) { @@ -34,15 +74,6 @@ ApiProxy.post(async (req, res) => { }); } - const appealData = getFieldValue(req.body?.appealData); - const containerID = getFieldValue(req.body?.containerID); - const casefolderID = getFieldValue(req.body?.casefolderID); - const repOrAppealRaw = getFieldValue(req.body?.repOrAppeal); - const repOrAppeal = - repOrAppealRaw === true || - repOrAppealRaw === "true" || - repOrAppealRaw === "1"; - if ( typeof containerID === "undefined" || containerID.length === 0 || diff --git a/pages/api/file/uploadsinglefile.js b/pages/api/file/uploadsinglefile.js index a89d1961..558065c3 100644 --- a/pages/api/file/uploadsinglefile.js +++ b/pages/api/file/uploadsinglefile.js @@ -6,6 +6,7 @@ import { consoleLogger } from "../../../actions/core/logger"; import { hashAPIPath } from "../../../actions/core/hash"; import { respondError, respondSuccess } from "../middleware/apiResponse"; import { fileTypeFromBuffer } from "file-type"; +import { getSession } from "next-auth/react"; import fs from "fs"; import path from "path"; @@ -36,7 +37,40 @@ function validateFilenameServer(originalFilename) { const ApiProxy = nextConnect(); ApiProxy.use(middleware); +const unauthenticated = (res) => + respondError(res, { + status: 401, + code: "UNAUTHENTICATED", + message: "Authentication required" + }); + +const forbidden = (res) => + respondError(res, { + status: 403, + code: "FORBIDDEN", + message: "Request not permitted" + }); + ApiProxy.post(async (req, res) => { + const session = await getSession({ req }); + if ( + typeof session?.user?.id !== "string" || + session.user.id.length === 0 + ) { + return unauthenticated(res); + } + + const containerID = req.body?.containerID?.[0]; + const casefolderID = req.body?.casefolderID?.[0]; + + if ( + typeof containerID !== "undefined" && + containerID.length > 0 && + session.user.id !== containerID + ) { + return forbidden(res); + } + const checkHash = req.query.hash; if (typeof checkHash === "undefined" || checkHash.length === 0) { @@ -63,9 +97,6 @@ ApiProxy.post(async (req, res) => { }); } - const containerID = req.body?.containerID?.[0]; - const casefolderID = req.body?.casefolderID?.[0]; - if ( typeof containerID === "undefined" || containerID.length === 0 || diff --git a/tests/phase21/file-handler-contract.test.cjs b/tests/phase21/file-handler-contract.test.cjs index b6272595..8c432892 100644 --- a/tests/phase21/file-handler-contract.test.cjs +++ b/tests/phase21/file-handler-contract.test.cjs @@ -16,6 +16,7 @@ test("upload handler returns HASH_REQUIRED for missing hash", async () => { const mod = loadModule("pages/api/file/upload.js", { nextConnect: createNextConnectMock(), middleware: () => {}, + getSession: async () => ({ user: { id: "c1" } }), hashAPIPath: () => "?hash=expected", respondError: respondErrorMock, respondSuccess: respondSuccessMock, @@ -35,6 +36,7 @@ test("upload handler returns INVALID_HASH for wrong hash", async () => { const mod = loadModule("pages/api/file/upload.js", { nextConnect: createNextConnectMock(), middleware: () => {}, + getSession: async () => ({ user: { id: "c1" } }), hashAPIPath: () => "?hash=expected", respondError: respondErrorMock, respondSuccess: respondSuccessMock, @@ -57,6 +59,7 @@ test("upload handler accepts alternate canonical hash candidate", async () => { const mod = loadModule("pages/api/file/upload.js", { nextConnect: createNextConnectMock(), middleware: () => {}, + getSession: async () => ({ user: { id: "c1" } }), hashAPIPath: (queryPath) => queryPath === "/api/file/upload?" ? "?hash=expected" @@ -85,6 +88,130 @@ test("upload handler accepts alternate canonical hash candidate", async () => { }); }); +const loadUploadAuthorization = (overrides = {}) => { + const calls = { createBlob: [], createRepBlob: [], hash: [] }; + const mod = loadModule("pages/api/file/upload.js", { + nextConnect: createNextConnectMock(), + middleware: () => {}, + getSession: async () => ({ user: { id: "user-container" } }), + hashAPIPath: (queryPath) => { + calls.hash.push(queryPath); + return "?hash=expected"; + }, + respondError: respondErrorMock, + respondSuccess: respondSuccessMock, + createBlob: async (...args) => { + calls.createBlob.push(args); + return { id: "appeal-blob" }; + }, + createRepBlob: async (...args) => { + calls.createRepBlob.push(args); + return { id: "representation-blob" }; + }, + ...overrides + }); + + return { mod, calls }; +}; + +const uploadAuthorizationRequest = (overrides = {}) => ({ + query: { hash: "expected" }, + body: { + appealData: [JSON.stringify({ key: "value" })], + containerID: ["user-container"], + casefolderID: ["CASE-1"], + repOrAppeal: ["false"] + }, + files: {}, + ...overrides +}); + +const assertNoUploadWrites = (calls) => { + assert.strictEqual(calls.createBlob.length, 0); + assert.strictEqual(calls.createRepBlob.length, 0); +}; + +test("upload rejects a missing NextAuth session before HMAC or Azure write", async () => { + const loaded = loadUploadAuthorization({ getSession: async () => null }); + const res = createRes(); + + await loaded.mod.default.handler(uploadAuthorizationRequest(), res); + + assert.strictEqual(res.state.statusCode, 401); + assert.strictEqual(res.state.jsonBody.error.code, "UNAUTHENTICATED"); + assert.strictEqual(loaded.calls.hash.length, 0); + assertNoUploadWrites(loaded.calls); +}); + +test("upload rejects an unusable NextAuth session before HMAC or Azure write", async () => { + const loaded = loadUploadAuthorization({ + getSession: async () => ({ user: {} }) + }); + const res = createRes(); + + await loaded.mod.default.handler(uploadAuthorizationRequest(), res); + + assert.strictEqual(res.state.statusCode, 401); + assert.strictEqual(res.state.jsonBody.error.code, "UNAUTHENTICATED"); + assert.strictEqual(loaded.calls.hash.length, 0); + assertNoUploadWrites(loaded.calls); +}); + +test("upload forbids another user's container before HMAC or Azure write", async () => { + const loaded = loadUploadAuthorization(); + const req = uploadAuthorizationRequest(); + req.body.containerID = ["other-container"]; + const res = createRes(); + + await loaded.mod.default.handler(req, res); + + assert.strictEqual(res.state.statusCode, 403); + assert.strictEqual(res.state.jsonBody.error.code, "FORBIDDEN"); + assert.strictEqual(loaded.calls.hash.length, 0); + assertNoUploadWrites(loaded.calls); +}); + +test("upload rejects invalid HMAC for the owning user without Azure write", async () => { + const loaded = loadUploadAuthorization({ + hashAPIPath: () => "?hash=different" + }); + const res = createRes(); + + await loaded.mod.default.handler(uploadAuthorizationRequest(), res); + + assert.strictEqual(res.state.statusCode, 400); + assert.strictEqual(res.state.jsonBody.error.code, "INVALID_HASH"); + assertNoUploadWrites(loaded.calls); +}); + +test("upload requires HMAC for the authenticated owning user", async () => { + const loaded = loadUploadAuthorization(); + const req = uploadAuthorizationRequest({ query: {} }); + const res = createRes(); + + await loaded.mod.default.handler(req, res); + + assert.strictEqual(res.state.statusCode, 400); + assert.strictEqual(res.state.jsonBody.error.code, "HASH_REQUIRED"); + assertNoUploadWrites(loaded.calls); +}); + +test("upload preserves the authorised owner success contract and helper arguments", async () => { + const loaded = loadUploadAuthorization(); + const res = createRes(); + + await loaded.mod.default.handler(uploadAuthorizationRequest(), res); + + assert.strictEqual(res.state.statusCode, 200); + assert.deepStrictEqual(JSON.parse(JSON.stringify(res.state.jsonBody)), { + data: { id: "appeal-blob" } + }); + assert.deepStrictEqual(loaded.calls.createBlob, [ + [JSON.stringify({ key: "value" }), "user-container", "CASE-1"] + ]); + assert.strictEqual(loaded.calls.createRepBlob.length, 0); +}); + test("deleteblob handler returns MISSING_REQUIRED_QUERY when blobname missing", async () => { const mod = loadModule("pages/api/file/deleteblob.js", { nextConnect: createNextConnectMock(), @@ -1249,6 +1376,7 @@ test("uploadsinglefile handler returns HASH_REQUIRED when hash missing", async ( const mod = loadModule("pages/api/file/uploadsinglefile.js", { nextConnect: createNextConnectMock(), middleware: () => {}, + getSession: async () => ({ user: { id: "c1" } }), hashAPIPath: () => "?hash=expected", respondError: respondErrorMock, respondSuccess: respondSuccessMock, @@ -1270,6 +1398,7 @@ test("uploadsinglefile handler dependency failure returns UPLOAD_SINGLE_FILE_FAI const mod = loadModule("pages/api/file/uploadsinglefile.js", { nextConnect: createNextConnectMock(), middleware: () => {}, + getSession: async () => ({ user: { id: "c1" } }), hashAPIPath: () => "?hash=expected", respondError: respondErrorMock, respondSuccess: respondSuccessMock, @@ -1307,6 +1436,176 @@ test("uploadsinglefile handler dependency failure returns UPLOAD_SINGLE_FILE_FAI ); }); +const loadSingleFileUploadAuthorization = (overrides = {}) => { + const calls = { upload: [], hash: [], fileReads: 0, typeChecks: 0 }; + const mod = loadModule("pages/api/file/uploadsinglefile.js", { + nextConnect: createNextConnectMock(), + middleware: () => {}, + getSession: async () => ({ user: { id: "user-container" } }), + hashAPIPath: (queryPath) => { + calls.hash.push(queryPath); + return "?hash=expected"; + }, + respondError: respondErrorMock, + respondSuccess: respondSuccessMock, + consoleLogger: () => {}, + uploadSingleFile: async (...args) => { + calls.upload.push(args); + return [{ file: "document.pdf" }]; + }, + fileTypeFromBuffer: async () => { + calls.typeChecks += 1; + return { mime: "application/pdf" }; + }, + fs: { + readFileSync: () => { + calls.fileReads += 1; + return Buffer.from("synthetic-pdf"); + } + }, + path: { basename: (value) => value }, + ...overrides + }); + + return { mod, calls }; +}; + +const singleFileUploadAuthorizationRequest = (overrides = {}) => ({ + query: { hash: "expected" }, + body: { + containerID: ["user-container"], + casefolderID: ["CASE-1"] + }, + files: { + document: [ + { + fieldName: "document.pdf", + originalFilename: "document.pdf", + path: "/tmp/document.pdf", + size: 1, + headers: { "content-type": "application/pdf" } + } + ] + }, + ...overrides +}); + +const assertNoSingleFileWork = (calls, { noHash = false } = {}) => { + assert.strictEqual(calls.upload.length, 0); + assert.strictEqual(calls.fileReads, 0); + assert.strictEqual(calls.typeChecks, 0); + if (noHash) assert.strictEqual(calls.hash.length, 0); +}; + +test("uploadsinglefile rejects a missing NextAuth session before HMAC, file processing or Azure write", async () => { + const loaded = loadSingleFileUploadAuthorization({ + getSession: async () => null + }); + const res = createRes(); + + await loaded.mod.default.handler( + singleFileUploadAuthorizationRequest(), + res + ); + + assert.strictEqual(res.state.statusCode, 401); + assert.strictEqual(res.state.jsonBody.error.code, "UNAUTHENTICATED"); + assertNoSingleFileWork(loaded.calls, { noHash: true }); +}); + +test("uploadsinglefile rejects an unusable NextAuth session before HMAC, file processing or Azure write", async () => { + const loaded = loadSingleFileUploadAuthorization({ + getSession: async () => ({ user: { id: "" } }) + }); + const res = createRes(); + + await loaded.mod.default.handler( + singleFileUploadAuthorizationRequest(), + res + ); + + assert.strictEqual(res.state.statusCode, 401); + assert.strictEqual(res.state.jsonBody.error.code, "UNAUTHENTICATED"); + assertNoSingleFileWork(loaded.calls, { noHash: true }); +}); + +test("uploadsinglefile forbids another user's container before HMAC, file processing or Azure write", async () => { + const loaded = loadSingleFileUploadAuthorization(); + const req = singleFileUploadAuthorizationRequest(); + req.body.containerID = ["other-container"]; + const res = createRes(); + + await loaded.mod.default.handler(req, res); + + assert.strictEqual(res.state.statusCode, 403); + assert.strictEqual(res.state.jsonBody.error.code, "FORBIDDEN"); + assertNoSingleFileWork(loaded.calls, { noHash: true }); +}); + +test("uploadsinglefile rejects invalid HMAC for the owning user before file processing or Azure write", async () => { + const loaded = loadSingleFileUploadAuthorization({ + hashAPIPath: () => "?hash=different" + }); + const res = createRes(); + + await loaded.mod.default.handler( + singleFileUploadAuthorizationRequest(), + res + ); + + assert.strictEqual(res.state.statusCode, 400); + assert.strictEqual(res.state.jsonBody.error.code, "INVALID_HASH"); + assertNoSingleFileWork(loaded.calls); +}); + +test("uploadsinglefile requires HMAC for the authenticated owning user", async () => { + const loaded = loadSingleFileUploadAuthorization(); + const req = singleFileUploadAuthorizationRequest({ query: {} }); + const res = createRes(); + + await loaded.mod.default.handler(req, res); + + assert.strictEqual(res.state.statusCode, 400); + assert.strictEqual(res.state.jsonBody.error.code, "HASH_REQUIRED"); + assertNoSingleFileWork(loaded.calls); +}); + +test("uploadsinglefile preserves multipart owner success response and exact Azure helper arguments", async () => { + const loaded = loadSingleFileUploadAuthorization(); + const res = createRes(); + + await loaded.mod.default.handler( + singleFileUploadAuthorizationRequest(), + res + ); + + assert.strictEqual(res.state.statusCode, 200); + assert.deepStrictEqual(JSON.parse(JSON.stringify(res.state.jsonBody)), { + data: [{ file: "document.pdf" }], + invalidFiles: [] + }); + assert.strictEqual(loaded.calls.upload.length, 1); + assert.strictEqual(loaded.calls.upload[0][1], "user-container"); + assert.strictEqual(loaded.calls.upload[0][2], "CASE-1"); + assert.deepStrictEqual( + JSON.parse(JSON.stringify(loaded.calls.upload[0][0])), + { + document: [ + { + fieldName: "document.pdf", + originalFilename: "document.pdf", + path: "/tmp/document.pdf", + size: 1, + headers: { "content-type": "application/pdf" }, + contentType: "application/pdf" + } + ] + } + ); + assert.strictEqual(loaded.calls.fileReads, 1); + assert.strictEqual(loaded.calls.typeChecks, 1); +}); + const loadOrdinaryRepCompletion = (overrides = {}) => { const calls = { hash: [], queue: [] }; const payload = { messageId: "ordinary-message" }; @@ -2597,6 +2896,7 @@ test("upload handler success returns data payload with 200", async () => { const mod = loadModule("pages/api/file/upload.js", { nextConnect: createNextConnectMock(), middleware: () => {}, + getSession: async () => ({ user: { id: "c1" } }), hashAPIPath: () => "?hash=expected", respondError: respondErrorMock, respondSuccess: respondSuccessMock, @@ -2699,6 +2999,7 @@ test("upload handler dependency failure returns UPLOAD_FAILED", async () => { const mod = loadModule("pages/api/file/upload.js", { nextConnect: createNextConnectMock(), middleware: () => {}, + getSession: async () => ({ user: { id: "c1" } }), hashAPIPath: () => "?hash=expected", respondError: respondErrorMock, respondSuccess: respondSuccessMock,