[IMPROVE] validateParams to accept different validators per request m…

…ethod (#26357)

Co-authored-by: Murtaza Patrawala <[email protected]>
Co-authored-by: Guilherme Gazzo <[email protected]>

apps/meteor/app/api/server/api.d.ts

@@ -68,7 +68,7 @@ type Options = (
 			twoFactorOptions?: ITwoFactorOptions;
 	  }
 ) & {
-	validateParams?: ValidateFunction;
+	validateParams?: ValidateFunction | { [key in Method]?: ValidateFunction };
 	authOrAnonRequired?: true;
 };
 
@@ -93,6 +93,8 @@ type ActionThis<TMethod extends Method, TPathPattern extends PathPattern, TOptio
 	// TODO make it unsafe
 	readonly queryParams: TMethod extends 'GET'
 		? TOptions extends { validateParams: ValidateFunction<infer T> }
+			? T
+			: TOptions extends { validateParams: { GET: ValidateFunction<infer T> } }
 			? T
 			: Partial<OperationParams<TMethod, TPathPattern>>
 		: Record<string, string>;
@@ -101,6 +103,10 @@ type ActionThis<TMethod extends Method, TPathPattern extends PathPattern, TOptio
 		? Record<string, unknown>
 		: TOptions extends { validateParams: ValidateFunction<infer T> }
 		? T
+		: TOptions extends { validateParams: infer V }
+		? V extends { [key in TMethod]: ValidateFunction<infer T> }
+			? T
+			: Partial<OperationParams<TMethod, TPathPattern>>
 		: Partial<OperationParams<TMethod, TPathPattern>>;
 	readonly request: Request;
 

apps/meteor/app/api/server/api.js

