Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Allow to use the token from room.v when requesting transcript instead of finding visitor #33244

Merged
merged 1 commit into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@
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 All @@ -22,7 +23,7 @@

const logger = new Logger('Livechat-SendTranscript');

export async function sendTranscript({

Check warning on line 26 in apps/meteor/app/livechat/server/lib/sendTranscript.ts

View workflow job for this annotation

GitHub Actions / 🔎 Code Check / Code Lint

Async function 'sendTranscript' has a complexity of 32. Maximum allowed is 31
token,
rid,
email,
Expand All @@ -41,16 +42,12 @@

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 @@
}

// 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;
});
});
Loading