From 745af4184f630174450f9c56a95ca65357eb212d Mon Sep 17 00:00:00 2001 From: Michilis Date: Sun, 23 Aug 2026 06:35:57 +0000 Subject: [PATCH] Restrict whole-event door takings to admin and organizer. The door session sheet showed every staff member what the event had taken overall, by tender and against pre-sale. That is management information, not door information, and it matches the convention already applied to the other revenue aggregates (admin/analytics, admin/export/financial are both admin-only). Door staff keep their own shift cash-up: the "This session" totals are computed on the device from its own action log, so nothing they need to reconcile at the end of the night is lost. The gate is on GET /api/events/:eventId/door-summary, not only on the section that renders it -- hiding the panel while the endpoint still returned the figures would leave them one network response away. The client skips the request entirely for staff rather than provoking a 403. The test auth mock previously waved every role through, so it could not have caught a wrong gate; it now honours the role list, which also puts several already-written assertions onto real code paths. Co-Authored-By: Claude Opus 5 --- backend/src/routes/door.integration.test.ts | 67 +++++++++++++++++-- backend/src/routes/door.ts | 10 ++- .../scanner/_components/SessionSheet.tsx | 24 ++++--- frontend/src/app/admin/scanner/page.tsx | 9 ++- 4 files changed, 92 insertions(+), 18 deletions(-) diff --git a/backend/src/routes/door.integration.test.ts b/backend/src/routes/door.integration.test.ts index 929b557..7846676 100644 --- a/backend/src/routes/door.integration.test.ts +++ b/backend/src/routes/door.integration.test.ts @@ -14,17 +14,36 @@ process.env.BETTER_AUTH_SECRET = 'door-test-secret-0123456789abcdef'; delete process.env.REDIS_URL; const STAFF = { id: 'staff-user-id', name: 'Door Staff', role: 'staff' }; +const ADMIN = { id: 'admin-user-id', name: 'The Admin', role: 'admin' }; +const ORGANIZER = { id: 'organizer-user-id', name: 'The Organizer', role: 'organizer' }; + +// Who the next request is from. Session auth itself is Better Auth's concern and +// has its own integration suite; this mock keeps the *role* check real so the +// tests can prove which endpoints door staff may reach. +let currentUser: { id: string; name: string; role: string } = STAFF; -// The door endpoints are behind staff auth; the flows under test are the writes, -// not Better Auth, which has its own integration suite. vi.mock('../lib/auth.js', () => ({ - requireAuth: () => async (c: any, next: any) => { - c.set('user', STAFF); + requireAuth: (roles?: string[]) => async (c: any, next: any) => { + if (roles && !roles.includes(currentUser.role)) { + return c.json({ error: 'Forbidden' }, 403); + } + c.set('user', currentUser); await next(); }, - getAuthUser: async () => STAFF, + getAuthUser: async () => currentUser, })); +/** Run one request as a given role, always restoring the default afterwards. */ +async function as(user: typeof STAFF, fn: () => Promise): Promise { + const previous = currentUser; + currentUser = user; + try { + return await fn(); + } finally { + currentUser = previous; + } +} + // Walk-ins with an email trigger a confirmation send; keep it out of the test. vi.mock('../lib/email.js', () => ({ default: { sendBookingConfirmation: vi.fn(async () => ({ success: true })) }, @@ -362,9 +381,45 @@ describe('undo', () => { }); }); +describe('door-summary access', () => { + it('is hidden from door staff — whole-event takings are not door information', async () => { + const { status, body } = await get(`/api/events/${EVENT_ID}/door-summary`); + expect(status).toBe(403); + // The numbers must not leak in the body either: hiding the section in the UI + // alone would still expose them to anyone reading the network response. + expect(body).not.toHaveProperty('door'); + expect(body).not.toHaveProperty('presale'); + }); + + it('is available to admin and organizer', async () => { + for (const role of [ADMIN, ORGANIZER]) { + const { status } = await as(role, () => get(`/api/events/${EVENT_ID}/door-summary`)); + expect(status, `${role.role} should see door takings`).toBe(200); + } + }); + + it('still lets door staff do their job — list, check in and undo', async () => { + expect((await get(`/api/events/${EVENT_ID}/door-attendees`)).status).toBe(200); + + // Comp, so this ticket stays out of the revenue totals asserted below and + // the two tests cannot drift into each other through the shared database. + seedTicket({ id: 'tkt-role', first: 'Role', last: 'Check', status: 'confirmed', paymentStatus: 'comp' }); + const checkin = await post(`/api/events/${EVENT_ID}/door-checkin`, { + ticketId: 'tkt-role', + idempotencyKey: 'key-role-check', + }); + expect(checkin.status).toBe(201); + + const undo = await post(`/api/events/${EVENT_ID}/door-checkin/undo`, { + idempotencyKey: 'key-role-check', + }); + expect(undo.status).toBe(200); + }); +}); + describe('door-summary', () => { it('totals door takings by tender and splits them from pre-sale', async () => { - const { status, body } = await get(`/api/events/${EVENT_ID}/door-summary`); + const { status, body } = await as(ADMIN, () => get(`/api/events/${EVENT_ID}/door-summary`)); expect(status).toBe(200); // Cash: tkt-unpaid + the 'Walk' and 'Overflow' walk-ins (the undone ones are diff --git a/backend/src/routes/door.ts b/backend/src/routes/door.ts index 608e51c..d532a84 100644 --- a/backend/src/routes/door.ts +++ b/backend/src/routes/door.ts @@ -8,7 +8,8 @@ // POST /:eventId/door-checkin the single write endpoint — checks in, settles // payment, or creates a walk-in, atomically // POST /:eventId/door-checkin/undo reverses exactly what one keyed action did -// GET /:eventId/door-summary end-of-night cash-up + pre-sale/door revenue split +// GET /:eventId/door-summary end-of-night cash-up + pre-sale/door revenue +// split (admin/organizer only) // // Every write carries a client-generated idempotencyKey. The key is inserted in // the same transaction as the writes, so a double tap or a retry after a timeout @@ -35,6 +36,11 @@ import emailService from '../lib/email.js'; const doorRouter = new Hono(); const STAFF_ROLES = ['admin', 'organizer', 'staff'] as const; +// Whole-event money is management information, not door information: door staff +// reconcile their own shift from the session feed the client keeps locally, and +// never see what the event took overall. Matches the existing convention for +// revenue aggregates (admin/export/financial, admin/analytics). +const REVENUE_ROLES = ['admin', 'organizer'] as const; const IDEMPOTENCY_SCOPE = 'door-checkin'; // ==================== Shared helpers ==================== @@ -573,7 +579,7 @@ doorRouter.post( // End-of-night reconciliation: what was taken at the door, by tender, plus the // pre-sale/door split the event dashboard shows. -doorRouter.get('/:eventId/door-summary', requireAuth([...STAFF_ROLES]), async (c) => { +doorRouter.get('/:eventId/door-summary', requireAuth([...REVENUE_ROLES]), async (c) => { const eventId = c.req.param('eventId'); const event = await loadEvent(eventId); diff --git a/frontend/src/app/admin/scanner/_components/SessionSheet.tsx b/frontend/src/app/admin/scanner/_components/SessionSheet.tsx index 995584b..8ccb2a0 100644 --- a/frontend/src/app/admin/scanner/_components/SessionSheet.tsx +++ b/frontend/src/app/admin/scanner/_components/SessionSheet.tsx @@ -96,6 +96,7 @@ export function SessionSheet({ entries, summary, summaryLoading, + showEventTotals, currency, onRefresh, onClose, @@ -103,6 +104,8 @@ export function SessionSheet({ entries: SessionEntry[]; summary: DoorSummary | null; summaryLoading: boolean; + /** Whole-event takings are admin/organizer only; door staff see their own shift. */ + showEventTotals: boolean; currency: string; onRefresh: () => void; onClose: () => void; @@ -118,13 +121,15 @@ export function SessionSheet({

{liveEntries.length} checked in from this device

- + {showEventTotals && ( + + )}