diff --git a/packages/backend/src/controllers/api/v1/admin/roles/update-role.ee.js b/packages/backend/src/controllers/api/v1/admin/roles/update-role.ee.js new file mode 100644 index 00000000..de8a9aa4 --- /dev/null +++ b/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, + }; +}; diff --git a/packages/backend/src/controllers/api/v1/admin/roles/update-role.ee.test.js b/packages/backend/src/controllers/api/v1/admin/roles/update-role.ee.test.js new file mode 100644 index 00000000..8d6a3636 --- /dev/null +++ b/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', + }, + }); + }); +}); diff --git a/packages/backend/src/models/permission.js b/packages/backend/src/models/permission.js index 7ebd79c0..6ef9ae0d 100644 --- a/packages/backend/src/models/permission.js +++ b/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; diff --git a/packages/backend/src/models/role.js b/packages/backend/src/models/role.js index 0b0d7852..2c74bdd4 100644 --- a/packages/backend/src/models/role.js +++ b/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; diff --git a/packages/backend/src/routes/api/v1/admin/roles.ee.js b/packages/backend/src/routes/api/v1/admin/roles.ee.js index 6ec88967..d429462d 100644 --- a/packages/backend/src/routes/api/v1/admin/roles.ee.js +++ b/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; diff --git a/packages/backend/test/mocks/rest/api/v1/admin/roles/update-role.ee.js b/packages/backend/test/mocks/rest/api/v1/admin/roles/update-role.ee.js new file mode 100644 index 00000000..164ea4b5 --- /dev/null +++ b/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;