From 54cde7961b527e2b081c44a8e454d561303c981e Mon Sep 17 00:00:00 2001 From: Thomas Joise Date: Sat, 18 Jul 2026 00:20:51 +0530 Subject: [PATCH] fix(sec/d24): gate destructive/sensitive ops to managerial MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Red-team authorization findings: - New requireManagerial middleware. Applied to POST /documents/:id/cancel, /credit-note (financially destructive — reverse money / consume a number) and /documents/:id/share (mints an UNAUTHENTICATED public PDF link — the IDOR exfiltration high). Any signed-in staff could previously do all three. - PATCH /interactions/:id now 403s unless the viewer is the interaction's author or managerial — staff can no longer overwrite another employee's logged call/visit. New authz-gates test locks both. Note (deliberate): general client read/edit stays open to staff — a shared support-team book is the intended model; the genuinely sensitive credential reveals are already managerial + rate-limited. Co-Authored-By: Claude Fable 5 --- apps/hq/src/api.ts | 25 +++++++++---- apps/hq/src/auth.ts | 12 ++++++ apps/hq/test/authz-gates.test.ts | 64 ++++++++++++++++++++++++++++++++ 3 files changed, 94 insertions(+), 7 deletions(-) create mode 100644 apps/hq/test/authz-gates.test.ts diff --git a/apps/hq/src/api.ts b/apps/hq/src/api.ts index 977447b..fc96072 100644 --- a/apps/hq/src/api.ts +++ b/apps/hq/src/api.ts @@ -1,6 +1,6 @@ import { Router, type Request, type RequestHandler, type Response } from 'express' import { fyOf, validateGstin } from '@sims/domain' -import { isManagerial, login, requireAuth, requireOwner } from './auth' +import { isManagerial, login, requireAuth, requireManagerial, requireOwner } from './auth' import type { DB } from './db' import { createClient, getClient, listClients, revealClientDbPassword, setClientDbPassword, @@ -947,7 +947,8 @@ export function apiRouter( } return convertDocument(db, staffId(res), id, to) })) - r.post('/documents/:id/cancel', requireAuth, docAction((id, res) => + // Cancel is financially destructive (consumes a document number) — managerial only (D24). + r.post('/documents/:id/cancel', requireAuth, requireManagerial, docAction((id, res) => cancelDocument(db, staffId(res), id))) // Supersede & recreate (spec §8): proforma-only cancel + fresh linked draft. r.post('/documents/:id/supersede', requireAuth, docAction((id, res) => @@ -1008,15 +1009,18 @@ export function apiRouter( } })() }) - r.post('/documents/:id/credit-note', requireAuth, docAction((id, res, req) => { + // A credit note reverses money on an issued invoice — managerial only (D24). + r.post('/documents/:id/credit-note', requireAuth, requireManagerial, docAction((id, res, req) => { const { lines } = req.body as { lines?: DraftLineInput[] } return createCreditNote(db, staffId(res), id, lines) })) // ---------- shareable links ---------- - // Owner mints an opaque public token for one document (default +30d expiry). The - // token is returned to the authenticated owner (they build the link) but never logged. - r.post('/documents/:id/share', requireAuth, async (req, res) => { + // Owner/manager mints an opaque public token for one document (default +30d expiry). + // Minting an UNAUTHENTICATED public link to a client's document is privileged (D24) — + // gated managerial so any staff account cannot exfiltrate any invoice as a public PDF. + // The token is returned to the caller (they build the link) but never logged. + r.post('/documents/:id/share', requireAuth, requireManagerial, async (req, res) => { const id = String(req.params['id'] ?? '') if ((await getDocument(db, id)) === null) { res.status(404).json({ ok: false, error: 'Document not found' }); return } try { @@ -1232,7 +1236,14 @@ export function apiRouter( }) r.patch('/interactions/:id', requireAuth, async (req, res) => { const id = String(req.params['id'] ?? '') - if ((await getInteraction(db, id)) === null) { res.status(404).json({ ok: false, error: 'Interaction not found' }); return } + const existing = await getInteraction(db, id) + if (existing === null) { res.status(404).json({ ok: false, error: 'Interaction not found' }); return } + // D24: only the author or a managerial role may edit a logged interaction — + // staff cannot overwrite another employee's call/visit record. + const viewer = res.locals['staff'] as { id: string; role: string } + if (!isManagerial(viewer.role) && existing.staffId !== viewer.id) { + res.status(403).json({ ok: false, error: 'You can only edit interactions you logged' }); return + } try { res.json({ ok: true, interaction: await updateInteraction(db, staffId(res), id, req.body as InteractionPatch) }) } catch (err) { diff --git a/apps/hq/src/auth.ts b/apps/hq/src/auth.ts index a3cf663..01271fa 100644 --- a/apps/hq/src/auth.ts +++ b/apps/hq/src/auth.ts @@ -107,6 +107,18 @@ export const requireOwner: RequestHandler = (_req, res, next) => { next() } +/** + * Gate for owner|manager (D24 red-team): financially destructive or externally-visible + * actions — cancel a document, raise a credit note, mint a public share link — are + * reserved for managerial roles, not open to every signed-in staff user. + */ +export const requireManagerial: RequestHandler = (_req, res, next) => { + if (!isManagerial((res.locals['staff'] as { role: string }).role)) { + res.status(403).json({ ok: false, error: 'Owner or manager only' }); return + } + next() +} + /** True for the managerial roles (owner|manager) allowed to see/act beyond their own rows. */ export const isManagerial = (role: string): boolean => role === 'owner' || role === 'manager' diff --git a/apps/hq/test/authz-gates.test.ts b/apps/hq/test/authz-gates.test.ts new file mode 100644 index 0000000..fe012d3 --- /dev/null +++ b/apps/hq/test/authz-gates.test.ts @@ -0,0 +1,64 @@ +// apps/hq/test/authz-gates.test.ts — D24 (red-team): destructive/sensitive ops gated managerial. +import express from 'express' +import { describe, it, expect, afterAll } from 'vitest' +import { openDb } from '../src/db' +import { seedIfEmpty } from '../src/seed' +import { createStaff } from '../src/auth' +import { createClient } from '../src/repos-clients' +import { createModule, setPrice } from '../src/repos-modules' +import { createDraft, issueDocument } from '../src/repos-documents' +import { createInteraction } from '../src/repos-interactions' +import { apiRouter } from '../src/api' + +const servers: { close: () => void }[] = [] +afterAll(() => { for (const s of servers) s.close() }) + +async function ctx() { + const db = openDb(':memory:'); await seedIfEmpty(db) + await createStaff(db, { email: 'owner@t.in', displayName: 'O', role: 'owner', password: 'owner-pass-1' }) + await createStaff(db, { email: 'staff@t.in', displayName: 'S', role: 'staff', password: 'staff-pass-1' }) + const staffB = await createStaff(db, { email: 'staff2@t.in', displayName: 'S2', role: 'staff', password: 'staff-pass-2' }) + const c = await createClient(db, 'u1', { name: 'Bank', stateCode: '32' }) + const m = await createModule(db, 'u1', { code: 'CORE', name: 'Core' }) + await setPrice(db, 'u1', { moduleId: m.id, kind: 'yearly', pricePaise: 10_000_00, effectiveFrom: '2026-01-01' }) + const inv = await issueDocument(db, 'u1', (await createDraft(db, 'u1', { + docType: 'INVOICE', clientId: c.id, lines: [{ moduleId: m.id, qty: 1, kind: 'yearly' }], + })).id) + const app = express(); app.use(express.json()); app.locals['db'] = db + app.set('trust proxy', 1) + app.use('/api', apiRouter(db, { keyHex: '11'.repeat(32) })) + const server = app.listen(0); servers.push(server) + const base = `http://localhost:${(server.address() as { port: number }).port}/api` + const tok = async (e: string, p: string) => ((await (await fetch(`${base}/auth/login`, { + method: 'POST', headers: { 'content-type': 'application/json' }, body: JSON.stringify({ email: e, password: p }), + })).json()) as { token: string }).token + return { db, c, m, inv, staffB, base, tok } +} +const H = (t: string) => ({ 'content-type': 'application/json', authorization: `Bearer ${t}` }) + +describe('D24 authorization gates', () => { + it('cancel / credit-note / public share are managerial-only; owner can', async () => { + const x = await ctx() + const staff = await x.tok('staff@t.in', 'staff-pass-1') + const owner = await x.tok('owner@t.in', 'owner-pass-1') + for (const path of [`/documents/${x.inv.id}/cancel`, `/documents/${x.inv.id}/credit-note`, `/documents/${x.inv.id}/share`]) { + const staffRes = await fetch(`${x.base}${path}`, { method: 'POST', headers: H(staff), body: '{}' }) + expect(staffRes.status, `${path} staff`).toBe(403) + } + // Owner can mint a share (the one that has no other preconditions). + const share = await fetch(`${x.base}/documents/${x.inv.id}/share`, { method: 'POST', headers: H(owner), body: '{}' }) + expect(share.status).toBe(200) + }) + + it('a staff user cannot edit another employee’s interaction; the author can', async () => { + const x = await ctx() + // Author = staffB; log it directly with staffB's id. + const it2 = await createInteraction(x.db, x.staffB.id, { clientId: x.c.id, typeCode: 'call', onDate: '2026-07-10' }) + const other = await x.tok('staff@t.in', 'staff-pass-1') // different staff + const author = await x.tok('staff2@t.in', 'staff-pass-2') // the author + const deny = await fetch(`${x.base}/interactions/${it2.id}`, { method: 'PATCH', headers: H(other), body: JSON.stringify({ notes: 'tampered' }) }) + expect(deny.status).toBe(403) + const ok = await fetch(`${x.base}/interactions/${it2.id}`, { method: 'PATCH', headers: H(author), body: JSON.stringify({ notes: 'my note' }) }) + expect(ok.status).toBe(200) + }) +})