[permissions] Update updateRole logic + disallow self role-assignment (#10476)

In this PR

- updateWorkspaceMemberRole api was changed to stop allowing null as a
valid value for roleId. it is not possible anymore to just unassign a
role from a user. instead it is only possible to assign a different role
to a user, which will unassign them from their previous role. For this
reason in the FE the bins icons next to the workspaceMember on a role
page were removed
- updateWorkspaceMemberRole will throw if a user attempts to update
their own role
- tests tests tests!

packages/twenty-server/src/engine/metadata-modules/permissions/permissions.exception.ts

@@ -18,6 +18,7 @@ export enum PermissionsExceptionCode {
   CANNOT_UNASSIGN_LAST_ADMIN = 'CANNOT_UNASSIGN_LAST_ADMIN',
   UNKNOWN_OPERATION_NAME = 'UNKNOWN_OPERATION_NAME',
   UNKNOWN_REQUIRED_PERMISSION = 'UNKNOWN_REQUIRED_PERMISSION',
+  CANNOT_UPDATE_SELF_ROLE = 'CANNOT_UPDATE_SELF_ROLE',
 }
 
 export enum PermissionsExceptionMessage {
@@ -29,7 +30,8 @@ export enum PermissionsExceptionMessage {
   USER_WORKSPACE_ALREADY_HAS_ROLE = 'User workspace already has role',
   WORKSPACE_MEMBER_NOT_FOUND = 'Workspace member not found',
   ROLE_NOT_FOUND = 'Role not found',
-  CANNOT_UNASSIGN_LAST_ADMIN = 'Cannot unassign last admin',
+  CANNOT_UNASSIGN_LAST_ADMIN = 'Cannot unassign admin role from last admin of the workspace',
   UNKNOWN_OPERATION_NAME = 'Unknown operation name, cannot determine required permission',
   UNKNOWN_REQUIRED_PERMISSION = 'Unknown required permission',
+  CANNOT_UPDATE_SELF_ROLE = 'Cannot update self role',
 }

packages/twenty-server/src/engine/metadata-modules/permissions/utils/permission-graphql-api-exception-handler.util.ts

@@ -14,6 +14,7 @@ export const permissionGraphqlApiExceptionHandler = (
   switch (error.code) {
     case PermissionsExceptionCode.PERMISSION_DENIED:
     case PermissionsExceptionCode.CANNOT_UNASSIGN_LAST_ADMIN:
+    case PermissionsExceptionCode.CANNOT_UPDATE_SELF_ROLE:
       throw new ForbiddenError(error.message);
     case PermissionsExceptionCode.ROLE_NOT_FOUND:
     case PermissionsExceptionCode.USER_WORKSPACE_NOT_FOUND:

packages/twenty-server/src/engine/metadata-modules/role/role.resolver.ts

@@ -8,14 +8,18 @@ import {
   Resolver,
 } from '@nestjs/graphql';
 
-import { isDefined } from 'twenty-shared';
-
 import { UserWorkspaceService } from 'src/engine/core-modules/user-workspace/user-workspace.service';
 import { WorkspaceMember } from 'src/engine/core-modules/user/dtos/workspace-member.dto';
 import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity';
+import { AuthWorkspaceMemberId } from 'src/engine/decorators/auth/auth-workspace-member-id.decorator';
 import { AuthWorkspace } from 'src/engine/decorators/auth/auth-workspace.decorator';
 import { SettingsPermissionsGuard } from 'src/engine/guards/settings-permissions.guard';
 import { SettingsPermissions } from 'src/engine/metadata-modules/permissions/constants/settings-permissions.constants';
+import {
+  PermissionsException,
+  PermissionsExceptionCode,
+  PermissionsExceptionMessage,
+} from 'src/engine/metadata-modules/permissions/permissions.exception';
 import { PermissionsGraphqlApiExceptionFilter } from 'src/engine/metadata-modules/permissions/utils/permissions-graphql-api-exception.filter';
 import { RoleDTO } from 'src/engine/metadata-modules/role/dtos/role.dto';
 import { RoleService } from 'src/engine/metadata-modules/role/role.service';
@@ -41,9 +45,17 @@ export class RoleResolver {
   async updateWorkspaceMemberRole(
     @AuthWorkspace() workspace: Workspace,
     @Args('workspaceMemberId') workspaceMemberId: string,
-    @Args('roleId', { type: () => String, nullable: true })
-    roleId: string | null,
+    @Args('roleId', { type: () => String }) roleId: string,
+    @AuthWorkspaceMemberId()
+    updatorWorkspaceMemberId: string,
   ): Promise<WorkspaceMember> {
+    if (updatorWorkspaceMemberId === workspaceMemberId) {
+      throw new PermissionsException(
+        PermissionsExceptionMessage.CANNOT_UPDATE_SELF_ROLE,
+        PermissionsExceptionCode.CANNOT_UPDATE_SELF_ROLE,
+      );
+    }
+
     const workspaceMember =
       await this.userWorkspaceService.getWorkspaceMemberOrThrow({
         workspaceMemberId,
@@ -56,18 +68,11 @@ export class RoleResolver {
         workspaceId: workspace.id,
       });
 
-    if (!isDefined(roleId)) {
-      await this.userRoleService.unassignAllRolesFromUserWorkspace({
-        userWorkspaceId: userWorkspace.id,
-        workspaceId: workspace.id,
-      });
-    } else {
-      await this.userRoleService.assignRoleToUserWorkspace({
-        userWorkspaceId: userWorkspace.id,
-        workspaceId: workspace.id,
-        roleId,
-      });
-    }
+    await this.userRoleService.assignRoleToUserWorkspace({
+      userWorkspaceId: userWorkspace.id,
+      workspaceId: workspace.id,
+      roleId,
+    });
 
     const roles = await this.userRoleService
       .getRolesByUserWorkspaces({

packages/twenty-server/src/engine/metadata-modules/user-role/user-role.service.ts

@@ -1,14 +1,15 @@
 import { InjectRepository } from '@nestjs/typeorm';
 
 import { isDefined } from 'twenty-shared';
-import { In, Repository } from 'typeorm';
+import { In, Not, Repository } from 'typeorm';
 
 import { UserWorkspace } from 'src/engine/core-modules/user-workspace/user-workspace.entity';
+import { ADMIN_ROLE_LABEL } from 'src/engine/metadata-modules/permissions/constants/admin-role-label.constants';
 import {
   PermissionsException,
   PermissionsExceptionCode,
+  PermissionsExceptionMessage,
 } from 'src/engine/metadata-modules/permissions/permissions.exception';
-import { RoleDTO } from 'src/engine/metadata-modules/role/dtos/role.dto';
 import { RoleEntity } from 'src/engine/metadata-modules/role/role.entity';
 import { UserWorkspaceRoleEntity } from 'src/engine/metadata-modules/role/user-workspace-role.entity';
 import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager';
@@ -34,72 +35,22 @@ export class UserRoleService {
     userWorkspaceId: string;
     roleId: string;
   }): Promise<void> {
-    const userWorkspace = await this.userWorkspaceRepository.findOne({
-      where: {
-        id: userWorkspaceId,
-      },
-    });
-
-    if (!isDefined(userWorkspace)) {
-      throw new PermissionsException(
-        'User workspace not found',
-        PermissionsExceptionCode.USER_WORKSPACE_NOT_FOUND,
-      );
-    }
-
-    const role = await this.roleRepository.findOne({
-      where: {
-        id: roleId,
-      },
-    });
-
-    if (!isDefined(role)) {
-      throw new PermissionsException(
-        'Role not found',
-        PermissionsExceptionCode.ROLE_NOT_FOUND,
-      );
-    }
-
-    const roles = await this.getRolesByUserWorkspaces({
-      userWorkspaceIds: [userWorkspace.id],
+    await this.validateAssignRoleInput({
+      userWorkspaceId,
       workspaceId,
-    });
-
-    const currentRole = roles.get(userWorkspace.id)?.[0];
-
-    if (currentRole?.id === roleId) {
-      return;
-    }
-
-    await this.unassignAllRolesFromUserWorkspace({
-      userWorkspaceId: userWorkspace.id,
-      workspaceId,
-    });
-
-    await this.userWorkspaceRoleRepository.save({
       roleId,
-      userWorkspaceId: userWorkspace.id,
+    });
+
+    const newUserWorkspaceRole = await this.userWorkspaceRoleRepository.save({
+      roleId,
+      userWorkspaceId,
       workspaceId,
     });
-  }
-
-  public async unassignAllRolesFromUserWorkspace({
-    userWorkspaceId,
-    workspaceId,
-  }: {
-    userWorkspaceId: string;
-    workspaceId: string;
-  }): Promise<void> {
-    await this.validatesUserWorkspaceIsNotLastAdminIfUnassigningAdminRoleOrThrow(
-      {
-        userWorkspaceId,
-        workspaceId,
-      },
-    );
 
     await this.userWorkspaceRoleRepository.delete({
       userWorkspaceId,
       workspaceId,
+      id: Not(newUserWorkspaceRole.id),
     });
   }
 
@@ -186,34 +137,64 @@ export class UserRoleService {
     return workspaceMembers;
   }
 
-  private async validatesUserWorkspaceIsNotLastAdminIfUnassigningAdminRoleOrThrow({
+  private async validateAssignRoleInput({
     userWorkspaceId,
     workspaceId,
+    roleId,
   }: {
     userWorkspaceId: string;
     workspaceId: string;
-  }): Promise<void> {
+    roleId: string;
+  }) {
+    const userWorkspace = await this.userWorkspaceRepository.findOne({
+      where: {
+        id: userWorkspaceId,
+      },
+    });
+
+    if (!isDefined(userWorkspace)) {
+      throw new PermissionsException(
+        'User workspace not found',
+        PermissionsExceptionCode.USER_WORKSPACE_NOT_FOUND,
+      );
+    }
+
+    const role = await this.roleRepository.findOne({
+      where: {
+        id: roleId,
+      },
+    });
+
+    if (!isDefined(role)) {
+      throw new PermissionsException(
+        'Role not found',
+        PermissionsExceptionCode.ROLE_NOT_FOUND,
+      );
+    }
+
     const roles = await this.getRolesByUserWorkspaces({
-      userWorkspaceIds: [userWorkspaceId],
+      userWorkspaceIds: [userWorkspace.id],
       workspaceId,
     });
 
-    const currentRoles = roles.get(userWorkspaceId);
+    const currentRole = roles.get(userWorkspace.id)?.[0];
 
-    const adminRole = currentRoles?.find(
-      (role: RoleDTO) => role.isEditable === false,
-    );
+    if (currentRole?.id === roleId) {
+      return;
+    }
 
-    if (isDefined(adminRole)) {
-      const workspaceMembersWithAdminRole =
-        await this.getWorkspaceMembersAssignedToRole(adminRole.id, workspaceId);
+    if (!(currentRole?.label === ADMIN_ROLE_LABEL)) {
+      return;
+    }
 
-      if (workspaceMembersWithAdminRole.length === 1) {
-        throw new PermissionsException(
-          `Cannot unassign admin role as there is only one admin in the workspace`,
-          PermissionsExceptionCode.CANNOT_UNASSIGN_LAST_ADMIN,
-        );
-      }
+    const workspaceMembersWithAdminRole =
+      await this.getWorkspaceMembersAssignedToRole(currentRole.id, workspaceId);
+
+    if (workspaceMembersWithAdminRole.length === 1) {
+      throw new PermissionsException(
+        PermissionsExceptionMessage.CANNOT_UNASSIGN_LAST_ADMIN,
+        PermissionsExceptionCode.CANNOT_UNASSIGN_LAST_ADMIN,
+      );
     }
   }
 }