From 2425a90f40e5b2b61fd6449bbc07b24df69d2a90 Mon Sep 17 00:00:00 2001 From: Matteo Susca Date: Thu, 29 Jan 2026 12:43:32 +0100 Subject: [PATCH 1/6] PIX-58 validate lobby access --- .../src/middlewares/permissionMiddleware.ts | 9 ++-- server/src/services/lobby.service.ts | 28 +++++++++++ server/src/sockets/index.ts | 49 ++++++++++++------- server/src/utils/socketUtils.ts | 33 +++++++++++++ 4 files changed, 97 insertions(+), 22 deletions(-) create mode 100644 server/src/utils/socketUtils.ts diff --git a/server/src/middlewares/permissionMiddleware.ts b/server/src/middlewares/permissionMiddleware.ts index c2f2bb3..9feeb07 100644 --- a/server/src/middlewares/permissionMiddleware.ts +++ b/server/src/middlewares/permissionMiddleware.ts @@ -2,6 +2,7 @@ import { Response, NextFunction } from "express"; import { AuthRequest } from "./authMiddleware.js"; import { Lobby } from "../models/Lobby.js"; import { Types } from "mongoose"; +import { LobbyService } from "../services/lobby.service.js"; /** * Middleware to check if the user is a System Administrator. @@ -61,9 +62,11 @@ export const requireLobbyAccess = async (req: AuthRequest, res: Response, next: return res.status(404).json({ error: "Lobby not found" }); } - // Check if user is banned - if (lobby.bannedUsers.some((id: Types.ObjectId) => id.toString() === userId)) { - return res.status(403).json({ error: "Access denied. You are banned from this lobby." }); + // reused logic from Service + try { + LobbyService.validateJoinAccess(lobby, userId); + } catch (e: any) { + return res.status(403).json({ error: e.message }); } // If we implement private lobbies in the future, check allowedUsers here diff --git a/server/src/services/lobby.service.ts b/server/src/services/lobby.service.ts index ec257e5..00b03da 100644 --- a/server/src/services/lobby.service.ts +++ b/server/src/services/lobby.service.ts @@ -35,4 +35,32 @@ export class LobbyService { static async delete(id: string) { return await Lobby.findByIdAndDelete(id); } + + // Common validation logic for joining/accessing a lobby + static validateJoinAccess(lobby: ILobby, userId: string): void { + if (lobby.bannedUsers.some((id: any) => id.toString() === userId)) { + throw new Error("Access denied. You are banned from this lobby."); + } + } + + static validateCapacity(lobby: ILobby, currentCount: number): void { + if (currentCount >= lobby.maxCollaborators) { + throw new Error(`Lobby is full`); + } + } + + static async verifyCanJoin(lobbyName: string, userId: string, currentCount: number) { + const lobby = await this.getByName(lobbyName); + if (!lobby) { + throw new Error("Lobby not found"); + } + + // 1. Check Banned + this.validateJoinAccess(lobby, userId); + + // 2. Check Capacity + this.validateCapacity(lobby, currentCount); + + return lobby; + } } \ No newline at end of file diff --git a/server/src/sockets/index.ts b/server/src/sockets/index.ts index 238aa39..4a518a0 100644 --- a/server/src/sockets/index.ts +++ b/server/src/sockets/index.ts @@ -4,6 +4,7 @@ import { CONFIG } from '../config.js'; import { DrawPayload, DrawBatchPayload, AuthenticatedSocket } from './types.js'; import { LobbyService } from '../services/lobby.service.js'; import jwt from 'jsonwebtoken'; +import { getLobbyUserCount, getUsersInLobby, broadcastToLobby, broadcastToOthers } from '../utils/socketUtils.js'; export const setupSocket = (io: Server) => { // Authentication Middleware @@ -33,26 +34,36 @@ export const setupSocket = (io: Server) => { socket.on(CONFIG.EVENTS.CLIENT.JOIN_LOBBY, async (lobbyName: string) => { console.log(`[Socket] ${socket.id} joining lobby: ${lobbyName}`); - // 1. Join the Socket.io room channel - socket.join(lobbyName); + try { + // 1. Get User + const user = (socket as AuthenticatedSocket).user; + if (!user || !user.id) { + console.error(`[Socket] Error: User not found on socket ${socket.id}`); + return; + } - const user = (socket as AuthenticatedSocket).user; - if (!user) { - console.error(`[Socket] Error: User not found on socket ${socket.id}`); - return; - } + // 2. Validate Join (Service checks existence, ban status, and capacity) + const currentCount = getLobbyUserCount(io, lobbyName); - try { - // 2. Broadcast to others that a new user joined (First, to avoid race conditions) - socket.to(lobbyName).emit(CONFIG.EVENTS.SERVER.USER_JOINED, user); + try { + await LobbyService.verifyCanJoin(lobbyName, user.id, currentCount); + } catch (e: any) { + console.error(`[Socket] Join blocked: ${e.message}`); + socket.emit(CONFIG.EVENTS.SERVER.ERROR, { message: e.message }); + return; + } + + // 3. Join the Socket.io room channel + socket.join(lobbyName); + + // 4. Broadcast to others that a new user joined + broadcastToOthers(socket, lobbyName, CONFIG.EVENTS.SERVER.USER_JOINED, user); - // 3. Send list of connected users to the new user - // We fetch sockets AFTER joining so the user is included in the list - const sockets = await io.in(lobbyName).fetchSockets(); - const users = sockets.map(s => s.data.user).filter(u => u); + // 5. Send list of connected users to the new user + const users = await getUsersInLobby(io, lobbyName); socket.emit(CONFIG.EVENTS.SERVER.LOBBY_USERS, users); - // 4. Get current state from Service & Send to user + // 6. Get current state from Service & Send to user const state = await CanvasService.getState(lobbyName); socket.emit(CONFIG.EVENTS.SERVER.INIT_STATE, state); } catch (error) { @@ -75,7 +86,7 @@ export const setupSocket = (io: Server) => { // 2. Broadcast if successful if (success) { // Send to everyone in the room INCLUDING the sender (for consistency) - io.to(lobbyName).emit(CONFIG.EVENTS.SERVER.PIXEL_UPDATE, { x, y, color }); + broadcastToLobby(io, lobbyName, CONFIG.EVENTS.SERVER.PIXEL_UPDATE, { x, y, color }); } }); @@ -105,7 +116,7 @@ export const setupSocket = (io: Server) => { // 2. Broadcast batch if any successful if (successfulUpdates.length > 0) { // Send to everyone in the room INCLUDING the sender (for consistency) - io.to(lobbyName).emit(CONFIG.EVENTS.SERVER.PIXEL_UPDATE_BATCH, { pixels: successfulUpdates }); + broadcastToLobby(io, lobbyName, CONFIG.EVENTS.SERVER.PIXEL_UPDATE_BATCH, { pixels: successfulUpdates }); } }); @@ -115,10 +126,10 @@ export const setupSocket = (io: Server) => { // Notify rooms that user is leaving for (const room of socket.rooms) { if (room !== socket.id) { - socket.to(room).emit(CONFIG.EVENTS.SERVER.USER_LEFT, (socket as AuthenticatedSocket).user); + broadcastToOthers(socket, room, CONFIG.EVENTS.SERVER.USER_LEFT, (socket as AuthenticatedSocket).user); // Check if room is empty (excluding this socket) - const socketsInRoom = await io.in(room).fetchSockets(); + const socketsInRoom = await getUsersInLobby(io, room); const remainingUsers = socketsInRoom.length - 1; // fetchSockets includes the disconnecting socket if (remainingUsers <= 0) { diff --git a/server/src/utils/socketUtils.ts b/server/src/utils/socketUtils.ts new file mode 100644 index 0000000..86237fb --- /dev/null +++ b/server/src/utils/socketUtils.ts @@ -0,0 +1,33 @@ +import { Server, Socket } from 'socket.io'; + +/** + * Gets the number of connected users in a specific lobby. + * @param io The Socket.IO server instance + * @param lobbyName The name of the lobby (room) + * @returns The number of clients in the room + */ +export const getLobbyUserCount = (io: Server, lobbyName: string): number => { + return io.sockets.adapter.rooms.get(lobbyName)?.size || 0; +}; + +/** + * Fetches all users currently connected to a lobby. + */ +export const getUsersInLobby = async (io: Server, lobbyName: string): Promise => { + const sockets = await io.in(lobbyName).fetchSockets(); + return sockets.map(s => s.data.user).filter(u => u); +}; + +/** + * Broadcasts an event to all users in a lobby (including sender). + */ +export const broadcastToLobby = (io: Server, lobbyName: string, event: string, data: any) => { + io.to(lobbyName).emit(event, data); +}; + +/** + * Broadcasts an event to all other users in a lobby (excluding sender). + */ +export const broadcastToOthers = (socket: Socket, lobbyName: string, event: string, data: any) => { + socket.to(lobbyName).emit(event, data); +}; From ccee2c66a30d18e7adadefb115b3a2817f821a2e Mon Sep 17 00:00:00 2001 From: Matteo Susca Date: Thu, 29 Jan 2026 15:19:36 +0100 Subject: [PATCH 2/6] PIX-58 fix join race condition and simplify validation logic --- client/src/views/PlayView.vue | 3 +- server/src/services/canvas.service.ts | 20 ++++ server/src/sockets/index.ts | 128 ++++++-------------------- 3 files changed, 51 insertions(+), 100 deletions(-) diff --git a/client/src/views/PlayView.vue b/client/src/views/PlayView.vue index f269f72..de69db3 100644 --- a/client/src/views/PlayView.vue +++ b/client/src/views/PlayView.vue @@ -8,6 +8,7 @@ import MobileNavBar from '@/components/lobbies/MobileNavBar.vue'; import LobbyHeader from '@/components/lobbies/LobbyHeader.vue'; import { useRoute } from 'vue-router'; import { getLobbyById } from '../services/api'; +import { onUnmounted } from 'vue'; // Setup Store const store = useEditorStore(); @@ -35,8 +36,6 @@ onMounted(async () => { store.init(lobbyName); }); -import { onUnmounted } from 'vue'; - onUnmounted(() => { store.cleanup(); }); diff --git a/server/src/services/canvas.service.ts b/server/src/services/canvas.service.ts index 4fdaa12..e8d9269 100644 --- a/server/src/services/canvas.service.ts +++ b/server/src/services/canvas.service.ts @@ -58,6 +58,26 @@ export class CanvasService { return changed; } + // Updates multiple pixels + static drawBatch(lobbyName: string, pixels: { x: number, y: number, color: number }[]): { x: number, y: number, color: number }[] { + const successfulUpdates: { x: number, y: number, color: number }[] = []; + let anyChanged = false; + + for (const p of pixels) { + // Basic validation + if (!p || typeof p.x !== 'number' || typeof p.y !== 'number' || typeof p.color !== 'number') continue; + + const changed = canvasStore.modifyPixelColor(lobbyName, p.x, p.y, p.color); + if (changed) { + successfulUpdates.push(p); + anyChanged = true; + } + } + + if (anyChanged) this.scheduleSave(lobbyName); + return successfulUpdates; + } + // Schedules a DB save if one isn't already pending private static scheduleSave(lobbyName: string) { if (this.saveTimers.has(lobbyName)) { diff --git a/server/src/sockets/index.ts b/server/src/sockets/index.ts index 4a518a0..734aea8 100644 --- a/server/src/sockets/index.ts +++ b/server/src/sockets/index.ts @@ -7,19 +7,11 @@ import jwt from 'jsonwebtoken'; import { getLobbyUserCount, getUsersInLobby, broadcastToLobby, broadcastToOthers } from '../utils/socketUtils.js'; export const setupSocket = (io: Server) => { - // Authentication Middleware io.use((socket, next) => { const token = socket.handshake.auth.token; - - if (!token) { - return next(new Error("Authentication error: No token provided")); - } - + if (!token) return next(new Error("Authentication error: No token provided")); jwt.verify(token, CONFIG.JWT.SECRET, (err: any, decoded: any) => { - if (err) { - return next(new Error("Authentication error: Invalid token")); - } - // Attach user info to socket + if (err) return next(new Error("Authentication error: Invalid token")); (socket as AuthenticatedSocket).user = decoded; socket.data.user = decoded; next(); @@ -29,121 +21,61 @@ export const setupSocket = (io: Server) => { io.on('connection', (socket: Socket) => { console.log(`[Socket] New connection: ${socket.id}`); - // --- EVENT: JOIN_LOBBY --- - // User requests to enter a specific room socket.on(CONFIG.EVENTS.CLIENT.JOIN_LOBBY, async (lobbyName: string) => { - console.log(`[Socket] ${socket.id} joining lobby: ${lobbyName}`); - try { - // 1. Get User const user = (socket as AuthenticatedSocket).user; - if (!user || !user.id) { - console.error(`[Socket] Error: User not found on socket ${socket.id}`); - return; - } + if (!user?.id) return console.error(`[Socket] User not found: ${socket.id}`); - // 2. Validate Join (Service checks existence, ban status, and capacity) - const currentCount = getLobbyUserCount(io, lobbyName); + const lobby = await LobbyService.getByName(lobbyName); + if (!lobby) return socket.emit(CONFIG.EVENTS.SERVER.ERROR, { message: "Lobby not found" }); try { - await LobbyService.verifyCanJoin(lobbyName, user.id, currentCount); + LobbyService.validateJoinAccess(lobby, user.id); } catch (e: any) { - console.error(`[Socket] Join blocked: ${e.message}`); - socket.emit(CONFIG.EVENTS.SERVER.ERROR, { message: e.message }); - return; + return socket.emit(CONFIG.EVENTS.SERVER.ERROR, { message: e.message }); } - // 3. Join the Socket.io room channel - socket.join(lobbyName); - - // 4. Broadcast to others that a new user joined - broadcastToOthers(socket, lobbyName, CONFIG.EVENTS.SERVER.USER_JOINED, user); + socket.join(lobbyName); // Optimistic Join - // 5. Send list of connected users to the new user - const users = await getUsersInLobby(io, lobbyName); - socket.emit(CONFIG.EVENTS.SERVER.LOBBY_USERS, users); + const currentCount = getLobbyUserCount(io, lobbyName); + try { + LobbyService.validateCapacity(lobby, currentCount - 1); + } catch (e: any) { + socket.leave(lobbyName); + return socket.emit(CONFIG.EVENTS.SERVER.ERROR, { message: "Lobby is full" }); + } - // 6. Get current state from Service & Send to user - const state = await CanvasService.getState(lobbyName); - socket.emit(CONFIG.EVENTS.SERVER.INIT_STATE, state); + broadcastToOthers(socket, lobbyName, CONFIG.EVENTS.SERVER.USER_JOINED, user); + socket.emit(CONFIG.EVENTS.SERVER.LOBBY_USERS, await getUsersInLobby(io, lobbyName)); + socket.emit(CONFIG.EVENTS.SERVER.INIT_STATE, await CanvasService.getState(lobbyName)); + console.log(`[Socket] ${socket.id} joined ${lobbyName}`); } catch (error) { - console.error(`[Socket] Error joining lobby ${lobbyName}:`, error); + console.error(`[Socket] Join Error:`, error); socket.emit(CONFIG.EVENTS.SERVER.ERROR, { message: "Failed to join lobby" }); } }); - // --- EVENT: DRAW --- - // User wants to color a pixel - socket.on(CONFIG.EVENTS.CLIENT.DRAW, (payload: DrawPayload) => { - // Payload validation could happen here or in a DTO - const { lobbyName, x, y, color } = payload; - - if (!lobbyName) return; - - // 1. Process Logic via Service - const success = CanvasService.draw(lobbyName, x, y, color); - - // 2. Broadcast if successful - if (success) { - // Send to everyone in the room INCLUDING the sender (for consistency) + socket.on(CONFIG.EVENTS.CLIENT.DRAW, ({ lobbyName, x, y, color }: DrawPayload) => { + if (lobbyName && CanvasService.draw(lobbyName, x, y, color)) { broadcastToLobby(io, lobbyName, CONFIG.EVENTS.SERVER.PIXEL_UPDATE, { x, y, color }); } }); - // --- EVENT: DRAW_BATCH --- - // User wants to color multiple pixels (stroke) - socket.on(CONFIG.EVENTS.CLIENT.DRAW_BATCH, (payload: DrawBatchPayload) => { - const { lobbyName, pixels } = payload; - // pixels: { x, y, color }[] - + socket.on(CONFIG.EVENTS.CLIENT.DRAW_BATCH, ({ lobbyName, pixels }: DrawBatchPayload) => { if (!lobbyName || !Array.isArray(pixels)) return; - - const successfulUpdates: any[] = []; - - // 1. Process Logic via Service for each pixel - for (const p of pixels) { - // Validation: ensure p is an object and has required properties - if (!p || typeof p !== 'object' || typeof p.x !== 'number' || typeof p.y !== 'number' || typeof p.color !== 'number') { - continue; - } - const { x, y, color } = p; - const success = CanvasService.draw(lobbyName, x, y, color); - if (success) { - successfulUpdates.push({ x, y, color }); - } - } - - // 2. Broadcast batch if any successful - if (successfulUpdates.length > 0) { - // Send to everyone in the room INCLUDING the sender (for consistency) - broadcastToLobby(io, lobbyName, CONFIG.EVENTS.SERVER.PIXEL_UPDATE_BATCH, { pixels: successfulUpdates }); - } + const updates = CanvasService.drawBatch(lobbyName, pixels); + if (updates.length) broadcastToLobby(io, lobbyName, CONFIG.EVENTS.SERVER.PIXEL_UPDATE_BATCH, { pixels: updates }); }); - - // --- DISCONNECTING --- socket.on('disconnecting', async () => { - // Notify rooms that user is leaving for (const room of socket.rooms) { - if (room !== socket.id) { - broadcastToOthers(socket, room, CONFIG.EVENTS.SERVER.USER_LEFT, (socket as AuthenticatedSocket).user); - - // Check if room is empty (excluding this socket) - const socketsInRoom = await getUsersInLobby(io, room); - const remainingUsers = socketsInRoom.length - 1; // fetchSockets includes the disconnecting socket - - if (remainingUsers <= 0) { - // Room is empty, unload from hot storage - await CanvasService.unloadLobby(room); - } - } + if (room === socket.id) continue; + broadcastToOthers(socket, room, CONFIG.EVENTS.SERVER.USER_LEFT, (socket as AuthenticatedSocket).user); + const users = await getUsersInLobby(io, room); + if (users.length - 1 <= 0) await CanvasService.unloadLobby(room); } }); - // --- DISCONNECT --- - socket.on('disconnect', () => { - // Cleanup logic if needed (e.g., updating user count) - console.log(`[Socket] Disconnected: ${socket.id}`); - }); + socket.on('disconnect', () => console.log(`[Socket] Disconnected: ${socket.id}`)); }); }; \ No newline at end of file From c1de01167ccfa62cf975425a3c7d29673de1778d Mon Sep 17 00:00:00 2001 From: matteosusca <39494514+matteosusca@users.noreply.github.com> Date: Thu, 29 Jan 2026 15:27:02 +0100 Subject: [PATCH 3/6] PIX-58 Update server/src/middlewares/permissionMiddleware.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- server/src/middlewares/permissionMiddleware.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/middlewares/permissionMiddleware.ts b/server/src/middlewares/permissionMiddleware.ts index 9feeb07..02362d7 100644 --- a/server/src/middlewares/permissionMiddleware.ts +++ b/server/src/middlewares/permissionMiddleware.ts @@ -1,7 +1,6 @@ import { Response, NextFunction } from "express"; import { AuthRequest } from "./authMiddleware.js"; import { Lobby } from "../models/Lobby.js"; -import { Types } from "mongoose"; import { LobbyService } from "../services/lobby.service.js"; /** From 064ffa9ec0934a1151a7e992db75087de29895e8 Mon Sep 17 00:00:00 2001 From: matteosusca <39494514+matteosusca@users.noreply.github.com> Date: Thu, 29 Jan 2026 15:27:42 +0100 Subject: [PATCH 4/6] PIX-58 Update server/src/sockets/index.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- server/src/sockets/index.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/server/src/sockets/index.ts b/server/src/sockets/index.ts index 734aea8..dd03dba 100644 --- a/server/src/sockets/index.ts +++ b/server/src/sockets/index.ts @@ -24,7 +24,12 @@ export const setupSocket = (io: Server) => { socket.on(CONFIG.EVENTS.CLIENT.JOIN_LOBBY, async (lobbyName: string) => { try { const user = (socket as AuthenticatedSocket).user; - if (!user?.id) return console.error(`[Socket] User not found: ${socket.id}`); + if (!user?.id) { + console.error(`[Socket] User not found: ${socket.id}`); + socket.emit(CONFIG.EVENTS.SERVER.ERROR, { message: "User not authenticated" }); + socket.disconnect(true); + return; + } const lobby = await LobbyService.getByName(lobbyName); if (!lobby) return socket.emit(CONFIG.EVENTS.SERVER.ERROR, { message: "Lobby not found" }); From 03fad62af3554ef823b2ef8c299506941a291b5e Mon Sep 17 00:00:00 2001 From: matteosusca <39494514+matteosusca@users.noreply.github.com> Date: Thu, 29 Jan 2026 15:29:06 +0100 Subject: [PATCH 5/6] Update server/src/services/canvas.service.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- server/src/services/canvas.service.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/services/canvas.service.ts b/server/src/services/canvas.service.ts index e8d9269..7d30e5a 100644 --- a/server/src/services/canvas.service.ts +++ b/server/src/services/canvas.service.ts @@ -69,7 +69,8 @@ export class CanvasService { const changed = canvasStore.modifyPixelColor(lobbyName, p.x, p.y, p.color); if (changed) { - successfulUpdates.push(p); + // Sanitize the object we return to avoid echoing unexpected client properties + successfulUpdates.push({ x: p.x, y: p.y, color: p.color }); anyChanged = true; } } From 7c73cce12d640e95bfabcfe57a670cdc70b4e7dc Mon Sep 17 00:00:00 2001 From: matteosusca <39494514+matteosusca@users.noreply.github.com> Date: Thu, 29 Jan 2026 15:32:02 +0100 Subject: [PATCH 6/6] PIX-58 removed unused verifyCanJoin Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- server/src/services/lobby.service.ts | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/server/src/services/lobby.service.ts b/server/src/services/lobby.service.ts index 00b03da..6c8006d 100644 --- a/server/src/services/lobby.service.ts +++ b/server/src/services/lobby.service.ts @@ -49,18 +49,4 @@ export class LobbyService { } } - static async verifyCanJoin(lobbyName: string, userId: string, currentCount: number) { - const lobby = await this.getByName(lobbyName); - if (!lobby) { - throw new Error("Lobby not found"); - } - - // 1. Check Banned - this.validateJoinAccess(lobby, userId); - - // 2. Check Capacity - this.validateCapacity(lobby, currentCount); - - return lobby; - } } \ No newline at end of file