Skip to content

Commit 9825c8d

Browse files
authored
chore(outbound): Permissions & Validations for endpoints (#36709)
1 parent 1575089 commit 9825c8d

File tree

10 files changed

+286
-17
lines changed

10 files changed

+286
-17
lines changed

apps/meteor/ee/app/livechat-enterprise/server/api/outbound.ts

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,17 @@
1+
import { canSendOutboundMessage } from '@rocket.chat/omni-core-ee';
2+
import {
3+
validateBadRequestErrorResponse,
4+
validateForbiddenErrorResponse,
5+
validateUnauthorizedErrorResponse,
6+
} from '@rocket.chat/rest-typings';
7+
18
import { API } from '../../../../../app/api/server';
29
import {
310
GETOutboundProvidersResponseSchema,
411
GETOutboundProviderParamsSchema,
512
GETOutboundProviderBadRequestErrorSchema,
613
GETOutboundProviderMetadataSchema,
714
POSTOutboundMessageParams,
8-
POSTOutboundMessageErrorSchema,
915
POSTOutboundMessageSuccessSchema,
1016
} from '../outboundcomms/rest';
1117
import { outboundMessageProvider } from './lib/outbound';
@@ -18,8 +24,11 @@ const outboundCommsEndpoints = API.v1
1824
response: {
1925
200: GETOutboundProvidersResponseSchema,
2026
400: GETOutboundProviderBadRequestErrorSchema,
27+
401: validateUnauthorizedErrorResponse,
28+
403: validateForbiddenErrorResponse,
2129
},
2230
query: GETOutboundProviderParamsSchema,
31+
permissionsRequired: ['outbound.send-messages'],
2332
authRequired: true,
2433
license: ['outbound-messaging'],
2534
},
@@ -38,7 +47,10 @@ const outboundCommsEndpoints = API.v1
3847
response: {
3948
200: GETOutboundProviderMetadataSchema,
4049
400: GETOutboundProviderBadRequestErrorSchema,
50+
401: validateUnauthorizedErrorResponse,
51+
403: validateForbiddenErrorResponse,
4152
},
53+
permissionsRequired: ['outbound.send-messages'],
4254
authRequired: true,
4355
license: ['outbound-messaging'],
4456
},
@@ -54,13 +66,22 @@ const outboundCommsEndpoints = API.v1
5466
.post(
5567
'omnichannel/outbound/providers/:id/message',
5668
{
57-
response: { 200: POSTOutboundMessageSuccessSchema, 400: POSTOutboundMessageErrorSchema },
69+
response: {
70+
200: POSTOutboundMessageSuccessSchema,
71+
400: validateBadRequestErrorResponse,
72+
401: validateUnauthorizedErrorResponse,
73+
403: validateForbiddenErrorResponse,
74+
},
5875
authRequired: true,
76+
permissionsRequired: ['outbound.send-messages'],
5977
body: POSTOutboundMessageParams,
6078
license: ['outbound-messaging'],
6179
},
6280
async function action() {
6381
const { id } = this.urlParams;
82+
const { departmentId, agentId } = this.bodyParams;
83+
84+
await canSendOutboundMessage(this.userId, agentId, departmentId);
6485

6586
await outboundMessageProvider.sendMessage(id, this.bodyParams);
6687
return API.v1.success();

apps/meteor/ee/app/livechat-enterprise/server/outboundcomms/rest.ts

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -178,33 +178,22 @@ const POSTOutboundMessageSchema = {
178178
},
179179
additionalProperties: false,
180180
},
181+
agentId: { type: 'string' },
182+
departmentId: { type: 'string' },
181183
},
182184
additionalProperties: false,
183185
};
184186

185187
export const POSTOutboundMessageParams = ajv.compile<POSTOutboundMessageParamsType>(POSTOutboundMessageSchema);
186188

