From b9be188d275de0b7210c8c885e55486944c76c95 Mon Sep 17 00:00:00 2001 From: robbond Date: Fri, 6 Nov 2026 10:55:47 +0000 Subject: [PATCH] security: enforce CRM contact ownership and account field allowlist --- pages/api/endpoint/updateaccount_api.js | 132 ++++++-- .../endpoint-handler-contract.test.cjs | 318 +++++++++++++++++- 2 files changed, 422 insertions(+), 28 deletions(-) diff --git a/pages/api/endpoint/updateaccount_api.js b/pages/api/endpoint/updateaccount_api.js index d1747e83..1f3149f7 100644 --- a/pages/api/endpoint/updateaccount_api.js +++ b/pages/api/endpoint/updateaccount_api.js @@ -1,31 +1,68 @@ -/** - * @swagger - * /api/endpoint/updateaccount_api: - * patch: - * tags: [Account] - * description: Update account - * summary: Phase 2 - * responses: - * 200: - * description: Success - */ - import { consoleLogger } from "../../../actions/core/logger"; import { respondError, respondSuccess } from "../middleware/apiResponse"; import { updateCrmContact } from "../../../lib/server/updateCrmContact"; +import { getServerSession } from "next-auth/next"; +import { authOptions } from "../auth/[...nextauth]"; +import { getToken } from "../../../actions/core/token"; +import { relayGetData } from "../middleware/relayForwarding"; +import { azureHeaders } from "../../../actions/core/headers"; +import { escapeODataString } from "../../../actions/core/guards"; + +const GUID_PATTERN = + /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + +const ALLOWED_ACCOUNT_FIELDS = new Set([ + "pinswg_preferredlanguage", + "firstname", + "lastname", + "telephone1", + "pinswg_companyname", + "address1_line1", + "address1_line2", + "address1_city", + "address1_county", + "address1_postalcode" +]); export default async function ApiProxy(req, res) { - const contactId = req.query.contactId; - - if (typeof contactId !== "string" || contactId.trim().length === 0) { + if (req.method !== "POST") { + res.setHeader("Allow", "POST"); return respondError(res, { - status: 400, - code: "CONTACT_ID_REQUIRED", - message: "contactId is required" + status: 405, + code: "METHOD_NOT_ALLOWED", + message: "Method not allowed" }); } - if (!req.body || Object.keys(req.body).length === 0) { + const session = await getServerSession(req, res, authOptions(req, res)); + + if ( + typeof session?.user?.email !== "string" || + session.user.email.trim().length === 0 + ) { + return respondError(res, { + status: 401, + code: "UNAUTHENTICATED", + message: "Authentication required" + }); + } + + const contactId = req.query.contactId; + + if (typeof contactId !== "string" || !GUID_PATTERN.test(contactId.trim())) { + return respondError(res, { + status: 400, + code: "INVALID_CONTACT_ID", + message: "A valid contactId is required" + }); + } + + if ( + !req.body || + typeof req.body !== "object" || + Array.isArray(req.body) || + Object.keys(req.body).length === 0 + ) { return respondError(res, { status: 400, code: "ACCOUNT_UPDATE_PAYLOAD_REQUIRED", @@ -34,7 +71,62 @@ export default async function ApiProxy(req, res) { } try { - const data = await updateCrmContact(contactId, req.body); + const token = await getToken(); + + const escapedEmail = escapeODataString(session.user.email); + + const contactQueryUrl = + "contacts?$filter=emailaddress1 eq '" + + escapedEmail + + "' and statuscode eq 1&$select=contactid"; + + const contactResponse = await relayGetData({ + queryUrl: contactQueryUrl, + accessToken: token.access_token, + requestOptionsBuilder: (accessToken) => azureHeaders(accessToken) + }); + + const contacts = contactResponse?.data?.value; + + if ( + !Array.isArray(contacts) || + contacts.length !== 1 || + typeof contacts[0]?.contactid !== "string" || + !GUID_PATTERN.test(contacts[0].contactid.trim()) + ) { + return respondError(res, { + status: 403, + code: "CONTACT_IDENTITY_NOT_RESOLVED", + message: "Request not permitted" + }); + } + + if ( + contactId.trim().toLowerCase() !== + contacts[0].contactid.trim().toLowerCase() + ) { + return respondError(res, { + status: 403, + code: "CONTACT_OWNERSHIP_MISMATCH", + message: "Request not permitted" + }); + } + + const updatePayload = Object.fromEntries( + Object.entries(req.body).filter(([field]) => + ALLOWED_ACCOUNT_FIELDS.has(field) + ) + ); + + if (Object.keys(updatePayload).length === 0) { + return respondError(res, { + status: 400, + code: "NO_ALLOWED_ACCOUNT_FIELDS", + message: "No permitted account fields supplied" + }); + } + + const data = await updateCrmContact(contactId.trim(), updatePayload); return respondSuccess(res, data); } catch (error) { consoleLogger(error); diff --git a/tests/phase21/endpoint-handler-contract.test.cjs b/tests/phase21/endpoint-handler-contract.test.cjs index 1b34a7da..437383f6 100644 --- a/tests/phase21/endpoint-handler-contract.test.cjs +++ b/tests/phase21/endpoint-handler-contract.test.cjs @@ -831,20 +831,40 @@ test("createaccount catch path returns ACCOUNT_CREATE_FAILED", async () => { assert.strictEqual(res.state.jsonBody.error.code, "ACCOUNT_CREATE_FAILED"); }); -test("updateaccount returns CONTACT_ID_REQUIRED when contactId missing", async () => { +test("updateaccount returns INVALID_CONTACT_ID when contactId missing", async () => { const mod = loadModule("pages/api/endpoint/updateaccount_api.js", { respondError: respondErrorMock, respondSuccess: respondSuccessMock, updateCrmContact: async () => ({ contactid: "c1" }), - consoleLogger: () => {} + consoleLogger: () => {}, + getServerSession: async () => ({ + user: { email: "owner@example.com" } + }), + authOptions: () => ({}), + getToken: async () => ({ + access_token: "test-token" + }), + escapeODataString: (value) => value, + azureHeaders: (token) => ({ + headers: { Authorization: `Bearer ${token}` } + }), + relayGetData: async () => ({ + data: { + value: [ + { + contactid: "00000000-0000-0000-0000-000000000001" + } + ] + } + }) }); - const req = { query: {}, body: { firstname: "A" } }; + const req = { method: "POST", query: {}, body: { firstname: "A" } }; const res = createRes(); await mod.default(req, res); assert.strictEqual(res.state.statusCode, 400); - assert.strictEqual(res.state.jsonBody.error.code, "CONTACT_ID_REQUIRED"); + assert.strictEqual(res.state.jsonBody.error.code, "INVALID_CONTACT_ID"); }); test("updateaccount returns ACCOUNT_UPDATE_PAYLOAD_REQUIRED when body missing", async () => { @@ -852,10 +872,34 @@ test("updateaccount returns ACCOUNT_UPDATE_PAYLOAD_REQUIRED when body missing", respondError: respondErrorMock, respondSuccess: respondSuccessMock, updateCrmContact: async () => ({ contactid: "c1" }), - consoleLogger: () => {} + consoleLogger: () => {}, + getServerSession: async () => ({ + user: { email: "owner@example.com" } + }), + authOptions: () => ({}), + getToken: async () => ({ + access_token: "test-token" + }), + escapeODataString: (value) => value, + azureHeaders: (token) => ({ + headers: { Authorization: `Bearer ${token}` } + }), + relayGetData: async () => ({ + data: { + value: [ + { + contactid: "00000000-0000-0000-0000-000000000001" + } + ] + } + }) }); - const req = { query: { contactId: "c1" }, body: undefined }; + const req = { + method: "POST", + query: { contactId: "00000000-0000-0000-0000-000000000001" }, + body: undefined + }; const res = createRes(); await mod.default(req, res); @@ -873,10 +917,34 @@ test("updateaccount catch path returns ACCOUNT_UPDATE_FAILED", async () => { updateCrmContact: async () => { throw new Error("relay failed"); }, - consoleLogger: () => {} + consoleLogger: () => {}, + getServerSession: async () => ({ + user: { email: "owner@example.com" } + }), + authOptions: () => ({}), + getToken: async () => ({ + access_token: "test-token" + }), + escapeODataString: (value) => value, + azureHeaders: (token) => ({ + headers: { Authorization: `Bearer ${token}` } + }), + relayGetData: async () => ({ + data: { + value: [ + { + contactid: "00000000-0000-0000-0000-000000000001" + } + ] + } + }) }); - const req = { query: { contactId: "c1" }, body: { firstname: "A" } }; + const req = { + method: "POST", + query: { contactId: "00000000-0000-0000-0000-000000000001" }, + body: { firstname: "A" } + }; const res = createRes(); await mod.default(req, res); @@ -884,6 +952,240 @@ test("updateaccount catch path returns ACCOUNT_UPDATE_FAILED", async () => { assert.strictEqual(res.state.jsonBody.error.code, "ACCOUNT_UPDATE_FAILED"); }); +test("updateaccount rejects a different CRM contact before updating", async () => { + let updateCalls = 0; + + const mod = loadModule("pages/api/endpoint/updateaccount_api.js", { + respondError: respondErrorMock, + respondSuccess: respondSuccessMock, + consoleLogger: () => {}, + + getServerSession: async () => ({ + user: { email: "owner@example.com" } + }), + authOptions: () => ({}), + + getToken: async () => ({ + access_token: "test-token" + }), + escapeODataString: (value) => value, + azureHeaders: (token) => ({ + headers: { Authorization: `Bearer ${token}` } + }), + + relayGetData: async () => ({ + data: { + value: [ + { + contactid: "00000000-0000-0000-0000-000000000001" + } + ] + } + }), + + updateCrmContact: async () => { + updateCalls++; + return {}; + } + }); + + const req = { + method: "POST", + query: { + contactId: "00000000-0000-0000-0000-000000000002" + }, + body: { + firstname: "Unauthorised Update" + } + }; + + const res = createRes(); + + await mod.default(req, res); + + assert.strictEqual(res.state.statusCode, 403); + assert.strictEqual( + res.state.jsonBody.error.code, + "CONTACT_OWNERSHIP_MISMATCH" + ); + assert.strictEqual(updateCalls, 0); +}); + +test("updateaccount rejects unresolved CRM identity before updating", async () => { + let updateCalls = 0; + + const mod = loadModule("pages/api/endpoint/updateaccount_api.js", { + respondError: respondErrorMock, + respondSuccess: respondSuccessMock, + consoleLogger: () => {}, + + getServerSession: async () => ({ + user: { email: "owner@example.com" } + }), + authOptions: () => ({}), + + getToken: async () => ({ + access_token: "test-token" + }), + escapeODataString: (value) => value, + azureHeaders: (token) => ({ + headers: { Authorization: `Bearer ${token}` } + }), + + relayGetData: async () => ({ + data: { + value: [] + } + }), + + updateCrmContact: async () => { + updateCalls++; + return {}; + } + }); + + const req = { + method: "POST", + query: { + contactId: "00000000-0000-0000-0000-000000000001" + }, + body: { + firstname: "Unauthorised Update" + } + }; + + const res = createRes(); + + await mod.default(req, res); + + assert.strictEqual(res.state.statusCode, 403); + assert.strictEqual( + res.state.jsonBody.error.code, + "CONTACT_IDENTITY_NOT_RESOLVED" + ); + assert.strictEqual(updateCalls, 0); +}); + +test("updateaccount strips prohibited CRM fields before updating", async () => { + const contactId = "00000000-0000-0000-0000-000000000001"; + let capturedContactId; + let capturedPayload; + let updateCalls = 0; + + const mod = loadModule("pages/api/endpoint/updateaccount_api.js", { + respondError: respondErrorMock, + respondSuccess: respondSuccessMock, + consoleLogger: () => {}, + + getServerSession: async () => ({ + user: { email: "owner@example.com" } + }), + authOptions: () => ({}), + + getToken: async () => ({ + access_token: "test-token" + }), + escapeODataString: (value) => value, + azureHeaders: (token) => ({ + headers: { Authorization: `Bearer ${token}` } + }), + + relayGetData: async () => ({ + data: { + value: [{ contactid: contactId }] + } + }), + + updateCrmContact: async (id, payload) => { + updateCalls++; + capturedContactId = id; + capturedPayload = payload; + return { contactid: id }; + } + }); + + const req = { + method: "POST", + query: { contactId }, + body: { + firstname: "Robert", + telephone1: "01234567890", + emailaddress1: "attacker@example.com", + statuscode: 2, + contactid: "00000000-0000-0000-0000-000000000002" + } + }; + + const res = createRes(); + + await mod.default(req, res); + + assert.strictEqual(res.state.statusCode, 200); + assert.strictEqual(updateCalls, 1); + assert.strictEqual(capturedContactId, contactId); + + assert.deepStrictEqual(JSON.parse(JSON.stringify(capturedPayload)), { + firstname: "Robert", + telephone1: "01234567890" + }); +}); + +test("updateaccount rejects payload containing only prohibited fields", async () => { + const contactId = "00000000-0000-0000-0000-000000000001"; + let updateCalls = 0; + + const mod = loadModule("pages/api/endpoint/updateaccount_api.js", { + respondError: respondErrorMock, + respondSuccess: respondSuccessMock, + consoleLogger: () => {}, + + getServerSession: async () => ({ + user: { email: "owner@example.com" } + }), + authOptions: () => ({}), + + getToken: async () => ({ + access_token: "test-token" + }), + escapeODataString: (value) => value, + azureHeaders: (token) => ({ + headers: { Authorization: `Bearer ${token}` } + }), + + relayGetData: async () => ({ + data: { + value: [{ contactid: contactId }] + } + }), + + updateCrmContact: async () => { + updateCalls++; + return {}; + } + }); + + const req = { + method: "POST", + query: { contactId }, + body: { + emailaddress1: "attacker@example.com", + statuscode: 2, + contactid: "00000000-0000-0000-0000-000000000002" + } + }; + + const res = createRes(); + + await mod.default(req, res); + + assert.strictEqual(res.state.statusCode, 400); + assert.strictEqual( + res.state.jsonBody.error.code, + "NO_ALLOWED_ACCOUNT_FIELDS" + ); + assert.strictEqual(updateCalls, 0); +}); + test("getbasicsearch returns SEARCH_STRING_REQUIRED when searchString missing", async () => { const mod = loadModule("pages/api/endpoint/getbasicsearch_api.js", { respondError: respondErrorMock,