Skip to content

Commit

Permalink
fix: Invalid association between visitors and contacts. (#33868)
Browse files Browse the repository at this point in the history
* fix: Invalid association between visitors and contacts.

* types

* types

* some tests

* removed pointless tests

* fix api test

* uneeded import

* fixed some tests

* fixed some tests

* fixing tests

* added tests with different invalid associations

* do not migrate visitors with no rooms

* tests

* tests

* Update apps/meteor/ee/app/livechat-enterprise/server/api/contacts.ts

Co-authored-by: Rafael Tapia <[email protected]>

* no need for array

* adjusting tests

---------

Co-authored-by: Rafael Tapia <[email protected]>
  • Loading branch information
pierre-lehnen-rc and Rafael Tapia authored Nov 5, 2024
1 parent 33f955c commit 7f46d97
Show file tree
Hide file tree
Showing 62 changed files with 960 additions and 545 deletions.
11 changes: 5 additions & 6 deletions apps/meteor/app/apps/server/bridges/contact.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import type { IAppServerOrchestrator } from '@rocket.chat/apps';
import type { ILivechatContact } from '@rocket.chat/apps-engine/definition/livechat';
import { ContactBridge } from '@rocket.chat/apps-engine/server/bridges';
import type { IVisitor } from '@rocket.chat/core-typings';
import { LivechatContacts } from '@rocket.chat/models';

import { addContactEmail } from '../../../livechat/server/lib/contacts/addContactEmail';
import { verifyContactChannel } from '../../../livechat/server/lib/contacts/verifyContactChannel';
Expand All @@ -12,9 +10,9 @@ export class AppContactBridge extends ContactBridge {
super();
}

async getByVisitorId(visitorId: IVisitor['_id'], appId: string): Promise<ILivechatContact | null> {
async getById(contactId: ILivechatContact['_id'], appId: string): Promise<ILivechatContact | undefined> {
this.orch.debugLog(`The app ${appId} is fetching a contact`);
return LivechatContacts.findOneByVisitorId<ILivechatContact>(visitorId);
return this.orch.getConverters().get('contacts').convertById(contactId);
}

async verifyContact(
Expand All @@ -33,8 +31,9 @@ export class AppContactBridge extends ContactBridge {
await verifyContactChannel(verifyContactChannelParams);
}

protected addContactEmail(contactId: ILivechatContact['_id'], email: string, appId: string): Promise<ILivechatContact> {
protected async addContactEmail(contactId: ILivechatContact['_id'], email: string, appId: string): Promise<ILivechatContact> {
this.orch.debugLog(`The app ${appId} is adding a new email to the contact`);
return addContactEmail(contactId, email);
const contact = await addContactEmail(contactId, email);
return this.orch.getConverters().get('contacts').convertContact(contact);
}
}
126 changes: 126 additions & 0 deletions apps/meteor/app/apps/server/converters/contacts.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
import type { IAppContactsConverter, IAppsLivechatContact } from '@rocket.chat/apps';
import type { ILivechatContact } from '@rocket.chat/core-typings';
import { LivechatContacts } from '@rocket.chat/models';
import cloneDeep from 'lodash.clonedeep';

import { transformMappedData } from './transformMappedData';

export class AppContactsConverter implements IAppContactsConverter {
async convertById(contactId: ILivechatContact['_id']): Promise<IAppsLivechatContact | undefined> {
const contact = await LivechatContacts.findOneById(contactId);
if (!contact) {
return;
}

return this.convertContact(contact);
}

async convertContact(contact: undefined | null): Promise<undefined>;

async convertContact(contact: ILivechatContact): Promise<IAppsLivechatContact>;

async convertContact(contact: ILivechatContact | undefined | null): Promise<IAppsLivechatContact | undefined> {
if (!contact) {
return;
}

return cloneDeep(contact);
}

convertAppContact(contact: undefined | null): Promise<undefined>;

convertAppContact(contact: IAppsLivechatContact): Promise<ILivechatContact>;

async convertAppContact(contact: IAppsLivechatContact | undefined | null): Promise<ILivechatContact | undefined> {
if (!contact) {
return;
}

// Map every attribute individually to ensure there are no extra data coming from the app and leaking into anything else.
const map = {
_id: '_id',
_updatedAt: '_updatedAt',
name: 'name',
phones: {
from: 'phones',
list: true,
map: {
phoneNumber: 'phoneNumber',
},
},
emails: {
from: 'emails',
list: true,
map: {
address: 'address',
},
},
contactManager: 'contactManager',
unknown: 'unknown',
conflictingFields: {
from: 'conflictingFields',
list: true,
map: {
field: 'field',
value: 'value',
},
},
customFields: 'customFields',
channels: {
from: 'channels',
list: true,
map: {
name: 'name',
verified: 'verified',
visitor: {
from: 'visitor',
map: {
visitorId: 'visitorId',
source: {
from: 'source',
map: {
type: 'type',
id: 'id',
},
},
},
},
blocked: 'blocked',
field: 'field',
value: 'value',
verifiedAt: 'verifiedAt',
details: {
from: 'details',
map: {
type: 'type',
id: 'id',
alias: 'alias',
label: 'label',
sidebarIcon: 'sidebarIcon',
defaultIcon: 'defaultIcon',
destination: 'destination',
},
},
lastChat: {
from: 'lastChat',
map: {
_id: '_id',
ts: 'ts',
},
},
},
},
createdAt: 'createdAt',
lastChat: {
from: 'lastChat',
map: {
_id: '_id',
ts: 'ts',
},
},
importIds: 'importIds',
};

return transformMappedData(contact, map);
}
}
2 changes: 2 additions & 0 deletions apps/meteor/app/apps/server/converters/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { AppContactsConverter } from './contacts';
import { AppDepartmentsConverter } from './departments';
import { AppMessagesConverter } from './messages';
import { AppRolesConverter } from './roles';
Expand All @@ -9,6 +10,7 @@ import { AppVideoConferencesConverter } from './videoConferences';
import { AppVisitorsConverter } from './visitors';

export {
AppContactsConverter,
AppMessagesConverter,
AppRoomsConverter,
AppSettingsConverter,
Expand Down
61 changes: 53 additions & 8 deletions apps/meteor/app/apps/server/converters/transformMappedData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,16 +62,41 @@ import cloneDeep from 'lodash.clonedeep';
* @returns Object The data after transformations have been applied
*/

export const transformMappedData = async <
ResultType extends {
-readonly [p in keyof MapType]: MapType[p] extends keyof DataType
? DataType[MapType[p]]
: MapType[p] extends (...args: any[]) => any
? Awaited<ReturnType<MapType[p]>>
type MapFor<DataType> = {
[p in string]:
| string
| ((data: DataType) => Promise<unknown>)
| ((data: DataType) => unknown)
| { from: string; list: true }
| { from: string; map: MapFor<DataType[keyof DataType]>; list?: boolean };
};

type ResultFor<DataType extends Record<string, any>, MapType extends MapFor<DataType>> = {
-readonly [p in keyof MapType]: MapType[p] extends keyof DataType
? DataType[MapType[p]]
: MapType[p] extends (...args: any[]) => any
? Awaited<ReturnType<MapType[p]>>
: MapType[p] extends { from: infer KeyName; map?: Record<string, any>; list?: boolean }
? KeyName extends keyof DataType
? MapType[p]['list'] extends true
? DataType[KeyName] extends any[]
? MapType[p]['map'] extends MapFor<DataType[KeyName][number]>
? ResultFor<DataType[KeyName][number], MapType[p]['map']>[]
: DataType[KeyName]
: DataType[KeyName][]
: DataType[KeyName] extends object
? MapType[p]['map'] extends MapFor<DataType[KeyName]>
? ResultFor<DataType[KeyName], MapType[p]['map']>
: never
: never
: never
: never;
},
};

export const transformMappedData = async <
ResultType extends ResultFor<DataType, MapType>,
DataType extends Record<string, any>,
MapType extends { [p in string]: string | ((data: DataType) => Promise<unknown>) | ((data: DataType) => unknown) },
MapType extends MapFor<DataType>,
UnmappedProperties extends {
[p in keyof DataType as Exclude<p, MapType[keyof MapType]>]: DataType[p];
},
Expand All @@ -94,6 +119,26 @@ export const transformMappedData = async <
transformedData[to] = originalData[from];
}
delete originalData[from];
} else if (typeof from === 'object' && 'from' in from) {
const { from: fromName } = from;

if (from.list) {
if (Array.isArray(originalData[fromName])) {
if ('map' in from && from.map) {
if (typeof originalData[fromName] === 'object') {
transformedData[to] = await Promise.all(originalData[fromName].map((item) => transformMappedData(item, from.map)));
}
} else {
transformedData[to] = [...originalData[fromName]];
}
} else if (originalData[fromName] !== undefined && originalData[fromName] !== null) {
transformedData[to] = [originalData[fromName]];
}
} else {
transformedData[to] = await transformMappedData(originalData[fromName], from.map);
}

delete originalData[fromName];
}
}

Expand Down
20 changes: 20 additions & 0 deletions apps/meteor/app/livechat/lib/isSameChannel.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import type { ILivechatContactVisitorAssociation } from '@rocket.chat/core-typings';

export function isSameChannel(channel1: ILivechatContactVisitorAssociation, channel2: ILivechatContactVisitorAssociation): boolean {
if (!channel1 || !channel2) {
return false;
}

if (channel1.visitorId !== channel2.visitorId) {
return false;
}
if (channel1.source.type !== channel2.source.type) {
return false;
}

if ((channel1.source.id || channel2.source.id) && channel1.source.id !== channel2.source.id) {
return false;
}

return true;
}
14 changes: 1 addition & 13 deletions apps/meteor/app/livechat/server/api/lib/visitors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import type { FindOptions } from 'mongodb';

import { callbacks } from '../../../../../lib/callbacks';
import { canAccessRoomAsync } from '../../../../authorization/server/functions/canAccessRoom';
import { getContactIdByVisitorId } from '../../lib/contacts/getContactIdByVisitorId';
import { migrateVisitorToContactId } from '../../lib/contacts/migrateVisitorToContactId';

export async function findVisitorInfo({ visitorId }: { visitorId: IVisitor['_id'] }) {
const visitor = await LivechatVisitors.findOneEnabledById(visitorId);
Expand All @@ -14,17 +12,7 @@ export async function findVisitorInfo({ visitorId }: { visitorId: IVisitor['_id'
}

return {
visitor: await addContactIdToVisitor(visitor),
};
}

export async function addContactIdToVisitor(visitor: ILivechatVisitor): Promise<ILivechatVisitor & { contactId?: string }> {
const contactId = await getContactIdByVisitorId(visitor._id);

// If the visitor doesn't have a contactId yet, create a new contact for it using the same _id
return {
...visitor,
contactId: contactId || (await migrateVisitorToContactId(visitor, undefined, true)) || undefined,
visitor,
};
}

Expand Down
18 changes: 5 additions & 13 deletions apps/meteor/app/livechat/server/api/v1/contact.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import type { ILivechatContact } from '@rocket.chat/core-typings';
import { LivechatContacts, LivechatCustomField, LivechatVisitors } from '@rocket.chat/models';
import {
isPOSTOmnichannelContactsProps,
Expand All @@ -15,7 +14,7 @@ import { Meteor } from 'meteor/meteor';
import { API } from '../../../../api/server';
import { getPaginationItems } from '../../../../api/server/helpers/getPaginationItems';
import { createContact } from '../../lib/contacts/createContact';
import { getContact } from '../../lib/contacts/getContact';
import { getContactByChannel } from '../../lib/contacts/getContactByChannel';
import { getContactChannelsGrouped } from '../../lib/contacts/getContactChannelsGrouped';
import { getContactHistory } from '../../lib/contacts/getContactHistory';
import { getContacts } from '../../lib/contacts/getContacts';
Expand Down Expand Up @@ -134,20 +133,13 @@ API.v1.addRoute(
{ authRequired: true, permissionsRequired: ['view-livechat-contact'], validateParams: isGETOmnichannelContactsProps },
{
async get() {
const { contactId, email, phone } = this.queryParams;
let contact: ILivechatContact | null = null;
const { contactId, visitor } = this.queryParams;

if (contactId) {
contact = await getContact(contactId);
}

if (email) {
contact = await LivechatContacts.findOne({ 'emails.address': email });
if (!contactId && !visitor) {
return API.v1.notFound();
}

if (phone) {
contact = await LivechatContacts.findOne({ 'phones.phoneNumber': phone });
}
const contact = await (contactId ? LivechatContacts.findOneById(contactId) : getContactByChannel(visitor));

if (!contact) {
return API.v1.notFound();
Expand Down
3 changes: 1 addition & 2 deletions apps/meteor/app/livechat/server/api/v1/visitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { API } from '../../../../api/server';
import { settings } from '../../../../settings/server';
import { Livechat as LivechatTyped } from '../../lib/LivechatTyped';
import { findGuest, normalizeHttpHeaderData } from '../lib/livechat';
import { addContactIdToVisitor } from '../lib/visitors';

API.v1.addRoute(
'livechat/visitor',
Expand Down Expand Up @@ -145,7 +144,7 @@ API.v1.addRoute('livechat/visitor/:token', {
throw new Meteor.Error('invalid-token');
}

return API.v1.success({ visitor: await addContactIdToVisitor(visitor) });
return API.v1.success({ visitor });
},
async delete() {
check(this.urlParams, {
Expand Down
10 changes: 9 additions & 1 deletion apps/meteor/app/livechat/server/hooks/saveContactLastChat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ callbacks.add(
const {
_id,
v: { _id: guestId, contactId },
source,
} = room;

const lastChat = {
Expand All @@ -21,7 +22,14 @@ callbacks.add(
};
await LivechatVisitors.setLastChatById(guestId, lastChat);
if (contactId) {
await LivechatContacts.updateLastChatById(contactId, guestId, lastChat);
await LivechatContacts.updateLastChatById(
contactId,
{
visitorId: guestId,
source,
},
lastChat,
);
}
},
callbacks.priority.MEDIUM,
Expand Down
Loading

0 comments on commit 7f46d97

Please sign in to comment.