diff --git a/packages/backend/src/controllers/api/v1/admin/roles/delete-role.ee.test.js b/packages/backend/src/controllers/api/v1/admin/roles/delete-role.ee.test.js index a9396bf0..839acc19 100644 --- a/packages/backend/src/controllers/api/v1/admin/roles/delete-role.ee.test.js +++ b/packages/backend/src/controllers/api/v1/admin/roles/delete-role.ee.test.js @@ -92,21 +92,4 @@ describe('DELETE /api/v1/admin/roles/:roleId', () => { }, }); }); - - 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); - }); }); diff --git a/packages/backend/src/models/__snapshots__/role.test.js.snap b/packages/backend/src/models/__snapshots__/role.test.js.snap new file mode 100644 index 00000000..1a06bee0 --- /dev/null +++ b/packages/backend/src/models/__snapshots__/role.test.js.snap @@ -0,0 +1,33 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`Role model > jsonSchema should have correct validations 1`] = ` +{ + "properties": { + "createdAt": { + "type": "string", + }, + "description": { + "maxLength": 255, + "type": [ + "string", + "null", + ], + }, + "id": { + "format": "uuid", + "type": "string", + }, + "name": { + "minLength": 1, + "type": "string", + }, + "updatedAt": { + "type": "string", + }, + }, + "required": [ + "name", + ], + "type": "object", +} +`; diff --git a/packages/backend/src/models/role.js b/packages/backend/src/models/role.js index dec80c5b..d8b353cf 100644 --- a/packages/backend/src/models/role.js +++ b/packages/backend/src/models/role.js @@ -52,56 +52,73 @@ class Role extends Base { return await this.query().findOne({ name: 'Admin' }); } - async updateWithPermissions(data) { - if (this.isAdmin) { + async preventAlteringAdmin() { + const currentRole = await Role.query().findById(this.id); + + if (currentRole.isAdmin) { throw new NotAuthorizedError('The admin role cannot be altered!'); } + } + async deletePermissions() { + return await this.$relatedQuery('permissions').delete(); + } + + async createPermissions(permissions) { + if (permissions?.length) { + const validPermissions = Permission.filter(permissions).map( + (permission) => ({ + ...permission, + roleId: this.id, + }) + ); + + await Permission.query().insert(validPermissions); + } + } + + async updatePermissions(permissions) { + await this.deletePermissions(); + + await this.createPermissions(permissions); + } + + async updateWithPermissions(data) { const { name, description, permissions } = data; - return await Role.transaction(async (trx) => { - await this.$relatedQuery('permissions', trx).delete(); + await this.updatePermissions(permissions); - if (permissions?.length) { - const validPermissions = Permission.filter(permissions).map( - (permission) => ({ - ...permission, - roleId: this.id, - }) - ); - - await Permission.query().insert(validPermissions); - } - - await this.$query(trx).patch({ - name, - description, - }); - - return await this.$query(trx) - .leftJoinRelated({ - permissions: true, - }) - .withGraphFetched({ - permissions: true, - }); + await this.$query().patchAndFetch({ + id: this.id, + name, + description, }); + + return await this.$query() + .leftJoinRelated({ + permissions: true, + }) + .withGraphFetched({ + permissions: true, + }); } async deleteWithPermissions() { - return await Role.transaction(async (trx) => { - await this.$relatedQuery('permissions', trx).delete(); + await this.deletePermissions(); - return await this.$query(trx).delete(); - }); + return await this.$query().delete(); + } + + async $beforeUpdate(opt, queryContext) { + await super.$beforeUpdate(opt, queryContext); + + await this.preventAlteringAdmin(); } async $beforeDelete(queryContext) { await super.$beforeDelete(queryContext); - if (this.isAdmin) { - throw new NotAuthorizedError('The admin role cannot be deleted!'); - } + await this.preventAlteringAdmin(); const userCount = await this.$relatedQuery('users').limit(1).resultSize(); const hasUsers = userCount > 0; diff --git a/packages/backend/src/models/role.test.js b/packages/backend/src/models/role.test.js new file mode 100644 index 00000000..f665713c --- /dev/null +++ b/packages/backend/src/models/role.test.js @@ -0,0 +1,206 @@ +import { describe, it, expect, vi } from 'vitest'; +import Role from './role'; +import Base from './base.js'; +import Permission from './permission.js'; +import User from './user.js'; +import { createRole } from '../../test/factories/role.js'; +import { createPermission } from '../../test/factories/permission.js'; + +describe('Role model', () => { + it('tableName should return correct name', () => { + expect(Role.tableName).toBe('roles'); + }); + + it('jsonSchema should have correct validations', () => { + expect(Role.jsonSchema).toMatchSnapshot(); + }); + + it('relationMappingsshould return correct associations', () => { + const relationMappings = Role.relationMappings(); + + const expectedRelations = { + users: { + relation: Base.HasManyRelation, + modelClass: User, + join: { + from: 'roles.id', + to: 'users.role_id', + }, + }, + permissions: { + relation: Base.HasManyRelation, + modelClass: Permission, + join: { + from: 'roles.id', + to: 'permissions.role_id', + }, + }, + }; + + expect(relationMappings).toStrictEqual(expectedRelations); + }); + + it('virtualAttributes should return correct attributes', () => { + expect(Role.virtualAttributes).toStrictEqual(['isAdmin']); + }); + + describe('isAdmin', () => { + it('should return true for admin named role', () => { + const role = new Role(); + role.name = 'Admin'; + + expect(role.isAdmin).toBe(true); + }); + + it('should return false for not admin named roles', () => { + const role = new Role(); + role.name = 'User'; + + expect(role.isAdmin).toBe(false); + }); + }); + + it('findAdmin should return admin role', async () => { + const createdAdminRole = await createRole({ name: 'Admin' }); + + const adminRole = await Role.findAdmin(); + + expect(createdAdminRole).toStrictEqual(adminRole); + }); + + describe('preventAlteringAdmin', () => { + it('preventAlteringAdmin should throw an error when altering admin role', async () => { + const role = await createRole({ name: 'Admin' }); + + await expect(() => role.preventAlteringAdmin()).rejects.toThrowError( + 'The admin role cannot be altered!' + ); + }); + + it('preventAlteringAdmin should not throw an error when altering non-admin roles', async () => { + const role = await createRole({ name: 'User' }); + + expect(await role.preventAlteringAdmin()).toBe(undefined); + }); + }); + + it("deletePermissions should delete role's permissions", async () => { + const role = await createRole({ name: 'User' }); + await createPermission({ roleId: role.id }); + + await role.deletePermissions(); + + expect(await role.$relatedQuery('permissions')).toStrictEqual([]); + }); + + describe('createPermissions', () => { + it('should create permissions', async () => { + const role = await createRole({ name: 'User' }); + + await role.createPermissions([ + { action: 'read', subject: 'Flow', conditions: [] }, + ]); + + expect(await role.$relatedQuery('permissions')).toMatchObject([ + { + action: 'read', + subject: 'Flow', + conditions: [], + }, + ]); + }); + + it('should call Permission.filter', async () => { + const role = await createRole({ name: 'User' }); + + const permissions = [{ action: 'read', subject: 'Flow', conditions: [] }]; + + const permissionFilterSpy = vi + .spyOn(Permission, 'filter') + .mockReturnValue(permissions); + + await role.createPermissions(permissions); + + expect(permissionFilterSpy).toHaveBeenCalledWith(permissions); + }); + }); + + it('updatePermissions should delete existing permissions and create new permissions', async () => { + const permissionsData = [ + { action: 'read', subject: 'Flow', conditions: [] }, + ]; + + const deletePermissionsSpy = vi + .spyOn(Role.prototype, 'deletePermissions') + .mockResolvedValueOnce(); + const createPermissionsSpy = vi + .spyOn(Role.prototype, 'createPermissions') + .mockResolvedValueOnce(); + + const role = await createRole({ name: 'User' }); + + await role.updatePermissions(permissionsData); + + expect(deletePermissionsSpy.mock.invocationCallOrder[0]).toBeLessThan( + createPermissionsSpy.mock.invocationCallOrder[0] + ); + + expect(deletePermissionsSpy).toHaveBeenNthCalledWith(1); + expect(createPermissionsSpy).toHaveBeenNthCalledWith(1, permissionsData); + }); + + describe('updateWithPermissions', () => { + it('should update role along with given permissions', async () => { + const role = await createRole({ name: 'User' }); + await createPermission({ + roleId: role.id, + subject: 'Flow', + action: 'read', + conditions: [], + }); + + const newRoleData = { + name: 'Updated user', + description: 'Updated description', + permissions: [ + { + action: 'update', + subject: 'Flow', + conditions: [], + }, + ], + }; + + await role.updateWithPermissions(newRoleData); + + const roleWithPermissions = await role + .$query() + .leftJoinRelated({ permissions: true }) + .withGraphFetched({ permissions: true }); + + expect(roleWithPermissions).toMatchObject(newRoleData); + }); + }); + + describe('deleteWithPermissions', () => { + it('should delete role along with given permissions', async () => { + const role = await createRole({ name: 'User' }); + await createPermission({ + roleId: role.id, + subject: 'Flow', + action: 'read', + conditions: [], + }); + + await role.deleteWithPermissions(); + + const refetchedRole = await role.$query(); + const rolePermissions = await Permission.query().where({ + roleId: role.id, + }); + + expect(refetchedRole).toBe(undefined); + expect(rolePermissions).toStrictEqual([]); + }); + }); +});