From f392fe2e347a3a37a0c3e66c9627bd4e1f2a7c04 Mon Sep 17 00:00:00 2001 From: Jake Bromberg Date: Sat, 21 Feb 2026 08:32:01 -0800 Subject: [PATCH 1/4] fix: apply showMemberMiddleware to flowsheet write routes The middleware was imported but never applied, allowing any authenticated DJ to modify flowsheet entries regardless of show membership. Co-authored-by: Cursor --- apps/backend/app.ts | 1 - apps/backend/middleware/checkShowMember.ts | 21 ++--- apps/backend/routes/flowsheet.route.ts | 4 + tests/unit/middleware/checkShowMember.test.ts | 84 +++++++++++++++++++ 4 files changed, 99 insertions(+), 11 deletions(-) create mode 100644 tests/unit/middleware/checkShowMember.test.ts diff --git a/apps/backend/app.ts b/apps/backend/app.ts index e06f5108..38feb401 100644 --- a/apps/backend/app.ts +++ b/apps/backend/app.ts @@ -10,7 +10,6 @@ import { library_route } from './routes/library.route.js'; import { schedule_route } from './routes/schedule.route.js'; import { events_route } from './routes/events.route.js'; import { request_line_route } from './routes/requestLine.route.js'; -import { showMemberMiddleware } from './middleware/checkShowMember.js'; import { activeShow } from './middleware/checkActiveShow.js'; import errorHandler from './middleware/errorHandler.js'; import { requirePermissions } from '@wxyc/authentication'; diff --git a/apps/backend/middleware/checkShowMember.ts b/apps/backend/middleware/checkShowMember.ts index 8d07269f..85a8875a 100644 --- a/apps/backend/middleware/checkShowMember.ts +++ b/apps/backend/middleware/checkShowMember.ts @@ -2,16 +2,17 @@ import { RequestHandler } from 'express'; import { getDJsInCurrentShow } from '../services/flowsheet.service.js'; export const showMemberMiddleware: RequestHandler = async (req, res, next) => { - const show_djs = await getDJsInCurrentShow(); - // Get user ID from JWT - check both req.auth (from better-auth middleware) and res.locals (legacy) - const user_id = req.auth?.id || req.auth?.sub || res.locals.decodedJWT?.id || res.locals.decodedJWT?.userId; - const dj_in_show = show_djs.filter((dj) => { - return dj.id === user_id; - }).length; + try { + const show_djs = await getDJsInCurrentShow(); + const user_id = req.auth?.id || req.auth?.sub || res.locals.decodedJWT?.id || res.locals.decodedJWT?.userId; + const dj_in_show = show_djs.some((dj) => dj.id === user_id); - if (dj_in_show > 0) { - next(); - } else { - res.status(400).json({ message: 'Bad Request: DJ not a member of show' }); + if (dj_in_show) { + next(); + } else { + res.status(400).json({ message: 'Bad Request: DJ not a member of show' }); + } + } catch { + res.status(500).json({ message: 'Internal server error checking show membership' }); } }; diff --git a/apps/backend/routes/flowsheet.route.ts b/apps/backend/routes/flowsheet.route.ts index 38385dc5..07f968e1 100644 --- a/apps/backend/routes/flowsheet.route.ts +++ b/apps/backend/routes/flowsheet.route.ts @@ -3,6 +3,7 @@ import { Router } from 'express'; import * as flowsheetController from '../controllers/flowsheet.controller'; import { flowsheetMirror } from '../middleware/legacy/flowsheet.mirror'; import { conditionalGet } from '../middleware/conditionalGet'; +import { showMemberMiddleware } from '../middleware/checkShowMember'; export const flowsheet_route = Router(); @@ -11,6 +12,7 @@ flowsheet_route.get('/', conditionalGet, flowsheetMirror.getEntries, flowsheetCo flowsheet_route.post( '/', requirePermissions({ flowsheet: ['write'] }), + showMemberMiddleware, flowsheetMirror.addEntry, flowsheetController.addEntry ); @@ -18,6 +20,7 @@ flowsheet_route.post( flowsheet_route.patch( '/', requirePermissions({ flowsheet: ['write'] }), + showMemberMiddleware, flowsheetMirror.updateEntry, flowsheetController.updateEntry ); @@ -25,6 +28,7 @@ flowsheet_route.patch( flowsheet_route.delete( '/', requirePermissions({ flowsheet: ['write'] }), + showMemberMiddleware, flowsheetMirror.deleteEntry, flowsheetController.deleteEntry ); diff --git a/tests/unit/middleware/checkShowMember.test.ts b/tests/unit/middleware/checkShowMember.test.ts new file mode 100644 index 00000000..9c42e16e --- /dev/null +++ b/tests/unit/middleware/checkShowMember.test.ts @@ -0,0 +1,84 @@ +import { jest } from '@jest/globals'; +import type { Request, Response, NextFunction } from 'express'; + +const mockGetDJsInCurrentShow = jest.fn<() => Promise<{ id: string }[]>>(); + +jest.mock('../../../apps/backend/services/flowsheet.service', () => ({ + getDJsInCurrentShow: mockGetDJsInCurrentShow, +})); + +import { showMemberMiddleware } from '../../../apps/backend/middleware/checkShowMember'; + +function createMockReqResNext(userId: string) { + const req = { + auth: { id: userId }, + } as unknown as Request; + + const res = { + status: jest.fn().mockReturnThis(), + json: jest.fn().mockReturnThis(), + locals: {}, + } as unknown as Response; + + const next = jest.fn() as unknown as NextFunction; + + return { req, res, next }; +} + +describe('showMemberMiddleware', () => { + it('rejects a DJ who is not in the current show', async () => { + mockGetDJsInCurrentShow.mockResolvedValue([ + { id: 'dj-alice' }, + { id: 'dj-bob' }, + ]); + + const { req, res, next } = createMockReqResNext('dj-charlie'); + + await showMemberMiddleware(req, res, next); + + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith({ + message: 'Bad Request: DJ not a member of show', + }); + expect(next).not.toHaveBeenCalled(); + }); + + it('allows a DJ who is in the current show', async () => { + mockGetDJsInCurrentShow.mockResolvedValue([ + { id: 'dj-alice' }, + { id: 'dj-bob' }, + ]); + + const { req, res, next } = createMockReqResNext('dj-alice'); + + await showMemberMiddleware(req, res, next); + + expect(next).toHaveBeenCalled(); + expect(res.status).not.toHaveBeenCalled(); + }); + + it('rejects when there are no DJs in the current show', async () => { + mockGetDJsInCurrentShow.mockResolvedValue([]); + + const { req, res, next } = createMockReqResNext('dj-alice'); + + await showMemberMiddleware(req, res, next); + + expect(res.status).toHaveBeenCalledWith(400); + expect(next).not.toHaveBeenCalled(); + }); + + it('returns 500 when getDJsInCurrentShow throws', async () => { + mockGetDJsInCurrentShow.mockRejectedValue(new Error('DB connection lost')); + + const { req, res, next } = createMockReqResNext('dj-alice'); + + await showMemberMiddleware(req, res, next); + + expect(res.status).toHaveBeenCalledWith(500); + expect(res.json).toHaveBeenCalledWith({ + message: 'Internal server error checking show membership', + }); + expect(next).not.toHaveBeenCalled(); + }); +}); From edda8a2dbe68f6e25b46bcac392c111386252036 Mon Sep 17 00:00:00 2001 From: Jake Bromberg Date: Fri, 27 Feb 2026 14:03:00 -0800 Subject: [PATCH 2/4] fix: resolve lint errors --- tests/unit/middleware/checkShowMember.test.ts | 38 +++++++++---------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/tests/unit/middleware/checkShowMember.test.ts b/tests/unit/middleware/checkShowMember.test.ts index 9c42e16e..206dc01b 100644 --- a/tests/unit/middleware/checkShowMember.test.ts +++ b/tests/unit/middleware/checkShowMember.test.ts @@ -14,69 +14,65 @@ function createMockReqResNext(userId: string) { auth: { id: userId }, } as unknown as Request; + const statusMock = jest.fn().mockReturnThis(); + const jsonMock = jest.fn().mockReturnThis(); const res = { - status: jest.fn().mockReturnThis(), - json: jest.fn().mockReturnThis(), + status: statusMock, + json: jsonMock, locals: {}, } as unknown as Response; const next = jest.fn() as unknown as NextFunction; - return { req, res, next }; + return { req, res, next, statusMock, jsonMock }; } describe('showMemberMiddleware', () => { it('rejects a DJ who is not in the current show', async () => { - mockGetDJsInCurrentShow.mockResolvedValue([ - { id: 'dj-alice' }, - { id: 'dj-bob' }, - ]); + mockGetDJsInCurrentShow.mockResolvedValue([{ id: 'dj-alice' }, { id: 'dj-bob' }]); - const { req, res, next } = createMockReqResNext('dj-charlie'); + const { req, res, next, statusMock, jsonMock } = createMockReqResNext('dj-charlie'); await showMemberMiddleware(req, res, next); - expect(res.status).toHaveBeenCalledWith(400); - expect(res.json).toHaveBeenCalledWith({ + expect(statusMock).toHaveBeenCalledWith(400); + expect(jsonMock).toHaveBeenCalledWith({ message: 'Bad Request: DJ not a member of show', }); expect(next).not.toHaveBeenCalled(); }); it('allows a DJ who is in the current show', async () => { - mockGetDJsInCurrentShow.mockResolvedValue([ - { id: 'dj-alice' }, - { id: 'dj-bob' }, - ]); + mockGetDJsInCurrentShow.mockResolvedValue([{ id: 'dj-alice' }, { id: 'dj-bob' }]); - const { req, res, next } = createMockReqResNext('dj-alice'); + const { req, res, next, statusMock } = createMockReqResNext('dj-alice'); await showMemberMiddleware(req, res, next); expect(next).toHaveBeenCalled(); - expect(res.status).not.toHaveBeenCalled(); + expect(statusMock).not.toHaveBeenCalled(); }); it('rejects when there are no DJs in the current show', async () => { mockGetDJsInCurrentShow.mockResolvedValue([]); - const { req, res, next } = createMockReqResNext('dj-alice'); + const { req, res, next, statusMock } = createMockReqResNext('dj-alice'); await showMemberMiddleware(req, res, next); - expect(res.status).toHaveBeenCalledWith(400); + expect(statusMock).toHaveBeenCalledWith(400); expect(next).not.toHaveBeenCalled(); }); it('returns 500 when getDJsInCurrentShow throws', async () => { mockGetDJsInCurrentShow.mockRejectedValue(new Error('DB connection lost')); - const { req, res, next } = createMockReqResNext('dj-alice'); + const { req, res, next, statusMock, jsonMock } = createMockReqResNext('dj-alice'); await showMemberMiddleware(req, res, next); - expect(res.status).toHaveBeenCalledWith(500); - expect(res.json).toHaveBeenCalledWith({ + expect(statusMock).toHaveBeenCalledWith(500); + expect(jsonMock).toHaveBeenCalledWith({ message: 'Internal server error checking show membership', }); expect(next).not.toHaveBeenCalled(); From 18222d92ca46962af1d4d51301d366dc71120cbc Mon Sep 17 00:00:00 2001 From: Jake Bromberg Date: Fri, 27 Feb 2026 15:05:22 -0800 Subject: [PATCH 3/4] fix: bypass showMemberMiddleware when AUTH_BYPASS is true The showMemberMiddleware checks req.auth for the user ID, but when AUTH_BYPASS=true the requirePermissions middleware skips JWT verification and never sets req.auth. This caused all flowsheet write routes to return 400 in integration tests. Follow the same AUTH_BYPASS pattern used by requirePermissions. --- apps/backend/middleware/checkShowMember.ts | 4 ++++ tests/unit/middleware/checkShowMember.test.ts | 23 +++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/apps/backend/middleware/checkShowMember.ts b/apps/backend/middleware/checkShowMember.ts index 85a8875a..703d885e 100644 --- a/apps/backend/middleware/checkShowMember.ts +++ b/apps/backend/middleware/checkShowMember.ts @@ -2,6 +2,10 @@ import { RequestHandler } from 'express'; import { getDJsInCurrentShow } from '../services/flowsheet.service.js'; export const showMemberMiddleware: RequestHandler = async (req, res, next) => { + if (process.env.AUTH_BYPASS === 'true') { + return next(); + } + try { const show_djs = await getDJsInCurrentShow(); const user_id = req.auth?.id || req.auth?.sub || res.locals.decodedJWT?.id || res.locals.decodedJWT?.userId; diff --git a/tests/unit/middleware/checkShowMember.test.ts b/tests/unit/middleware/checkShowMember.test.ts index 206dc01b..e64d819f 100644 --- a/tests/unit/middleware/checkShowMember.test.ts +++ b/tests/unit/middleware/checkShowMember.test.ts @@ -28,7 +28,30 @@ function createMockReqResNext(userId: string) { } describe('showMemberMiddleware', () => { + const originalAuthBypass = process.env.AUTH_BYPASS; + + afterEach(() => { + if (originalAuthBypass !== undefined) { + process.env.AUTH_BYPASS = originalAuthBypass; + } else { + delete process.env.AUTH_BYPASS; + } + }); + + it('skips show membership check when AUTH_BYPASS is true', async () => { + process.env.AUTH_BYPASS = 'true'; + + const { req, res, next, statusMock } = createMockReqResNext('unknown-user'); + + await showMemberMiddleware(req, res, next); + + expect(next).toHaveBeenCalled(); + expect(statusMock).not.toHaveBeenCalled(); + expect(mockGetDJsInCurrentShow).not.toHaveBeenCalled(); + }); + it('rejects a DJ who is not in the current show', async () => { + delete process.env.AUTH_BYPASS; mockGetDJsInCurrentShow.mockResolvedValue([{ id: 'dj-alice' }, { id: 'dj-bob' }]); const { req, res, next, statusMock, jsonMock } = createMockReqResNext('dj-charlie'); From a002084622c0305e39661da95c5f4de9b246ec3f Mon Sep 17 00:00:00 2001 From: Jake Bromberg Date: Fri, 27 Feb 2026 15:26:26 -0800 Subject: [PATCH 4/4] test: fix AUTH_BYPASS env restoration in showMemberMiddleware tests Use beforeEach to clear AUTH_BYPASS before each test and afterAll to restore it once after the suite. The previous afterEach approach restored AUTH_BYPASS to 'true' between tests in CI, causing later tests to hit the bypass path instead of exercising the middleware. --- tests/unit/middleware/checkShowMember.test.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/unit/middleware/checkShowMember.test.ts b/tests/unit/middleware/checkShowMember.test.ts index e64d819f..8e825996 100644 --- a/tests/unit/middleware/checkShowMember.test.ts +++ b/tests/unit/middleware/checkShowMember.test.ts @@ -30,7 +30,11 @@ function createMockReqResNext(userId: string) { describe('showMemberMiddleware', () => { const originalAuthBypass = process.env.AUTH_BYPASS; - afterEach(() => { + beforeEach(() => { + delete process.env.AUTH_BYPASS; + }); + + afterAll(() => { if (originalAuthBypass !== undefined) { process.env.AUTH_BYPASS = originalAuthBypass; } else { @@ -51,7 +55,6 @@ describe('showMemberMiddleware', () => { }); it('rejects a DJ who is not in the current show', async () => { - delete process.env.AUTH_BYPASS; mockGetDJsInCurrentShow.mockResolvedValue([{ id: 'dj-alice' }, { id: 'dj-bob' }]); const { req, res, next, statusMock, jsonMock } = createMockReqResNext('dj-charlie');