fix: Remove model-level query restrictions for monitors & improve transfer error handling

.changeset/pretty-starfishes-battle.md

@@ -0,0 +1,6 @@
+---
+"@rocket.chat/meteor": patch
+---
+
+fix: Remove model-level query restrictions for monitors
+fix: `onAssignmentFailed` & `onTransferFailure` callbacks not working properly

apps/meteor/app/livechat/server/lib/Helper.js

@@ -483,7 +483,13 @@ export const forwardRoomToDepartment = async (room, guest, transferData) => {
 			throw new Error('error-no-agents-online-in-department');
 		}
 		// if a chat has a fallback department, attempt to redirect chat to there [EE]
-		return !!callbacks.run('livechat:onTransferFailure', { room, guest, transferData });
+		const transferSuccess = !!(await callbacks.run('livechat:onTransferFailure', room, { guest, transferData }));
+
+		// On CE theres no callback so it will return the room
+		if (typeof transferSuccess !== 'boolean' || !transferSuccess) {
+			logger.debug(`Cannot forward room ${room._id}. Unable to delegate inquiry`);
+			return false;
+		}
 	}
 
 	await Livechat.saveTransferHistory(room, transferData);

apps/meteor/app/livechat/server/lib/RoutingManager.ts

@@ -252,9 +252,8 @@ export const RoutingManager: Routing = {
 
 		if (!agent) {
 			logger.debug(`Cannot take Inquiry ${inquiry._id}: Precondition failed for agent`);
-			const cbRoom = await callbacks.run<'livechat.onAgentAssignmentFailed'>('livechat.onAgentAssignmentFailed', {
+			const cbRoom = await callbacks.run<'livechat.onAgentAssignmentFailed'>('livechat.onAgentAssignmentFailed', room, {
 				inquiry,
-				room,
 				options,
 			});
 			return cbRoom;

apps/meteor/ee/app/livechat-enterprise/server/hooks/applyDepartmentRestrictions.ts

@@ -4,16 +4,18 @@ import type { ILivechatDepartment } from '@rocket.chat/core-typings';
 import { callbacks } from '../../../../../lib/callbacks';
 import { hasRoleAsync } from '../../../../../app/authorization/server/functions/hasRole';
 import { cbLogger } from '../lib/logger';
-import { getUnitsFromUser } from '../lib/units';
+import { getUnitsFromUser } from '../methods/getUnitsFromUserRoles';
 
-export const addQueryRestrictionsToDepartmentsModel = async (originalQuery: FilterOperators<ILivechatDepartment> = {}) => {
+export const addQueryRestrictionsToDepartmentsModel = async (originalQuery: FilterOperators<ILivechatDepartment> = {}, userId: string) => {
 	const query: FilterOperators<ILivechatDepartment> = { ...originalQuery, type: { $ne: 'u' } };
 
-	const units = await getUnitsFromUser();
+	const units = await getUnitsFromUser(userId);
 	if (Array.isArray(units)) {
 		query.ancestors = { $in: units };
+		query._id = { $in: units };
 	}
 
+	cbLogger.debug({ msg: 'Applying department query restrictions', userId, units });
 	return query;
 };
 
@@ -25,8 +27,7 @@ callbacks.add(
 			return originalQuery;
 		}
 
-		cbLogger.debug('Applying department query restrictions');
-		return addQueryRestrictionsToDepartmentsModel(originalQuery);
+		return addQueryRestrictionsToDepartmentsModel(originalQuery, userId);
 	},
 	callbacks.priority.HIGH,
 	'livechat-apply-department-restrictions',

apps/meteor/ee/app/livechat-enterprise/server/hooks/applyRoomRestrictions.ts

@@ -19,13 +19,13 @@ export const restrictQuery = async (originalQuery: FilterOperators<IOmnichannelR
 	};
 	query.$and = [condition, ...expressions];
 
+	cbLogger.debug({ msg: 'Applying room query restrictions', units });
 	return query;
 };
 
 callbacks.add(
 	'livechat.applyRoomRestrictions',
 	async (originalQuery: FilterOperators<IOmnichannelRoom> = {}) => {
-		cbLogger.debug('Applying room query restrictions');
 		return restrictQuery(originalQuery);
 	},
 	callbacks.priority.HIGH,

apps/meteor/ee/app/livechat-enterprise/server/hooks/onAgentAssignmentFailed.ts

@@ -1,19 +1,22 @@
+import type { IOmnichannelRoom } from '@rocket.chat/core-typings';
+
 import { callbacks } from '../../../../../lib/callbacks';
 import { settings } from '../../../../../app/settings/server';
 import { cbLogger } from '../lib/logger';
 
-const handleOnAgentAssignmentFailed = async ({
-	inquiry,
-	room,
-	options,
-}: {
-	inquiry: any;
-	room: any;
-	options: {
-		forwardingToDepartment?: { oldDepartmentId: string; transferData: any };
-		clientAction?: boolean;
-	};
-}): Promise<any> => {
+const handleOnAgentAssignmentFailed = async (
+	room: IOmnichannelRoom,
+	{
+		inquiry,
+		options,
+	}: {
+		inquiry: any;
+		options: {
+			forwardingToDepartment?: { oldDepartmentId?: string; transferData?: any };
+			clientAction?: boolean;
+		};
+	},
+) => {
 	if (!inquiry || !room) {
 		cbLogger.debug('Skipping callback. No inquiry or room provided');
 		return;
@@ -37,8 +40,7 @@ const handleOnAgentAssignmentFailed = async ({
 		return;
 	}
 
-	room.chatQueued = true;
-	return room;
+	return { ...room, chatQueued: true } as IOmnichannelRoom & { chatQueued: boolean };
 };
 
 callbacks.add(

apps/meteor/ee/app/livechat-enterprise/server/hooks/onTransferFailure.ts

@@ -6,15 +6,16 @@ import { callbacks } from '../../../../../lib/callbacks';
 import { forwardRoomToDepartment } from '../../../../../app/livechat/server/lib/Helper';
 import { cbLogger } from '../lib/logger';
 
-const onTransferFailure = async ({
-	room,
-	guest,
-	transferData,
-}: {
-	room: IRoom;
-	guest: ILivechatVisitor;
-	transferData: { [k: string]: string | any };
-}) => {
+const onTransferFailure = async (
+	room: IRoom,
+	{
+		guest,
+		transferData,
+	}: {
+		guest: ILivechatVisitor;
+		transferData: any;
+	},
+) => {
 	cbLogger.debug(`Attempting to transfer room ${room._id} using fallback departments`);
 	const { departmentId } = transferData;
 	const department = (await LivechatDepartment.findOneById(departmentId, {
@@ -26,13 +27,17 @@ const onTransferFailure = async ({
 	}
 
 	cbLogger.debug(`Fallback department ${department.fallbackForwardDepartment} found for department ${department._id}. Redirecting`);
+	const fallbackDepartmentDb = await LivechatDepartment.findOneById<Pick<ILivechatDepartment, '_id' | 'name'>>(
+		department.fallbackForwardDepartment,
+		{
+			projection: { name: 1, _id: 1 },
+		},
+	);
 	const transferDataFallback = {
 		...transferData,
 		prevDepartment: department.name,
 		departmentId: department.fallbackForwardDepartment,
-		department: await LivechatDepartment.findOneById(department.fallbackForwardDepartment, {
-			fields: { name: 1, _id: 1 },
-		}),
+		department: fallbackDepartmentDb,
 	};
 
 	const forwardSuccess = await forwardRoomToDepartment(room, guest, transferDataFallback);

apps/meteor/ee/app/livechat-enterprise/server/methods/getUnitsFromUserRoles.ts

@@ -1,7 +1,7 @@
 import { Meteor } from 'meteor/meteor';
 import mem from 'mem';
 import type { ServerMethods } from '@rocket.chat/ui-contexts';
-import { LivechatUnit } from '@rocket.chat/models';
+import { LivechatUnit, LivechatDepartmentAgents } from '@rocket.chat/models';
 
 import { hasAnyRoleAsync } from '../../../../../app/authorization/server/functions/hasRole';
 
@@ -17,7 +17,30 @@ async function getUnitsFromUserRoles(user: string | null): Promise<string[] | un
 	return LivechatUnit.findByMonitorId(user);
 }
 
+async function getDepartmentsFromUserRoles(user: string | null): Promise<string[] | undefined> {
+	if (!user || (await hasAnyRoleAsync(user, ['admin', 'livechat-manager']))) {
+		return;
+	}
+
+	if (!(await hasAnyRoleAsync(user, ['livechat-monitor']))) {
+		return;
+	}
+
+	return (await LivechatDepartmentAgents.findByAgentId(user).toArray()).map((department) => department.departmentId);
+}
+
 const memoizedGetUnitFromUserRoles = mem(getUnitsFromUserRoles, { maxAge: 10000 });
+const memoizedGetDepartmentsFromUserRoles = mem(getDepartmentsFromUserRoles, { maxAge: 5000 });
+
+export const getUnitsFromUser = async (user: string | null) => {
+	const units = await memoizedGetUnitFromUserRoles(user);
+	if (!units?.length) {
+		return;
+	}
+
+	const departments = (await memoizedGetDepartmentsFromUserRoles(user)) || [];
+	return [...units, ...departments];
+};
 
 declare module '@rocket.chat/ui-contexts' {
 	// eslint-disable-next-line @typescript-eslint/naming-convention
@@ -27,8 +50,9 @@ declare module '@rocket.chat/ui-contexts' {
 }
 
 Meteor.methods<ServerMethods>({
-	'livechat:getUnitsFromUser'(): Promise<string[] | undefined> {
+	async 'livechat:getUnitsFromUser'(): Promise<string[] | undefined> {
 		const user = Meteor.userId();
-		return memoizedGetUnitFromUserRoles(user);
+
+		return getUnitsFromUser(user);
 	},
 });

apps/meteor/ee/server/models/raw/LivechatRooms.ts

@@ -10,7 +10,6 @@ import type { FindCursor, UpdateResult, Document, FindOptions, Db, Collection, F
 
 import { LivechatRoomsRaw } from '../../../../server/models/raw/LivechatRooms';
 import { queriesLogger } from '../../../app/livechat-enterprise/server/lib/logger';
-import { addQueryRestrictionsToRoomsModel } from '../../../app/livechat-enterprise/server/lib/query.helper';
 
 declare module '@rocket.chat/model-typings' {
 	interface ILivechatRoomsModel {
@@ -264,33 +263,4 @@ export class LivechatRoomsRawEE extends LivechatRoomsRaw implements ILivechatRoo
 		const update = departmentAncestors ? { $set: { departmentAncestors } } : { $unset: { departmentAncestors: 1 } };
 		return this.updateOne(query, update);
 	}
-
-	/** @deprecated Use updateOne or updateMany instead */
-	async update(...args: Parameters<LivechatRoomsRaw['update']>) {
-		const [query, ...restArgs] = args;
-		const restrictedQuery = await addQueryRestrictionsToRoomsModel(query);
-		queriesLogger.debug({ msg: 'LivechatRoomsRawEE.update', query: restrictedQuery });
-		return super.update(restrictedQuery, ...restArgs);
-	}
-
-	async updateOne(...args: [...Parameters<LivechatRoomsRaw['updateOne']>, { bypassUnits?: boolean }?]) {
-		const [query, update, opts, extraOpts] = args;
-		if (extraOpts?.bypassUnits) {
-			// When calling updateOne from a service, we cannot call the meteor code inside the query restrictions
-			// So the solution now is to pass a bypassUnits flag to the updateOne method which prevents checking
-			// units restrictions on the query, but just for the query the service is actually using
-			// We need to find a way of remove the meteor dependency when fetching units, and then, we can remove this flag
-			return super.updateOne(query, update, opts);
-		}
-		const restrictedQuery = await addQueryRestrictionsToRoomsModel(query);
-		queriesLogger.debug({ msg: 'LivechatRoomsRawEE.updateOne', query: restrictedQuery });
-		return super.updateOne(restrictedQuery, update, opts);
-	}
-
-	async updateMany(...args: Parameters<LivechatRoomsRaw['updateMany']>) {
-		const [query, ...restArgs] = args;
-		const restrictedQuery = await addQueryRestrictionsToRoomsModel(query);
-		queriesLogger.debug({ msg: 'LivechatRoomsRawEE.updateMany', query: restrictedQuery });
-		return super.updateMany(restrictedQuery, ...restArgs);
-	}
 }

apps/meteor/ee/server/models/raw/LivechatUnit.ts

@@ -54,16 +54,6 @@ export class LivechatUnitRaw extends BaseRaw<IOmnichannelBusinessUnit> implement
 		return this.col.findOne(query, options);
 	}
 
-	async update(
-		originalQuery: Filter<IOmnichannelBusinessUnit>,
-		update: Filter<IOmnichannelBusinessUnit>,
-		options: FindOptions<IOmnichannelBusinessUnit>,
-	): Promise<UpdateResult> {
-		const query = await addQueryRestrictions(originalQuery);
-		queriesLogger.debug({ msg: 'LivechatUnit.update', query });
-		return this.col.updateOne(query, update, options);
-	}
-
 	remove(query: Filter<IOmnichannelBusinessUnit>): Promise<DeleteResult> {
 		return this.deleteMany(query);
 	}

apps/meteor/lib/callbacks.ts

@@ -136,11 +136,10 @@ type ChainedCallbackSignatures = {
 	'afterDeleteRoom': (rid: IRoom['_id']) => IRoom['_id'];
 	'livechat:afterOnHold': (room: Pick<IOmnichannelRoom, '_id'>) => Pick<IOmnichannelRoom, '_id'>;
 	'livechat:afterOnHoldChatResumed': (room: Pick<IOmnichannelRoom, '_id'>) => Pick<IOmnichannelRoom, '_id'>;
-	'livechat:onTransferFailure': (params: { room: IRoom; guest: ILivechatVisitor; transferData: { [k: string]: string | any } }) => {
-		room: IRoom;
-		guest: ILivechatVisitor;
-		transferData: { [k: string]: string | any };
-	};
+	'livechat:onTransferFailure': (
+		room: IRoom,
+		params: { guest: ILivechatVisitor; transferData: any },
+	) => IOmnichannelRoom | Promise<boolean>;
 	'livechat.afterForwardChatToAgent': (params: {
 		rid: IRoom['_id'];
 		servedBy: { _id: string; ts: Date; username?: string };
@@ -201,15 +200,17 @@ type ChainedCallbackSignatures = {
 	'livechat.chatQueued': (room: IOmnichannelRoom) => IOmnichannelRoom;
 	'livechat.leadCapture': (room: IOmnichannelRoom) => IOmnichannelRoom;
 	'beforeSendMessageNotifications': (message: string) => string;
-	'livechat.onAgentAssignmentFailed': (params: {
-		inquiry: {
-			_id: string;
-			rid: string;
-			status: string;
-		};
-		room: IOmnichannelRoom;
-		options: { forwardingToDepartment?: { oldDepartmentId: string; transferData: any }; clientAction?: boolean };
-	}) => (IOmnichannelRoom & { chatQueued: boolean }) | void;
+	'livechat.onAgentAssignmentFailed': (
+		room: IOmnichannelRoom,
+		params: {
+			inquiry: {
+				_id: string;
+				rid: string;
+				status: string;
+			};
+			options: { forwardingToDepartment?: { oldDepartmentId?: string; transferData?: any }; clientAction?: boolean };
+		},
+	) => Promise<(IOmnichannelRoom & { chatQueued: boolean }) | undefined>;
 	'roomNameChanged': (room: IRoom) => void;
 	'roomTopicChanged': (room: IRoom) => void;
 	'roomAnnouncementChanged': (room: IRoom) => void;

apps/meteor/server/models/raw/LivechatRooms.ts

@@ -1509,11 +1509,6 @@ export class LivechatRoomsRaw extends BaseRaw<IOmnichannelRoom> implements ILive
 			{
 				$set: { pdfTranscriptRequested: true },
 			},
-			{},
-			// @ts-expect-error - extra arg not on base types
-			{
-				bypassUnits: true,
-			},
 		);
 	}
 
@@ -1525,11 +1520,6 @@ export class LivechatRoomsRaw extends BaseRaw<IOmnichannelRoom> implements ILive
 			{
 				$unset: { pdfTranscriptRequested: 1 },
 			},
-			{},
-			// @ts-expect-error - extra arg not on base types
-			{
-				bypassUnits: true,
-			},
 		);
 	}
 
@@ -1541,11 +1531,6 @@ export class LivechatRoomsRaw extends BaseRaw<IOmnichannelRoom> implements ILive
 			{
 				$set: { pdfTranscriptFileId: fileId },
 			},
-			{},
-			// @ts-expect-error - extra arg not on base types
-			{
-				bypassUnits: true,
-			},
 		);
 	}