From 52c56ef89577808550b03cef6d4432361277b9c8 Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Mon, 29 Apr 2024 11:53:40 -0600 Subject: [PATCH 1/8] Fix file upload and accessing from automated PDF transcript feature --- .../app/file-upload/server/lib/FileUpload.ts | 8 +- .../src/OmnichannelTranscript.ts | 76 +++++++++++++------ 2 files changed, 61 insertions(+), 23 deletions(-) diff --git a/apps/meteor/app/file-upload/server/lib/FileUpload.ts b/apps/meteor/app/file-upload/server/lib/FileUpload.ts index 4458f9d61881..faa020beb0da 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'; @@ -468,6 +468,12 @@ export const FileUpload = { return true; } + // Special check for managers/monitors that can access rooms even when no subscription exist for them + // And also for agents that have the permission to access the room even after it is closed + if (await canAccessRoomIdAsync(file.rid, user._id)) { + return true; + } + const subscription = await Subscriptions.findOneByRoomIdAndUserId(file.rid, user._id, { projection: { _id: 1 } }); if (subscription) { diff --git a/ee/packages/omnichannel-services/src/OmnichannelTranscript.ts b/ee/packages/omnichannel-services/src/OmnichannelTranscript.ts index 02816b7aebc3..d75bd19c8172 100644 --- a/ee/packages/omnichannel-services/src/OmnichannelTranscript.ts +++ b/ee/packages/omnichannel-services/src/OmnichannelTranscript.ts @@ -323,22 +323,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 }); } @@ -367,7 +360,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) { @@ -375,17 +410,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' }), - ]); + 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'), @@ -393,9 +425,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), From 3f722de4391e1f5d7cda9604b3b13f6d8d0a9594 Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Tue, 30 Apr 2024 08:00:35 -0600 Subject: [PATCH 2/8] Add new setting --- .../app/file-upload/server/lib/FileUpload.ts | 22 +++++++++------ apps/meteor/server/settings/file-upload.ts | 28 ++++++++++++++++--- packages/i18n/src/locales/en.i18n.json | 2 ++ 3 files changed, 39 insertions(+), 13 deletions(-) diff --git a/apps/meteor/app/file-upload/server/lib/FileUpload.ts b/apps/meteor/app/file-upload/server/lib/FileUpload.ts index faa020beb0da..9e234a89961e 100644 --- a/apps/meteor/app/file-upload/server/lib/FileUpload.ts +++ b/apps/meteor/app/file-upload/server/lib/FileUpload.ts @@ -464,20 +464,24 @@ export const FileUpload = { return false; } - if (!settings.get('FileUpload_Restrict_to_room_members') || !file?.rid) { + if (!file?.rid) { return true; } - // Special check for managers/monitors that can access rooms even when no subscription exist for them - // And also for agents that have the permission to access the room even after it is closed - if (await canAccessRoomIdAsync(file.rid, user._id)) { - return true; - } + const fileUploadRestrictedToMembers = settings.get('FileUpload_Restrict_to_room_members'); + const fileUploadRestrictToUsersWhoCanAccessRoom = settings.get('FileUpload_Restrict_to_users_who_can_access_room'); + + if (fileUploadRestrictedToMembers && !fileUploadRestrictToUsersWhoCanAccessRoom) { + const sub = await Subscriptions.findOneByRoomIdAndUserId(file.rid, user._id, { projection: { _id: 1 } }); + if (!sub) { + return false; + } - const subscription = await Subscriptions.findOneByRoomIdAndUserId(file.rid, user._id, { projection: { _id: 1 } }); + return !!sub; + } - if (subscription) { - return true; + 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 4be9dfd117a7..6a6946544cdb 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/packages/i18n/src/locales/en.i18n.json b/packages/i18n/src/locales/en.i18n.json index 24fb91023e1a..abf20d7d70a0 100644 --- a/packages/i18n/src/locales/en.i18n.json +++ b/packages/i18n/src/locales/en.i18n.json @@ -2344,6 +2344,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", From f4a6a37fa73635a5aef4f09c5b278d3a8c5c5e54 Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Tue, 30 Apr 2024 08:20:30 -0600 Subject: [PATCH 3/8] boolean --- apps/meteor/app/file-upload/server/lib/FileUpload.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/meteor/app/file-upload/server/lib/FileUpload.ts b/apps/meteor/app/file-upload/server/lib/FileUpload.ts index 9e234a89961e..ada91fd729dc 100644 --- a/apps/meteor/app/file-upload/server/lib/FileUpload.ts +++ b/apps/meteor/app/file-upload/server/lib/FileUpload.ts @@ -468,8 +468,8 @@ export const FileUpload = { return true; } - const fileUploadRestrictedToMembers = settings.get('FileUpload_Restrict_to_room_members'); - const fileUploadRestrictToUsersWhoCanAccessRoom = settings.get('FileUpload_Restrict_to_users_who_can_access_room'); + const fileUploadRestrictedToMembers = settings.get('FileUpload_Restrict_to_room_members'); + const fileUploadRestrictToUsersWhoCanAccessRoom = settings.get('FileUpload_Restrict_to_users_who_can_access_room'); if (fileUploadRestrictedToMembers && !fileUploadRestrictToUsersWhoCanAccessRoom) { const sub = await Subscriptions.findOneByRoomIdAndUserId(file.rid, user._id, { projection: { _id: 1 } }); From 8c35a58a143f806e83f3f745b9c151c9b91b4d41 Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Tue, 30 Apr 2024 12:03:12 -0600 Subject: [PATCH 4/8] tests --- .../app/file-upload/server/lib/FileUpload.ts | 4 ++ apps/meteor/tests/end-to-end/api/09-rooms.js | 40 +++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/apps/meteor/app/file-upload/server/lib/FileUpload.ts b/apps/meteor/app/file-upload/server/lib/FileUpload.ts index ada91fd729dc..d7d19a59ea99 100644 --- a/apps/meteor/app/file-upload/server/lib/FileUpload.ts +++ b/apps/meteor/app/file-upload/server/lib/FileUpload.ts @@ -471,6 +471,10 @@ export const FileUpload = { const fileUploadRestrictedToMembers = settings.get('FileUpload_Restrict_to_room_members'); const fileUploadRestrictToUsersWhoCanAccessRoom = settings.get('FileUpload_Restrict_to_users_who_can_access_room'); + if (!fileUploadRestrictToUsersWhoCanAccessRoom && !fileUploadRestrictedToMembers) { + return true; + } + if (fileUploadRestrictedToMembers && !fileUploadRestrictToUsersWhoCanAccessRoom) { const sub = await Subscriptions.findOneByRoomIdAndUserId(file.rid, user._id, { projection: { _id: 1 } }); if (!sub) { 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 ba1ff2c9d729..9dc99b7c8dcc 100644 --- a/apps/meteor/tests/end-to-end/api/09-rooms.js +++ b/apps/meteor/tests/end-to-end/api/09-rooms.js @@ -84,6 +84,8 @@ describe('[Rooms]', function () { let user; let userCredentials; let blockedMediaTypes; + let testPrivateChannel; + before(async () => { user = await createUser({ joinDefaultChannels: false }); userCredentials = await login(user.username, password); @@ -100,6 +102,7 @@ describe('[Rooms]', function () { user = undefined; await updateSetting('FileUpload_Restrict_to_room_members', false); + await updateSetting('FileUpload_Restrict_to_users_who_can_access_room', false); await updateSetting('FileUpload_ProtectFiles', true); await updateSetting('FileUpload_MediaTypeBlackList', blockedMediaTypes); }); @@ -112,6 +115,14 @@ describe('[Rooms]', function () { done(); }); }); + it('create a private channel', async () => { + const { + body: { group }, + } = await createRoom({ type: 'p', name: `channel.test.private.${Date.now()}-${Math.random()}` }); + + testPrivateChannel = group; + }); + it("don't upload a file to room with file field other than file", (done) => { request .post(api(`rooms.upload/${testChannel._id}`)) @@ -223,6 +234,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); }); @@ -239,6 +251,34 @@ 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(credentials).expect('Content-Type', 'image/png').expect(200); + await request.get(fileOldUrl).set(credentials).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_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); From 4b45160eac5ba2bfd1bb54ad108e18b90fd34209 Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Tue, 30 Apr 2024 14:23:58 -0600 Subject: [PATCH 5/8] Create rude-llamas-notice.md --- .changeset/rude-llamas-notice.md | 8 ++++++++ 1 file changed, 8 insertions(+) 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. From 778d8ba6989de4af840a58a8f7c0123039fd2b4e Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Mon, 13 May 2024 08:28:25 -0600 Subject: [PATCH 6/8] cr --- apps/meteor/app/file-upload/server/lib/FileUpload.ts | 4 ---- apps/meteor/tests/end-to-end/api/09-rooms.js | 6 ++++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/apps/meteor/app/file-upload/server/lib/FileUpload.ts b/apps/meteor/app/file-upload/server/lib/FileUpload.ts index d7d19a59ea99..703154c94951 100644 --- a/apps/meteor/app/file-upload/server/lib/FileUpload.ts +++ b/apps/meteor/app/file-upload/server/lib/FileUpload.ts @@ -477,10 +477,6 @@ export const FileUpload = { if (fileUploadRestrictedToMembers && !fileUploadRestrictToUsersWhoCanAccessRoom) { const sub = await Subscriptions.findOneByRoomIdAndUserId(file.rid, user._id, { projection: { _id: 1 } }); - if (!sub) { - return false; - } - return !!sub; } 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 9dc99b7c8dcc..84d19e4385bd 100644 --- a/apps/meteor/tests/end-to-end/api/09-rooms.js +++ b/apps/meteor/tests/end-to-end/api/09-rooms.js @@ -105,6 +105,7 @@ describe('[Rooms]', function () { await updateSetting('FileUpload_Restrict_to_users_who_can_access_room', false); await updateSetting('FileUpload_ProtectFiles', true); await updateSetting('FileUpload_MediaTypeBlackList', blockedMediaTypes); + await deleteRoom({ roomId: testPrivateChannel._id, type: 'p' }); }); const testChannelName = `channel.test.upload.${Date.now()}-${Math.random()}`; @@ -255,8 +256,8 @@ describe('[Rooms]', function () { await updateSetting('FileUpload_Restrict_to_room_members', false); await updateSetting('FileUpload_Restrict_to_users_who_can_access_room', true); - await request.get(fileNewUrl).set(credentials).expect('Content-Type', 'image/png').expect(200); - await request.get(fileOldUrl).set(credentials).expect('Content-Type', 'image/png').expect(200); + 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 () => { @@ -273,6 +274,7 @@ describe('[Rooms]', function () { }); 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); From 29d83ed0d2589315c23bb3e45ac8571de420929d Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Fri, 17 May 2024 14:26:54 -0600 Subject: [PATCH 7/8] Update ee/packages/omnichannel-services/src/OmnichannelTranscript.ts Co-authored-by: Marcos Spessatto Defendi --- ee/packages/omnichannel-services/src/OmnichannelTranscript.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/packages/omnichannel-services/src/OmnichannelTranscript.ts b/ee/packages/omnichannel-services/src/OmnichannelTranscript.ts index d75bd19c8172..131fba769f2c 100644 --- a/ee/packages/omnichannel-services/src/OmnichannelTranscript.ts +++ b/ee/packages/omnichannel-services/src/OmnichannelTranscript.ts @@ -410,7 +410,7 @@ export class OmnichannelTranscript extends ServiceClass implements IOmnichannelT } // Send the file to the livechat room where this was requested, to keep it in context try { - LivechatRooms.setPdfTranscriptFileIdById(details.rid, transcriptFile._id); + 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([ From 80a467852d3c16196321a1a6ad7aafdf8b64fb32 Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Mon, 20 May 2024 07:49:52 -0600 Subject: [PATCH 8/8] conflicts killed test --- apps/meteor/tests/end-to-end/api/09-rooms.js | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) 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 9592ffdf811f..94318ce66225 100644 --- a/apps/meteor/tests/end-to-end/api/09-rooms.js +++ b/apps/meteor/tests/end-to-end/api/09-rooms.js @@ -85,15 +85,15 @@ describe('[Rooms]', function () { let testChannel; let user; let userCredentials; + const testChannelName = `channel.test.upload.${Date.now()}-${Math.random()}`; let blockedMediaTypes; let testPrivateChannel; - const testChannelName = `channel.test.upload.${Date.now()}-${Math.random()}`; - 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(',') @@ -114,20 +114,6 @@ describe('[Rooms]', function () { ]), ); - it('create an channel', (done) => { - createRoom({ type: 'c', name: testChannelName }).end((err, res) => { - testChannel = res.body.channel; - done(); - }); - }); - it('create a private channel', async () => { - const { - body: { group }, - } = await createRoom({ type: 'p', name: `channel.test.private.${Date.now()}-${Math.random()}` }); - - testPrivateChannel = group; - }); - it("don't upload a file to room with file field other than file", (done) => { request .post(api(`rooms.upload/${testChannel._id}`))