From 7c4b5c6724b8281658785e2ded85461a98e659d3 Mon Sep 17 00:00:00 2001 From: juancarmore Date: Fri, 23 Jan 2026 19:10:56 +0100 Subject: [PATCH] backend: streamline recording access validation and error handling for ZIP downloads and bulk delete --- .../src/controllers/recording.controller.ts | 51 +++---- meet-ce/backend/src/models/error.model.ts | 4 +- .../backend/src/services/recording.service.ts | 142 ++++++------------ 3 files changed, 72 insertions(+), 125 deletions(-) diff --git a/meet-ce/backend/src/controllers/recording.controller.ts b/meet-ce/backend/src/controllers/recording.controller.ts index 60234f71..2ea5731e 100644 --- a/meet-ce/backend/src/controllers/recording.controller.ts +++ b/meet-ce/backend/src/controllers/recording.controller.ts @@ -1,18 +1,17 @@ +import { MeetRecordingInfo } from '@openvidu-meet/typings'; import archiver from 'archiver'; import { Request, Response } from 'express'; import { Readable } from 'stream'; import { container } from '../config/dependency-injector.config.js'; import { INTERNAL_CONFIG } from '../config/internal-config.js'; -import { RecordingHelper } from '../helpers/recording.helper.js'; import { - errorRecordingsNotFromSameRoom, + errorRecordingsZipEmpty, handleError, internalError, rejectRequestFromMeetError } from '../models/error.model.js'; import { LoggerService } from '../services/logger.service.js'; import { RecordingService } from '../services/recording.service.js'; -import { RequestSessionService } from '../services/request-session.service.js'; import { getBaseUrl } from '../utils/url.utils.js'; export const startRecording = async (req: Request, res: Response) => { @@ -77,16 +76,12 @@ export const getRecordings = async (req: Request, res: Response) => { export const bulkDeleteRecordings = async (req: Request, res: Response) => { const logger = container.get(LoggerService); const recordingService = container.get(RecordingService); - const requestSessionService = container.get(RequestSessionService); const { recordingIds } = req.query as { recordingIds: string[] }; logger.info(`Deleting recordings: ${recordingIds}`); try { - // If room member token is present, delete only recordings for the room associated with the token - const roomId = requestSessionService.getRoomIdFromMember(); - - const { deleted, failed } = await recordingService.bulkDeleteRecordings(recordingIds, roomId); + const { deleted, failed } = await recordingService.bulkDeleteRecordings(recordingIds); // All recordings were successfully deleted if (deleted.length > 0 && failed.length === 0) { @@ -237,35 +232,28 @@ export const getRecordingUrl = async (req: Request, res: Response) => { export const downloadRecordingsZip = async (req: Request, res: Response) => { const logger = container.get(LoggerService); const recordingService = container.get(RecordingService); - const requestSessionService = container.get(RequestSessionService); const { recordingIds } = req.query as { recordingIds: string[] }; - let validRecordingIds = recordingIds; + const validRecordings: MeetRecordingInfo[] = []; - // If room member token is present, download only recordings for the room associated with the token - const roomId = requestSessionService.getRoomIdFromMember(); + logger.info(`Preparing ZIP download for recordings: ${recordingIds}`); - if (roomId) { - validRecordingIds = recordingIds.filter((recordingId) => { - const { roomId: recRoomId } = RecordingHelper.extractInfoFromRecordingId(recordingId); - const isValid = recRoomId === roomId; - - if (!isValid) { - logger.warn(`Skipping recording '${recordingId}' as it does not belong to room '${roomId}'`); - } - - return isValid; - }); + // Validate each recording: first check existence, then permissions + for (const recordingId of recordingIds) { + try { + const recordingInfo = await recordingService.validateRecordingAccess(recordingId, 'canRetrieveRecordings'); + validRecordings.push(recordingInfo); + } catch (error) { + logger.warn(`Skipping recording '${recordingId}' for ZIP`); + } } - if (validRecordingIds.length === 0) { - logger.warn(`None of the provided recording IDs belong to room '${roomId}'`); - const error = errorRecordingsNotFromSameRoom(roomId!); + if (validRecordings.length === 0) { + logger.error(`None of the provided recording IDs are available for ZIP download`); + const error = errorRecordingsZipEmpty(); return rejectRequestFromMeetError(res, error); } - logger.info(`Creating ZIP for recordings: ${recordingIds}`); - res.setHeader('Content-Type', 'application/zip'); res.setHeader('Content-Disposition', 'attachment; filename="recordings.zip"'); @@ -280,13 +268,14 @@ export const downloadRecordingsZip = async (req: Request, res: Response) => { // Pipe the archive to the response archive.pipe(res); - for (const recordingId of validRecordingIds) { + for (const recording of validRecordings) { + const recordingId = recording.recordingId; + try { logger.debug(`Adding recording '${recordingId}' to ZIP`); const result = await recordingService.getRecordingAsStream(recordingId); - const recordingInfo = await recordingService.getRecording(recordingId, 'filename'); - const filename = recordingInfo.filename || `${recordingId}.mp4`; + const filename = recording.filename || `${recordingId}.mp4`; archive.append(result.fileStream, { name: filename }); } catch (error) { logger.error(`Error adding recording '${recordingId}' to ZIP: ${error}`); diff --git a/meet-ce/backend/src/models/error.model.ts b/meet-ce/backend/src/models/error.model.ts index d06346d7..1c641f86 100644 --- a/meet-ce/backend/src/models/error.model.ts +++ b/meet-ce/backend/src/models/error.model.ts @@ -175,10 +175,10 @@ export const errorInvalidRecordingSecret = (recordingId: string, secret: string) ); }; -export const errorRecordingsNotFromSameRoom = (roomId: string): OpenViduMeetError => { +export const errorRecordingsZipEmpty = (): OpenViduMeetError => { return new OpenViduMeetError( 'Recording Error', - `None of the provided recording IDs belong to room '${roomId}'`, + 'None of the provided recordings are available for ZIP download', 400 ); }; diff --git a/meet-ce/backend/src/services/recording.service.ts b/meet-ce/backend/src/services/recording.service.ts index c8008c59..8e920eab 100644 --- a/meet-ce/backend/src/services/recording.service.ts +++ b/meet-ce/backend/src/services/recording.service.ts @@ -3,7 +3,8 @@ import { MeetRecordingInfo, MeetRecordingStatus, MeetRoom, - MeetRoomConfig + MeetRoomConfig, + MeetRoomMemberPermissions } from '@openvidu-meet/typings'; import { inject, injectable } from 'inversify'; import { EgressStatus, EncodedFileOutput, EncodedFileType, RoomCompositeOptions } from 'livekit-server-sdk'; @@ -17,6 +18,7 @@ import { RecordingHelper } from '../helpers/recording.helper.js'; import { MeetLock } from '../helpers/redis.helper.js'; import { DistributedEventType } from '../models/distributed-event.model.js'; import { + errorInsufficientPermissions, errorRecordingAlreadyStarted, errorRecordingAlreadyStopped, errorRecordingCannotBeStoppedWhileStarting, @@ -264,70 +266,64 @@ export class RecordingService { } } + /** + * Validates if the authenticated user has permission to access a specific recording. + * First checks if the recording exists, then validates user permissions. + * + * @param recordingId The recording identifier to validate. + * @param permission The permission to check. + * @returns The recording info if accessible. + * @throws Error if recording not found or insufficient permissions. + */ + async validateRecordingAccess( + recordingId: string, + permission: keyof MeetRoomMemberPermissions + ): Promise { + // First, check if the recording exists + const recordingInfo = await this.recordingRepository.findByRecordingId(recordingId); + + if (!recordingInfo) { + throw errorRecordingNotFound(recordingId); + } + + // Extract roomId from the recording info + const { roomId: recRoomId } = recordingInfo; + + // Check room member permissions for the room associated with the recording + const roomService = await this.getRoomService(); + const permissions = await roomService.getAuthenticatedRoomMemberPermissions(recRoomId); + + if (!permissions[permission]) { + this.logger.warn(`Insufficient permissions to access recording '${recordingId}'`); + throw errorInsufficientPermissions(); + } + + return recordingInfo; + } + /** * Deletes multiple recordings in bulk from MongoDB and blob storage. * For each provided recordingId, the metadata and recording file are deleted (only if the status is stopped). * * @param recordingIds Array of recording identifiers. - * @param roomId Optional room identifier to delete only recordings from a specific room. * @returns An object containing: * - `deleted`: An array of successfully deleted recording IDs. - * - `notDeleted`: An array of objects containing recording IDs and error messages for those that could not be deleted. + * - `failed`: An array of objects containing recording IDs and error messages for those that could not be deleted. */ async bulkDeleteRecordings( - recordingIds: string[], - roomId?: string + recordingIds: string[] ): Promise<{ deleted: string[]; failed: { recordingId: string; error: string }[] }> { - const roomService = await this.getRoomService(); - const validRecordingIds: Set = new Set(); const deletedRecordings: Set = new Set(); const failedRecordings: Set<{ recordingId: string; error: string }> = new Set(); + // Process each recording: first check existence, then permissions, then deletability for (const recordingId of recordingIds) { - const { roomId: recRoomId } = RecordingHelper.extractInfoFromRecordingId(recordingId); - - // If a roomId is provided, only process recordings from that room - if (roomId) { - if (recRoomId !== roomId) { - this.logger.warn(`Skipping recording '${recordingId}' as it does not belong to room '${roomId}'`); - failedRecordings.add({ - recordingId, - error: `Recording '${recordingId}' does not belong to room '${roomId}'` - }); - continue; - } - } else { - // Check room member permissions for each recording if no roomId filter is applied - try { - const permissions = await roomService.getAuthenticatedRoomMemberPermissions(recRoomId); - - 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: `Room associated with recording '${recordingId}' not found` - }); - continue; - } - } - try { - // Check if the recording exists and can be deleted - const recordingInfo = await this.recordingRepository.findByRecordingId(recordingId); - - if (!recordingInfo) { - throw errorRecordingNotFound(recordingId); - } + // Validate recording exists and user has permission to delete + const recordingInfo = await this.validateRecordingAccess(recordingId, 'canDeleteRecordings'); + // Check if the recording can be deleted (must be stopped) if (!RecordingHelper.canBeDeleted(recordingInfo)) { throw errorRecordingNotStopped(recordingId); } @@ -397,7 +393,6 @@ export class RecordingService { if (recording.status !== MeetRecordingStatus.COMPLETE) { this.logger.warn(`Recording '${recordingId}' did not complete successfully`); - this.logger.warn(`ABORTING RECORDING '${recordingId}'`); await this.updateRecordingStatus(recordingId, MeetRecordingStatus.ABORTED); } @@ -423,49 +418,12 @@ export class RecordingService { `Found ${allRecordingIds.length} recordings for room '${roomId}', proceeding with deletion` ); - // Attempt initial deletion - let remainingRecordings = [...allRecordingIds]; - let retryCount = 0; - const maxRetries = 3; - const retryDelayMs = 1000; - - while (remainingRecordings.length > 0 && retryCount < maxRetries) { - if (retryCount > 0) { - this.logger.info( - `Retry ${retryCount}/${maxRetries}: attempting to delete ${remainingRecordings.length} remaining recordings` - ); - await new Promise((resolve) => setTimeout(resolve, retryDelayMs * retryCount)); - } - - const { failed } = await this.bulkDeleteRecordings(remainingRecordings, roomId); - - if (failed.length === 0) { - this.logger.info(`Successfully deleted all recordings for room '${roomId}'`); - return; - } - - // Prepare for retry with failed recordings - remainingRecordings = failed.map((failed) => failed.recordingId); - retryCount++; - - this.logger.warn( - `${failed.length} recordings failed to delete for room '${roomId}': ${remainingRecordings.join(', ')}` - ); - - if (retryCount < maxRetries) { - this.logger.info(`Will retry deletion in ${retryDelayMs * retryCount}ms`); - } - } - - // Final check and logging - if (remainingRecordings.length > 0) { - this.logger.error( - `Failed to delete ${remainingRecordings.length} recordings for room '${roomId}' after ${maxRetries} attempts: ${remainingRecordings.join(', ')}` - ); - throw new Error( - `Failed to delete all recordings for room '${roomId}'. ${remainingRecordings.length} recordings could not be deleted.` - ); - } + // Delete recordings metadata from MongoDB and media files from blob storage + await Promise.all([ + this.recordingRepository.deleteByRecordingIds(allRecordingIds), + this.blobStorageService.deleteRecordingMediaBatch(allRecordingIds) + ]); + this.logger.info(`Successfully deleted all recordings for room '${roomId}'`); } catch (error) { this.logger.error(`Error deleting all recordings for room '${roomId}': ${error}`); throw error;