Merge pull request #2064 from automatisch/aut-1237

feat: write and implement REST API endpoint to delete role

packages/backend/src/controllers/api/v1/admin/roles/delete-role.ee.js

@@ -0,0 +1,11 @@
+import Role from '../../../../../models/role.js';
+
+export default async (request, response) => {
+  const role = await Role.query()
+    .findById(request.params.roleId)
+    .throwIfNotFound();
+
+  await role.deleteWithPermissions();
+
+  response.status(204).end();
+};

packages/backend/src/controllers/api/v1/admin/roles/delete-role.ee.test.js

@@ -0,0 +1,112 @@
+import Crypto from 'node:crypto';
+import { vi, describe, it, expect, beforeEach } from 'vitest';
+import request from 'supertest';
+import app from '../../../../../app.js';
+import createAuthTokenByUserId from '../../../../../helpers/create-auth-token-by-user-id.js';
+import { createRole } from '../../../../../../test/factories/role.js';
+import { createPermission } from '../../../../../../test/factories/permission.js';
+import { createSamlAuthProvider } from '../../../../../../test/factories/saml-auth-provider.ee.js';
+import { createUser } from '../../../../../../test/factories/user.js';
+import * as license from '../../../../../helpers/license.ee.js';
+
+describe('DELETE /api/v1/admin/roles/:roleId', () => {
+  let adminRole, currentUser, token;
+
+  beforeEach(async () => {
+    vi.spyOn(license, 'hasValidLicense').mockResolvedValue(true);
+
+    adminRole = await createRole({ name: 'Admin' });
+
+    currentUser = await createUser({ roleId: adminRole.id });
+
+    token = await createAuthTokenByUserId(currentUser.id);
+  });
+
+  it('should return HTTP 204 for unused role', async () => {
+    const role = await createRole();
+    const permission = await createPermission({ roleId: role.id });
+
+    await request(app)
+      .delete(`/api/v1/admin/roles/${role.id}`)
+      .set('Authorization', token)
+      .expect(204);
+
+    const refetchedRole = await role.$query();
+    const refetchedPermission = await permission.$query();
+
+    expect(refetchedRole).toBeUndefined();
+    expect(refetchedPermission).toBeUndefined();
+  });
+
+  it('should return HTTP 404 for not existing role UUID', async () => {
+    const notExistingRoleUUID = Crypto.randomUUID();
+
+    await request(app)
+      .delete(`/api/v1/admin/roles/${notExistingRoleUUID}`)
+      .set('Authorization', token)
+      .expect(404);
+  });
+
+  it('should return not authorized response for deleting admin role', async () => {
+    await request(app)
+      .delete(`/api/v1/admin/roles/${adminRole.id}`)
+      .set('Authorization', token)
+      .expect(403);
+  });
+
+  it('should return unprocessable entity response for role used by users', async () => {
+    const role = await createRole();
+    await createUser({ roleId: role.id });
+
+    const response = await request(app)
+      .delete(`/api/v1/admin/roles/${role.id}`)
+      .set('Authorization', token)
+      .expect(422);
+
+    expect(response.body).toStrictEqual({
+      errors: {
+        role: [`All users must be migrated away from the "${role.name}" role.`],
+      },
+      meta: {
+        type: 'ValidationError',
+      },
+    });
+  });
+
+  it('should return unprocessable entity response for role used by saml auth providers', async () => {
+    const samlAuthProvider = await createSamlAuthProvider();
+
+    const response = await request(app)
+      .delete(`/api/v1/admin/roles/${samlAuthProvider.defaultRoleId}`)
+      .set('Authorization', token)
+      .expect(422);
+
+    expect(response.body).toStrictEqual({
+      errors: {
+        samlAuthProvider: [
+          'You need to change the default role in the SAML configuration before deleting this role.',
+        ],
+      },
+      meta: {
+        type: 'ValidationError',
+      },
+    });
+  });
+
+  it('should not delete role and permissions on unsuccessful response', async () => {
+    const role = await createRole();
+    const permission = await createPermission({ roleId: role.id });
+    await createUser({ roleId: role.id });
+
+    await request(app)
+      .delete(`/api/v1/admin/roles/${role.id}`)
+      .set('Authorization', token)
+      .expect(422);
+
+    const refetchedRole = await role.$query();
+    const refetchedPermission = await permission.$query();
+
+    expect(refetchedRole).toStrictEqual(role);
+    expect(refetchedPermission).toStrictEqual(permission);
+  });
+});

packages/backend/src/graphql/mutation-resolvers.js

