From 84a0b2ac6e791f4f8acdfcabfa2c0c3b58ec06e3 Mon Sep 17 00:00:00 2001 From: juancarmore Date: Fri, 23 Jan 2026 19:13:51 +0100 Subject: [PATCH] backend: streamline room and recording middlewares, enhance permission checks and error handling --- meet-ce/backend/src/helpers/room.helper.ts | 34 ---- .../src/middlewares/recording.middleware.ts | 172 ++++++++++-------- .../src/middlewares/room-member.middleware.ts | 43 +++-- .../src/middlewares/room.middleware.ts | 6 +- .../backend/src/routes/recording.routes.ts | 22 +-- meet-ce/backend/src/services/room.service.ts | 26 +-- 6 files changed, 142 insertions(+), 161 deletions(-) diff --git a/meet-ce/backend/src/helpers/room.helper.ts b/meet-ce/backend/src/helpers/room.helper.ts index cd08bb42..f6cac38d 100644 --- a/meet-ce/backend/src/helpers/room.helper.ts +++ b/meet-ce/backend/src/helpers/room.helper.ts @@ -1,7 +1,5 @@ import { MeetRoom, MeetRoomOptions } from '@openvidu-meet/typings'; -import { Request } from 'express'; import { MEET_ENV } from '../environment.js'; -import { RecordingHelper } from './recording.helper.js'; export class MeetRoomHelper { private constructor() { @@ -117,36 +115,4 @@ export class MeetRoomHelper { return false; } } - - /** - * Extracts the room ID from the request object. - * It checks the following locations in order: - * 1. req.params.roomId - * 2. req.body.roomId - * 3. req.params.recordingId (extracts roomId from it) - * - * @param req - The express request object - * @returns The extracted room ID or undefined if not found - */ - static getRoomIdFromRequest(req: Request): string | undefined { - // 1. Check params - if (req.params.roomId) { - return req.params.roomId; - } - - // 2. Check body - if (req.body.roomId) { - return req.body.roomId; - } - - // 3. Check recordingId in params - const recordingId = req.params.recordingId; - - if (recordingId) { - const { roomId } = RecordingHelper.extractInfoFromRecordingId(recordingId); - return roomId; - } - - return undefined; - } } diff --git a/meet-ce/backend/src/middlewares/recording.middleware.ts b/meet-ce/backend/src/middlewares/recording.middleware.ts index 46ea1d40..228e5de4 100644 --- a/meet-ce/backend/src/middlewares/recording.middleware.ts +++ b/meet-ce/backend/src/middlewares/recording.middleware.ts @@ -1,7 +1,6 @@ import { MeetRoomMemberPermissions, MeetUserRole } from '@openvidu-meet/typings'; import { NextFunction, Request, Response } from 'express'; import { container } from '../config/dependency-injector.config.js'; -import { MeetRoomHelper } from '../helpers/room.helper.js'; import { errorInsufficientPermissions, errorInvalidRecordingSecret, @@ -12,7 +11,6 @@ import { import { LoggerService } from '../services/logger.service.js'; import { RecordingService } from '../services/recording.service.js'; import { RequestSessionService } from '../services/request-session.service.js'; -import { RoomMemberService } from '../services/room-member.service.js'; import { RoomService } from '../services/room.service.js'; import { allowAnonymous, @@ -22,12 +20,15 @@ import { withAuth } from './auth.middleware.js'; +/** + * Middleware to ensure that recording is enabled for the specified room. + */ export const withRecordingEnabled = async (req: Request, res: Response, next: NextFunction) => { const logger = container.get(LoggerService); const roomService = container.get(RoomService); try { - const roomId = MeetRoomHelper.getRoomIdFromRequest(req); + const { roomId } = req.body as { roomId: string }; const room = await roomService.getMeetRoom(roomId!); if (!room.config.recording.enabled) { @@ -38,7 +39,7 @@ export const withRecordingEnabled = async (req: Request, res: Response, next: Ne return next(); } catch (error) { - handleError(res, error, 'checking recording config'); + handleError(res, error, 'checking room recording config'); } }; @@ -95,91 +96,104 @@ export const setupRecordingAuthentication = async (req: Request, res: Response, }; /** - * Middleware to authorize recording access (retrieval or deletion). + * Middleware to authorize access (retrieval or deletion) for a single recording. * - * - If a secret is provided in the request query, and allowSecretAccess is true, - * it assumes the secret has been validated and grants access. - * - If a Room Member Token is used, it checks that the token's roomId matches the requested roomId - * and that the member has the required permission. - * - If a registered user is authenticated, it checks their role and whether they are the owner or a member of the room - * with the required permission. - * - If neither a valid token nor an authenticated user is present, it rejects the request. + * - If a secret is provided in the request query, it is assumed to have been validated already. + * In that case, access is granted directly for retrieval requests. + * - If no secret is provided, the recording's existence and permissions are checked + * based on the authenticated context (room member token or registered user). * * @param permission - The permission to check (canRetrieveRecordings or canDeleteRecordings). - * @param allowSecretAccess - Whether to allow access based on a valid secret in the query. */ -export const authorizeRecordingAccess = (permission: keyof MeetRoomMemberPermissions, allowSecretAccess = false) => { +export const authorizeRecordingAccess = (permission: keyof MeetRoomMemberPermissions) => { return async (req: Request, res: Response, next: NextFunction) => { - const roomId = MeetRoomHelper.getRoomIdFromRequest(req); - const secret = req.query.secret as string; + const recordingId = req.params.recordingId as string; + const secret = req.query.secret as string | undefined; - // If allowSecretAccess is true and a secret is provided, - // we assume it has been validated by setupRecordingAuthentication. - if (allowSecretAccess && secret) { + // If a secret is provided, we assume it has been validated by setupRecordingAuthentication. + // In that case, grant access directly for retrieval requests. + if (secret && permission === 'canRetrieveRecordings') { return next(); } - const requestSessionService = container.get(RequestSessionService); - const roomService = container.get(RoomService); - const roomMemberService = container.get(RoomMemberService); - - const memberRoomId = requestSessionService.getRoomIdFromMember(); - const user = requestSessionService.getAuthenticatedUser(); - - const forbiddenError = errorInsufficientPermissions(); - - // Case 1: Room Member Token - if (memberRoomId) { - const permissions = requestSessionService.getRoomMemberPermissions(); - - if (!permissions) { - return rejectRequestFromMeetError(res, forbiddenError); - } - - const sameRoom = roomId ? memberRoomId === roomId : true; - - if (!sameRoom || !permissions[permission]) { - return rejectRequestFromMeetError(res, forbiddenError); - } - + try { + // Check recording existence and permissions based on the authenticated context + const recordingService = container.get(RecordingService); + await recordingService.validateRecordingAccess(recordingId, permission); return next(); + } catch (error) { + return handleError(res, error, 'checking recording permissions'); } - - // Case 2: Authenticated User - if (user) { - // If no roomId is specified, we are in a listing/bulk request - // Each recording's room ownership and permissions will be checked individually - if (!roomId) { - return next(); - } - - // Admins can always access - if (user.role === MeetUserRole.ADMIN) { - return next(); - } - - try { - // Check if owner - const isOwner = await roomService.isRoomOwner(roomId, user.userId); - - if (isOwner) { - return next(); - } - - // Check if member with permissions - const member = await roomMemberService.getRoomMember(roomId, user.userId); - - if (member && member.effectivePermissions[permission]) { - return next(); - } - - return rejectRequestFromMeetError(res, forbiddenError); - } catch (error) { - return handleError(res, error, 'checking user access to room'); - } - } - - // Otherwise, reject the request - return rejectRequestFromMeetError(res, forbiddenError); }; }; + +/** + * Middleware to authorize access (retrieval or deletion) for multiple recordings. + * + * - If a room member token is present, checks if the member has the specified permission. + * - If no room member token is present, each recording's permissions will be checked individually later. + * + * @param permission - The permission to check (canRetrieveRecordings or canDeleteRecordings). + */ +export const authorizeBulkRecordingAccess = (permission: keyof MeetRoomMemberPermissions) => { + return async (_req: Request, res: Response, next: NextFunction) => { + const requestSessionService = container.get(RequestSessionService); + const memberRoomId = requestSessionService.getRoomIdFromMember(); + + // If there is no room member token, + // each recording's permissions will be checked individually later + if (!memberRoomId) { + return next(); + } + + // If there is a room member token, check permissions now + // because they have the same permissions for all recordings in the room associated with the token + const permissions = requestSessionService.getRoomMemberPermissions(); + + if (!permissions || !permissions[permission]) { + const forbiddenError = errorInsufficientPermissions(); + return rejectRequestFromMeetError(res, forbiddenError); + } + + return next(); + }; +}; + +/** + * Middleware to authorize control actions (start/stop) for recordings. + * + * - For starting a recording, checks if the authenticated user has 'canRecord' permission in the target room. + * - For stopping a recording, checks if the recording exists and if the authenticated user has 'canRecord' permission. + */ +export const authorizeRecordingControl = async (req: Request, res: Response, next: NextFunction) => { + const recordingId = req.params.recordingId as string | undefined; + + if (!recordingId) { + // Start recording + const { roomId } = req.body as { roomId: string }; + + try { + // Check that the authenticated user has 'canRecord' permission in the target room + const roomService = container.get(RoomService); + const permissions = await roomService.getAuthenticatedRoomMemberPermissions(roomId); + + if (!permissions['canRecord']) { + throw errorInsufficientPermissions(); + } + + return next(); + } catch (error) { + return handleError(res, error, 'checking recording permissions'); + } + } else { + // Stop recording + try { + // Check that the recording exists and the authenticated user has 'canRecord' permission + const recordingService = container.get(RecordingService); + await recordingService.validateRecordingAccess(recordingId, 'canRecord'); + return next(); + } catch (error) { + return handleError(res, error, 'checking recording permissions'); + } + } +}; diff --git a/meet-ce/backend/src/middlewares/room-member.middleware.ts b/meet-ce/backend/src/middlewares/room-member.middleware.ts index d025c8b8..368a8beb 100644 --- a/meet-ce/backend/src/middlewares/room-member.middleware.ts +++ b/meet-ce/backend/src/middlewares/room-member.middleware.ts @@ -1,7 +1,6 @@ import { MeetRoomMemberPermissions, MeetRoomMemberTokenOptions, MeetUserRole } from '@openvidu-meet/typings'; import { NextFunction, Request, Response } from 'express'; import { container } from '../config/dependency-injector.config.js'; -import { MeetRoomHelper } from '../helpers/room.helper.js'; import { errorInsufficientPermissions, errorInvalidRoomSecret, @@ -46,20 +45,21 @@ export const authorizeRoomMemberAccess = async (req: Request, res: Response, nex const forbiddenError = errorInsufficientPermissions(); - // Scenario 1: Room Member Token + // Room Member Token if (memberRoomId) { // Check if the token belongs to the requested room - if (memberRoomId !== roomId) { + // and if the memberId matches the requested member + const isSameRoom = memberRoomId === roomId; + const isSameMember = currentMemberId === memberId; + + if (!isSameRoom || !isSameMember) { return rejectRequestFromMeetError(res, forbiddenError); } - // Check if the token belongs to the requested member (self access) - if (currentMemberId === memberId) { - return next(); - } + return next(); } - // Scenario 2: Registered User + // Registered User if (user) { // Allow if user is admin if (user.role === MeetUserRole.ADMIN) { @@ -67,18 +67,17 @@ export const authorizeRoomMemberAccess = async (req: Request, res: Response, nex } try { - // Allow if user is room owner + // Check if user is room owner + // or is accessing their own member info const roomService = container.get(RoomService); const isOwner = await roomService.isRoomOwner(roomId, user.userId); + const isSameMember = user.userId === memberId; - if (isOwner) { - return next(); + if (!isOwner && !isSameMember) { + return rejectRequestFromMeetError(res, forbiddenError); } - // Allow if user is accessing their own member info - if (user.userId === memberId) { - return next(); - } + return next(); } catch (error) { return handleError(res, error, 'checking room ownership'); } @@ -110,7 +109,7 @@ export const setupRoomMemberTokenAuthentication = async (req: Request, res: Resp * Middleware to authorize the generation of a room member token. * * - If a secret is provided, it checks if it matches a valid room secret (anonymous access) or if it corresponds to a room member. - * - If no secret is provided, it checks if the authenticated user has permissions to access the room (Admin, Owner, or Member). + * - If no secret is provided, it checks if the authenticated user has permissions to access the room (admin, owner, or member). */ export const authorizeRoomMemberTokenGeneration = async (req: Request, res: Response, next: NextFunction) => { const { roomId } = req.params; @@ -180,7 +179,7 @@ export const authorizeRoomMemberTokenGeneration = async (req: Request, res: Resp */ export const withRoomMemberPermission = (permission: keyof MeetRoomMemberPermissions) => { return async (req: Request, res: Response, next: NextFunction) => { - const roomId = MeetRoomHelper.getRoomIdFromRequest(req); + const roomId = req.params.roomId as string; const roomService = container.get(RoomService); const roomExists = await roomService.meetRoomExists(roomId!); @@ -195,12 +194,12 @@ export const withRoomMemberPermission = (permission: keyof MeetRoomMemberPermiss const memberRoomId = requestSessionService.getRoomIdFromMember(); const permissions = requestSessionService.getRoomMemberPermissions(); - if (!memberRoomId || !permissions) { - const error = errorInsufficientPermissions(); - return rejectRequestFromMeetError(res, error); - } + // Check if room member belongs to the requested room + // and has the required permission + const sameRoom = memberRoomId === roomId; + const hasPermission = permissions && permissions[permission]; - if (memberRoomId !== roomId || !permissions[permission]) { + if (!sameRoom || !hasPermission) { const error = errorInsufficientPermissions(); return rejectRequestFromMeetError(res, error); } diff --git a/meet-ce/backend/src/middlewares/room.middleware.ts b/meet-ce/backend/src/middlewares/room.middleware.ts index 72cfdc91..dd60ca46 100644 --- a/meet-ce/backend/src/middlewares/room.middleware.ts +++ b/meet-ce/backend/src/middlewares/room.middleware.ts @@ -97,11 +97,11 @@ export const authorizeRoomManagement = async (req: Request, res: Response, next: try { const isOwner = await roomService.isRoomOwner(roomId, user.userId); - if (isOwner) { - return next(); + if (!isOwner) { + return rejectRequestFromMeetError(res, forbiddenError); } - return rejectRequestFromMeetError(res, forbiddenError); + return next(); } catch (error) { return handleError(res, error, 'checking room ownership'); } diff --git a/meet-ce/backend/src/routes/recording.routes.ts b/meet-ce/backend/src/routes/recording.routes.ts index 0d893e5a..e601a9e5 100644 --- a/meet-ce/backend/src/routes/recording.routes.ts +++ b/meet-ce/backend/src/routes/recording.routes.ts @@ -9,7 +9,9 @@ import { withAuth } from '../middlewares/auth.middleware.js'; import { + authorizeBulkRecordingAccess, authorizeRecordingAccess, + authorizeRecordingControl, setupRecordingAuthentication, withRecordingEnabled } from '../middlewares/recording.middleware.js'; @@ -22,7 +24,6 @@ import { validateStartRecordingReq, withValidRecordingId } from '../middlewares/request-validators/recording-validator.middleware.js'; -import { withRoomMemberPermission } from '../middlewares/room-member.middleware.js'; export const recordingRouter: Router = Router(); recordingRouter.use(bodyParser.urlencoded({ extended: true })); @@ -37,7 +38,7 @@ recordingRouter.get( tokenAndRoleValidator(MeetUserRole.ADMIN, MeetUserRole.USER, MeetUserRole.ROOM_MEMBER) ), validateGetRecordingsReq, - authorizeRecordingAccess('canRetrieveRecordings'), + authorizeBulkRecordingAccess('canRetrieveRecordings'), recordingCtrl.getRecordings ); recordingRouter.delete( @@ -48,7 +49,7 @@ recordingRouter.delete( tokenAndRoleValidator(MeetUserRole.ADMIN, MeetUserRole.USER, MeetUserRole.ROOM_MEMBER) ), validateBulkDeleteRecordingsReq, - authorizeRecordingAccess('canDeleteRecordings'), + authorizeBulkRecordingAccess('canDeleteRecordings'), recordingCtrl.bulkDeleteRecordings ); recordingRouter.get( @@ -59,14 +60,14 @@ recordingRouter.get( tokenAndRoleValidator(MeetUserRole.ADMIN, MeetUserRole.USER, MeetUserRole.ROOM_MEMBER) ), validateBulkDeleteRecordingsReq, - authorizeRecordingAccess('canRetrieveRecordings'), + authorizeBulkRecordingAccess('canRetrieveRecordings'), recordingCtrl.downloadRecordingsZip ); recordingRouter.get( '/:recordingId', validateGetRecordingReq, setupRecordingAuthentication, - authorizeRecordingAccess('canRetrieveRecordings', true), + authorizeRecordingAccess('canRetrieveRecordings'), recordingCtrl.getRecording ); recordingRouter.delete( @@ -84,7 +85,7 @@ recordingRouter.get( '/:recordingId/media', validateGetRecordingMediaReq, setupRecordingAuthentication, - authorizeRecordingAccess('canRetrieveRecordings', true), + authorizeRecordingAccess('canRetrieveRecordings'), recordingCtrl.getRecordingMedia ); recordingRouter.get( @@ -106,17 +107,16 @@ internalRecordingRouter.use(bodyParser.json()); internalRecordingRouter.post( '/', + withAuth(roomMemberTokenValidator), validateStartRecordingReq, withRecordingEnabled, - withAuth(roomMemberTokenValidator), - withRoomMemberPermission('canRecord'), + authorizeRecordingControl, recordingCtrl.startRecording ); internalRecordingRouter.post( '/:recordingId/stop', - withValidRecordingId, - withRecordingEnabled, withAuth(roomMemberTokenValidator), - withRoomMemberPermission('canRecord'), + withValidRecordingId, + authorizeRecordingControl, recordingCtrl.stopRecording ); diff --git a/meet-ce/backend/src/services/room.service.ts b/meet-ce/backend/src/services/room.service.ts index 561d9cd2..bcf638e9 100644 --- a/meet-ce/backend/src/services/room.service.ts +++ b/meet-ce/backend/src/services/room.service.ts @@ -832,11 +832,11 @@ export class RoomService { /** * Retrieves the permissions of the authenticated room member. * - * - 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 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). * * @param roomId The ID of the room. * @returns A promise that resolves to the MeetRoomMemberPermissions object. @@ -846,14 +846,13 @@ export class RoomService { const user = this.requestSessionService.getAuthenticatedUser(); const memberRoomId = this.requestSessionService.getRoomIdFromMember(); - if (!user && !memberRoomId) { - return roomMemberService.getAllPermissions(); - } - // Room member token - if (memberRoomId === roomId) { - const permissions = this.requestSessionService.getRoomMemberPermissions(); - return permissions!; + if (memberRoomId) { + if (memberRoomId !== roomId) { + return roomMemberService.getNoPermissions(); + } + + return this.requestSessionService.getRoomMemberPermissions()!; } // Registered user @@ -868,12 +867,15 @@ export class RoomService { const member = await roomMemberService.getRoomMember(roomId, user.userId); - if (member) { - return member.effectivePermissions; + if (!member) { + return roomMemberService.getNoPermissions(); } + + return member.effectivePermissions; } - return roomMemberService.getNoPermissions(); + // No authenticated user nor room member token - return all permissions for system processes + return roomMemberService.getAllPermissions(); } /**