diff --git a/app/controllers/timeentrycontroller.js b/app/controllers/timeentrycontroller.js index d05badc..49d0105 100644 --- a/app/controllers/timeentrycontroller.js +++ b/app/controllers/timeentrycontroller.js @@ -135,8 +135,13 @@ exports.getById = async (req, res) => { const isMaster = await IsMaster(authKey); if (!isMaster) { const companyId = await GetCompanyId(authKey); + // Cross-tenant access is reported as 404, not 403 — otherwise + // a scoped caller can enumerate which TimeEntry ids are + // populated across the whole tenant table by status code. + // Same secure-404 pattern as the prior 13 entities (#174 + // through #234). if (companyId === -1 || entry.teCompId !== companyId) { - return res.status(403).json({ message: "Invalid Authorization Key." }); + return res.status(404).json({ message: "Not found." }); } } return res.status(200).json({ message: "Found.", timeEntry: entry }); @@ -244,8 +249,9 @@ exports.update = async (req, res) => { const isMaster = await IsMaster(authKey); if (!isMaster) { const companyId = await GetCompanyId(authKey); + // Secure-404 on PATCH for the same reason as GET. if (companyId === -1 || entry.teCompId !== companyId) { - return res.status(403).json({ message: "Invalid Authorization Key." }); + return res.status(404).json({ message: "Not found." }); } } @@ -313,8 +319,9 @@ exports.remove = async (req, res) => { const isMaster = await IsMaster(authKey); if (!isMaster) { const companyId = await GetCompanyId(authKey); + // Secure-404 on DELETE for the same reason as GET / PATCH. if (companyId === -1 || entry.teCompId !== companyId) { - return res.status(403).json({ message: "Invalid Authorization Key." }); + return res.status(404).json({ message: "Not found." }); } } diff --git a/tests/api/timeentry.test.js b/tests/api/timeentry.test.js index 600cab0..f3f9493 100644 --- a/tests/api/timeentry.test.js +++ b/tests/api/timeentry.test.js @@ -269,3 +269,87 @@ describe('computeMinutes helper', () => { expect(computeMinutes('not-a-date', 'also-not')).toBe(null); }); }); + +describe('TimeEntry tenant-enumeration defense (secure 404)', () => { + // Direct-company-scoped via teCompId. Spy on auth.isMaster / + // auth.getCompanyId so the caller appears scoped to a different + // company than the entry's teCompId. + 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/timeentrycontroller.js'); + const isMasterSpy = vi.spyOn(auth, 'isMaster').mockResolvedValue(false); + const getCompanyIdSpy = vi.spyOn(auth, 'getCompanyId').mockResolvedValue(7); + try { + const db = require('../../app/config/db.config.js'); + db.TimeEntry.findByPk = vi.fn().mockResolvedValue({ + teId: 42, teCompId: 99, teArch: 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(); + } + }); + + 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/timeentrycontroller.js'); + const isMasterSpy = vi.spyOn(auth, 'isMaster').mockResolvedValue(false); + const getCompanyIdSpy = vi.spyOn(auth, 'getCompanyId').mockResolvedValue(7); + try { + const db = require('../../app/config/db.config.js'); + db.TimeEntry.findByPk = vi.fn().mockResolvedValue({ + teId: 42, teCompId: 99, teArch: false, update: vi.fn(), + }); + const req = { + get: (h) => (h === 'authKey' ? 'scoped-to-7' : undefined), + params: { id: 42 }, + body: { teDescription: '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(); + } + }); + + 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/timeentrycontroller.js'); + const isMasterSpy = vi.spyOn(auth, 'isMaster').mockResolvedValue(false); + const getCompanyIdSpy = vi.spyOn(auth, 'getCompanyId').mockResolvedValue(7); + try { + const db = require('../../app/config/db.config.js'); + db.TimeEntry.findByPk = vi.fn().mockResolvedValue({ + teId: 42, teCompId: 99, teArch: 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(); + } + }); +});