@@ -1,7 +1,6 @@
 import createConnection from './mutations/create-connection.js';
 import createUser from './mutations/create-user.ee.js';
 import deleteFlow from './mutations/delete-flow.js';
-import deleteRole from './mutations/delete-role.ee.js';
 import duplicateFlow from './mutations/duplicate-flow.js';
 import generateAuthUrl from './mutations/generate-auth-url.js';
 import registerUser from './mutations/register-user.ee.js';
@@ -25,7 +24,6 @@ const mutationResolvers = {
   createUser,
   deleteCurrentUser,
   deleteFlow,
-  deleteRole,
   deleteStep,
   duplicateFlow,
   executeFlow,

packages/backend/src/graphql/mutations/delete-role.ee.js

@@ -1,36 +0,0 @@
-import Role from '../../models/role.js';
-import SamlAuthProvider from '../../models/saml-auth-provider.ee.js';
-
-const deleteRole = async (_parent, params, context) => {
-  context.currentUser.can('delete', 'Role');
-
-  const role = await Role.query().findById(params.input.id).throwIfNotFound();
-  const count = await role.$relatedQuery('users').resultSize();
-
-  if (count > 0) {
-    throw new Error('All users must be migrated away from the role!');
-  }
-
-  if (role.isAdmin) {
-    throw new Error('Admin role cannot be deleted!');
-  }
-
-  const samlAuthProviderUsingDefaultRole = await SamlAuthProvider.query()
-    .where({ default_role_id: role.id })
-    .limit(1)
-    .first();
-
-  if (samlAuthProviderUsingDefaultRole) {
-    throw new Error(
-      'You need to change the default role in the SAML configuration before deleting this role.'
-    );
-  }
-
-  // delete permissions first
-  await role.$relatedQuery('permissions').delete();
-  await role.$query().delete();
-
-  return true;
-};
-
-export default deleteRole;

packages/backend/src/graphql/schema.graphql

@@ -7,7 +7,6 @@ type Mutation {
   createUser(input: CreateUserInput): UserWithAcceptInvitationUrl
   deleteCurrentUser: Boolean
   deleteFlow(input: DeleteFlowInput): Boolean
-  deleteRole(input: DeleteRoleInput): Boolean
   deleteStep(input: DeleteStepInput): Step
   duplicateFlow(input: DuplicateFlowInput): Flow
   executeFlow(input: ExecuteFlowInput): executeFlowType
@@ -309,10 +308,6 @@ input UpdateCurrentUserInput {
   fullName: String
 }
 
-input DeleteRoleInput {
-  id: String!
-}
-
 """
 The `JSONObject` scalar type represents JSON objects as specified by [ECMA-404](http://www.ecma-international.org/publications/files/ECMA-ST/ECMA-404.pdf).
 """

packages/backend/src/models/role.js

@@ -1,6 +1,8 @@
+import { ValidationError } from 'objection';
 import Base from './base.js';
 import Permission from './permission.js';
 import User from './user.js';
+import SamlAuthProvider from './saml-auth-provider.ee.js';
 import NotAuthorizedError from '../errors/not-authorized.js';
 
 class Role extends Base {
@@ -85,6 +87,59 @@ class Role extends Base {
         });
     });
   }
+
+  async deleteWithPermissions() {
+    return await Role.transaction(async (trx) => {
+      await this.$relatedQuery('permissions', trx).delete();
+
+      return await this.$query(trx).delete();
+    });
+  }
+
+  async $beforeDelete(queryContext) {
+    await super.$beforeDelete(queryContext);
+
+    if (this.isAdmin) {
+      throw new NotAuthorizedError('The admin role cannot be deleted!');
+    }
+
+    const userCount = await this.$relatedQuery('users').limit(1).resultSize();
+    const hasUsers = userCount > 0;
+
+    if (hasUsers) {
+      throw new ValidationError({
+        data: {
+          role: [
+            {
+              message: `All users must be migrated away from the "${this.name}" role.`,
+            },
+          ],
+        },
+        type: 'ValidationError',
+      });
+    }
+
+    const samlAuthProviderUsingDefaultRole = await SamlAuthProvider.query()
+      .where({
+        default_role_id: this.id,
+      })
+      .limit(1)
+      .first();
+
+    if (samlAuthProviderUsingDefaultRole) {
+      throw new ValidationError({
+        data: {
+          samlAuthProvider: [
+            {
+              message:
+                'You need to change the default role in the SAML configuration before deleting this role.',
+            },
+          ],
+        },
+        type: 'ValidationError',
+      });
+    }
+  }
 }
 
 export default Role;

packages/backend/src/routes/api/v1/admin/roles.ee.js

