fix: use authenticated user ID for bin operations instead of client-supplied dj_id

.github/workflows/test.yml

@@ -26,6 +26,8 @@ env:
   DB_PASSWORD: test-pw
   DB_NAME: wxyc_db
   AUTH_BYPASS: true
+  AUTH_USERNAME: test_dj1
+  AUTH_PASSWORD: testpassword123
 
 jobs:
   # Detect what changed to conditionally run jobs

apps/backend/controllers/djs.controller.ts

@@ -10,12 +10,12 @@ export type binBody = {
 };
 
 export const addToBin: RequestHandler<object, unknown, binBody> = async (req, res, next) => {
-  if (req.body.album_id === undefined || req.body.dj_id === undefined) {
+  if (req.body.album_id === undefined) {
     console.error('Bad Request, Missing Album Identifier: album_id');
-    res.status(400).send('Bad Request, Missing DJ or album identifier: album_id');
+    res.status(400).send('Bad Request, Missing album identifier: album_id');
   } else {
     const bin_entry: NewBinEntry = {
-      dj_id: req.body.dj_id,
+      dj_id: req.auth!.id!,
       album_id: req.body.album_id,
       track_title: req.body.track_title === undefined ? null : req.body.track_title,
     };
@@ -38,13 +38,12 @@ export type binQuery = {
 };
 
 export const deleteFromBin: RequestHandler<object, unknown, unknown, binQuery> = async (req, res, next) => {
-  if (req.query.album_id === undefined || req.query.dj_id === undefined) {
-    console.error('Bad Request, Missing Bin Entry Identifier: album_id or dj_id');
-    res.status(400).send('Bad Request, Missing Bin Entry Identifier: album_id or dj_id');
+  if (req.query.album_id === undefined) {
+    console.error('Bad Request, Missing Bin Entry Identifier: album_id');
+    res.status(400).send('Bad Request, Missing Bin Entry Identifier: album_id');
   } else {
     try {
-      //check that the dj_id === dj_id of bin entry
-      const removed_bin_item = await DJService.removeFromBin(parseInt(req.query.album_id), req.query.dj_id);
+      const removed_bin_item = await DJService.removeFromBin(parseInt(req.query.album_id), req.auth!.id!);
       res.status(200).json(removed_bin_item);
     } catch (e) {
       console.error(e);
@@ -53,19 +52,14 @@ export const deleteFromBin: RequestHandler<object, unknown, unknown, binQuery> =
   }
 };
 
-export const getBin: RequestHandler<object, unknown, object, { dj_id: string }> = async (req, res, next) => {
-  if (req.query.dj_id === undefined) {
-    console.error('Bad Request, Missing DJ Identifier: dj_id');
-    res.status(400).send('Bad Request, Missing DJ Identifier: dj_id');
-  } else {
-    try {
-      const dj_bin = await DJService.getBinFromDB(req.query.dj_id);
-      res.status(200).json(dj_bin);
-    } catch (e) {
-      console.error("Error: Failed to retrieve dj's bin");
-      console.error(e);
-      next(e);
-    }
+export const getBin: RequestHandler = async (req, res, next) => {
+  try {
+    const dj_bin = await DJService.getBinFromDB(req.auth!.id!);
+    res.status(200).json(dj_bin);
+  } catch (e) {
+    console.error("Error: Failed to retrieve dj's bin");
+    console.error(e);
+    next(e);
   }
 };
 

shared/authentication/src/auth.middleware.ts

@@ -1,5 +1,5 @@
 import { NextFunction, Request, Response } from 'express';
-import { createRemoteJWKSet, jwtVerify, type JWTPayload } from 'jose';
+import { createRemoteJWKSet, decodeJwt, jwtVerify, type JWTPayload } from 'jose';
 import { AccessControlStatement, WXYCRole, WXYCRoles, normalizeRole } from './auth.roles';
 
 // JWT payload structure expected from better-auth JWT plugin
@@ -53,6 +53,26 @@ export type RequiredPermissions = {
 export function requirePermissions(required: RequiredPermissions) {
   return async (req: Request, res: Response, next: NextFunction) => {
     if (process.env.AUTH_BYPASS === 'true') {
+      // In bypass mode, try to decode the JWT (without verification) so that
+      // req.auth is populated for controllers that rely on req.auth.id.
+      // If the token is not a valid JWT (e.g. integration tests pass a raw
+      // user ID), fall back to using the token value as the user ID directly.
+      const authHeader = req.headers.authorization;
+      if (authHeader) {
+        const token = authHeader.startsWith('Bearer ') ? authHeader.slice(7).trim() : authHeader.trim();
+        if (token) {
+          try {
+            const payload = decodeJwt(token) as WXYCAuthJwtPayload;
+            const userId = payload.id || payload.sub;
+            if (userId) {
+              req.auth = { ...payload, id: userId } as WXYCAuthJwtPayload;
+            }
+          } catch {
+            // Token is not a valid JWT -- treat it as a raw user ID.
+            req.auth = { id: token } as WXYCAuthJwtPayload;
+          }
+        }
+      }
       return next();
     }
 

tests/integration/djs.spec.js

@@ -6,9 +6,9 @@ const { createAuthRequest, expectErrorContains, expectFields, expectArray } = re
  * DJ Endpoints Integration Tests
  *
  * Tests for:
- * - GET /djs/bin - Retrieve DJ's bin
- * - POST /djs/bin - Add entry to bin
- * - DELETE /djs/bin - Remove entry from bin
+ * - GET /djs/bin - Retrieve authenticated DJ's bin
+ * - POST /djs/bin - Add entry to authenticated DJ's bin
+ * - DELETE /djs/bin - Remove entry from authenticated DJ's bin
  * - GET /djs/playlists - Get playlists for a DJ
  */
 
@@ -20,20 +20,8 @@ describe('DJ Bin', () => {
   });
 
   describe('GET /djs/bin', () => {
-    test('returns array for DJ', async () => {
-      const res = await auth.get('/djs/bin').query({ dj_id: global.primary_dj_id }).expect(200);
-
-      expectArray(res);
-    });
-
-    test('returns 400 when dj_id is missing', async () => {
-      const res = await auth.get('/djs/bin').expect(400);
-
-      expectErrorContains(res, 'Missing DJ Identifier');
-    });
-
-    test('returns empty array for DJ with no bin entries', async () => {
-      const res = await auth.get('/djs/bin').query({ dj_id: global.secondary_dj_id }).expect(200);
+    test('returns array for authenticated DJ', async () => {
+      const res = await auth.get('/djs/bin').expect(200);
 
       expectArray(res);
     });
@@ -42,28 +30,25 @@ describe('DJ Bin', () => {
   describe('POST /djs/bin', () => {
     afterEach(async () => {
       // Clean up any bin entries created during tests
-      await auth.delete('/djs/bin').query({ dj_id: global.primary_dj_id, album_id: 1 });
+      await auth.delete('/djs/bin').query({ album_id: 1 });
     });
 
     test('adds entry to bin successfully', async () => {
       const res = await auth
         .post('/djs/bin')
         .send({
-          dj_id: global.primary_dj_id,
           album_id: 1,
         })
         .expect(200);
 
       expectFields(res.body, 'album_id', 'dj_id');
       expect(res.body.album_id).toBe(1);
-      expect(res.body.dj_id).toBe(global.primary_dj_id);
     });
 
     test('adds entry with track title to bin', async () => {
       const res = await auth
         .post('/djs/bin')
         .send({
-          dj_id: global.primary_dj_id,
           album_id: 1,
           track_title: 'Carry the Zero',
         })
@@ -75,23 +60,7 @@ describe('DJ Bin', () => {
     });
 
     test('returns 400 when album_id is missing', async () => {
-      const res = await auth
-        .post('/djs/bin')
-        .send({
-          dj_id: global.primary_dj_id,
-        })
-        .expect(400);
-
-      expectErrorContains(res, 'Missing');
-    });
-
-    test('returns 400 when dj_id is missing', async () => {
-      const res = await auth
-        .post('/djs/bin')
-        .send({
-          album_id: 1,
-        })
-        .expect(400);
+      const res = await auth.post('/djs/bin').send({}).expect(400);
 
       expectErrorContains(res, 'Missing');
     });
@@ -101,31 +70,24 @@ describe('DJ Bin', () => {
     beforeEach(async () => {
       // Set up a bin entry to delete
       await auth.post('/djs/bin').send({
-        dj_id: global.primary_dj_id,
         album_id: 2,
       });
     });
 
     test('removes entry from bin successfully', async () => {
-      const res = await auth.delete('/djs/bin').query({ dj_id: global.primary_dj_id, album_id: 2 }).expect(200);
+      const res = await auth.delete('/djs/bin').query({ album_id: 2 }).expect(200);
 
       expect(res.body).toBeDefined();
 
       // Verify it was removed
-      const binRes = await auth.get('/djs/bin').query({ dj_id: global.primary_dj_id }).expect(200);
+      const binRes = await auth.get('/djs/bin').expect(200);
 
       const entry = binRes.body.find((e) => e.album_id === 2);
       expect(entry).toBeUndefined();
     });
 
     test('returns 400 when album_id is missing', async () => {
-      const res = await auth.delete('/djs/bin').query({ dj_id: global.primary_dj_id }).expect(400);
-
-      expectErrorContains(res, 'Missing');
-    });
-
-    test('returns 400 when dj_id is missing', async () => {
-      const res = await auth.delete('/djs/bin').query({ album_id: 2 }).expect(400);
+      const res = await auth.delete('/djs/bin').expect(400);
 
       expectErrorContains(res, 'Missing');
     });

tests/mocks/database.mock.ts

@@ -86,6 +86,7 @@ export const flowsheet = {
   message: 'message',
   add_time: 'add_time',
 };
+export const bins = {};
 export const shows = {};
 export const show_djs = {};
 export const user = {};
@@ -131,3 +132,11 @@ export type NewFSEntry = Partial<FSEntry>;
 export type Show = Record<string, unknown>;
 export type ShowDJ = Record<string, unknown>;
 export type User = Record<string, unknown>;
+
+export type BinEntry = {
+  id: number;
+  dj_id: string;
+  album_id: number;
+  track_title: string | null;
+};
+export type NewBinEntry = Omit<BinEntry, 'id'>;

tests/setup/integration.setup.js

@@ -49,10 +49,5 @@ async function getUserIdsFromDatabase() {
 // Note: Service readiness is checked in globalSetup.js (runs once for all tests)
 beforeAll(async () => {
   await getUserIdsFromDatabase();
-
-  if (process.env.AUTH_BYPASS === 'true') {
-    global.access_token = 'Auth Bypass Enabled';
-  } else {
-    global.access_token = await get_access_token();
-  }
+  global.access_token = await get_access_token();
 });

tests/unit/authentication/auth.middleware.test.ts

@@ -6,17 +6,19 @@ process.env.BETTER_AUTH_ISSUER = 'https://test.example.com';
 process.env.BETTER_AUTH_AUDIENCE = 'https://test.example.com';
 
 // Mock jose -- the middleware calls createRemoteJWKSet at module scope
-// and jwtVerify on each request
+// and jwtVerify on each request. decodeJwt is used in AUTH_BYPASS mode.
 jest.mock('jose', () => ({
   createRemoteJWKSet: jest.fn(() => jest.fn()),
   jwtVerify: jest.fn(),
+  decodeJwt: jest.fn(),
 }));
 
-import { jwtVerify } from 'jose';
+import { jwtVerify, decodeJwt } from 'jose';
 import { requirePermissions } from '../../../shared/authentication/src/auth.middleware';
 import type { Request, Response, NextFunction } from 'express';
 
 const mockedJwtVerify = jwtVerify as jest.MockedFunction<typeof jwtVerify>;
+const mockedDecodeJwt = decodeJwt as jest.MockedFunction<typeof decodeJwt>;
 
 function createMocks(authHeader?: string) {
   const req = {
@@ -62,8 +64,11 @@ describe('requirePermissions middleware', () => {
   });
 
   describe('AUTH_BYPASS', () => {
-    it('should skip all validation when AUTH_BYPASS is "true"', async () => {
+    beforeEach(() => {
       process.env.AUTH_BYPASS = 'true';
+    });
+
+    it('should skip all validation when AUTH_BYPASS is "true"', async () => {
       const { req, res, next } = createMocks();
       const middleware = requirePermissions({ catalog: ['read'] });
 
@@ -72,6 +77,35 @@ describe('requirePermissions middleware', () => {
       expect(next).toHaveBeenCalled();
       expect(res.status).not.toHaveBeenCalled();
     });
+
+    it('should populate req.auth from decoded JWT when token is a valid JWT', async () => {
+      mockedDecodeJwt.mockReturnValue({
+        sub: 'jwt-user-id',
+        email: 'test@wxyc.org',
+        role: 'dj',
+      } as any);
+      const { req, res, next } = createMocks('Bearer some.jwt.token');
+      const middleware = requirePermissions({ bin: ['read'] });
+
+      await middleware(req, res, next);
+
+      expect(next).toHaveBeenCalled();
+      expect(req.auth?.id).toBe('jwt-user-id');
+    });
+
+    it('should use raw token as user ID when token is not a valid JWT', async () => {
+      mockedDecodeJwt.mockImplementation(() => {
+        throw new Error('Invalid token');
+      });
+      const userId = 'abc-123-user-id';
+      const { req, res, next } = createMocks(`Bearer ${userId}`);
+      const middleware = requirePermissions({ bin: ['read'] });
+
+      await middleware(req, res, next);
+
+      expect(next).toHaveBeenCalled();
+      expect(req.auth?.id).toBe(userId);
+    });
   });
 
   describe('token validation', () => {

tests/unit/controllers/djs.bin-idor.test.ts

@@ -0,0 +1,62 @@
+import { jest } from '@jest/globals';
+import type { Request, Response, NextFunction } from 'express';
+
+jest.mock('../../../apps/backend/services/djs.service', () => ({
+  addToBin: jest.fn().mockResolvedValue({ id: 1, dj_id: 'user-A', album_id: 1, track_title: null } as never),
+  removeFromBin: jest.fn().mockResolvedValue({ id: 1, dj_id: 'user-A', album_id: 1, track_title: null } as never),
+  getBinFromDB: jest.fn().mockResolvedValue([] as never),
+}));
+
+import * as DJService from '../../../apps/backend/services/djs.service';
+import { addToBin, deleteFromBin, getBin } from '../../../apps/backend/controllers/djs.controller';
+
+function mockResponse(): Response {
+  const res = {} as Response;
+  res.status = jest.fn().mockReturnValue(res) as unknown as Response['status'];
+  res.json = jest.fn().mockReturnValue(res) as unknown as Response['json'];
+  res.send = jest.fn().mockReturnValue(res) as unknown as Response['send'];
+  return res;
+}
+
+const next: NextFunction = jest.fn();
+
+describe('djs.controller bin endpoints – IDOR protection', () => {
+  describe('addToBin', () => {
+    it('uses authenticated user ID, not client-supplied dj_id', async () => {
+      const req = {
+        auth: { id: 'user-A' },
+        body: { dj_id: 'user-B', album_id: 1 },
+      } as unknown as Request;
+
+      await addToBin(req, mockResponse(), next);
+
+      expect(DJService.addToBin).toHaveBeenCalledWith(expect.objectContaining({ dj_id: 'user-A' }));
+    });
+  });
+
+  describe('deleteFromBin', () => {
+    it('uses authenticated user ID, not client-supplied dj_id', async () => {
+      const req = {
+        auth: { id: 'user-A' },
+        query: { dj_id: 'user-B', album_id: '1' },
+      } as unknown as Request;
+
+      await deleteFromBin(req, mockResponse(), next);
+
+      expect(DJService.removeFromBin).toHaveBeenCalledWith(1, 'user-A');
+    });
+  });
+
+  describe('getBin', () => {
+    it('uses authenticated user ID, not client-supplied dj_id', async () => {
+      const req = {
+        auth: { id: 'user-A' },
+        query: { dj_id: 'user-B' },
+      } as unknown as Request;
+
+      await getBin(req, mockResponse(), next);
+
+      expect(DJService.getBinFromDB).toHaveBeenCalledWith('user-A');
+    });
+  });
+});