From 30399688fce5bb1482b7737c34cfa92a896ea877 Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Tue, 18 Jun 2024 13:45:38 -0600 Subject: [PATCH] fix: PDFs uploaded by "PDF transcript" feature were returnig 403 when attempting to download (#32329) --- .changeset/rude-llamas-notice.md | 8 ++ .../app/file-upload/server/lib/FileUpload.ts | 18 ++++- apps/meteor/server/settings/file-upload.ts | 28 ++++++- apps/meteor/tests/end-to-end/api/09-rooms.js | 34 +++++++++ .../src/OmnichannelTranscript.ts | 76 +++++++++++++------ packages/i18n/src/locales/en.i18n.json | 2 + 6 files changed, 136 insertions(+), 30 deletions(-) create mode 100644 .changeset/rude-llamas-notice.md diff --git a/.changeset/rude-llamas-notice.md b/.changeset/rude-llamas-notice.md new file mode 100644 index 000000000000..90c0ca3bd20a --- /dev/null +++ b/.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. diff --git a/apps/meteor/app/file-upload/server/lib/FileUpload.ts b/apps/meteor/app/file-upload/server/lib/FileUpload.ts index c824ba6c31a5..08e2ccb0a52b 100644 --- a/apps/meteor/app/file-upload/server/lib/FileUpload.ts +++ b/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('FileUpload_Restrict_to_room_members'); + const fileUploadRestrictToUsersWhoCanAccessRoom = settings.get('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; }, diff --git a/apps/meteor/server/settings/file-upload.ts b/apps/meteor/server/settings/file-upload.ts index 76e788cda0e2..505520b73e57 100644 --- a/apps/meteor/server/settings/file-upload.ts +++ b/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, { diff --git a/apps/meteor/tests/end-to-end/api/09-rooms.js b/apps/meteor/tests/end-to-end/api/09-rooms.js index dc8a6a209300..72bd5819593e 100644 --- a/apps/meteor/tests/end-to-end/api/09-rooms.js +++ b/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); diff --git a/ee/packages/omnichannel-services/src/OmnichannelTranscript.ts b/ee/packages/omnichannel-services/src/OmnichannelTranscript.ts index 2d273991508b..5f99269f671b 100644 --- a/ee/packages/omnichannel-services/src/OmnichannelTranscript.ts +++ b/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,7 +373,49 @@ export class OmnichannelTranscript extends ServiceClass implements IOmnichannelT }); } - private async pdfComplete({ details, file }: { details: WorkDetailsWithSource; file: IUpload }): Promise { + private async uploadFiles({ + details, + buffer, + roomIds, + data, + transcriptText, + }: { + details: WorkDetailsWithSource; + buffer: Buffer; + roomIds: string[]; + data: any; + transcriptText: string; + }): Promise { + 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 { this.log.info(`Transcript for room ${details.rid} by user ${details.userId} - Complete`); const user = await Users.findOneById(details.userId); if (!user) { @@ -388,17 +423,14 @@ export class OmnichannelTranscript extends ServiceClass implements IOmnichannelT } // 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'), @@ -406,9 +438,9 @@ export class OmnichannelTranscript extends ServiceClass implements IOmnichannelT }), // 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), diff --git a/packages/i18n/src/locales/en.i18n.json b/packages/i18n/src/locales/en.i18n.json index 779c71c9d755..836b534df601 100644 --- a/packages/i18n/src/locales/en.i18n.json +++ b/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",