From c3fa764534f3af3a63fb3aea8198c7925f0b73c4 Mon Sep 17 00:00:00 2001 From: Carlos Santos <4a.santos@gmail.com> Date: Mon, 14 Apr 2025 17:57:18 +0200 Subject: [PATCH] backend: Refactor bulkDeleteRooms to improve response handling and update sanitization logic --- backend/src/controllers/room.controller.ts | 32 +++++-- .../room-validator.middleware.ts | 85 ++++++++++++------- backend/src/services/room.service.ts | 20 ++--- 3 files changed, 86 insertions(+), 51 deletions(-) diff --git a/backend/src/controllers/room.controller.ts b/backend/src/controllers/room.controller.ts index 05e5484..c427288 100644 --- a/backend/src/controllers/room.controller.ts +++ b/backend/src/controllers/room.controller.ts @@ -92,21 +92,35 @@ export const bulkDeleteRooms = async (req: Request, res: Response) => { logger.info(`Deleting rooms: ${roomIds}`); try { - const roomIdsArray = (roomIds as string).split(','); - const { deleted, markedAsDeleted } = await roomService.bulkDeleteRooms(roomIdsArray, forceDelete); + const roomIdsArray = roomIds as string[]; - if (roomIdsArray.length === 1) { + const { deleted, markedForDeletion } = await roomService.bulkDeleteRooms(roomIdsArray, forceDelete); + const isSingleRoom = roomIdsArray.length === 1; + + if (isSingleRoom) { + // For a single room, no content is sent if fully deleted. if (deleted.length > 0) { - return res.status(204).send(); + return res.sendStatus(204); } - return res.status(202).json({ message: `Room ${roomIds} marked as deleted` }); + // For a single room marked as deleted, return a message. + return res.status(202).json({ message: `Room ${roomIdsArray[0]} marked as deleted` }); } - return res.status(200).json({ - deleted, - markedAsDeleted - }); + // For multiple rooms + if (deleted.length > 0 && markedForDeletion.length === 0) { + // All rooms were deleted + return res.sendStatus(204); + } + + if (deleted.length === 0 && markedForDeletion.length > 0) { + // All rooms were marked as deleted + return res + .status(202) + .json({ message: `Rooms ${markedForDeletion.join(', ')} marked for deletion`, markedForDeletion }); + } + + return res.status(200).json({ deleted, markedForDeletion }); } catch (error) { logger.error(`Error deleting rooms: ${error}`); handleError(res, error); diff --git a/backend/src/middlewares/request-validators/room-validator.middleware.ts b/backend/src/middlewares/request-validators/room-validator.middleware.ts index 56a798f..2860593 100644 --- a/backend/src/middlewares/request-validators/room-validator.middleware.ts +++ b/backend/src/middlewares/request-validators/room-validator.middleware.ts @@ -8,25 +8,37 @@ import { } from '@typings-ce'; import { Request, Response, NextFunction } from 'express'; import { z } from 'zod'; +import ms from 'ms'; +import INTERNAL_CONFIG from '../../config/internal-config.js'; +/** + * Sanitizes an identifier by removing/replacing invalid characters + * and normalizing format. + * + * @param val The string to sanitize + * @returns A sanitized string safe for use as an identifier + */ const sanitizeId = (val: string): string => { - return val - .trim() // Remove leading and trailing spaces - .replace(/\s+/g, '-') // Replace spaces with hyphens - .replace(/[^a-zA-Z0-9_-]/g, ''); // Remove special characters (allow alphanumeric, hyphens and underscores) + let transformed = val + .trim() // Remove leading/trailing spaces + .replace(/\s+/g, '') // Remove all spaces + .replace(/[^a-zA-Z0-9_-]/g, '') // Allow alphanumeric, underscores and hyphens + .replace(/-+/g, '-') // Replace multiple consecutive hyphens + .replace(/-+$/, ''); // Remove trailing hyphens + + // Remove leading hyphens + if (transformed.startsWith('-')) { + transformed = transformed.substring(1); + } + + return transformed; }; const nonEmptySanitizedString = (fieldName: string) => z .string() .min(1, { message: `${fieldName} is required and cannot be empty` }) - .transform((val) => { - let transformed = sanitizeId(val); - - if (transformed.startsWith('-')) transformed = transformed.substring(1); - - return transformed; - }) + .transform(sanitizeId) .refine((data) => data !== '', { message: `${fieldName} cannot be empty after sanitization` }); @@ -64,25 +76,14 @@ const RoomRequestOptionsSchema: z.ZodType = z.object({ autoDeletionDate: z .number() .positive('autoDeletionDate must be a positive integer') - .refine((date) => date >= Date.now() + 60 * 60 * 1000, 'autoDeletionDate must be at least 1 hour in the future') + .refine( + (date) => date >= Date.now() + ms(INTERNAL_CONFIG.MIN_FUTURE_TIME_FOR_ROOM_AUTODELETION_DATE), + `autoDeletionDate must be at least ${INTERNAL_CONFIG.MIN_FUTURE_TIME_FOR_ROOM_AUTODELETION_DATE} in the future` + ) .optional(), roomIdPrefix: z .string() - .transform((val) => { - let transformed = val - .trim() // Remove leading and trailing spaces - .replace(/\s+/g, '') // Remove all whitespace instead of replacing it with hyphens - .replace(/[^a-zA-Z0-9-]/g, '') // Remove any character except letters, numbers, and hyphens - .replace(/-+/g, '-') // Replace multiple consecutive hyphens with a single one - .replace(/-+$/, ''); // Remove trailing hyphens - - // If the transformed string starts with a hyphen, remove it. - if (transformed.startsWith('-')) { - transformed = transformed.substring(1); - } - - return transformed; - }) + .transform(sanitizeId) .optional() .default(''), preferences: RoomPreferencesSchema.optional().default({ @@ -116,17 +117,37 @@ const GetRoomFiltersSchema: z.ZodType = z.object({ const BulkDeleteRoomsSchema = z.object({ roomIds: z.preprocess( (arg) => { + // First, convert input to array of strings + let roomIds: string[] = []; + if (typeof arg === 'string') { - // If the argument is a string, it is expected to be a comma-separated list of recording IDs. - return arg + roomIds = arg .split(',') .map((s) => s.trim()) .filter((s) => s !== ''); + } else if (Array.isArray(arg)) { + roomIds = arg.map((item) => String(item)).filter((s) => s !== ''); } - return arg; + // Apply sanitization BEFORE validation and deduplicate + // This prevents identical IDs from being processed separately + const sanitizedIds = new Set(); + + // Pre-sanitize to check for duplicates that would become identical + for (const id of roomIds) { + const transformed = sanitizeId(id); + + // Only add non-empty IDs + if (transformed !== '') { + sanitizedIds.add(transformed); + } + } + + return Array.from(sanitizedIds); }, - z.array(nonEmptySanitizedString('roomId')).default([]) + z.array(z.string()).min(1, { + message: 'At least one valid roomId is required after sanitization' + }) ), force: validForceQueryParam() }); @@ -186,7 +207,7 @@ export const withValidRoomBulkDeleteRequest = (req: Request, res: Response, next return rejectRequest(res, error); } - req.query.roomIds = data.roomIds.join(','); + req.query.roomIds = data.roomIds as any; req.query.force = data.force ? 'true' : 'false'; next(); }; diff --git a/backend/src/services/room.service.ts b/backend/src/services/room.service.ts index 7b13a73..cd7588d 100644 --- a/backend/src/services/room.service.ts +++ b/backend/src/services/room.service.ts @@ -159,7 +159,7 @@ export class RoomService { async bulkDeleteRooms( roomIds: string[], forceDelete: boolean - ): Promise<{ deleted: string[]; markedAsDeleted: string[] }> { + ): Promise<{ deleted: string[]; markedForDeletion: string[] }> { try { const results = await Promise.allSettled( roomIds.map(async (roomId) => { @@ -184,26 +184,26 @@ export class RoomService { ); const deleted: string[] = []; - const markedAsDeleted: string[] = []; + const markedForDeletion: string[] = []; results.forEach((result) => { if (result.status === 'fulfilled') { if (result.value.status === 'deleted') { deleted.push(result.value.roomId); } else if (result.value.status === 'marked') { - markedAsDeleted.push(result.value.roomId); + markedForDeletion.push(result.value.roomId); } } else { this.logger.error(`Failed to process deletion for a room: ${result.reason}`); } }); - if (deleted.length === 0 && markedAsDeleted.length === 0) { + if (deleted.length === 0 && markedForDeletion.length === 0) { this.logger.error('No rooms were deleted or marked as deleted.'); throw internalError('No rooms were deleted or marked as deleted.'); } - return { deleted, markedAsDeleted }; + return { deleted, markedForDeletion }; } catch (error) { this.logger.error('Error deleting rooms:', error); throw error; @@ -287,7 +287,7 @@ export class RoomService { protected async deleteExpiredRooms(): Promise { let nextPageToken: string | undefined; const deletedRooms: string[] = []; - const markedAsDeletedRooms: string[] = []; + const markedForDeletionRooms: string[] = []; this.logger.verbose(`Checking expired rooms at ${new Date(Date.now()).toISOString()}`); try { @@ -306,10 +306,10 @@ export class RoomService { `Trying to delete ${expiredRoomIds.length} expired Meet rooms: ${expiredRoomIds.join(', ')}` ); - const { deleted, markedAsDeleted } = await this.bulkDeleteRooms(expiredRoomIds, false); + const { deleted, markedForDeletion } = await this.bulkDeleteRooms(expiredRoomIds, false); deletedRooms.push(...deleted); - markedAsDeletedRooms.push(...markedAsDeleted); + markedForDeletionRooms.push(...markedForDeletion); } } while (nextPageToken); @@ -317,8 +317,8 @@ export class RoomService { this.logger.verbose(`Successfully deleted ${deletedRooms.length} expired rooms`); } - if (markedAsDeletedRooms.length > 0) { - this.logger.verbose(`Marked as deleted ${markedAsDeletedRooms.length} expired rooms`); + if (markedForDeletionRooms.length > 0) { + this.logger.verbose(`Marked for deletion ${markedForDeletionRooms.length} expired rooms`); } } catch (error) { this.logger.error('Error deleting expired rooms:', error);