Fix critical socket authorization issues

server/socket/index.js

@@ -39,7 +39,7 @@ const init = async () => {
       credentials: true,
     },
     cookie: {
-      name: config.auth.secret,
+      name: 'io',
       httpOnly: true,
       secure: process.env.NODE_ENV === 'prod',
     },

server/socket/lib/roomAuthorization.js

@@ -0,0 +1,31 @@
+const SUPER_ADMIN_ID = 8184;
+
+const canDeleteRoom = (userId, room) => {
+  if (+userId === SUPER_ADMIN_ID) {
+    return true;
+  }
+
+  if (!userId || !room) {
+    return false;
+  }
+
+  return [room.owner, room.admin].some((user) => user && +user.id === +userId);
+};
+
+const canAccessRoom = (userId, room) => {
+  if (!room) {
+    return false;
+  }
+
+  if (!userId) {
+    return true;
+  }
+
+  const userKey = userId.toString();
+  return !!room.inRoom.get(userKey) && !room.banned.get(userKey);
+};
+
+module.exports = {
+  canAccessRoom,
+  canDeleteRoom,
+};

server/socket/lib/roomAuthorization.test.js

@@ -0,0 +1,42 @@
+/* eslint-env jest */
+
+const { canAccessRoom, canDeleteRoom } = require('./roomAuthorization');
+
+describe('canDeleteRoom', () => {
+  const room = {
+    owner: { id: 101 },
+    admin: { id: 202 },
+  };
+
+  it('allows the room owner and current admin', () => {
+    expect(canDeleteRoom(101, room)).toBe(true);
+    expect(canDeleteRoom('202', room)).toBe(true);
+  });
+
+  it('rejects users without authority over the target room', () => {
+    expect(canDeleteRoom(303, room)).toBe(false);
+    expect(canDeleteRoom(null, room)).toBe(false);
+  });
+
+  it('allows the global administrator', () => {
+    expect(canDeleteRoom(8184, null)).toBe(true);
+  });
+});
+
+describe('canAccessRoom', () => {
+  const room = ({ inRoom = true, banned = false } = {}) => ({
+    inRoom: new Map([['101', inRoom]]),
+    banned: new Map([['101', banned]]),
+  });
+
+  it('allows anonymous spectators and active room users', () => {
+    expect(canAccessRoom(null, room())).toBe(true);
+    expect(canAccessRoom(101, room())).toBe(true);
+  });
+
+  it('rejects missing rooms, removed users, and banned users', () => {
+    expect(canAccessRoom(101, null)).toBe(false);
+    expect(canAccessRoom(101, room({ inRoom: false }))).toBe(false);
+    expect(canAccessRoom(101, room({ banned: true }))).toBe(false);
+  });
+});

server/socket/lib/roomSockets.js

@@ -0,0 +1,10 @@
+const { encodeUserRoom } = require('../utils');
+
+const removeUserFromRoomSockets = (namespace, userId, room) => {
+  const userRoom = encodeUserRoom(userId, room._id);
+  namespace.in(userRoom).socketsLeave([room.accessCode, userRoom]);
+};
+
+module.exports = {
+  removeUserFromRoomSockets,
+};

server/socket/lib/roomSockets.test.js

@@ -0,0 +1,20 @@
+/* eslint-env jest */
+
+const { removeUserFromRoomSockets } = require('./roomSockets');
+
+describe('removeUserFromRoomSockets', () => {
+  it('uses Socket.IO 4 socketsLeave for every socket in the user-room', () => {
+    const socketsLeave = jest.fn();
+    const inRoom = jest.fn(() => ({ socketsLeave }));
+    const namespace = { in: inRoom };
+    const room = { _id: 'room-123', accessCode: 'access-123' };
+
+    removeUserFromRoomSockets(namespace, 101, room);
+
+    expect(inRoom).toHaveBeenCalledWith('user-room/101-room-123');
+    expect(socketsLeave).toHaveBeenCalledWith([
+      'access-123',
+      'user-room/101-room-123',
+    ]);
+  });
+});

server/socket/lib/socketHandler.js

@@ -0,0 +1,31 @@
+const createSafeSocketHandler = (socket, logger, errorEvent) => (event, handler) => {
+  socket.on(event, (...args) => {
+    Promise.resolve()
+      .then(() => handler(...args))
+      .catch((err) => {
+        logger.error(err);
+
+        const response = {
+          statusCode: 500,
+          event,
+          message: 'Socket event failed',
+        };
+        const acknowledgment = args[args.length - 1];
+
+        if (typeof acknowledgment === 'function') {
+          acknowledgment(response);
+        } else if (socket.connected) {
+          socket.emit(errorEvent, response);
+        }
+      });
+  });
+};
+
+const optionalAcknowledgment = (acknowledgment) => (
+  typeof acknowledgment === 'function' ? acknowledgment : () => {}
+);
+
+module.exports = {
+  createSafeSocketHandler,
+  optionalAcknowledgment,
+};

server/socket/lib/socketHandler.test.js

@@ -0,0 +1,73 @@
+/* eslint-env jest */
+
+const {
+  createSafeSocketHandler,
+  optionalAcknowledgment,
+} = require('./socketHandler');
+
+describe('createSafeSocketHandler', () => {
+  const flushPromises = () => new Promise(setImmediate);
+
+  it('reports rejected handlers without leaving an unhandled rejection', async () => {
+    const listeners = {};
+    const socket = {
+      connected: true,
+      emit: jest.fn(),
+      on: jest.fn((event, handler) => {
+        listeners[event] = handler;
+      }),
+    };
+    const logger = { error: jest.fn() };
+    const on = createSafeSocketHandler(socket, logger, 'errorrr');
+    const failure = new Error('bad payload');
+
+    on('join_room', async () => {
+      throw failure;
+    });
+    listeners.join_room();
+    await flushPromises();
+
+    expect(logger.error).toHaveBeenCalledWith(failure);
+    expect(socket.emit).toHaveBeenCalledWith('errorrr', {
+      statusCode: 500,
+      event: 'join_room',
+      message: 'Socket event failed',
+    });
+  });
+
+  it('uses an acknowledgement callback when one was provided', async () => {
+    const listeners = {};
+    const socket = {
+      connected: true,
+      emit: jest.fn(),
+      on: jest.fn((event, handler) => {
+        listeners[event] = handler;
+      }),
+    };
+    const logger = { error: jest.fn() };
+    const acknowledgment = jest.fn();
+    const on = createSafeSocketHandler(socket, logger, 'errorrr');
+
+    on('delete_room', () => {
+      throw new Error('missing room');
+    });
+    listeners.delete_room('room-123', acknowledgment);
+    await flushPromises();
+
+    expect(acknowledgment).toHaveBeenCalledWith({
+      statusCode: 500,
+      event: 'delete_room',
+      message: 'Socket event failed',
+    });
+    expect(socket.emit).not.toHaveBeenCalled();
+  });
+});
+
+describe('optionalAcknowledgment', () => {
+  it('preserves callbacks and substitutes a no-op for missing callbacks', () => {
+    const acknowledgment = jest.fn();
+
+    expect(optionalAcknowledgment(acknowledgment)).toBe(acknowledgment);
+    expect(() => optionalAcknowledgment(undefined)({ statusCode: 400 })).not.toThrow();
+  });
+});