Merge pull request #2060 from automatisch/aut-1233

feat: write and implement REST API endpoint to update role

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

@@ -0,0 +1,24 @@
+import { renderObject } from '../../../../../helpers/renderer.js';
+import Role from '../../../../../models/role.js';
+
+export default async (request, response) => {
+  const role = await Role.query()
+    .findById(request.params.roleId)
+    .throwIfNotFound();
+
+  const updatedRoleWithPermissions = await role.updateWithPermissions(
+    roleParams(request)
+  );
+
+  renderObject(response, updatedRoleWithPermissions);
+};
+
+const roleParams = (request) => {
+  const { name, description, permissions } = request.body;
+
+  return {
+    name,
+    description,
+    permissions,
+  };
+};

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

@@ -0,0 +1,177 @@
+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 { createUser } from '../../../../../../test/factories/user.js';
+import updateRoleMock from '../../../../../../test/mocks/rest/api/v1/admin/roles/update-role.ee.js';
+import * as license from '../../../../../helpers/license.ee.js';
+
+describe('PATCH /api/v1/admin/roles/:roleId', () => {
+  let adminRole, viewerRole, currentUser, token;
+
+  beforeEach(async () => {
+    vi.spyOn(license, 'hasValidLicense').mockResolvedValue(true);
+
+    adminRole = await createRole({ name: 'Admin' });
+    viewerRole = await createRole({ name: 'Viewer' });
+
+    await createPermission({
+      action: 'read',
+      subject: 'Connection',
+    });
+
+    await createPermission({
+      action: 'read',
+      subject: 'Flow',
+    });
+
+    currentUser = await createUser({ roleId: adminRole.id });
+
+    token = await createAuthTokenByUserId(currentUser.id);
+  });
+
+  it('should return the updated role along with permissions', async () => {
+    const roleData = {
+      name: 'Updated role name',
+      description: 'A new description',
+      permissions: [
+        {
+          action: 'read',
+          subject: 'Execution',
+          conditions: ['isCreator'],
+        },
+      ],
+    };
+
+    const response = await request(app)
+      .patch(`/api/v1/admin/roles/${viewerRole.id}`)
+      .set('Authorization', token)
+      .send(roleData)
+      .expect(200);
+
+    const refetchedViewerRole = await viewerRole
+      .$query()
+      .withGraphFetched({ permissions: true });
+
+    const expectedPayload = await updateRoleMock(
+      {
+        ...refetchedViewerRole,
+        ...roleData,
+        isAdmin: false,
+      },
+      [
+        {
+          ...refetchedViewerRole.permissions[0],
+          ...roleData.permissions[0],
+        },
+      ]
+    );
+
+    expect(response.body).toStrictEqual(expectedPayload);
+  });
+
+  it('should return the updated role with sanitized permissions', async () => {
+    const validPermission = {
+      action: 'create',
+      subject: 'Connection',
+      conditions: ['isCreator'],
+    };
+
+    const invalidPermission = {
+      action: 'publish',
+      subject: 'Connection',
+      conditions: ['isCreator'],
+    };
+
+    const roleData = {
+      permissions: [validPermission, invalidPermission],
+    };
+
+    const response = await request(app)
+      .patch(`/api/v1/admin/roles/${viewerRole.id}`)
+      .set('Authorization', token)
+      .send(roleData)
+      .expect(200);
+
+    const refetchedViewerRole = await viewerRole.$query().withGraphFetched({
+      permissions: true,
+    });
+
+    const expectedPayload = updateRoleMock(refetchedViewerRole, [
+      {
+        ...refetchedViewerRole.permissions[0],
+        ...validPermission,
+      },
+    ]);
+
+    expect(response.body).toStrictEqual(expectedPayload);
+  });
+
+  it('should return not authorized response for updating admin role', async () => {
+    const roleData = {
+      name: 'Updated role name',
+      description: 'A new description',
+      permissions: [
+        {
+          action: 'read',
+          subject: 'Execution',
+          conditions: ['isCreator'],
+        },
+      ],
+    };
+
+    await request(app)
+      .patch(`/api/v1/admin/roles/${adminRole.id}`)
+      .set('Authorization', token)
+      .send(roleData)
+      .expect(403);
+  });
+
+  it('should return unprocessable entity response for invalid role data', async () => {
+    const roleData = {
+      description: 123,
+      permissions: [],
+    };
+
+    const response = await request(app)
+      .patch(`/api/v1/admin/roles/${viewerRole.id}`)
+      .set('Authorization', token)
+      .send(roleData)
+      .expect(422);
+
+    expect(response.body).toStrictEqual({
+      errors: {
+        description: ['must be string,null'],
+      },
+      meta: {
+        type: 'ModelValidation',
+      },
+    });
+  });
+
+  it('should return unique violation response for duplicate role data', async () => {
+    await createRole({ name: 'Editor' });
+
+    const roleData = {
+      name: 'Editor',
+      permissions: [],
+    };
+
+    const response = await request(app)
+      .patch(`/api/v1/admin/roles/${viewerRole.id}`)
+      .set('Authorization', token)
+      .send(roleData)
+      .expect(422);
+
+    expect(response.body).toStrictEqual({
+      errors: {
+        name: ["'name' must be unique."],
+      },
+      meta: {
+        type: 'UniqueViolationError',
+      },
+    });
+  });
+});

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

