From 35498a5854bc33d4f2a05d0de789ca52e43af75f Mon Sep 17 00:00:00 2001 From: juancarmore Date: Wed, 21 Jan 2026 19:37:57 +0100 Subject: [PATCH] backend: enhance permission checks and error handling in room and recording services --- .../backend/src/services/recording.service.ts | 17 +++++++-- .../src/services/room-member.service.ts | 11 +++--- meet-ce/backend/src/services/room.service.ts | 38 +++++++++++-------- 3 files changed, 42 insertions(+), 24 deletions(-) diff --git a/meet-ce/backend/src/services/recording.service.ts b/meet-ce/backend/src/services/recording.service.ts index 30328352..c8008c59 100644 --- a/meet-ce/backend/src/services/recording.service.ts +++ b/meet-ce/backend/src/services/recording.service.ts @@ -299,13 +299,22 @@ export class RecordingService { } } else { // Check room member permissions for each recording if no roomId filter is applied - const permissions = await roomService.getAuthenticatedRoomMemberPermissions(recRoomId); + try { + const permissions = await roomService.getAuthenticatedRoomMemberPermissions(recRoomId); - if (!permissions.canDeleteRecordings) { - this.logger.warn(`Insufficient permissions to delete recording '${recordingId}'`); + if (!permissions.canDeleteRecordings) { + this.logger.warn(`Insufficient permissions to delete recording '${recordingId}'`); + failedRecordings.add({ + recordingId, + error: `Insufficient permissions to delete recording '${recordingId}'` + }); + continue; + } + } catch (error) { + this.logger.error(`Error checking permissions for recording '${recordingId}': ${error}`); failedRecordings.add({ recordingId, - error: `Insufficient permissions to delete recording '${recordingId}'` + error: `Room associated with recording '${recordingId}' not found` }); continue; } diff --git a/meet-ce/backend/src/services/room-member.service.ts b/meet-ce/backend/src/services/room-member.service.ts index 5fa2a83a..476739ba 100644 --- a/meet-ce/backend/src/services/room-member.service.ts +++ b/meet-ce/backend/src/services/room-member.service.ts @@ -123,8 +123,11 @@ export class RoomMemberService { * @param roomId - The ID of the room * @param memberId - The ID of the member * @returns A promise that resolves to true if the user is a member, false otherwise + * @throws Error if room not found */ async isRoomMember(roomId: string, memberId: string): Promise { + // Verify room exists first + await this.roomService.getMeetRoom(roomId); const member = await this.roomMemberRepository.findByRoomAndMemberId(roomId, memberId); return !!member; } @@ -487,7 +490,7 @@ export class RoomMemberService { } } - const livekitPermissions = await this.getLiveKitPermissions(roomId, effectivePermissions); + const livekitPermissions = this.getLiveKitPermissions(roomId, effectivePermissions); const tokenMetadata: MeetRoomMemberTokenMetadata = { livekitUrl: MEET_ENV.LIVEKIT_URL, roomId, @@ -539,6 +542,7 @@ export class RoomMemberService { * @param roomId - The unique identifier of the room to check * @param secret - The secret to validate against the room's moderator and speaker secrets * @returns A promise that resolves to the room member role (MODERATOR or SPEAKER) if the secret is valid + * @throws Error if room not found * @throws Error if the moderator or speaker secrets cannot be extracted from their URLs * @throws Error if the provided secret doesn't match any of the room's secrets (unauthorized) */ @@ -606,10 +610,7 @@ export class RoomMemberService { * @param roomId - The ID of the room * @returns The LiveKit permissions for the room member */ - protected async getLiveKitPermissions( - roomId: string, - permissions: MeetRoomMemberPermissions - ): Promise { + protected getLiveKitPermissions(roomId: string, permissions: MeetRoomMemberPermissions): LiveKitPermissions { const canPublishSources: TrackSource[] = []; if (permissions.canPublishAudio) { diff --git a/meet-ce/backend/src/services/room.service.ts b/meet-ce/backend/src/services/room.service.ts index c2579e52..561d9cd2 100644 --- a/meet-ce/backend/src/services/room.service.ts +++ b/meet-ce/backend/src/services/room.service.ts @@ -808,10 +808,11 @@ export class RoomService { * @param roomId - The ID of the room * @param userId - The ID of the user * @returns A promise that resolves to true if the user is the owner, false otherwise + * @throws Error if room not found */ async isRoomOwner(roomId: string, userId: string): Promise { - const room = await this.roomRepository.findByRoomId(roomId); - return room?.owner === userId; + const room = await this.getMeetRoom(roomId); + return room.owner === userId; } /** @@ -820,12 +821,10 @@ export class RoomService { * @param roomId - The ID of the room * @param secret - The secret to validate * @returns A promise that resolves to true if the secret is valid, false otherwise + * @throws Error if room not found */ async isValidRoomSecret(roomId: string, secret: string): Promise { - const room = await this.roomRepository.findByRoomId(roomId); - - if (!room) return false; - + const room = await this.getMeetRoom(roomId); const { moderatorSecret, speakerSecret } = MeetRoomHelper.extractSecretsFromRoom(room); return secret === moderatorSecret || secret === speakerSecret; } @@ -835,9 +834,9 @@ export class RoomService { * * - If there's no authenticated user nor room member token, returns all permissions. * This is necessary for methods invoked by system processes (e.g., room auto-deletion). + * - If the user is authenticated via room member token, their permissions are obtained from the token metadata. * - If the user is admin or the room owner, they have all permissions. * - If the user is a registered room member, their permissions are obtained from their room member info. - * - If the user is authenticated via room member token, their permissions are obtained from the token metadata. * * @param roomId The ID of the room. * @returns A promise that resolves to the MeetRoomMemberPermissions object. @@ -851,6 +850,12 @@ export class RoomService { return roomMemberService.getAllPermissions(); } + // Room member token + if (memberRoomId === roomId) { + const permissions = this.requestSessionService.getRoomMemberPermissions(); + return permissions!; + } + // Registered user if (user) { const isAdmin = user.role === MeetUserRole.ADMIN; @@ -868,12 +873,6 @@ export class RoomService { } } - // Room member token - if (memberRoomId === roomId) { - const permissions = this.requestSessionService.getRoomMemberPermissions(); - return permissions!; - } - return roomMemberService.getNoPermissions(); } @@ -883,17 +882,26 @@ export class RoomService { * @param roomId The ID of the room to check access for. * @param user The user object containing user details and role. * @returns A promise that resolves to true if the user can access the room, false otherwise. + * @throws Error if room not found */ async canUserAccessRoom(roomId: string, user: MeetUser): Promise { + // Verify room exists first (throws 404 if not found) + const room = await this.getMeetRoom(roomId); + if (user.role === MeetUserRole.ADMIN) { // Admins can access all rooms return true; } // Users can access rooms they own or are members of - const isOwner = await this.isRoomOwner(roomId, user.userId); + const isOwner = room.owner === user.userId; + + if (isOwner) { + return true; + } + const roomMemberService = await this.getRoomMemberService(); const isMember = await roomMemberService.isRoomMember(roomId, user.userId); - return isOwner || isMember; + return isMember; } }