From 9825c8d5180455a0791fde83d7db2c149341ab9d Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Mon, 1 Sep 2025 11:17:17 -0600 Subject: [PATCH] chore(outbound): Permissions & Validations for endpoints (#36709) --- .../server/api/outbound.ts | 25 ++- .../server/outboundcomms/rest.ts | 19 +- .../livechat-enterprise/server/permissions.ts | 4 + ee/packages/omni-core-ee/src/index.ts | 1 + .../src/outbound-communication/index.ts | 1 + .../validators/canSendMessage.spec.ts | 190 ++++++++++++++++++ .../validators/canSendMessage.ts | 51 +++++ .../outboundComunication/IOutboundMessage.ts | 2 + .../core-typings/src/omnichannel/outbound.ts | 2 + packages/i18n/src/locales/en.i18n.json | 8 + 10 files changed, 286 insertions(+), 17 deletions(-) create mode 100644 ee/packages/omni-core-ee/src/outbound-communication/index.ts create mode 100644 ee/packages/omni-core-ee/src/outbound-communication/validators/canSendMessage.spec.ts create mode 100644 ee/packages/omni-core-ee/src/outbound-communication/validators/canSendMessage.ts diff --git a/apps/meteor/ee/app/livechat-enterprise/server/api/outbound.ts b/apps/meteor/ee/app/livechat-enterprise/server/api/outbound.ts index 296d4f06d15..eccdf2ac2fd 100644 --- a/apps/meteor/ee/app/livechat-enterprise/server/api/outbound.ts +++ b/apps/meteor/ee/app/livechat-enterprise/server/api/outbound.ts @@ -1,3 +1,10 @@ +import { canSendOutboundMessage } from '@rocket.chat/omni-core-ee'; +import { + validateBadRequestErrorResponse, + validateForbiddenErrorResponse, + validateUnauthorizedErrorResponse, +} from '@rocket.chat/rest-typings'; + import { API } from '../../../../../app/api/server'; import { GETOutboundProvidersResponseSchema, @@ -5,7 +12,6 @@ import { GETOutboundProviderBadRequestErrorSchema, GETOutboundProviderMetadataSchema, POSTOutboundMessageParams, - POSTOutboundMessageErrorSchema, POSTOutboundMessageSuccessSchema, } from '../outboundcomms/rest'; import { outboundMessageProvider } from './lib/outbound'; @@ -18,8 +24,11 @@ const outboundCommsEndpoints = API.v1 response: { 200: GETOutboundProvidersResponseSchema, 400: GETOutboundProviderBadRequestErrorSchema, + 401: validateUnauthorizedErrorResponse, + 403: validateForbiddenErrorResponse, }, query: GETOutboundProviderParamsSchema, + permissionsRequired: ['outbound.send-messages'], authRequired: true, license: ['outbound-messaging'], }, @@ -38,7 +47,10 @@ const outboundCommsEndpoints = API.v1 response: { 200: GETOutboundProviderMetadataSchema, 400: GETOutboundProviderBadRequestErrorSchema, + 401: validateUnauthorizedErrorResponse, + 403: validateForbiddenErrorResponse, }, + permissionsRequired: ['outbound.send-messages'], authRequired: true, license: ['outbound-messaging'], }, @@ -54,13 +66,22 @@ const outboundCommsEndpoints = API.v1 .post( 'omnichannel/outbound/providers/:id/message', { - response: { 200: POSTOutboundMessageSuccessSchema, 400: POSTOutboundMessageErrorSchema }, + response: { + 200: POSTOutboundMessageSuccessSchema, + 400: validateBadRequestErrorResponse, + 401: validateUnauthorizedErrorResponse, + 403: validateForbiddenErrorResponse, + }, authRequired: true, + permissionsRequired: ['outbound.send-messages'], body: POSTOutboundMessageParams, license: ['outbound-messaging'], }, async function action() { const { id } = this.urlParams; + const { departmentId, agentId } = this.bodyParams; + + await canSendOutboundMessage(this.userId, agentId, departmentId); await outboundMessageProvider.sendMessage(id, this.bodyParams); return API.v1.success(); diff --git a/apps/meteor/ee/app/livechat-enterprise/server/outboundcomms/rest.ts b/apps/meteor/ee/app/livechat-enterprise/server/outboundcomms/rest.ts index 9d1d8a053c9..1b40a6584ee 100644 --- a/apps/meteor/ee/app/livechat-enterprise/server/outboundcomms/rest.ts +++ b/apps/meteor/ee/app/livechat-enterprise/server/outboundcomms/rest.ts @@ -178,33 +178,22 @@ const POSTOutboundMessageSchema = { }, additionalProperties: false, }, + agentId: { type: 'string' }, + departmentId: { type: 'string' }, }, additionalProperties: false, }; export const POSTOutboundMessageParams = ajv.compile(POSTOutboundMessageSchema); -const POSTOutboundMessageError = { +const POSTOutboundMessageSuccess = { type: 'object', properties: { - success: { - type: 'boolean', - }, - message: { - type: 'string', - }, + success: { type: 'boolean', enum: [true] }, }, additionalProperties: false, }; -export const POSTOutboundMessageErrorSchema = ajv.compile(POSTOutboundMessageError); - -const POSTOutboundMessageSuccess = { - type: 'object', - properties: {}, - additionalProperties: false, -}; - export const POSTOutboundMessageSuccessSchema = ajv.compile(POSTOutboundMessageSuccess); const OutboundProviderMetadataSchema = { diff --git a/apps/meteor/ee/app/livechat-enterprise/server/permissions.ts b/apps/meteor/ee/app/livechat-enterprise/server/permissions.ts index 03e43c07ced..c37ab01cdfc 100644 --- a/apps/meteor/ee/app/livechat-enterprise/server/permissions.ts +++ b/apps/meteor/ee/app/livechat-enterprise/server/permissions.ts @@ -20,6 +20,10 @@ export const omnichannelEEPermissions = [ { _id: 'view-livechat-reports', roles: [adminRole, livechatManagerRole, livechatMonitorRole] }, { _id: 'block-livechat-contact', roles: [adminRole, livechatManagerRole, livechatMonitorRole, livechatAgentRole] }, { _id: 'unblock-livechat-contact', roles: [adminRole, livechatManagerRole, livechatMonitorRole, livechatAgentRole] }, + { _id: 'outbound.send-messages', roles: [adminRole, livechatManagerRole, livechatMonitorRole, livechatAgentRole] }, + { _id: 'outbound.can-assign-queues', roles: [adminRole, livechatManagerRole] }, + { _id: 'outbound.can-assign-any-agent', roles: [adminRole, livechatManagerRole, livechatMonitorRole] }, + { _id: 'outbound.can-assign-self-only', roles: [livechatAgentRole] }, ]; export const createPermissions = async (): Promise => { diff --git a/ee/packages/omni-core-ee/src/index.ts b/ee/packages/omni-core-ee/src/index.ts index 34ebbaefcc6..d4f79be0be9 100644 --- a/ee/packages/omni-core-ee/src/index.ts +++ b/ee/packages/omni-core-ee/src/index.ts @@ -6,4 +6,5 @@ export function patchOmniCore(): void { applyDepartmentRestrictionsPatch(); } +export * from './outbound-communication'; export * from './units/getUnitsFromUser'; diff --git a/ee/packages/omni-core-ee/src/outbound-communication/index.ts b/ee/packages/omni-core-ee/src/outbound-communication/index.ts new file mode 100644 index 00000000000..fd24c26a5d1 --- /dev/null +++ b/ee/packages/omni-core-ee/src/outbound-communication/index.ts @@ -0,0 +1 @@ +export * from './validators/canSendMessage'; diff --git a/ee/packages/omni-core-ee/src/outbound-communication/validators/canSendMessage.spec.ts b/ee/packages/omni-core-ee/src/outbound-communication/validators/canSendMessage.spec.ts new file mode 100644 index 00000000000..ed71b1db4a7 --- /dev/null +++ b/ee/packages/omni-core-ee/src/outbound-communication/validators/canSendMessage.spec.ts @@ -0,0 +1,190 @@ +import { Authorization } from '@rocket.chat/core-services'; +import { LivechatDepartment, LivechatDepartmentAgents, Users } from '@rocket.chat/models'; + +import { canSendOutboundMessage, validateAgentAssignPermissions } from './canSendMessage'; + +jest.mock('@rocket.chat/core-services', () => ({ + Authorization: { + hasPermission: jest.fn(), + }, +})); + +jest.mock('@rocket.chat/models', () => ({ + LivechatDepartment: { + findOne: jest.fn(), + }, + LivechatDepartmentAgents: { + findOneByAgentIdAndDepartmentId: jest.fn(), + }, + Users: { + findOneAgentById: jest.fn(), + }, +})); + +const hasPermissionMock = Authorization.hasPermission as unknown as jest.Mock< + Promise, + [userId: string, permission: string, scope?: string] +>; + +const findDepartmentMock = LivechatDepartment.findOne as unknown as jest.Mock; +const findDepartmentAgentMock = LivechatDepartmentAgents.findOneByAgentIdAndDepartmentId as unknown as jest.Mock; +const findUserAgentMock = Users.findOneAgentById as unknown as jest.Mock; + +const setPermissions = (map: Record) => { + hasPermissionMock.mockImplementation(async (_userId: string, permission: string) => { + return Boolean(map[permission]); + }); +}; + +describe('canSendOutboundMessage', () => { + beforeEach(() => { + jest.clearAllMocks(); + // Default: no permissions granted + setPermissions({}); + }); + + describe('when departmentId is provided', () => { + test('resolves when user has assign-queues permission and department is enabled', async () => { + setPermissions({ 'outbound.can-assign-queues': true }); + findDepartmentMock.mockResolvedValue({ _id: 'dep1', enabled: true }); + + await expect(canSendOutboundMessage('u1', undefined, 'dep1')).resolves.toBeUndefined(); + + // Should not need to check requester department membership + expect(findDepartmentAgentMock).not.toHaveBeenCalled(); + }); + + test('throws error-invalid-department when department is disabled', async () => { + setPermissions({ 'outbound.can-assign-queues': true }); + findDepartmentMock.mockResolvedValue({ _id: 'dep1', enabled: false }); + + await expect(canSendOutboundMessage('u1', undefined, 'dep1')).rejects.toThrow('error-invalid-department'); + }); + + describe('and agentId is provided', () => { + test('throws error-invalid-agent if user lacks any-agent and has self-only but agentId != userId', async () => { + setPermissions({ + 'outbound.can-assign-queues': true, // so it skips requester membership + 'outbound.can-assign-any-agent': false, + 'outbound.can-assign-self-only': true, + }); + findDepartmentMock.mockResolvedValue({ _id: 'dep1', enabled: true }); + + await expect(canSendOutboundMessage('me', 'other', 'dep1')).rejects.toThrow('error-invalid-agent'); + + // Should not check selected agent membership after permission denial + expect(findDepartmentAgentMock).not.toHaveBeenCalled(); + }); + + test('throws error-agent-not-in-department if selected agent is not in the department (any-agent allowed)', async () => { + setPermissions({ + 'outbound.can-assign-queues': true, + 'outbound.can-assign-any-agent': true, + }); + findDepartmentMock.mockResolvedValue({ _id: 'dep1', enabled: true }); + findDepartmentAgentMock.mockResolvedValueOnce(null); // selected agent not in department + + await expect(canSendOutboundMessage('me', 'agentX', 'dep1')).rejects.toThrow('error-agent-not-in-department'); + + expect(findDepartmentAgentMock).toHaveBeenCalledWith('agentX', 'dep1', { projection: { _id: 1 } }); + }); + + test('resolves if selected agent is in the department (any-agent allowed)', async () => { + setPermissions({ + 'outbound.can-assign-queues': true, + 'outbound.can-assign-any-agent': true, + }); + findDepartmentMock.mockResolvedValue({ _id: 'dep1', enabled: true }); + findDepartmentAgentMock.mockResolvedValueOnce({ _id: 'relX' }); + + await expect(canSendOutboundMessage('me', 'agentInDep', 'dep1')).resolves.toBeUndefined(); + }); + + test('throws error-agent-not-in-department when self-only is allowed and agentId == userId but agent is not in department', async () => { + setPermissions({ + 'outbound.can-assign-queues': true, + 'outbound.can-assign-any-agent': false, + 'outbound.can-assign-self-only': true, + }); + findDepartmentMock.mockResolvedValue({ _id: 'dep1', enabled: true }); + findDepartmentAgentMock.mockResolvedValueOnce(null); // self not in department + + await expect(canSendOutboundMessage('me', 'me', 'dep1')).rejects.toThrow('error-agent-not-in-department'); + }); + }); + }); + + describe('when departmentId is not provided and agentId is provided', () => { + test('throws error-invalid-agent if user lacks any-agent and has self-only but agentId != userId', async () => { + setPermissions({ + 'outbound.can-assign-any-agent': false, + 'outbound.can-assign-self-only': true, + }); + + await expect(canSendOutboundMessage('me', 'other')).rejects.toThrow('error-invalid-agent'); + + // Should not query Users if permission check fails + expect(findUserAgentMock).not.toHaveBeenCalled(); + }); + + test('throws error-invalid-agent if selected agent is not an agent (any-agent allowed)', async () => { + setPermissions({ + 'outbound.can-assign-any-agent': true, + }); + findUserAgentMock.mockResolvedValueOnce(null); + + await expect(canSendOutboundMessage('me', 'notAnAgent')).rejects.toThrow('error-invalid-agent'); + + expect(findUserAgentMock).toHaveBeenCalledWith('notAnAgent', { projection: { _id: 1 } }); + }); + + test('resolves if selected agent exists (any-agent allowed)', async () => { + setPermissions({ + 'outbound.can-assign-any-agent': true, + }); + findUserAgentMock.mockResolvedValueOnce({ _id: 'agent1' }); + + await expect(canSendOutboundMessage('me', 'agent1')).resolves.toBeUndefined(); + }); + }); + + test('resolves when neither departmentId nor agentId are provided', async () => { + await expect(canSendOutboundMessage('u1')).resolves.toBeUndefined(); + + expect(hasPermissionMock).not.toHaveBeenCalled(); + expect(findDepartmentMock).not.toHaveBeenCalled(); + expect(findDepartmentAgentMock).not.toHaveBeenCalled(); + expect(findUserAgentMock).not.toHaveBeenCalled(); + }); +}); + +describe('validateAgentAssignPermissions', () => { + beforeEach(() => { + jest.clearAllMocks(); + setPermissions({}); + }); + + test('resolves when user has any-agent permission', async () => { + setPermissions({ 'outbound.can-assign-any-agent': true }); + + await expect(validateAgentAssignPermissions('u1', 'other')).resolves.toBeUndefined(); + }); + + test('resolves when user has self-only and agentId equals userId', async () => { + setPermissions({ 'outbound.can-assign-self-only': true }); + + await expect(validateAgentAssignPermissions('me', 'me')).resolves.toBeUndefined(); + }); + + test('throws error-invalid-agent when self-only and agentId != userId', async () => { + setPermissions({ 'outbound.can-assign-self-only': true }); + + await expect(validateAgentAssignPermissions('me', 'other')).rejects.toThrow('error-invalid-agent'); + }); + + test('throws error-invalid-agent when user lacks both permissions', async () => { + setPermissions({}); + + await expect(validateAgentAssignPermissions('me', 'other')).rejects.toThrow('error-invalid-agent'); + }); +}); diff --git a/ee/packages/omni-core-ee/src/outbound-communication/validators/canSendMessage.ts b/ee/packages/omni-core-ee/src/outbound-communication/validators/canSendMessage.ts new file mode 100644 index 00000000000..e8cdcfd9489 --- /dev/null +++ b/ee/packages/omni-core-ee/src/outbound-communication/validators/canSendMessage.ts @@ -0,0 +1,51 @@ +import { Authorization } from '@rocket.chat/core-services'; +import type { ILivechatDepartment } from '@rocket.chat/core-typings'; +import { LivechatDepartment, LivechatDepartmentAgents, Users } from '@rocket.chat/models'; +import type { FilterOperators } from 'mongodb'; + +import { addQueryRestrictionsToDepartmentsModel } from '../../units/addRoleBasedRestrictionsToDepartment'; + +export async function validateAgentAssignPermissions(userId: string, agentId: string): Promise { + if (await Authorization.hasPermission(userId, 'outbound.can-assign-any-agent')) { + return; + } + + if ((await Authorization.hasPermission(userId, 'outbound.can-assign-self-only')) && agentId === userId) { + return; + } + + throw new Error('error-invalid-agent'); +} + +export async function canSendOutboundMessage(userId: string, agentId?: string, departmentId?: string): Promise { + // Case 1: Check department and check if agent is in department + if (departmentId) { + let query: FilterOperators = { _id: departmentId }; + if (!(await Authorization.hasPermission(userId, 'outbound.can-assign-queues'))) { + query = await addQueryRestrictionsToDepartmentsModel(query, userId); + } + + const department = await LivechatDepartment.findOne>(query, { _id: 1, enabled: 1 }); + if (!department?.enabled) { + throw new Error('error-invalid-department'); + } + + // Case 2: Agent & department: if agent is present, agent must be in department + if (agentId) { + await validateAgentAssignPermissions(userId, agentId); + // On here, we take a shortcut: if the user is here, we assume it's an agent (and we assume the collection is kept up to date :) ) + const agent = await LivechatDepartmentAgents.findOneByAgentIdAndDepartmentId(agentId, departmentId, { projection: { _id: 1 } }); + if (!agent) { + throw new Error('error-agent-not-in-department'); + } + } + // Case 3: Agent & no department: if agent is present and there's no department, agent must be an agent + } else if (agentId) { + await validateAgentAssignPermissions(userId, agentId); + + const agent = await Users.findOneAgentById(agentId, { projection: { _id: 1 } }); + if (!agent) { + throw new Error('error-invalid-agent'); + } + } +} diff --git a/packages/apps-engine/src/definition/outboundComunication/IOutboundMessage.ts b/packages/apps-engine/src/definition/outboundComunication/IOutboundMessage.ts index 9f8f7e3efec..86b0a1fc7f0 100644 --- a/packages/apps-engine/src/definition/outboundComunication/IOutboundMessage.ts +++ b/packages/apps-engine/src/definition/outboundComunication/IOutboundMessage.ts @@ -2,6 +2,8 @@ export interface IOutboundMessage { to: string; type: 'template'; templateProviderPhoneNumber: string; + agentId?: string; + departmentId?: string; template: { name: string; language: { diff --git a/packages/core-typings/src/omnichannel/outbound.ts b/packages/core-typings/src/omnichannel/outbound.ts index 7645ab29e3f..ad303e43d22 100644 --- a/packages/core-typings/src/omnichannel/outbound.ts +++ b/packages/core-typings/src/omnichannel/outbound.ts @@ -61,6 +61,8 @@ export interface IOutboundMessage { to: string; type: 'template'; templateProviderPhoneNumber: string; + departmentId?: string; + agentId?: string; template: { name: string; language: { diff --git a/packages/i18n/src/locales/en.i18n.json b/packages/i18n/src/locales/en.i18n.json index a4ddf35a051..8295458f0e3 100644 --- a/packages/i18n/src/locales/en.i18n.json +++ b/packages/i18n/src/locales/en.i18n.json @@ -6526,6 +6526,14 @@ "others": "others", "outbound-voip-calls": "Outbound Voip Calls", "outbound-voip-calls_description": "Permission to outbound voip calls", + "outbound.send-messages": "Send outbound messages", + "outbound.send-messages_description": "Permission to send outbound messages", + "outbound.can-assign-queues": "Can assign departments to receive outbound messages responses", + "outbound.can-assign-queues_description": "Permission to assign departments to receive outbound messages responses", + "outbound.can-assign-any-agent": "Can assign any agent to receive outbound messages responses", + "outbound.can-assign-any-agent_description": "Permission to assign any agent to receive outbound messages responses", + "outbound.can-assign-self-only": "Can assign self only to receive outbound messages responses", + "outbound.can-assign-self-only_description": "Permission to assign self only to receive outbound messages responses", "pdf_error_message": "Error generating PDF Transcript", "pdf_success_message": "PDF Transcript successfully generated", "pending": "pending",