diff --git a/.changeset/two-pets-knock.md b/.changeset/two-pets-knock.md new file mode 100644 index 00000000000..c568f32c9a6 --- /dev/null +++ b/.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 diff --git a/apps/meteor/app/api/server/v1/roles.ts b/apps/meteor/app/api/server/v1/roles.ts index 10c3a9fd783..8985e48c0af 100644 --- a/apps/meteor/app/api/server/v1/roles.ts +++ b/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); + return API.v1.failure('error-invalid-role-properties'); } - 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,21 +112,10 @@ API.v1.addRoute( } const options = { projection: { _id: 1 } }; - let roleData = await Roles.findOneById>(role, options); - if (!roleData) { - roleData = await Roles.findOneByName>(role, options); - if (!roleData) { - throw new Meteor.Error('error-invalid-roleId'); - } + const roleData = await Roles.findOneById>(role, options); - 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}`, - ); + if (!roleData) { + throw new Meteor.Error('error-invalid-roleId'); } const { cursor, totalCount } = await getUsersInRolePaginated(roleData._id, roomId, { @@ -191,14 +175,10 @@ 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); + return API.v1.failure('error-invalid-role-properties'); } const user = await Users.findOneByUsername(username); @@ -207,7 +187,7 @@ API.v1.addRoute( 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'); diff --git a/apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRolePage.tsx b/apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRolePage.tsx index 82f1d878773..c718ae1689e 100644 --- a/apps/meteor/client/views/admin/permissions/UsersInRole/UsersInRolePage.tsx +++ b/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 }); } }), ); diff --git a/apps/meteor/tests/end-to-end/api/roles.ts b/apps/meteor/tests/end-to-end/api/roles.ts index 130012f7535..e369aae9dd4 100644 --- a/apps/meteor/tests/end-to-end/api/roles.ts +++ b/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]', () => { diff --git a/packages/rest-typings/src/v1/roles.ts b/packages/rest-typings/src/v1/roles.ts index cf7cf0e7149..d8fe60b4782 100644 --- a/packages/rest-typings/src/v1/roles.ts +++ b/packages/rest-typings/src/v1/roles.ts @@ -24,9 +24,7 @@ export const isRoleDeleteProps = ajv.compile(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(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',