@@ -6,6 +6,7 @@ import createRoleAction from '../../../../controllers/api/v1/admin/roles/create-
 import getRolesAction from '../../../../controllers/api/v1/admin/roles/get-roles.ee.js';
 import getRoleAction from '../../../../controllers/api/v1/admin/roles/get-role.ee.js';
 import updateRoleAction from '../../../../controllers/api/v1/admin/roles/update-role.ee.js';
+import deleteRoleAction from '../../../../controllers/api/v1/admin/roles/delete-role.ee.js';
 
 const router = Router();
 
@@ -41,4 +42,12 @@ router.patch(
   updateRoleAction
 );
 
+router.delete(
+  '/:roleId',
+  authenticateUser,
+  authorizeAdmin,
+  checkIsEnterprise,
+  deleteRoleAction
+);
+
 export default router;

packages/e2e-tests/tests/admin/manage-roles.spec.js

@@ -221,7 +221,7 @@ test.describe('Role management page', () => {
         await adminRolesPage.snackbar.waitFor({
           state: 'attached',
         });
-        const snackbar = await adminRolesPage.getSnackbarData('snackbar-error');
+        const snackbar = await adminRolesPage.getSnackbarData('snackbar-delete-role-error');
         await expect(snackbar.variant).toBe('error');
         await adminRolesPage.closeSnackbar();
         await modal.close();

packages/web/src/components/DeleteRoleButton/index.ee.jsx

@@ -1,31 +1,26 @@
 import PropTypes from 'prop-types';
-import { useMutation } from '@apollo/client';
 import DeleteIcon from '@mui/icons-material/Delete';
 import IconButton from '@mui/material/IconButton';
 import useEnqueueSnackbar from 'hooks/useEnqueueSnackbar';
 import * as React from 'react';
-import { useQueryClient } from '@tanstack/react-query';
 
 import Can from 'components/Can';
 import ConfirmationDialog from 'components/ConfirmationDialog';
-import { DELETE_ROLE } from 'graphql/mutations/delete-role.ee';
 import useFormatMessage from 'hooks/useFormatMessage';
+import useAdminDeleteRole from 'hooks/useAdminDeleteRole';
 
 function DeleteRoleButton(props) {
   const { disabled, roleId } = props;
   const [showConfirmation, setShowConfirmation] = React.useState(false);
   const formatMessage = useFormatMessage();
   const enqueueSnackbar = useEnqueueSnackbar();
-  const queryClient = useQueryClient();
 
-  const [deleteRole] = useMutation(DELETE_ROLE, {
-    variables: { input: { id: roleId } },
-  });
+  const { mutateAsync: deleteRole } = useAdminDeleteRole(roleId);
 
   const handleConfirm = React.useCallback(async () => {
     try {
       await deleteRole();
-      queryClient.invalidateQueries({ queryKey: ['admin', 'roles'] });
+
       setShowConfirmation(false);
       enqueueSnackbar(formatMessage('deleteRoleButton.successfullyDeleted'), {
         variant: 'success',
@@ -34,9 +29,22 @@ function DeleteRoleButton(props) {
         },
       });
     } catch (error) {
+      const errors = Object.values(
+        error.response.data.errors || [['Failed while deleting!']],
+      );
+
+      for (const [error] of errors) {
+        enqueueSnackbar(error, {
+          variant: 'error',
+          SnackbarProps: {
+            'data-test': 'snackbar-delete-role-error',
+          },
+        });
+      }
+
       throw new Error('Failed while deleting!');
     }
-  }, [deleteRole, enqueueSnackbar, formatMessage, queryClient]);
+  }, [deleteRole, enqueueSnackbar, formatMessage]);
 
   return (
     <>

packages/web/src/graphql/mutations/delete-role.ee.js

@@ -1,6 +0,0 @@
-import { gql } from '@apollo/client';
-export const DELETE_ROLE = gql`
-  mutation DeleteRole($input: DeleteRoleInput) {
-    deleteRole(input: $input)
-  }
-`;

packages/web/src/hooks/useAdminDeleteRole.js

@@ -0,0 +1,21 @@
+import { useMutation, useQueryClient } from '@tanstack/react-query';
+import api from 'helpers/api';
+
+export default function useAdminDeleteRole(roleId) {
+  const queryClient = useQueryClient();
+
+  const query = useMutation({
+    mutationFn: async (payload) => {
+      const { data } = await api.delete(`/v1/admin/roles/${roleId}`);
+
+      return data;
+    },
+    onSuccess: () => {
+      queryClient.invalidateQueries({
+        queryKey: ['admin', 'roles'],
+      });
+    },
+  });
+
+  return query;
+}