From bb50b17277435d2ce3c2361284df38aa7536df63 Mon Sep 17 00:00:00 2001 From: robbond Date: Tue, 3 Nov 2026 09:25:02 +0000 Subject: [PATCH] fix(ithc): correct server-side session retrieval for file APIs --- pages/api/auth/[...nextauth].js | 2 +- pages/api/file/deleteblobcase.js | 5 +- pages/api/file/deleteblobrep.js | 5 +- pages/api/file/upload.js | 5 +- pages/api/file/uploadsinglefile.js | 5 +- tests/phase21/file-handler-contract.test.cjs | 252 +++++++++++++++++-- 6 files changed, 239 insertions(+), 35 deletions(-) diff --git a/pages/api/auth/[...nextauth].js b/pages/api/auth/[...nextauth].js index 96c5f8b2..7662326f 100644 --- a/pages/api/auth/[...nextauth].js +++ b/pages/api/auth/[...nextauth].js @@ -199,7 +199,7 @@ const resolveRequestLocale = (req) => { return resolved; }; -const authOptions = (req, res) => { +export const authOptions = (req, res) => { const requestLocale = resolveRequestLocale(req); const verificationTokenCompletion = createVerificationTokenCompletion(); diff --git a/pages/api/file/deleteblobcase.js b/pages/api/file/deleteblobcase.js index 9c969db7..002068a4 100644 --- a/pages/api/file/deleteblobcase.js +++ b/pages/api/file/deleteblobcase.js @@ -1,7 +1,8 @@ import { hashAPIPath } from "../../../actions/core/hash"; import { deleteBlobCase } from "../../../actions/azurestorage"; import { respondError, respondSuccess } from "../middleware/apiResponse"; -import { getSession } from "next-auth/react"; +import { getServerSession } from "next-auth/next"; +import { authOptions } from "../auth/[...nextauth]"; import nextConnect from "next-connect"; import middleware from "../middleware/middleware"; @@ -38,7 +39,7 @@ ApiProxy.get(async (req, res) => { const casefolderIDTrimmed = casefolderID.trim(); - const session = await getSession({ req }); + const session = await getServerSession(req, res, authOptions(req, res)); if ( typeof session?.user?.id !== "string" || session.user.id.length === 0 || diff --git a/pages/api/file/deleteblobrep.js b/pages/api/file/deleteblobrep.js index 405f0bab..d6c7f3f0 100644 --- a/pages/api/file/deleteblobrep.js +++ b/pages/api/file/deleteblobrep.js @@ -1,7 +1,8 @@ import { hashAPIPath } from "../../../actions/core/hash"; import { deleteBlobRep } from "../../../actions/azurestorage"; import { respondError, respondSuccess } from "../middleware/apiResponse"; -import { getSession } from "next-auth/react"; +import { getServerSession } from "next-auth/next"; +import { authOptions } from "../auth/[...nextauth]"; import nextConnect from "next-connect"; import middleware from "../middleware/middleware"; @@ -42,7 +43,7 @@ ApiProxy.get(async (req, res) => { const casefolderIDTrimmed = casefolderID.trim(); const repfileTrimmed = repfile.trim(); - const session = await getSession({ req }); + const session = await getServerSession(req, res, authOptions(req, res)); if ( typeof session?.user?.id !== "string" || session.user.id.length === 0 || diff --git a/pages/api/file/upload.js b/pages/api/file/upload.js index da97f5c9..f7687e43 100644 --- a/pages/api/file/upload.js +++ b/pages/api/file/upload.js @@ -1,7 +1,8 @@ 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 { getServerSession } from "next-auth/next"; +import { authOptions } from "../auth/[...nextauth]"; import nextConnect from "next-connect"; import middleware from "../middleware/middleware"; @@ -26,7 +27,7 @@ const forbidden = (res) => ApiProxy.post(async (req, res) => { const getFieldValue = (field) => (Array.isArray(field) ? field[0] : field); - const session = await getSession({ req }); + const session = await getServerSession(req, res, authOptions(req, res)); if ( typeof session?.user?.id !== "string" || session.user.id.length === 0 diff --git a/pages/api/file/uploadsinglefile.js b/pages/api/file/uploadsinglefile.js index 558065c3..8d3e554c 100644 --- a/pages/api/file/uploadsinglefile.js +++ b/pages/api/file/uploadsinglefile.js @@ -6,7 +6,8 @@ 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 { getServerSession } from "next-auth/next"; +import { authOptions } from "../auth/[...nextauth]"; import fs from "fs"; import path from "path"; @@ -52,7 +53,7 @@ const forbidden = (res) => }); ApiProxy.post(async (req, res) => { - const session = await getSession({ req }); + const session = await getServerSession(req, res, authOptions(req, res)); if ( typeof session?.user?.id !== "string" || session.user.id.length === 0 diff --git a/tests/phase21/file-handler-contract.test.cjs b/tests/phase21/file-handler-contract.test.cjs index 8c432892..b3d5e0f5 100644 --- a/tests/phase21/file-handler-contract.test.cjs +++ b/tests/phase21/file-handler-contract.test.cjs @@ -16,7 +16,8 @@ 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" } }), + getServerSession: async () => ({ user: { id: "c1" } }), + authOptions: () => ({}), hashAPIPath: () => "?hash=expected", respondError: respondErrorMock, respondSuccess: respondSuccessMock, @@ -36,7 +37,8 @@ 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" } }), + getServerSession: async () => ({ user: { id: "c1" } }), + authOptions: () => ({}), hashAPIPath: () => "?hash=expected", respondError: respondErrorMock, respondSuccess: respondSuccessMock, @@ -59,7 +61,8 @@ 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" } }), + getServerSession: async () => ({ user: { id: "c1" } }), + authOptions: () => ({}), hashAPIPath: (queryPath) => queryPath === "/api/file/upload?" ? "?hash=expected" @@ -89,11 +92,25 @@ test("upload handler accepts alternate canonical hash candidate", async () => { }); const loadUploadAuthorization = (overrides = {}) => { - const calls = { createBlob: [], createRepBlob: [], hash: [] }; + const calls = { + session: [], + options: [], + createBlob: [], + createRepBlob: [], + hash: [] + }; + const options = { id: "pedw-upload-options" }; const mod = loadModule("pages/api/file/upload.js", { nextConnect: createNextConnectMock(), middleware: () => {}, - getSession: async () => ({ user: { id: "user-container" } }), + authOptions: (req, res) => { + calls.options.push([req, res]); + return options; + }, + getServerSession: async (...args) => { + calls.session.push(args); + return { user: { id: "user-container" } }; + }, hashAPIPath: (queryPath) => { calls.hash.push(queryPath); return "?hash=expected"; @@ -111,7 +128,7 @@ const loadUploadAuthorization = (overrides = {}) => { ...overrides }); - return { mod, calls }; + return { mod, calls, options }; }; const uploadAuthorizationRequest = (overrides = {}) => ({ @@ -132,7 +149,7 @@ const assertNoUploadWrites = (calls) => { }; test("upload rejects a missing NextAuth session before HMAC or Azure write", async () => { - const loaded = loadUploadAuthorization({ getSession: async () => null }); + const loaded = loadUploadAuthorization({ getServerSession: async () => null }); const res = createRes(); await loaded.mod.default.handler(uploadAuthorizationRequest(), res); @@ -145,7 +162,7 @@ test("upload rejects a missing NextAuth session before HMAC or Azure write", asy test("upload rejects an unusable NextAuth session before HMAC or Azure write", async () => { const loaded = loadUploadAuthorization({ - getSession: async () => ({ user: {} }) + getServerSession: async () => ({ user: {} }) }); const res = createRes(); @@ -157,6 +174,20 @@ test("upload rejects an unusable NextAuth session before HMAC or Azure write", a assertNoUploadWrites(loaded.calls); }); +test("upload passes the request, response and PEDW options to getServerSession", async () => { + const loaded = loadUploadAuthorization(); + const req = uploadAuthorizationRequest(); + const res = createRes(); + + await loaded.mod.default.handler(req, res); + + assert.deepStrictEqual(loaded.calls.options, [[req, res]]); + assert.strictEqual(loaded.calls.session.length, 1); + assert.strictEqual(loaded.calls.session[0][0], req); + assert.strictEqual(loaded.calls.session[0][1], res); + assert.strictEqual(loaded.calls.session[0][2], loaded.options); +}); + test("upload forbids another user's container before HMAC or Azure write", async () => { const loaded = loadUploadAuthorization(); const req = uploadAuthorizationRequest(); @@ -300,7 +331,8 @@ test("deleteblobcase handler accepts encoded hash variant", async () => { const mod = loadModule("pages/api/file/deleteblobcase.js", { nextConnect: createNextConnectMock(), middleware: () => {}, - getSession: async () => ({ user: { id: "c1" } }), + getServerSession: async () => ({ user: { id: "c1" } }), + authOptions: () => ({}), hashAPIPath: (queryPath) => queryPath.includes("casefolderID=CASE%2FSUB") ? "&hash=expected" @@ -331,7 +363,8 @@ test("deleteblobcase handler dependency failure returns DELETE_BLOB_CASE_FAILED" const mod = loadModule("pages/api/file/deleteblobcase.js", { nextConnect: createNextConnectMock(), middleware: () => {}, - getSession: async () => ({ user: { id: "c1" } }), + getServerSession: async () => ({ user: { id: "c1" } }), + authOptions: () => ({}), hashAPIPath: () => "&hash=expected", respondError: respondErrorMock, respondSuccess: respondSuccessMock, @@ -362,7 +395,8 @@ test("deleteblobrep handler accepts encoded hash variant", async () => { const mod = loadModule("pages/api/file/deleteblobrep.js", { nextConnect: createNextConnectMock(), middleware: () => {}, - getSession: async () => ({ user: { id: "c1" } }), + getServerSession: async () => ({ user: { id: "c1" } }), + authOptions: () => ({}), hashAPIPath: (queryPath) => queryPath.includes("repfile=Rep%20Name") ? "&hash=expected" @@ -394,7 +428,8 @@ test("deleteblobrep handler dependency failure returns DELETE_BLOB_REP_FAILED", const mod = loadModule("pages/api/file/deleteblobrep.js", { nextConnect: createNextConnectMock(), middleware: () => {}, - getSession: async () => ({ user: { id: "c1" } }), + getServerSession: async () => ({ user: { id: "c1" } }), + authOptions: () => ({}), hashAPIPath: () => "&hash=expected", respondError: respondErrorMock, respondSuccess: respondSuccessMock, @@ -448,7 +483,8 @@ for (const deletionCase of ordinaryDeletionCases) { const mod = loadModule(deletionCase.route, { nextConnect: createNextConnectMock(), middleware: () => {}, - getSession: async () => null, + getServerSession: async () => null, + authOptions: () => ({}), hashAPIPath: () => "&hash=expected", respondError: respondErrorMock, respondSuccess: respondSuccessMock, @@ -470,7 +506,8 @@ for (const deletionCase of ordinaryDeletionCases) { const mod = loadModule(deletionCase.route, { nextConnect: createNextConnectMock(), middleware: () => {}, - getSession: async () => ({ user: {} }), + getServerSession: async () => ({ user: {} }), + authOptions: () => ({}), hashAPIPath: () => "&hash=expected", respondError: respondErrorMock, respondSuccess: respondSuccessMock, @@ -492,7 +529,8 @@ for (const deletionCase of ordinaryDeletionCases) { const mod = loadModule(deletionCase.route, { nextConnect: createNextConnectMock(), middleware: () => {}, - getSession: async () => ({ user: { id: "different-user" } }), + getServerSession: async () => ({ user: { id: "different-user" } }), + authOptions: () => ({}), hashAPIPath: () => { hashCalls += 1; return "&hash=expected"; @@ -512,12 +550,49 @@ for (const deletionCase of ordinaryDeletionCases) { assert.strictEqual(deletionCalls, 0); }); + test(`${deletionCase.name} passes the request, response and PEDW options to getServerSession`, async () => { + const sessionCalls = []; + const optionCalls = []; + const options = { id: `${deletionCase.name}-options` }; + const mod = loadModule(deletionCase.route, { + nextConnect: createNextConnectMock(), + middleware: () => {}, + authOptions: (req, res) => { + optionCalls.push([req, res]); + return options; + }, + getServerSession: async (...args) => { + sessionCalls.push(args); + return { user: { id: "user-container" } }; + }, + hashAPIPath: () => "&hash=expected", + respondError: respondErrorMock, + respondSuccess: respondSuccessMock, + [deletionCase.dependency]: async () => ({ deleted: true }) + }); + const req = { + query: deletionCase.query, + body: { parsed: ["multipart"] } + }; + const res = createRes(); + + await mod.default.handler(req, res); + + assert.deepStrictEqual(optionCalls, [[req, res]]); + assert.strictEqual(sessionCalls.length, 1); + assert.strictEqual(sessionCalls[0][0], req); + assert.strictEqual(sessionCalls[0][1], res); + assert.strictEqual(sessionCalls[0][2], options); + assert.strictEqual(res.state.statusCode, 200); + }); + test(`${deletionCase.name} rejects invalid HMAC for the owning user without deletion`, async () => { let deletionCalls = 0; const mod = loadModule(deletionCase.route, { nextConnect: createNextConnectMock(), middleware: () => {}, - getSession: async () => ({ user: { id: "user-container" } }), + getServerSession: async () => ({ user: { id: "user-container" } }), + authOptions: () => ({}), hashAPIPath: () => "&hash=different", respondError: respondErrorMock, respondSuccess: respondSuccessMock, @@ -539,7 +614,8 @@ for (const deletionCase of ordinaryDeletionCases) { const mod = loadModule(deletionCase.route, { nextConnect: createNextConnectMock(), middleware: () => {}, - getSession: async () => ({ user: { id: "user-container" } }), + getServerSession: async () => ({ user: { id: "user-container" } }), + authOptions: () => ({}), hashAPIPath: () => "&hash=expected", respondError: respondErrorMock, respondSuccess: respondSuccessMock, @@ -1376,7 +1452,8 @@ 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" } }), + getServerSession: async () => ({ user: { id: "c1" } }), + authOptions: () => ({}), hashAPIPath: () => "?hash=expected", respondError: respondErrorMock, respondSuccess: respondSuccessMock, @@ -1398,7 +1475,8 @@ 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" } }), + getServerSession: async () => ({ user: { id: "c1" } }), + authOptions: () => ({}), hashAPIPath: () => "?hash=expected", respondError: respondErrorMock, respondSuccess: respondSuccessMock, @@ -1437,11 +1515,26 @@ test("uploadsinglefile handler dependency failure returns UPLOAD_SINGLE_FILE_FAI }); const loadSingleFileUploadAuthorization = (overrides = {}) => { - const calls = { upload: [], hash: [], fileReads: 0, typeChecks: 0 }; + const calls = { + session: [], + options: [], + upload: [], + hash: [], + fileReads: 0, + typeChecks: 0 + }; + const options = { id: "pedw-request-options" }; const mod = loadModule("pages/api/file/uploadsinglefile.js", { nextConnect: createNextConnectMock(), middleware: () => {}, - getSession: async () => ({ user: { id: "user-container" } }), + authOptions: (req, res) => { + calls.options.push([req, res]); + return options; + }, + getServerSession: async (...args) => { + calls.session.push(args); + return { user: { id: "user-container" } }; + }, hashAPIPath: (queryPath) => { calls.hash.push(queryPath); return "?hash=expected"; @@ -1467,7 +1560,7 @@ const loadSingleFileUploadAuthorization = (overrides = {}) => { ...overrides }); - return { mod, calls }; + return { mod, calls, options }; }; const singleFileUploadAuthorizationRequest = (overrides = {}) => ({ @@ -1499,7 +1592,7 @@ const assertNoSingleFileWork = (calls, { noHash = false } = {}) => { test("uploadsinglefile rejects a missing NextAuth session before HMAC, file processing or Azure write", async () => { const loaded = loadSingleFileUploadAuthorization({ - getSession: async () => null + getServerSession: async () => null }); const res = createRes(); @@ -1515,7 +1608,7 @@ test("uploadsinglefile rejects a missing NextAuth session before HMAC, file proc test("uploadsinglefile rejects an unusable NextAuth session before HMAC, file processing or Azure write", async () => { const loaded = loadSingleFileUploadAuthorization({ - getSession: async () => ({ user: { id: "" } }) + getServerSession: async () => ({ user: { id: "" } }) }); const res = createRes(); @@ -1529,6 +1622,111 @@ test("uploadsinglefile rejects an unusable NextAuth session before HMAC, file pr assertNoSingleFileWork(loaded.calls, { noHash: true }); }); +test("uploadsinglefile passes the multipart request, response and PEDW request-scoped options to getServerSession", async () => { + const loaded = loadSingleFileUploadAuthorization(); + const req = singleFileUploadAuthorizationRequest(); + const res = createRes(); + + await loaded.mod.default.handler(req, res); + + assert.strictEqual(loaded.calls.options.length, 1); + assert.strictEqual(loaded.calls.options[0][0], req); + assert.strictEqual(loaded.calls.options[0][1], res); + assert.strictEqual(loaded.calls.session.length, 1); + assert.strictEqual(loaded.calls.session[0][0], req); + assert.strictEqual(loaded.calls.session[0][1], res); + assert.strictEqual(loaded.calls.session[0][2], loaded.options); +}); + +test("installed getServerSession resolves a database session cookie without treating multipart body as a session update", async () => { + const { getServerSession } = require("next-auth/next"); + const adapterCalls = []; + const callbackCalls = []; + const expires = new Date(Date.now() + 30 * 60 * 1000); + const req = { + method: "POST", + body: { + containerID: ["user-container"], + casefolderID: ["CASE-1"] + }, + headers: { host: "localhost:3000" }, + cookies: { "next-auth.session-token": "session-token" } + }; + const responseHeaders = {}; + const res = { + getHeader(name) { + return responseHeaders[name.toLowerCase()]; + }, + setHeader(name, value) { + responseHeaders[name.toLowerCase()] = value; + } + }; + const user = { + id: "user-container", + name: "Owner", + email: "owner@example.test", + image: null + }; + const session = { + sessionToken: "session-token", + userId: user.id, + expires + }; + const options = { + secret: "test-secret", + session: { + strategy: "database", + maxAge: 30 * 60, + updateAge: 5 * 60 + }, + cookies: { + sessionToken: { + name: "next-auth.session-token", + options: { + httpOnly: true, + sameSite: "lax", + path: "/", + secure: false + } + } + }, + adapter: { + async getSessionAndUser(sessionToken) { + adapterCalls.push(["getSessionAndUser", sessionToken]); + return { session, user }; + }, + async updateSession(updatedSession) { + adapterCalls.push(["updateSession", updatedSession]); + return updatedSession; + }, + async deleteSession(sessionToken) { + adapterCalls.push(["deleteSession", sessionToken]); + } + }, + callbacks: { + async session(callbackArguments) { + callbackCalls.push(callbackArguments); + return callbackArguments; + } + } + }; + + const resolvedSession = await getServerSession(req, res, options); + + assert.deepStrictEqual(adapterCalls[0], [ + "getSessionAndUser", + "session-token" + ]); + assert.strictEqual(callbackCalls.length, 1); + assert.strictEqual(callbackCalls[0].user.id, "user-container"); + assert.strictEqual(callbackCalls[0].newSession, undefined); + assert.strictEqual(resolvedSession.user.id, "user-container"); + assert.deepStrictEqual(req.body, { + containerID: ["user-container"], + casefolderID: ["CASE-1"] + }); +}); + test("uploadsinglefile forbids another user's container before HMAC, file processing or Azure write", async () => { const loaded = loadSingleFileUploadAuthorization(); const req = singleFileUploadAuthorizationRequest(); @@ -2896,7 +3094,8 @@ 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" } }), + getServerSession: async () => ({ user: { id: "c1" } }), + authOptions: () => ({}), hashAPIPath: () => "?hash=expected", respondError: respondErrorMock, respondSuccess: respondSuccessMock, @@ -2999,7 +3198,8 @@ 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" } }), + getServerSession: async () => ({ user: { id: "c1" } }), + authOptions: () => ({}), hashAPIPath: () => "?hash=expected", respondError: respondErrorMock, respondSuccess: respondSuccessMock,