From ed7398d93067cf92b48791b4ca5f20cb94c89131 Mon Sep 17 00:00:00 2001 From: "dionisio-bot[bot]" <117394943+dionisio-bot[bot]@users.noreply.github.com> Date: Wed, 11 Sep 2024 19:34:56 +0000 Subject: [PATCH] fix: Allow to use the token from `room.v` when requesting transcript instead of finding visitor (#33242) Co-authored-by: Kevin Aleman <11577696+KevLehman@users.noreply.github.com> --- .changeset/four-cherries-kneel.md | 5 +++ .../app/livechat/server/lib/sendTranscript.ts | 17 +++---- apps/meteor/tests/data/livechat/rooms.ts | 4 +- .../end-to-end/api/livechat/11-livechat.ts | 21 +++++++++ .../server/lib/sendTranscript.spec.ts | 45 +++++++++++++------ 5 files changed, 67 insertions(+), 25 deletions(-) create mode 100644 .changeset/four-cherries-kneel.md diff --git a/.changeset/four-cherries-kneel.md b/.changeset/four-cherries-kneel.md new file mode 100644 index 000000000000..095d5af0aa76 --- /dev/null +++ b/.changeset/four-cherries-kneel.md @@ -0,0 +1,5 @@ +--- +"@rocket.chat/meteor": patch +--- + +Allow to use the token from `room.v` when requesting transcript instead of visitor token. Visitors may change their tokens at any time, rendering old conversations impossible to access for them (or for APIs depending on token) as the visitor token won't match the `room.v` token. diff --git a/apps/meteor/app/livechat/server/lib/sendTranscript.ts b/apps/meteor/app/livechat/server/lib/sendTranscript.ts index 74032121ee50..bc7c06e0eaae 100644 --- a/apps/meteor/app/livechat/server/lib/sendTranscript.ts +++ b/apps/meteor/app/livechat/server/lib/sendTranscript.ts @@ -3,12 +3,13 @@ import { type IUser, type MessageTypesValues, type IOmnichannelSystemMessage, + type ILivechatVisitor, isFileAttachment, isFileImageAttachment, } from '@rocket.chat/core-typings'; import colors from '@rocket.chat/fuselage-tokens/colors'; import { Logger } from '@rocket.chat/logger'; -import { LivechatRooms, LivechatVisitors, Messages, Uploads, Users } from '@rocket.chat/models'; +import { LivechatRooms, Messages, Uploads, Users } from '@rocket.chat/models'; import { check } from 'meteor/check'; import moment from 'moment-timezone'; @@ -41,16 +42,12 @@ export async function sendTranscript({ const room = await LivechatRooms.findOneById(rid); - const visitor = await LivechatVisitors.getVisitorByToken(token, { - projection: { _id: 1, token: 1, language: 1, username: 1, name: 1 }, - }); - - if (!visitor) { - throw new Error('error-invalid-token'); + const visitor = room?.v as ILivechatVisitor; + if (token !== visitor?.token) { + throw new Error('error-invalid-visitor'); } - // @ts-expect-error - Visitor typings should include language? - const userLanguage = visitor?.language || settings.get('Language') || 'en'; + const userLanguage = settings.get('Language') || 'en'; const timezone = getTimezone(user); logger.debug(`Transcript will be sent using ${timezone} as timezone`); @@ -59,7 +56,7 @@ export async function sendTranscript({ } // allow to only user to send transcripts from their own chats - if (room.t !== 'l' || !room.v || room.v.token !== token) { + if (room.t !== 'l') { throw new Error('error-invalid-room'); } diff --git a/apps/meteor/tests/data/livechat/rooms.ts b/apps/meteor/tests/data/livechat/rooms.ts index 9532fd4214ab..b5d89762c614 100644 --- a/apps/meteor/tests/data/livechat/rooms.ts +++ b/apps/meteor/tests/data/livechat/rooms.ts @@ -33,10 +33,10 @@ export const createLivechatRoom = async (visitorToken: string, extraRoomParams?: return response.body.room; }; -export const createVisitor = (department?: string, visitorName?: string): Promise => +export const createVisitor = (department?: string, visitorName?: string, customEmail?: string): Promise => new Promise((resolve, reject) => { const token = getRandomVisitorToken(); - const email = `${token}@${token}.com`; + const email = customEmail || `${token}@${token}.com`; const phone = `${Math.floor(Math.random() * 10000000000)}`; void request.get(api(`livechat/visitor/${token}`)).end((err: Error, res: DummyResponse) => { if (!err && res && res.body && res.body.visitor) { diff --git a/apps/meteor/tests/end-to-end/api/livechat/11-livechat.ts b/apps/meteor/tests/end-to-end/api/livechat/11-livechat.ts index c07f7bcecc81..7ce582025538 100644 --- a/apps/meteor/tests/end-to-end/api/livechat/11-livechat.ts +++ b/apps/meteor/tests/end-to-end/api/livechat/11-livechat.ts @@ -283,6 +283,27 @@ describe('LIVECHAT - Utils', () => { .send({ token: visitor.token, rid: room._id, email: 'visitor@notadomain.com' }); expect(body).to.have.property('success', true); }); + it('should allow a visitor to get a transcript even if token changed by using an old token that matches room.v', async () => { + const visitor = await createVisitor(); + const room = await createLivechatRoom(visitor.token); + await closeOmnichannelRoom(room._id); + const visitor2 = await createVisitor(undefined, undefined, visitor.visitorEmails?.[0].address); + const room2 = await createLivechatRoom(visitor2.token); + await closeOmnichannelRoom(room2._id); + + expect(visitor.token !== visitor2.token).to.be.true; + const { body } = await request + .post(api('livechat/transcript')) + .set(credentials) + .send({ token: visitor.token, rid: room._id, email: 'visitor@notadomain.com' }); + expect(body).to.have.property('success', true); + + const { body: body2 } = await request + .post(api('livechat/transcript')) + .set(credentials) + .send({ token: visitor2.token, rid: room2._id, email: 'visitor@notadomain.com' }); + expect(body2).to.have.property('success', true); + }); }); describe('livechat/transcript/:rid', () => { diff --git a/apps/meteor/tests/unit/app/livechat/server/lib/sendTranscript.spec.ts b/apps/meteor/tests/unit/app/livechat/server/lib/sendTranscript.spec.ts index 64da050cfd88..ca39a64c21a9 100644 --- a/apps/meteor/tests/unit/app/livechat/server/lib/sendTranscript.spec.ts +++ b/apps/meteor/tests/unit/app/livechat/server/lib/sendTranscript.spec.ts @@ -6,9 +6,6 @@ const modelsMock = { LivechatRooms: { findOneById: sinon.stub(), }, - LivechatVisitors: { - getVisitorByToken: sinon.stub(), - }, Messages: { findLivechatClosingMessage: sinon.stub(), findVisibleByRoomIdNotContainingTypesBeforeTs: sinon.stub(), @@ -75,7 +72,6 @@ describe('Send transcript', () => { beforeEach(() => { checkMock.reset(); modelsMock.LivechatRooms.findOneById.reset(); - modelsMock.LivechatVisitors.getVisitorByToken.reset(); modelsMock.Messages.findLivechatClosingMessage.reset(); modelsMock.Messages.findVisibleByRoomIdNotContainingTypesBeforeTs.reset(); modelsMock.Users.findOneById.reset(); @@ -87,11 +83,9 @@ describe('Send transcript', () => { await expect(sendTranscript({})).to.be.rejectedWith(Error); }); it('should throw error when visitor not found', async () => { - modelsMock.LivechatVisitors.getVisitorByToken.resolves(null); await expect(sendTranscript({ rid: 'rid', email: 'email', logger: mockLogger })).to.be.rejectedWith(Error); }); it('should attempt to send an email when params are valid using default subject', async () => { - modelsMock.LivechatVisitors.getVisitorByToken.resolves({ language: null }); modelsMock.LivechatRooms.findOneById.resolves({ t: 'l', v: { token: 'token' } }); modelsMock.Messages.findVisibleByRoomIdNotContainingTypesBeforeTs.resolves([]); tStub.returns('Conversation Transcript'); @@ -117,7 +111,6 @@ describe('Send transcript', () => { ).to.be.true; }); it('should use provided subject', async () => { - modelsMock.LivechatVisitors.getVisitorByToken.resolves({ language: null }); modelsMock.LivechatRooms.findOneById.resolves({ t: 'l', v: { token: 'token' } }); modelsMock.Messages.findVisibleByRoomIdNotContainingTypesBeforeTs.resolves([]); @@ -143,7 +136,6 @@ describe('Send transcript', () => { ).to.be.true; }); it('should use subject from setting (when configured) when no subject provided', async () => { - modelsMock.LivechatVisitors.getVisitorByToken.resolves({ language: null }); modelsMock.LivechatRooms.findOneById.resolves({ t: 'l', v: { token: 'token' } }); modelsMock.Messages.findVisibleByRoomIdNotContainingTypesBeforeTs.resolves([]); mockSettingValues.Livechat_transcript_email_subject = 'A custom subject obtained from setting.get'; @@ -170,36 +162,63 @@ describe('Send transcript', () => { }); it('should fail if room provided is invalid', async () => { modelsMock.LivechatRooms.findOneById.resolves(null); - modelsMock.LivechatVisitors.getVisitorByToken.resolves({ language: null }); await expect(sendTranscript({ rid: 'rid', email: 'email', logger: mockLogger })).to.be.rejectedWith(Error); }); it('should fail if room provided is of different type', async () => { modelsMock.LivechatRooms.findOneById.resolves({ t: 'c' }); - modelsMock.LivechatVisitors.getVisitorByToken.resolves({ language: null }); await expect(sendTranscript({ rid: 'rid', email: 'email' })).to.be.rejectedWith(Error); }); it('should fail if room is of valid type, but doesnt doesnt have `v` property', async () => { - modelsMock.LivechatVisitors.getVisitorByToken.resolves({ language: null }); modelsMock.LivechatRooms.findOneById.resolves({ t: 'l' }); await expect(sendTranscript({ rid: 'rid', email: 'email' })).to.be.rejectedWith(Error); }); it('should fail if room is of valid type, has `v` prop, but it doesnt contain `token`', async () => { - modelsMock.LivechatVisitors.getVisitorByToken.resolves({ language: null }); modelsMock.LivechatRooms.findOneById.resolves({ t: 'l', v: { otherProp: 'xxx' } }); await expect(sendTranscript({ rid: 'rid', email: 'email' })).to.be.rejectedWith(Error); }); it('should fail if room is of valid type, has `v.token`, but its different from the one on param (room from another visitor)', async () => { - modelsMock.LivechatVisitors.getVisitorByToken.resolves({ language: null }); modelsMock.LivechatRooms.findOneById.resolves({ t: 'l', v: { token: 'xxx' } }); await expect(sendTranscript({ rid: 'rid', email: 'email', token: 'xveasdf' })).to.be.rejectedWith(Error); }); + + it('should throw an error when token is not the one on room.v', async () => { + modelsMock.LivechatRooms.findOneById.resolves({ t: 'l', v: { token: 'xxx' } }); + + await expect(sendTranscript({ rid: 'rid', email: 'email', token: 'xveasdf' })).to.be.rejectedWith(Error); + }); + it('should work when token matches room.v', async () => { + modelsMock.LivechatRooms.findOneById.resolves({ t: 'l', v: { token: 'token-123' } }); + modelsMock.Messages.findVisibleByRoomIdNotContainingTypesBeforeTs.resolves([]); + delete mockSettingValues.Livechat_transcript_email_subject; + tStub.returns('Conversation Transcript'); + + await sendTranscript({ + rid: 'rid', + token: 'token-123', + email: 'email', + user: { _id: 'x', name: 'x', utcOffset: '-6', username: 'x' }, + }); + + expect(getTimezoneMock.calledWith({ _id: 'x', name: 'x', utcOffset: '-6', username: 'x' })).to.be.true; + expect(modelsMock.Messages.findLivechatClosingMessage.calledWith('rid', { projection: { ts: 1 } })).to.be.true; + expect(modelsMock.Messages.findVisibleByRoomIdNotContainingTypesBeforeTs.called).to.be.true; + expect( + mailerMock.calledWith({ + to: 'email', + from: 'test@rocket.chat', + subject: 'Conversation Transcript', + replyTo: 'test@rocket.chat', + html: '
', + }), + ).to.be.true; + }); });