From 5a237bbb2478fe58ff52fa9400a9552798cc4ce6 Mon Sep 17 00:00:00 2001 From: "Aaron K. Clark" Date: Tue, 19 May 2026 05:24:42 -0500 Subject: [PATCH] =?UTF-8?q?fix(purchaseorderheader):=20return=20404=20(not?= =?UTF-8?q?=20403)=20on=20cross-tenant=20access=20=E2=80=94=20close=20tena?= =?UTF-8?q?nt-enumeration=20leak?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same class as the six direct-scoped-entity fixes that came before (#174 company, #188 billingtype, #192 worker, #196 inventoryitem, #200 purchaseordervendor, #204 inventorytransaction), now for the first vendor-cascade-scoped entity: PurchaseOrderHeader. A scoped (non-master) caller probing `pohId` values got 403 for existing-but-not-yours and 404 for absent ids — distinguishing them by status code. Collapse both into 404 with the same body so the populated-ids set can't be enumerated. Tests pin the cascade path: `auth.isMaster`, `auth.getCompanyId`, and `auth.getCompanyIdByPovId` all spied so the cascade resolves to a different company (99) than the caller's (7). Three tests covering getById / update / remove. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../purchaseorderheadercontroller.js | 14 ++- tests/api/purchaseorderheader.test.js | 90 +++++++++++++++++++ 2 files changed, 101 insertions(+), 3 deletions(-) diff --git a/app/controllers/purchaseorderheadercontroller.js b/app/controllers/purchaseorderheadercontroller.js index 4bc78b0..cb98cc0 100644 --- a/app/controllers/purchaseorderheadercontroller.js +++ b/app/controllers/purchaseorderheadercontroller.js @@ -79,8 +79,14 @@ exports.getById = async (req, res) => { if (!isMaster) { const authCompanyId = await GetCompanyId(authKey); const vendorCompanyId = await GetCompanyIdByPovId(header.pohPovId); + // Cross-tenant access is reported as 404, not 403 — otherwise + // a scoped caller can enumerate which PurchaseOrderHeader ids + // are populated across the whole tenant table by status code. + // Same secure-404 pattern as #174 (company), #188 (billingtype), + // #192 (worker), #196 (inventoryitem), #200 (purchaseordervendor), + // #204 (inventorytransaction). if (authCompanyId === -1 || vendorCompanyId === -1 || authCompanyId !== vendorCompanyId) { - return res.status(403).json({ message: "Invalid Authorization Key." }); + return res.status(404).json({ message: "Not found." }); } } return res.status(200).json({ message: "Found.", purchaseOrderHeader: header }); @@ -152,8 +158,9 @@ exports.update = async (req, res) => { if (!isMaster) { const authCompanyId = await GetCompanyId(authKey); const vendorCompanyId = await GetCompanyIdByPovId(header.pohPovId); + // Secure-404 on PATCH for the same reason as GET. if (authCompanyId === -1 || vendorCompanyId === -1 || authCompanyId !== vendorCompanyId) { - return res.status(403).json({ message: "Invalid Authorization Key." }); + return res.status(404).json({ message: "Not found." }); } } @@ -196,8 +203,9 @@ exports.remove = async (req, res) => { if (!isMaster) { const authCompanyId = await GetCompanyId(authKey); const vendorCompanyId = await GetCompanyIdByPovId(header.pohPovId); + // Secure-404 on DELETE for the same reason as GET / PATCH. if (authCompanyId === -1 || vendorCompanyId === -1 || authCompanyId !== vendorCompanyId) { - return res.status(403).json({ message: "Invalid Authorization Key." }); + return res.status(404).json({ message: "Not found." }); } } diff --git a/tests/api/purchaseorderheader.test.js b/tests/api/purchaseorderheader.test.js index c210008..6379e4f 100644 --- a/tests/api/purchaseorderheader.test.js +++ b/tests/api/purchaseorderheader.test.js @@ -66,3 +66,93 @@ describe('PurchaseOrderHeader body validation', () => { expect(res.status).toBe(400); }); }); + +describe('PurchaseOrderHeader tenant-enumeration defense (secure 404)', () => { + // Vendor-cascade-scoped: pohPovId → vendor.povCompId. Spy on + // getCompanyIdByPovId so the cascade resolves to a different + // company than the caller's, and verify the 404 fallback. + test('controller getById: existing-but-not-yours returns 404 to non-master', async () => { + const auth = require('../../app/middleware/auth.js'); + const controller = require('../../app/controllers/purchaseorderheadercontroller.js'); + const isMasterSpy = vi.spyOn(auth, 'isMaster').mockResolvedValue(false); + const getCompanyIdSpy = vi.spyOn(auth, 'getCompanyId').mockResolvedValue(7); + const getCompanyIdByPovIdSpy = vi.spyOn(auth, 'getCompanyIdByPovId').mockResolvedValue(99); + try { + const db = require('../../app/config/db.config.js'); + db.PurchaseOrderHeader.findByPk = vi.fn().mockResolvedValue({ + pohId: 42, pohPovId: 99, pohArch: false, + }); + const req = { get: (h) => (h === 'authKey' ? 'scoped-to-7' : undefined), params: { id: 42 } }; + let captured = null; + const res = { + status(code) { this._code = code; return this; }, + json(body) { captured = { code: this._code, body }; return this; }, + }; + await controller.getById(req, res); + expect(captured.code).toBe(404); + expect(captured.body.message).toMatch(/not found/i); + } finally { + isMasterSpy.mockRestore(); + getCompanyIdSpy.mockRestore(); + getCompanyIdByPovIdSpy.mockRestore(); + } + }); + + test('controller update: existing-but-not-yours returns 404 to non-master', async () => { + const auth = require('../../app/middleware/auth.js'); + const controller = require('../../app/controllers/purchaseorderheadercontroller.js'); + const isMasterSpy = vi.spyOn(auth, 'isMaster').mockResolvedValue(false); + const getCompanyIdSpy = vi.spyOn(auth, 'getCompanyId').mockResolvedValue(7); + const getCompanyIdByPovIdSpy = vi.spyOn(auth, 'getCompanyIdByPovId').mockResolvedValue(99); + try { + const db = require('../../app/config/db.config.js'); + db.PurchaseOrderHeader.findByPk = vi.fn().mockResolvedValue({ + pohId: 42, pohPovId: 99, pohArch: false, update: vi.fn(), + }); + const req = { + get: (h) => (h === 'authKey' ? 'scoped-to-7' : undefined), + params: { id: 42 }, + body: { pohReference: 'X' }, + }; + let captured = null; + const res = { + status(code) { this._code = code; return this; }, + json(body) { captured = { code: this._code, body }; return this; }, + }; + await controller.update(req, res); + expect(captured.code).toBe(404); + expect(captured.body.message).toMatch(/not found/i); + } finally { + isMasterSpy.mockRestore(); + getCompanyIdSpy.mockRestore(); + getCompanyIdByPovIdSpy.mockRestore(); + } + }); + + test('controller remove: existing-but-not-yours returns 404 to non-master', async () => { + const auth = require('../../app/middleware/auth.js'); + const controller = require('../../app/controllers/purchaseorderheadercontroller.js'); + const isMasterSpy = vi.spyOn(auth, 'isMaster').mockResolvedValue(false); + const getCompanyIdSpy = vi.spyOn(auth, 'getCompanyId').mockResolvedValue(7); + const getCompanyIdByPovIdSpy = vi.spyOn(auth, 'getCompanyIdByPovId').mockResolvedValue(99); + try { + const db = require('../../app/config/db.config.js'); + db.PurchaseOrderHeader.findByPk = vi.fn().mockResolvedValue({ + pohId: 42, pohPovId: 99, pohArch: false, update: vi.fn(), + }); + const req = { get: (h) => (h === 'authKey' ? 'scoped-to-7' : undefined), params: { id: 42 } }; + let captured = null; + const res = { + status(code) { this._code = code; return this; }, + json(body) { captured = { code: this._code, body }; return this; }, + }; + await controller.remove(req, res); + expect(captured.code).toBe(404); + expect(captured.body.message).toMatch(/not found/i); + } finally { + isMasterSpy.mockRestore(); + getCompanyIdSpy.mockRestore(); + getCompanyIdByPovIdSpy.mockRestore(); + } + }); +});