diff --git a/app/controllers/versioninfocontroller.js b/app/controllers/versioninfocontroller.js index 0d41048..b904053 100644 --- a/app/controllers/versioninfocontroller.js +++ b/app/controllers/versioninfocontroller.js @@ -33,6 +33,37 @@ async function requireMaster(authKey, res) { return true; } +/** + * Reject reads whose authKey header is missing OR present-but-unknown. + * + * VersionInfo reads were originally gated purely on "header present" + * — `getById` and `list` accepted any non-empty string in `authKey` + * and returned 200. The README's endpoint table says "yes (authKey)" + * for these endpoints, which the rest of the codebase consistently + * treats as "any VALID authKey, master or scoped"; a literal + * presence-check breaks that contract. + * + * attachAuth (mounted at /v1/*) has already populated req.isMaster + * and req.companyId from the supplied header, so this helper just + * reads the resolved state without a second DB round-trip. + * + * Master keys pass (isMaster=true, companyId=-1 by convention). + * Non-master keys pass when they resolved to a real company. An + * unknown header is the case we want to reject — !isMaster AND + * companyId===-1. + */ +function requireValidAuth(req, res) { + if (!req.authKey) { + res.status(403).json({ message: "Authorization key not sent." }); + return false; + } + if (!req.isMaster && req.companyId === -1) { + res.status(403).json({ message: "Invalid Authorization Key." }); + return false; + } + return true; +} + exports.create = async (req, res) => { const authKey = req.get('authKey'); if (!(await requireMaster(authKey, res))) return; @@ -52,10 +83,7 @@ exports.create = async (req, res) => { }; exports.getById = async (req, res) => { - const authKey = req.get('authKey'); - if (!authKey) { - return res.status(403).json({ message: "Authorization key not sent." }); - } + if (!requireValidAuth(req, res)) return; try { const v = await VersionInfo.findByPk(req.params.id); if (!v) return res.status(404).json({ message: "Not found." }); @@ -67,10 +95,7 @@ exports.getById = async (req, res) => { }; exports.list = async (req, res) => { - const authKey = req.get('authKey'); - if (!authKey) { - return res.status(403).json({ message: "Authorization key not sent." }); - } + if (!requireValidAuth(req, res)) return; const requestedLimit = parseInt(req.query.limit, 10); const limit = Number.isInteger(requestedLimit) && requestedLimit > 0 diff --git a/tests/api/versioninfo.test.js b/tests/api/versioninfo.test.js index 4378976..43dc02d 100644 --- a/tests/api/versioninfo.test.js +++ b/tests/api/versioninfo.test.js @@ -49,6 +49,22 @@ describe('VersionInfo auth contract', () => { test('DELETE 403 for non-master key', async () => { expect((await request(app).delete('/v1/versioninfo/1').set('authKey', 'not-master')).status).toBe(403); }); + + test('GET 403 for unknown authKey (header present but not in DB)', async () => { + // VersionInfo reads previously gated only on header-present, so any + // non-empty string in `authKey` returned 200. The README's endpoint + // table says "yes (authKey)" for reads, which the rest of the API + // consistently means "any VALID authKey". Pin the tightening so a + // future refactor can't accidentally drop the validation check. + const res = await request(app).get('/v1/versioninfo/1').set('authKey', 'unknown'); + expect(res.status).toBe(403); + expect(res.body.message).toMatch(/Invalid Authorization Key/i); + }); + test('GET list 403 for unknown authKey', async () => { + const res = await request(app).get('/v1/versioninfo').set('authKey', 'unknown'); + expect(res.status).toBe(403); + expect(res.body.message).toMatch(/Invalid Authorization Key/i); + }); }); describe('VersionInfo route mounting', () => {