fix(sec/d24): gate destructive/sensitive ops to managerial

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 <noreply@anthropic.com>
feat/client-detail-redesign
Thomas Joise 4 days ago
parent 10022965d1
commit 54cde7961b

@ -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) {

@ -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'

@ -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 employees 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)
})
})
Loading…
Cancel
Save