187-
const POSTOutboundMessageError = {
189+
const POSTOutboundMessageSuccess = {
188190
type: 'object',
189191
properties: {
190-
success: {
191-
type: 'boolean',
192-
},
193-
message: {
194-
type: 'string',
195-
},
192+
success: { type: 'boolean', enum: [true] },
196193
},
197194
additionalProperties: false,
198195
};
199196

200-
export const POSTOutboundMessageErrorSchema = ajv.compile<GenericErrorResponse>(POSTOutboundMessageError);
201-
202-
const POSTOutboundMessageSuccess = {
203-
type: 'object',
204-
properties: {},
205-
additionalProperties: false,
206-
};
207-
208197
export const POSTOutboundMessageSuccessSchema = ajv.compile<void>(POSTOutboundMessageSuccess);
209198

210199
const OutboundProviderMetadataSchema = {

apps/meteor/ee/app/livechat-enterprise/server/permissions.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ export const omnichannelEEPermissions = [
2020
{ _id: 'view-livechat-reports', roles: [adminRole, livechatManagerRole, livechatMonitorRole] },
2121
{ _id: 'block-livechat-contact', roles: [adminRole, livechatManagerRole, livechatMonitorRole, livechatAgentRole] },
2222
{ _id: 'unblock-livechat-contact', roles: [adminRole, livechatManagerRole, livechatMonitorRole, livechatAgentRole] },
23+
{ _id: 'outbound.send-messages', roles: [adminRole, livechatManagerRole, livechatMonitorRole, livechatAgentRole] },
24+
{ _id: 'outbound.can-assign-queues', roles: [adminRole, livechatManagerRole] },
25+
{ _id: 'outbound.can-assign-any-agent', roles: [adminRole, livechatManagerRole, livechatMonitorRole] },
26+
{ _id: 'outbound.can-assign-self-only', roles: [livechatAgentRole] },
2327
];
2428

2529
export const createPermissions = async (): Promise<void> => {

ee/packages/omni-core-ee/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,5 @@ export function patchOmniCore(): void {
66
applyDepartmentRestrictionsPatch();
77
}
88

9+
export * from './outbound-communication';
910
export * from './units/getUnitsFromUser';
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export * from './validators/canSendMessage';
Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
1+
import { Authorization } from '@rocket.chat/core-services';
2+
import { LivechatDepartment, LivechatDepartmentAgents, Users } from '@rocket.chat/models';
3+
4+
import { canSendOutboundMessage, validateAgentAssignPermissions } from './canSendMessage';
5+
6+
jest.mock('@rocket.chat/core-services', () => ({
7+
Authorization: {
8+
hasPermission: jest.fn(),
9+
},
10+
}));
11+
12+
jest.mock('@rocket.chat/models', () => ({
13+
LivechatDepartment: {
14+
findOne: jest.fn(),
15+
},
16+
LivechatDepartmentAgents: {
17+
findOneByAgentIdAndDepartmentId: jest.fn(),
18+
},
19+
Users: {
20+
findOneAgentById: jest.fn(),
21+
},
22+
}));
23+
24+
const hasPermissionMock = Authorization.hasPermission as unknown as jest.Mock<
25+
Promise<boolean>,
26+
[userId: string, permission: string, scope?: string]
27+
>;
28+
29+
const findDepartmentMock = LivechatDepartment.findOne as unknown as jest.Mock;
30+
const findDepartmentAgentMock = LivechatDepartmentAgents.findOneByAgentIdAndDepartmentId as unknown as jest.Mock;
31+
const findUserAgentMock = Users.findOneAgentById as unknown as jest.Mock;
32+
33+
const setPermissions = (map: Record<string, boolean>) => {
34+
hasPermissionMock.mockImplementation(async (_userId: string, permission: string) => {
35+
return Boolean(map[permission]);
36+
});
37+
};
38+
39+
describe('canSendOutboundMessage', () => {
40+
beforeEach(() => {
41+
jest.clearAllMocks();
42+
// Default: no permissions granted
43+
setPermissions({});
44+
});
45+
46+
describe('when departmentId is provided', () => {
47+
test('resolves when user has assign-queues permission and department is enabled', async () => {
48+
setPermissions({ 'outbound.can-assign-queues': true });
49+
findDepartmentMock.mockResolvedValue({ _id: 'dep1', enabled: true });
50+
51+
await expect(canSendOutboundMessage('u1', undefined, 'dep1')).resolves.toBeUndefined();
52+
53+
// Should not need to check requester department membership
54+
expect(findDepartmentAgentMock).not.toHaveBeenCalled();
55+
});
56+
57+
test('throws error-invalid-department when department is disabled', async () => {
58+
setPermissions({ 'outbound.can-assign-queues': true });
59+
findDepartmentMock.mockResolvedValue({ _id: 'dep1', enabled: false });
60+
61+
await expect(canSendOutboundMessage('u1', undefined, 'dep1')).rejects.toThrow('error-invalid-department');
62+
});
63+
64+
describe('and agentId is provided', () => {
65+
test('throws error-invalid-agent if user lacks any-agent and has self-only but agentId != userId', async () => {
66+
setPermissions({
67+
'outbound.can-assign-queues': true, // so it skips requester membership
68+
'outbound.can-assign-any-agent': false,
69+
'outbound.can-assign-self-only': true,
70+
});
71+
findDepartmentMock.mockResolvedValue({ _id: 'dep1', enabled: true });
72+
73+
await expect(canSendOutboundMessage('me', 'other', 'dep1')).rejects.toThrow('error-invalid-agent');
74+
75+
// Should not check selected agent membership after permission denial
76+
expect(findDepartmentAgentMock).not.toHaveBeenCalled();
77+
});
78+
79+
test('throws error-agent-not-in-department if selected agent is not in the department (any-agent allowed)', async () => {
80+
setPermissions({
81+
'outbound.can-assign-queues': true,
82+
'outbound.can-assign-any-agent': true,
83+
});
84+
findDepartmentMock.mockResolvedValue({ _id: 'dep1', enabled: true });
85+
findDepartmentAgentMock.mockResolvedValueOnce(null); // selected agent not in department
86+
87+
await expect(canSendOutboundMessage('me', 'agentX', 'dep1')).rejects.toThrow('error-agent-not-in-department');
88+
89+
expect(findDepartmentAgentMock).toHaveBeenCalledWith('agentX', 'dep1', { projection: { _id: 1 } });
90+
});
91+
92+
test('resolves if selected agent is in the department (any-agent allowed)', async () => {
93+
setPermissions({
94+
'outbound.can-assign-queues': true,
95+
'outbound.can-assign-any-agent': true,
96+
});
97+
findDepartmentMock.mockResolvedValue({ _id: 'dep1', enabled: true });
98+
findDepartmentAgentMock.mockResolvedValueOnce({ _id: 'relX' });
99+
100+
await expect(canSendOutboundMessage('me', 'agentInDep', 'dep1')).resolves.toBeUndefined();
101+
});
102+
103+
test('throws error-agent-not-in-department when self-only is allowed and agentId == userId but agent is not in department', async () => {
104+
setPermissions({
105+
'outbound.can-assign-queues': true,
106+
'outbound.can-assign-any-agent': false,
107+
'outbound.can-assign-self-only': true,
108+
});
109+
findDepartmentMock.mockResolvedValue({ _id: 'dep1', enabled: true });
110+
findDepartmentAgentMock.mockResolvedValueOnce(null); // self not in department
111+
112+
await expect(canSendOutboundMessage('me', 'me', 'dep1')).rejects.toThrow('error-agent-not-in-department');
113+
});
114+
});
115+
});
116+
117+
describe('when departmentId is not provided and agentId is provided', () => {
118+
test('throws error-invalid-agent if user lacks any-agent and has self-only but agentId != userId', async () => {
119+
setPermissions({
120+
'outbound.can-assign-any-agent': false,
121+
'outbound.can-assign-self-only': true,
122+
});
123+
124+
await expect(canSendOutboundMessage('me', 'other')).rejects.toThrow('error-invalid-agent');
125+
126+
// Should not query Users if permission check fails
127+
expect(findUserAgentMock).not.toHaveBeenCalled();
128+
});
129+
130+
test('throws error-invalid-agent if selected agent is not an agent (any-agent allowed)', async () => {
131+
setPermissions({
132+
'outbound.can-assign-any-agent': true,
133+
});
134+
findUserAgentMock.mockResolvedValueOnce(null);
135+
136+
await expect(canSendOutboundMessage('me', 'notAnAgent')).rejects.toThrow('error-invalid-agent');
137+
138+
expect(findUserAgentMock).toHaveBeenCalledWith('notAnAgent', { projection: { _id: 1 } });
139+
});
140+
141+
test('resolves if selected agent exists (any-agent allowed)', async () => {
142+
setPermissions({
143+
'outbound.can-assign-any-agent': true,
144+
});
145+
findUserAgentMock.mockResolvedValueOnce({ _id: 'agent1' });
146+
147+
await expect(canSendOutboundMessage('me', 'agent1')).resolves.toBeUndefined();
148+
});
149+
});
150+
151+
test('resolves when neither departmentId nor agentId are provided', async () => {
152+
await expect(canSendOutboundMessage('u1')).resolves.toBeUndefined();
153+
154+
expect(hasPermissionMock).not.toHaveBeenCalled();
155+
expect(findDepartmentMock).not.toHaveBeenCalled();
156+
expect(findDepartmentAgentMock).not.toHaveBeenCalled();
157+
expect(findUserAgentMock).not.toHaveBeenCalled();
158+
});
159+
});
160+
161+
describe('validateAgentAssignPermissions', () => {
162+
beforeEach(() => {
163+
jest.clearAllMocks();
164+
setPermissions({});
165+
});
166+
167+
test('resolves when user has any-agent permission', async () => {
168+
setPermissions({ 'outbound.can-assign-any-agent': true });
169+
170+
await expect(validateAgentAssignPermissions('u1', 'other')).resolves.toBeUndefined();
171+
});
172+
173+
test('resolves when user has self-only and agentId equals userId', async () => {
174+
setPermissions({ 'outbound.can-assign-self-only': true });
175+
176+
await expect(validateAgentAssignPermissions('me', 'me')).resolves.toBeUndefined();
177+
});
178+
179+
test('throws error-invalid-agent when self-only and agentId != userId', async () => {
180+
setPermissions({ 'outbound.can-assign-self-only': true });
181+
182+
await expect(validateAgentAssignPermissions('me', 'other')).rejects.toThrow('error-invalid-agent');
183+
});
184+
185+
test('throws error-invalid-agent when user lacks both permissions', async () => {
186+
setPermissions({});
187+
188+
await expect(validateAgentAssignPermissions('me', 'other')).rejects.toThrow('error-invalid-agent');
189+
});
190+
});
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
import { Authorization } from '@rocket.chat/core-services';
2+
import type { ILivechatDepartment } from '@rocket.chat/core-typings';
3+
import { LivechatDepartment, LivechatDepartmentAgents, Users } from '@rocket.chat/models';
4+
import type { FilterOperators } from 'mongodb';
5+
6+
import { addQueryRestrictionsToDepartmentsModel } from '../../units/addRoleBasedRestrictionsToDepartment';
7+
8+
export async function validateAgentAssignPermissions(userId: string, agentId: string): Promise<void> {
9+
if (await Authorization.hasPermission(userId, 'outbound.can-assign-any-agent')) {
10+
return;
11+
}
12+
13+
if ((await Authorization.hasPermission(userId, 'outbound.can-assign-self-only')) && agentId === userId) {
14+
return;
15+
}
16+
17+
throw new Error('error-invalid-agent');
18+
}
19+
20+
export async function canSendOutboundMessage(userId: string, agentId?: string, departmentId?: string): Promise<void> {
21+
// Case 1: Check department and check if agent is in department
22+
if (departmentId) {
23+
let query: FilterOperators<ILivechatDepartment> = { _id: departmentId };
24+
if (!(await Authorization.hasPermission(userId, 'outbound.can-assign-queues'))) {
25+
query = await addQueryRestrictionsToDepartmentsModel(query, userId);
26+
}
27+
28+
const department = await LivechatDepartment.findOne<Pick<ILivechatDepartment, '_id' | 'enabled'>>(query, { _id: 1, enabled: 1 });
29+
if (!department?.enabled) {
30+
throw new Error('error-invalid-department');
31+
}
32+
33+
// Case 2: Agent & department: if agent is present, agent must be in department
34+
if (agentId) {
35+
await validateAgentAssignPermissions(userId, agentId);
36+
// 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 :) )
37+
const agent = await LivechatDepartmentAgents.findOneByAgentIdAndDepartmentId(agentId, departmentId, { projection: { _id: 1 } });
38+
if (!agent) {
39+
throw new Error('error-agent-not-in-department');
40+
}
41+
}
42+
// Case 3: Agent & no department: if agent is present and there's no department, agent must be an agent
43+
} else if (agentId) {
44+
await validateAgentAssignPermissions(userId, agentId);
45+
46+
const agent = await Users.findOneAgentById(agentId, { projection: { _id: 1 } });
47+
if (!agent) {
48+
throw new Error('error-invalid-agent');
49+
}
50+
}
51+
}

packages/apps-engine/src/definition/outboundComunication/IOutboundMessage.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ export interface IOutboundMessage {
22
to: string;
33
type: 'template';
44
templateProviderPhoneNumber: string;
5+
agentId?: string;
6+
departmentId?: string;
57
template: {
68
name: string;
79
language: {

packages/core-typings/src/omnichannel/outbound.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ export interface IOutboundMessage {
6161
to: string;
6262
type: 'template';
6363
templateProviderPhoneNumber: string;
64+
departmentId?: string;
65+
agentId?: string;
6466
template: {
6567
name: string;
6668
language: {

packages/i18n/src/locales/en.i18n.json

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6526,6 +6526,14 @@
65266526
"others": "others",
65276527
"outbound-voip-calls": "Outbound Voip Calls",
65286528
"outbound-voip-calls_description": "Permission to outbound voip calls",
6529+
"outbound.send-messages": "Send outbound messages",
6530+
"outbound.send-messages_description": "Permission to send outbound messages",
6531+
"outbound.can-assign-queues": "Can assign departments to receive outbound messages responses",
6532+
"outbound.can-assign-queues_description": "Permission to assign departments to receive outbound messages responses",
6533+
"outbound.can-assign-any-agent": "Can assign any agent to receive outbound messages responses",
6534+
"outbound.can-assign-any-agent_description": "Permission to assign any agent to receive outbound messages responses",
6535+
"outbound.can-assign-self-only": "Can assign self only to receive outbound messages responses",
6536+
"outbound.can-assign-self-only_description": "Permission to assign self only to receive outbound messages responses",
65296537
"pdf_error_message": "Error generating PDF Transcript",
65306538
"pdf_success_message": "PDF Transcript successfully generated",
65316539
"pending": "pending",

0 commit comments

Comments
 (0)