fix: PDFs uploaded by "PDF transcript" feature were returnig 403 when…

… attempting to download (#32329)

.changeset/rude-llamas-notice.md

@@ -0,0 +1,8 @@
+---
+"@rocket.chat/meteor": patch
+"@rocket.chat/i18n": patch
+"@rocket.chat/omnichannel-services": patch
+---
+
+Added a new setting `Restrict files access to users who can access room` that controls file visibility. This new setting allows users that "can access a room" to also download the files that are there. This is specially important for users with livechat manager or monitor roles, or agents that have special permissions to view closed rooms, since this allows them to download files on the conversation even after the conversation is closed.
+New setting is disabled by default and it is mutually exclusive with the setting `Restrict file access to room members` since this allows _more_ types of users to download files.

apps/meteor/app/file-upload/server/lib/FileUpload.ts

@@ -28,7 +28,7 @@ import { roomCoordinator } from '../../../../server/lib/rooms/roomCoordinator';
 import { UploadFS } from '../../../../server/ufs';
 import { ufsComplete } from '../../../../server/ufs/ufs-methods';
 import type { Store, StoreOptions } from '../../../../server/ufs/ufs-store';
-import { canAccessRoomAsync } from '../../../authorization/server/functions/canAccessRoom';
+import { canAccessRoomAsync, canAccessRoomIdAsync } from '../../../authorization/server/functions/canAccessRoom';
 import { settings } from '../../../settings/server';
 import { mime } from '../../../utils/lib/mimeTypes';
 import { isValidJWT, generateJWT } from '../../../utils/server/lib/JWTHelper';
@@ -463,16 +463,26 @@ export const FileUpload = {
 			return false;
 		}
 
-		if (!settings.get('FileUpload_Restrict_to_room_members') || !file?.rid) {
+		if (!file?.rid) {
 			return true;
 		}
 
-		const subscription = await Subscriptions.findOneByRoomIdAndUserId(file.rid, user._id, { projection: { _id: 1 } });
+		const fileUploadRestrictedToMembers = settings.get<boolean>('FileUpload_Restrict_to_room_members');
+		const fileUploadRestrictToUsersWhoCanAccessRoom = settings.get<boolean>('FileUpload_Restrict_to_users_who_can_access_room');
 
-		if (subscription) {
+		if (!fileUploadRestrictToUsersWhoCanAccessRoom && !fileUploadRestrictedToMembers) {
 			return true;
 		}
 
+		if (fileUploadRestrictedToMembers && !fileUploadRestrictToUsersWhoCanAccessRoom) {
+			const sub = await Subscriptions.findOneByRoomIdAndUserId(file.rid, user._id, { projection: { _id: 1 } });
+			return !!sub;
+		}
+
+		if (fileUploadRestrictToUsersWhoCanAccessRoom && !fileUploadRestrictedToMembers) {
+			return canAccessRoomIdAsync(file.rid, user._id);
+		}
+
 		return false;
 	},
 

apps/meteor/server/settings/file-upload.ts

@@ -33,10 +33,30 @@ export const createFileUploadSettings = () =>
 
 		await this.add('FileUpload_Restrict_to_room_members', true, {
 			type: 'boolean',
-			enableQuery: {
-				_id: 'FileUpload_ProtectFiles',
-				value: true,
-			},
+			enableQuery: [
+				{
+					_id: 'FileUpload_ProtectFiles',
+					value: true,
+				},
+				{
+					_id: 'FileUpload_Restrict_to_users_who_can_access_room',
+					value: false,
+				},
+			],
+		});
+
+		await this.add('FileUpload_Restrict_to_users_who_can_access_room', false, {
+			type: 'boolean',
+			enableQuery: [
+				{
+					_id: 'FileUpload_ProtectFiles',
+					value: true,
+				},
+				{
+					_id: 'FileUpload_Restrict_to_room_members',
+					value: false,
+				},
+			],
 		});
 
 		await this.add('FileUpload_RotateImages', true, {

apps/meteor/tests/end-to-end/api/09-rooms.js

@@ -87,11 +87,13 @@ describe('[Rooms]', function () {
 		let userCredentials;
 		const testChannelName = `channel.test.upload.${Date.now()}-${Math.random()}`;
 		let blockedMediaTypes;
+		let testPrivateChannel;
 
 		before(async () => {
 			user = await createUser({ joinDefaultChannels: false });
 			userCredentials = await login(user.username, password);
 			testChannel = (await createRoom({ type: 'c', name: testChannelName })).body.channel;
+			testPrivateChannel = (await createRoom({ type: 'p', name: `channel.test.private.${Date.now()}-${Math.random()}` })).body.group;
 			blockedMediaTypes = await getSettingValueById('FileUpload_MediaTypeBlackList');
 			const newBlockedMediaTypes = blockedMediaTypes
 				.split(',')
@@ -105,8 +107,10 @@ describe('[Rooms]', function () {
 				deleteRoom({ type: 'c', roomId: testChannel._id }),
 				deleteUser(user),
 				updateSetting('FileUpload_Restrict_to_room_members', true),
+				updateSetting('FileUpload_Restrict_to_users_who_can_access_room', false),
 				updateSetting('FileUpload_ProtectFiles', true),
 				updateSetting('FileUpload_MediaTypeBlackList', blockedMediaTypes),
+				deleteRoom({ roomId: testPrivateChannel._id, type: 'p' }),
 			]),
 		);
 
@@ -221,6 +225,7 @@ describe('[Rooms]', function () {
 
 		it('should be able to get the file when no access to the room if setting allows it', async () => {
 			await updateSetting('FileUpload_Restrict_to_room_members', false);
+			await updateSetting('FileUpload_Restrict_to_users_who_can_access_room', false);
 			await request.get(fileNewUrl).set(userCredentials).expect('Content-Type', 'image/png').expect(200);
 			await request.get(fileOldUrl).set(userCredentials).expect('Content-Type', 'image/png').expect(200);
 		});
@@ -237,6 +242,35 @@ describe('[Rooms]', function () {
 			await request.get(fileOldUrl).set(credentials).expect('Content-Type', 'image/png').expect(200);
 		});
 
+		it('should be able to get the file if not member but can access room if setting allows', async () => {
+			await updateSetting('FileUpload_Restrict_to_room_members', false);
+			await updateSetting('FileUpload_Restrict_to_users_who_can_access_room', true);
+
+			await request.get(fileNewUrl).set(userCredentials).expect('Content-Type', 'image/png').expect(200);
+			await request.get(fileOldUrl).set(userCredentials).expect('Content-Type', 'image/png').expect(200);
+		});
+
+		it('should not be able to get the file if not member and cannot access room', async () => {
+			const { body } = await request
+				.post(api(`rooms.upload/${testPrivateChannel._id}`))
+				.set(credentials)
+				.attach('file', imgURL)
+				.expect('Content-Type', 'application/json')
+				.expect(200);
+
+			const fileUrl = `/file-upload/${body.message.file._id}/${body.message.file.name}`;
+
+			await request.get(fileUrl).set(userCredentials).expect(403);
+		});
+
+		it('should respect the setting with less permissions when both are true', async () => {
+			await updateSetting('FileUpload_ProtectFiles', true);
+			await updateSetting('FileUpload_Restrict_to_room_members', true);
+			await updateSetting('FileUpload_Restrict_to_users_who_can_access_room', true);
+			await request.get(fileNewUrl).set(userCredentials).expect(403);
+			await request.get(fileOldUrl).set(userCredentials).expect(403);
+		});
+
 		it('should not be able to get the file without credentials', async () => {
 			await request.get(fileNewUrl).attach('file', imgURL).expect(403);
 			await request.get(fileOldUrl).attach('file', imgURL).expect(403);

ee/packages/omnichannel-services/src/OmnichannelTranscript.ts

@@ -336,22 +336,15 @@ export class OmnichannelTranscript extends ServiceClass implements IOmnichannelT
 		const outBuff = await streamToBuffer(stream as Readable);
 
 		try {
-			const file = await uploadService.uploadFile({
-				userId: details.userId,
+			const { rid } = await roomService.createDirectMessage({ to: details.userId, from: 'rocket.cat' });
+			const [rocketCatFile, transcriptFile] = await this.uploadFiles({
+				details,
 				buffer: outBuff,
-				details: {
-					// transcript_{company-name)_{date}_{hour}.pdf
-					name: `${transcriptText}_${data.siteName}_${new Intl.DateTimeFormat('en-US').format(new Date())}_${
-						data.visitor?.name || data.visitor?.username || 'Visitor'
-					}.pdf`,
-					type: 'application/pdf',
-					rid: details.rid,
-					// Rocket.cat is the goat
-					userId: 'rocket.cat',
-					size: outBuff.length,
-				},
+				roomIds: [rid, details.rid],
+				data,
+				transcriptText,
 			});
-			await this.pdfComplete({ details, file });
+			await this.pdfComplete({ details, transcriptFile, rocketCatFile });
 		} catch (e: any) {
 			this.pdfFailed({ details, e });
 		}
@@ -380,35 +373,74 @@ export class OmnichannelTranscript extends ServiceClass implements IOmnichannelT
 		});
 	}
 
-	private async pdfComplete({ details, file }: { details: WorkDetailsWithSource; file: IUpload }): Promise<void> {
+	private async uploadFiles({
+		details,
+		buffer,
+		roomIds,
+		data,
+		transcriptText,
+	}: {
+		details: WorkDetailsWithSource;
+		buffer: Buffer;
+		roomIds: string[];
+		data: any;
+		transcriptText: string;
+	}): Promise<IUpload[]> {
+		return Promise.all(
+			roomIds.map((roomId) => {
+				return uploadService.uploadFile({
+					userId: details.userId,
+					buffer,
+					details: {
+						// transcript_{company-name}_{date}_{hour}.pdf
+						name: `${transcriptText}_${data.siteName}_${new Intl.DateTimeFormat('en-US').format(new Date())}_${
+							data.visitor?.name || data.visitor?.username || 'Visitor'
+						}.pdf`,
+						type: 'application/pdf',
+						rid: roomId,
+						// Rocket.cat is the goat
+						userId: 'rocket.cat',
+						size: buffer.length,
+					},
+				});
+			}),
+		);
+	}
+
+	private async pdfComplete({
+		details,
+		transcriptFile,
+		rocketCatFile,
+	}: {
+		details: WorkDetailsWithSource;
+		transcriptFile: IUpload;
+		rocketCatFile: IUpload;
+	}): Promise<void> {
 		this.log.info(`Transcript for room ${details.rid} by user ${details.userId} - Complete`);
 		const user = await Users.findOneById(details.userId);
 		if (!user) {
 			return;
 		}
 		// Send the file to the livechat room where this was requested, to keep it in context
 		try {
-			const [, { rid }] = await Promise.all([
-				LivechatRooms.setPdfTranscriptFileIdById(details.rid, file._id),
-				roomService.createDirectMessage({ to: details.userId, from: 'rocket.cat' }),
-			]);
+			await LivechatRooms.setPdfTranscriptFileIdById(details.rid, transcriptFile._id);
 
 			this.log.info(`Transcript for room ${details.rid} by user ${details.userId} - Sending success message to user`);
 			const result = await Promise.allSettled([
 				uploadService.sendFileMessage({
 					roomId: details.rid,
 					userId: 'rocket.cat',
-					file,
+					file: transcriptFile,
 					message: {
 						// Translate from service
 						msg: await translationService.translateToServerLanguage('pdf_success_message'),
 					},
 				}),
 				// Send the file to the user who requested it, so they can download it
 				uploadService.sendFileMessage({
-					roomId: rid,
+					roomId: rocketCatFile.rid || '',
 					userId: 'rocket.cat',
-					file,
+					file: rocketCatFile,
 					message: {
 						// Translate from service
 						msg: await translationService.translate('pdf_success_message', user),

packages/i18n/src/locales/en.i18n.json

@@ -2354,6 +2354,8 @@
   "FileUpload_Enable_json_web_token_for_files_description": "Appends a JWT to uploaded files urls",
   "FileUpload_Restrict_to_room_members": "Restrict files to rooms' members",
   "FileUpload_Restrict_to_room_members_Description": "Restrict the access of files uploaded on rooms to the rooms' members only",
+  "FileUpload_Restrict_to_users_who_can_access_room": "Restrict files to users who can access the room",
+  "FileUpload_Restrict_to_users_who_can_access_room_Description": "Restrict the access of files uploaded on rooms to the users who can access the room. This option is mutually exclusive with the \"Restrict files to rooms' members\" option as this one allows for users that are not part of some rooms but have special permissions that allow them to see it to access the files uploaded, for example, Omnichannel Managers & Monitors",
   "FileUpload_Enabled": "File Uploads Enabled",
   "FileUpload_Enabled_Direct": "File Uploads Enabled in Direct Messages ",
   "FileUpload_Error": "File Upload Error",