chore!: remove the ability to use role names as parameters on /api/v1/roles.* endpoints (#36896)

.changeset/two-pets-knock.md

@@ -0,0 +1,8 @@
+---
+"@rocket.chat/meteor": major
+"@rocket.chat/rest-typings": major
+---
+
+Removes the deprecated roleName parameter from /api/v1/roles.addUserToRole and /api/v1/roles.removeUserFromRole
+
+Removes the ability to pass a role name to the role parameter type from /api/v1/roles.getUsersInRole

apps/meteor/app/api/server/v1/roles.ts

@@ -10,7 +10,6 @@ import { getUsersInRolePaginated } from '../../../authorization/server/functions
 import { hasPermissionAsync } from '../../../authorization/server/functions/hasPermission';
 import { hasRoleAsync, hasAnyRoleAsync } from '../../../authorization/server/functions/hasRole';
 import { addUserToRole } from '../../../authorization/server/methods/addUserToRole';
-import { apiDeprecationLogger } from '../../../lib/server/lib/deprecationWarningLogger';
 import { notifyOnRoleChanged } from '../../../lib/server/lib/notifyListener';
 import { settings } from '../../../settings/server/index';
 import type { ExtractRoutesFromAPI } from '../ApiClass';
@@ -64,17 +63,13 @@ API.v1.addRoute(
 			}
 
 			const user = await getUserFromParams(this.bodyParams);
-			const { roleId, roleName, roomId } = this.bodyParams;
+			const { roleId, roomId } = this.bodyParams;
 
 			if (!roleId) {
-				if (!roleName) {
 				return API.v1.failure('error-invalid-role-properties');
 			}
 
-				apiDeprecationLogger.parameter(this.route, 'roleName', '7.0.0', this.response);
-			}
-
-			const role = roleId ? await Roles.findOneById(roleId) : await Roles.findOneByIdOrName(roleName as string);
+			const role = await Roles.findOneById(roleId);
 			if (!role) {
 				return API.v1.failure('error-role-not-found', 'Role not found');
 			}
@@ -117,23 +112,12 @@ API.v1.addRoute(
 			}
 
 			const options = { projection: { _id: 1 } };
-			let roleData = await Roles.findOneById<Pick<IRole, '_id'>>(role, options);
-			if (!roleData) {
-				roleData = await Roles.findOneByName<Pick<IRole, '_id'>>(role, options);
+			const roleData = await Roles.findOneById<Pick<IRole, '_id'>>(role, options);
+
 			if (!roleData) {
 				throw new Meteor.Error('error-invalid-roleId');
 			}
 
-				apiDeprecationLogger.deprecatedParameterUsage(
-					this.route,
-					'role',
-					'7.0.0',
-					this.response,
-					({ parameter, endpoint, version }) =>
-						`Querying \`${parameter}\` by name is deprecated in ${endpoint} and will be removed on the removed on version ${version}`,
-				);
-			}
-
 			const { cursor, totalCount } = await getUsersInRolePaginated(roleData._id, roomId, {
 				limit: count as number,
 				sort: { username: 1 },
@@ -191,23 +175,19 @@ API.v1.addRoute(
 				throw new Meteor.Error('error-invalid-role-properties', 'The role properties are invalid.');
 			}
 
-			const { roleId, roleName, username, scope } = bodyParams;
+			const { roleId, username, scope } = bodyParams;
 
 			if (!roleId) {
-				if (!roleName) {
 				return API.v1.failure('error-invalid-role-properties');
 			}
 
-				apiDeprecationLogger.parameter(this.route, 'roleName', '7.0.0', this.response);
-			}
-
 			const user = await Users.findOneByUsername(username);
 
 			if (!user) {
 				throw new Meteor.Error('error-invalid-user', 'There is no user with this username');
 			}
 
-			const role = roleId ? await Roles.findOneById(roleId) : await Roles.findOneByIdOrName(roleName as string);
+			const role = await Roles.findOneById(roleId);
 
 			if (!role) {
 				throw new Meteor.Error('error-invalid-roleId', 'This role does not exist');

apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRolePage.tsx

@@ -43,7 +43,7 @@ const UsersInRolePage = ({ role }: { role: IRole }): ReactElement => {
 			await Promise.all(
 				users.map(async (user) => {
 					if (user) {
-						await addUserToRoleEndpoint({ roleName: _id, username: user, roomId: rid });
+						await addUserToRoleEndpoint({ roleId: _id, username: user, roomId: rid });
 					}
 				}),
 			);

apps/meteor/tests/end-to-end/api/roles.ts

@@ -269,6 +269,21 @@ describe('[Roles]', function () {
 					expect(res.body).to.have.property('error', 'User does not have the permissions required for this action [error-unauthorized]');
 				});
 		});
+
+		(isEnterprise ? it : it.skip)('should fail getting a list of users in a role in case a role name is provided', async () => {
+			await request
+				.get(api('roles.getUsersInRole'))
+				.set(credentials)
+				.query({
+					role: testRoleName,
+				})
+				.expect('Content-Type', 'application/json')
+				.expect(400)
+				.expect((res: Response) => {
+					expect(res.body).to.have.property('success', false);
+					expect(res.body).to.have.property('errorType', 'error-invalid-roleId');
+				});
+		});
 	});
 
 	describe('[/roles.delete]', () => {

packages/rest-typings/src/v1/roles.ts

@@ -24,9 +24,7 @@ export const isRoleDeleteProps = ajv.compile<RoleDeleteProps>(roleDeletePropsSch
 
 type RoleAddUserToRoleProps = {
 	username: string;
-	// #ToDo: Make it non-optional on the next major release
-	roleId?: string;
-	roleName?: string;
+	roleId: string;
 	roomId?: string;
 };
 
@@ -38,11 +36,6 @@ const roleAddUserToRolePropsSchema = {
 		},
 		roleId: {
 			type: 'string',
-			nullable: true,
-		},
-		roleName: {
-			type: 'string',
-			nullable: true,
 		},
 		roomId: {
 			type: 'string',
@@ -57,9 +50,7 @@ export const isRoleAddUserToRoleProps = ajv.compile<RoleAddUserToRoleProps>(role
 
 type RoleRemoveUserFromRoleProps = {
 	username: string;
-	// #ToDo: Make it non-optional on the next major release
-	roleId?: string;
-	roleName?: string;
+	roleId: string;
 	roomId?: string;
 	scope?: string;
 };
@@ -72,11 +63,6 @@ const roleRemoveUserFromRolePropsSchema = {
 		},
 		roleId: {
 			type: 'string',
-			nullable: true,
-		},
-		roleName: {
-			type: 'string',
-			nullable: true,
 		},
 		roomId: {
 			type: 'string',