@@ -10,7 +10,6 @@ import resetConnection from './mutations/reset-connection.js';
 import updateConnection from './mutations/update-connection.js';
 import updateCurrentUser from './mutations/update-current-user.js';
 import updateFlowStatus from './mutations/update-flow-status.js';
-import updateRole from './mutations/update-role.ee.js';
 import updateStep from './mutations/update-step.js';
 import upsertSamlAuthProvidersRoleMappings from './mutations/upsert-saml-auth-providers-role-mappings.ee.js';
 
@@ -39,7 +38,6 @@ const mutationResolvers = {
   updateConnection,
   updateCurrentUser,
   updateFlowStatus,
-  updateRole,
   updateStep,
   updateUser,
   upsertSamlAuthProvidersRoleMappings,

packages/backend/src/graphql/schema.graphql

@@ -18,7 +18,6 @@ type Mutation {
   updateConnection(input: UpdateConnectionInput): Connection
   updateCurrentUser(input: UpdateCurrentUserInput): User
   updateFlowStatus(input: UpdateFlowStatusInput): Flow
-  updateRole(input: UpdateRoleInput): Role
   updateStep(input: UpdateStepInput): Step
   updateUser(input: UpdateUserInput): User
   upsertSamlAuthProvidersRoleMappings(
@@ -335,19 +334,6 @@ input UpdateCurrentUserInput {
   fullName: String
 }
 
-input PermissionInput {
-  action: String!
-  subject: String!
-  conditions: [String]
-}
-
-input UpdateRoleInput {
-  id: String!
-  name: String!
-  description: String
-  permissions: [PermissionInput]
-}
-
 input DeleteRoleInput {
   id: String!
 }

packages/backend/src/models/permission.js

@@ -1,4 +1,5 @@
 import Base from './base.js';
+import permissionCatalog from '../helpers/permission-catalog.ee.js';
 
 class Permission extends Base {
   static tableName = 'permissions';
@@ -17,6 +18,26 @@ class Permission extends Base {
       updatedAt: { type: 'string' },
     },
   };
+
+  static sanitize(permissions) {
+    const sanitizedPermissions = permissions.filter((permission) => {
+      const { action, subject, conditions } = permission;
+
+      const relevantAction = permissionCatalog.actions.find(
+        (actionCatalogItem) => actionCatalogItem.key === action
+      );
+      const validSubject = relevantAction.subjects.includes(subject);
+      const validConditions = conditions.every((condition) => {
+        return !!permissionCatalog.conditions.find(
+          (conditionCatalogItem) => conditionCatalogItem.key === condition
+        );
+      });
+
+      return validSubject && validConditions;
+    });
+
+    return sanitizedPermissions;
+  }
 }
 
 export default Permission;

packages/backend/src/models/role.js

@@ -1,6 +1,7 @@
 import Base from './base.js';
 import Permission from './permission.js';
 import User from './user.js';
+import NotAuthorizedError from '../errors/not-authorized.js';
 
 class Role extends Base {
   static tableName = 'roles';
@@ -48,6 +49,42 @@ class Role extends Base {
   static async findAdmin() {
     return await this.query().findOne({ name: 'Admin' });
   }
+
+  async updateWithPermissions(data) {
+    if (this.isAdmin) {
+      throw new NotAuthorizedError('The admin role cannot be altered!');
+    }
+
+    const { name, description, permissions } = data;
+
+    return await Role.transaction(async (trx) => {
+      await this.$relatedQuery('permissions', trx).delete();
+
+      if (permissions?.length) {
+        const sanitizedPermissions = Permission.sanitize(permissions).map(
+          (permission) => ({
+            ...permission,
+            roleId: this.id,
+          })
+        );
+
+        await Permission.query().insert(sanitizedPermissions);
+      }
+
+      await this.$query(trx).patch({
+        name,
+        description,
+      });
+
+      return await this.$query(trx)
+        .leftJoinRelated({
+          permissions: true,
+        })
+        .withGraphFetched({
+          permissions: true,
+        });
+    });
+  }
 }
 
 export default Role;

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

@@ -5,6 +5,7 @@ import { checkIsEnterprise } from '../../../../helpers/check-is-enterprise.js';
 import createRoleAction from '../../../../controllers/api/v1/admin/roles/create-role.ee.js';
 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';
 
 const router = Router();
 
@@ -32,4 +33,12 @@ router.get(
   getRoleAction
 );
 
+router.patch(
+  '/:roleId',
+  authenticateUser,
+  authorizeAdmin,
+  checkIsEnterprise,
+  updateRoleAction
+);
+
 export default router;

packages/backend/test/mocks/rest/api/v1/admin/roles/update-role.ee.js

@@ -0,0 +1,32 @@
+const updateRoleMock = (role, permissions = []) => {
+  const data = {
+    id: role.id,
+    name: role.name,
+    isAdmin: role.isAdmin,
+    description: role.description,
+    createdAt: role.createdAt.getTime(),
+    updatedAt: role.updatedAt.getTime(),
+    permissions: permissions.map((permission) => ({
+      id: permission.id,
+      action: permission.action,
+      conditions: permission.conditions,
+      roleId: permission.roleId,
+      subject: permission.subject,
+      createdAt: permission.createdAt.getTime(),
+      updatedAt: permission.updatedAt.getTime(),
+    })),
+  };
+
+  return {
+    data: data,
+    meta: {
+      count: 1,
+      currentPage: null,
+      isArray: false,
+      totalPages: null,
+      type: 'Role',
+    },
+  };
+};
+
+export default updateRoleMock;

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

@@ -1,16 +0,0 @@
-import { gql } from '@apollo/client';
-export const UPDATE_ROLE = gql`
-  mutation UpdateRole($input: UpdateRoleInput) {
-    updateRole(input: $input) {
-      id
-      name
-      description
-      permissions {
-        id
-        action
-        subject
-        conditions
-      }
-    }
-  }
-`;

packages/web/src/hooks/useAdminUpdateRole.js

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

packages/web/src/pages/EditRole/index.ee.jsx

@@ -1,4 +1,3 @@
-import { useMutation } from '@apollo/client';
 import LoadingButton from '@mui/lab/LoadingButton';
 import Grid from '@mui/material/Grid';
 import Skeleton from '@mui/material/Skeleton';
@@ -13,20 +12,21 @@ import PageTitle from 'components/PageTitle';
 import PermissionCatalogField from 'components/PermissionCatalogField/index.ee';
 import TextField from 'components/TextField';
 import * as URLS from 'config/urls';
-import { UPDATE_ROLE } from 'graphql/mutations/update-role.ee';
 import {
   getPermissions,
   getRoleWithComputedPermissions,
 } from 'helpers/computePermissions.ee';
 import useFormatMessage from 'hooks/useFormatMessage';
+import useAdminUpdateRole from 'hooks/useAdminUpdateRole';
 import useRole from 'hooks/useRole.ee';
 
 export default function EditRole() {
   const formatMessage = useFormatMessage();
-  const [updateRole, { loading }] = useMutation(UPDATE_ROLE);
   const navigate = useNavigate();
   const { roleId } = useParams();
   const { data, loading: isRoleLoading } = useRole({ roleId });
+  const { mutateAsync: updateRole, isPending: isUpdateRolePending } =
+    useAdminUpdateRole(roleId);
   const role = data?.data;
   const enqueueSnackbar = useEnqueueSnackbar();
 
@@ -34,14 +34,9 @@ export default function EditRole() {
     try {
       const newPermissions = getPermissions(roleData.computedPermissions);
       await updateRole({
-        variables: {
+        name: roleData.name,
-          input: {
+        description: roleData.description,
-            id: roleId,
+        permissions: newPermissions,
-            name: roleData.name,
-            description: roleData.description,
-            permissions: newPermissions,
-          },
-        },
       });
 
       enqueueSnackbar(formatMessage('editRole.successfullyUpdated'), {
@@ -111,7 +106,7 @@ export default function EditRole() {
                 variant="contained"
                 color="primary"
                 sx={{ boxShadow: 2 }}
-                loading={loading}
+                loading={isUpdateRolePending}
                 disabled={role?.isAdmin || isRoleLoading}
                 data-test="update-button"
               >