Skip to content

Commit

Permalink
Merge branch 'develop' into feat/disable-email-2FA-oauth
Browse files Browse the repository at this point in the history
  • Loading branch information
kodiakhq[bot] authored Sep 9, 2024
2 parents bb717b6 + 4146c39 commit b878dc7
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 25 deletions.
5 changes: 5 additions & 0 deletions .changeset/four-cherries-kneel.md
Original file line number Diff line number Diff line change
@@ -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.
17 changes: 7 additions & 10 deletions apps/meteor/app/livechat/server/lib/sendTranscript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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<string>('Language') || 'en';
const timezone = getTimezone(user);
logger.debug(`Transcript will be sent using ${timezone} as timezone`);

Expand All @@ -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');
}

Expand Down
4 changes: 2 additions & 2 deletions apps/meteor/tests/data/livechat/rooms.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ export const createLivechatRoom = async (visitorToken: string, extraRoomParams?:
return response.body.room;
};

export const createVisitor = (department?: string, visitorName?: string): Promise<ILivechatVisitor> =>
export const createVisitor = (department?: string, visitorName?: string, customEmail?: string): Promise<ILivechatVisitor> =>
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<ILivechatVisitor>) => {
if (!err && res && res.body && res.body.visitor) {
Expand Down
21 changes: 21 additions & 0 deletions apps/meteor/tests/end-to-end/api/livechat/11-livechat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,27 @@ describe('LIVECHAT - Utils', () => {
.send({ token: visitor.token, rid: room._id, email: '[email protected]' });
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: '[email protected]' });
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: '[email protected]' });
expect(body2).to.have.property('success', true);
});
});

describe('livechat/transcript/:rid', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@ const modelsMock = {
LivechatRooms: {
findOneById: sinon.stub(),
},
LivechatVisitors: {
getVisitorByToken: sinon.stub(),
},
Messages: {
findLivechatClosingMessage: sinon.stub(),
findVisibleByRoomIdNotContainingTypesBeforeTs: sinon.stub(),
Expand Down Expand Up @@ -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();
Expand All @@ -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');
Expand All @@ -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([]);

Expand All @@ -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';
Expand All @@ -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: '[email protected]',
subject: 'Conversation Transcript',
replyTo: '[email protected]',
html: '<div> <hr></div>',
}),
).to.be.true;
});
});

0 comments on commit b878dc7

Please sign in to comment.