@@ -425,9 +425,16 @@ export class APIClass extends Restivus {
 					try {
 						api.enforceRateLimit(objectForRateLimitMatch, this.request, this.response, this.userId);
 
-						if (_options.validateParams && !_options.validateParams(this.request.method === 'GET' ? this.queryParams : this.bodyParams)) {
-							throw new Meteor.Error('invalid-params', _options.validateParams.errors?.map((error) => error.message).join('\n '));
+						if (_options.validateParams) {
+							const requestMethod = this.request.method;
+							const validatorFunc =
+								typeof _options.validateParams === 'function' ? _options.validateParams : _options.validateParams[requestMethod];
+
+							if (validatorFunc && !validatorFunc(requestMethod === 'GET' ? this.queryParams : this.bodyParams)) {
+								throw new Meteor.Error('invalid-params', validatorFunc.errors?.map((error) => error.message).join('\n '));
+							}
 						}
+
 						if (shouldVerifyPermissions && (!this.userId || !hasAllPermission(this.userId, _options.permissionsRequired))) {
 							throw new Meteor.Error('error-unauthorized', 'User does not have the permissions required for this action', {
 								permissions: _options.permissionsRequired,

apps/meteor/app/livechat/imports/server/rest/departments.ts

@@ -1,4 +1,4 @@
-import { isLivechatDepartmentProps } from '@rocket.chat/rest-typings';
+import { isGETLivechatDepartmentProps, isPOSTLivechatDepartmentProps } from '@rocket.chat/rest-typings';
 import { Match, check } from 'meteor/check';
 
 import { API } from '../../../../api/server';
@@ -15,7 +15,7 @@ import {
 
 API.v1.addRoute(
 	'livechat/department',
-	{ authRequired: true, validateParams: isLivechatDepartmentProps },
+	{ authRequired: true, validateParams: { GET: isGETLivechatDepartmentProps, POST: isPOSTLivechatDepartmentProps } },
 	{
 		async get() {
 			if (!hasAtLeastOnePermission(this.userId, ['view-livechat-departments', 'view-l-room'])) {
@@ -27,50 +27,44 @@ API.v1.addRoute(
 
 			const { text, enabled, onlyMyDepartments, excludeDepartmentId } = this.queryParams;
 
-			const { departments, total } = Promise.await(
-				findDepartments({
-					userId: this.userId,
-					text,
-					enabled: enabled === 'true',
-					onlyMyDepartments: onlyMyDepartments === 'true',
-					excludeDepartmentId,
-					pagination: {
-						offset,
-						count,
-						// IMO, sort type shouldn't be record, but a generic of the model we're trying to sort
-						// or the form { [k: keyof T]: number | string }
-						sort: sort as any,
-					},
-				}),
-			);
+			const { departments, total } = await findDepartments({
+				userId: this.userId,
+				text,
+				enabled: enabled === 'true',
+				onlyMyDepartments: onlyMyDepartments === 'true',
+				excludeDepartmentId,
+				pagination: {
+					offset,
+					count,
+					// IMO, sort type shouldn't be record, but a generic of the model we're trying to sort
+					// or the form { [k: keyof T]: number | string }
+					sort: sort as any,
+				},
+			});
 
 			return API.v1.success({ departments, count: departments.length, offset, total });
 		},
-		post() {
+		async post() {
 			if (!hasPermission(this.userId, 'manage-livechat-departments')) {
 				return API.v1.unauthorized();
 			}
 
-			try {
-				check(this.bodyParams, {
-					department: Object,
-					agents: Match.Maybe(Array),
-				});
+			check(this.bodyParams, {
+				department: Object,
+				agents: Match.Maybe(Array),
+			});
 
-				const agents = this.bodyParams.agents ? { upsert: this.bodyParams.agents } : {};
-				const department = Livechat.saveDepartment(null, this.bodyParams.department, agents);
+			const agents = this.bodyParams.agents ? { upsert: this.bodyParams.agents } : {};
+			const department = Livechat.saveDepartment(null, this.bodyParams.department, agents);
 
-				if (department) {
-					return API.v1.success({
-						department,
-						agents: LivechatDepartmentAgents.find({ departmentId: department._id }).fetch(),
-					});
-				}
-
-				return API.v1.failure();
-			} catch (e) {
-				return API.v1.failure(e);
+			if (department) {
+				return API.v1.success({
+					department,
+					agents: LivechatDepartmentAgents.find({ departmentId: department._id }).fetch(),
+				});
 			}
+
+			return API.v1.failure();
 		},
 	},
 );
@@ -170,20 +164,22 @@ API.v1.addRoute(
 	'livechat/department.autocomplete',
 	{ authRequired: true },
 	{
-		get() {
+		async get() {
+			if (!hasAtLeastOnePermission(this.userId, ['view-livechat-departments', 'view-l-room'])) {
+				return API.v1.unauthorized();
+			}
+
 			const { selector, onlyMyDepartments } = this.queryParams;
 			if (!selector) {
 				return API.v1.failure("The 'selector' param is required");
 			}
 
 			return API.v1.success(
-				Promise.await(
-					findDepartmentsToAutocomplete({
-						uid: this.userId,
-						selector: JSON.parse(selector),
-						onlyMyDepartments: onlyMyDepartments === 'true',
-					}),
-				),
+				await findDepartmentsToAutocomplete({
+					uid: this.userId,
+					selector: JSON.parse(selector),
+					onlyMyDepartments: onlyMyDepartments === 'true',
+				}),
 			);
 		},
 	},
@@ -239,7 +235,11 @@ API.v1.addRoute(
 	'livechat/department.listByIds',
 	{ authRequired: true },
 	{
-		get() {
+		async get() {
+			if (!hasAtLeastOnePermission(this.userId, ['view-livechat-departments', 'view-l-room'])) {
+				return API.v1.unauthorized();
+			}
+
 			const { ids } = this.queryParams;
 			const { fields } = this.parseJsonQuery();
 			if (!ids) {
@@ -250,13 +250,10 @@ API.v1.addRoute(
 			}
 
 			return API.v1.success(
-				Promise.await(
-					findDepartmentsBetweenIds({
-						uid: this.userId,
-						ids,
-						fields,
-					}),
-				),
+				await findDepartmentsBetweenIds({
+					ids,
+					fields,
+				}),
 			);
 		},
 	},

apps/meteor/app/livechat/server/api/lib/departments.ts

@@ -110,9 +110,6 @@ export async function findDepartmentsToAutocomplete({
 	selector,
 	onlyMyDepartments = false,
 }: FindDepartmentToAutocompleteParams): Promise<{ items: ILivechatDepartmentRecord[] }> {
-	if (!(await hasPermissionAsync(uid, 'view-livechat-departments')) && !(await hasPermissionAsync(uid, 'view-l-room'))) {
-		return { items: [] };
-	}
 	const { exceptions = [] } = selector;
 	let { conditions = {} } = selector;
 
@@ -160,18 +157,12 @@ export async function findDepartmentAgents({
 }
 
 export async function findDepartmentsBetweenIds({
-	uid,
 	ids,
 	fields,
 }: {
-	uid: string;
 	ids: string[];
 	fields: Record<string, unknown>;
 }): Promise<{ departments: ILivechatDepartmentRecord[] }> {
-	if (!(await hasPermissionAsync(uid, 'view-livechat-departments')) && !(await hasPermissionAsync(uid, 'view-l-room'))) {
-		throw new Error('error-not-authorized');
-	}
-
 	const departments = await LivechatDepartment.findInIds(ids, fields).toArray();
 	return { departments };
 }

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

@@ -600,14 +600,15 @@ export const updateDepartmentAgents = (departmentId, agents, departmentEnabled)
 	}
 
 	upsert.forEach((agent) => {
-		if (!Users.findOneById(agent.agentId, { fields: { _id: 1 } })) {
+		const agentFromDb = Users.findOneById(agent.agentId, { fields: { _id: 1, username: 1 } });
+		if (!agentFromDb) {
 			return;
 		}
 
 		LivechatDepartmentAgents.saveAgent({
 			agentId: agent.agentId,
 			departmentId,
-			username: agent.username,
+			username: agentFromDb.username,
 			count: agent.count ? parseInt(agent.count) : 0,
 			order: agent.order ? parseInt(agent.order) : 0,
 			departmentEnabled,

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

@@ -1079,6 +1079,10 @@ export const Livechat = {
 			);
 		}
 
+		if (fallbackForwardDepartment && !LivechatDepartment.findOneById(fallbackForwardDepartment)) {
+			throw new Meteor.Error('error-fallback-department-not-found', 'Fallback department not found', { method: 'livechat:saveDepartment' });
+		}
+
 		const departmentDB = LivechatDepartment.createOrUpdateDepartment(_id, departmentData);
 		if (departmentDB && departmentAgents) {
 			updateDepartmentAgents(departmentDB._id, departmentAgents, departmentDB.enabled);

apps/meteor/tests/data/livechat/rooms.js

@@ -39,6 +39,21 @@ export const createVisitor = () =>
 		});
 	});
 
+export const createDepartment = () => {
+	return new Promise((resolve, reject) => {
+		request
+			.post(api('livechat/department'))
+			.set(credentials)
+			.send({ department: { name: `Department ${Date.now()}`, enabled: true, showOnOfflineForm: true, showOnRegistration: true, email: '[email protected]' } })
+			.end((err, res) => {
+				if (err) {
+					return reject(err);
+				}
+				resolve(res.body.department);
+			});
+	});
+}
+
 export const createAgent = () =>
 	new Promise((resolve, reject) => {
 		request

apps/meteor/tests/end-to-end/api/livechat/00-rooms.ts

@@ -1,3 +1,5 @@
+/* eslint-env mocha */
+
 import { expect } from 'chai';
 import { IOmnichannelRoom, IVisitor } from '@rocket.chat/core-typings';
 import { Response } from 'supertest';

apps/meteor/tests/end-to-end/api/livechat/visitors.ts → apps/meteor/tests/end-to-end/api/livechat/09-visitors.ts

@@ -1,3 +1,5 @@
+/* eslint-env mocha */
+
 import { expect } from 'chai';
 import type { ILivechatVisitor, IOmnichannelRoom } from '@rocket.chat/core-typings';
 import { Response } from 'supertest';