test(role): write model tests

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",
+}
+`;

packages/backend/src/models/role.js

@@ -52,16 +52,17 @@ class Role extends Base {
     return await this.query().findOne({ name: 'Admin' });
   }
 
-  async updateWithPermissions(data) {
+  preventAlteringAdmin() {
     if (this.isAdmin) {
       throw new NotAuthorizedError('The admin role cannot be altered!');
     }
+  }
 
-    const { name, description, permissions } = data;
+  async deletePermissions(trx) {
-
+    return await this.$relatedQuery('permissions', trx).delete();
-    return await Role.transaction(async (trx) => {
+  }
-      await this.$relatedQuery('permissions', trx).delete();
 
+  async createPermissions(permissions, trx) {
     if (permissions?.length) {
       const validPermissions = Permission.filter(permissions).map(
         (permission) => ({
@@ -70,14 +71,34 @@ class Role extends Base {
         })
       );
 
-        await Permission.query().insert(validPermissions);
+      await Permission.query(trx).insert(validPermissions);
     }
 
+    return this;
+  }
+
+  async overridePermissions(permissions, trx) {
+    await this.deletePermissions(trx);
+
+    await this.createPermissions(permissions, trx);
+
+    return this;
+  }
+
+  async updateWithPermissions(data) {
+    this.preventAlteringAdmin();
+
+    const { name, description, permissions } = data;
+
+    return await Role.transaction(async (trx) => {
+      await this.overridePermissions(permissions, trx);
+
       await this.$query(trx).patch({
         name,
         description,
       });
 
+      // TODO: consider removing returning permissions as they're not utilized
       return await this.$query(trx)
         .leftJoinRelated({
           permissions: true,
@@ -90,19 +111,13 @@ class Role extends Base {
 
   async deleteWithPermissions() {
     return await Role.transaction(async (trx) => {
-      await this.$relatedQuery('permissions', trx).delete();
+      await this.deletePermissions(trx);
 
       return await this.$query(trx).delete();
     });
   }
 
-  async $beforeDelete(queryContext) {
+  async assertNoRoleUserExists() {
-    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;
 
@@ -118,7 +133,9 @@ class Role extends Base {
         type: 'ValidationError',
       });
     }
+  }
 
+  async assertNoConfigurationUsage() {
     const samlAuthProviderUsingDefaultRole = await SamlAuthProvider.query()
       .where({
         default_role_id: this.id,
@@ -140,6 +157,20 @@ class Role extends Base {
       });
     }
   }
+
+  async assertRoleIsNotUsed() {
+    await this.assertNoRoleUserExists();
+
+    await this.assertNoConfigurationUsage();
+  }
+
+  async $beforeDelete(queryContext) {
+    await super.$beforeDelete(queryContext);
+
+    this.preventAlteringAdmin();
+
+    await this.assertRoleIsNotUsed();
+  }
 }
 
 export default Role;

packages/backend/src/models/role.test.js

@@ -0,0 +1,432 @@
+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';
+import { createUser } from '../../test/factories/user.js';
+import { createSamlAuthProvider } from '../../test/factories/saml-auth-provider.ee.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', () => {
+      const role = new Role();
+      role.name = 'Admin';
+
+      expect(() => role.preventAlteringAdmin()).toThrowError(
+        'The admin role cannot be altered!'
+      );
+    });
+
+    it('preventAlteringAdmin should not throw an error when altering non-admin roles', () => {
+      const role = new Role();
+      role.name = 'User';
+
+      expect(() => role.preventAlteringAdmin()).not.toThrowError();
+    });
+  });
+
+  describe('deletePermissions', () => {
+    it("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([]);
+    });
+
+    it('should accept transaction', async () => {
+      const transaction = vi.fn();
+
+      const relatedQuerySpy = vi
+        .spyOn(Role.prototype, '$relatedQuery')
+        .mockReturnValue({ delete: () => {} });
+
+      const role = await createRole({ name: 'User' });
+
+      await role.deletePermissions(transaction);
+
+      expect(relatedQuerySpy).toHaveBeenCalledWith('permissions', transaction);
+      1;
+    });
+  });
+
+  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('should accept transaction', async () => {
+      const transaction = vi.fn();
+
+      const permissionQuerySpy = vi
+        .spyOn(Permission, 'query')
+        .mockReturnValue({ insert: () => {} });
+
+      const role = await createRole({ name: 'User' });
+
+      await role.createPermissions(
+        [{ action: 'read', subject: 'Flow', conditions: [] }],
+        transaction
+      );
+
+      expect(permissionQuerySpy).toHaveBeenCalledWith(transaction);
+      1;
+    });
+  });
+
+  it('overridePermissions should delete existing permissions and create new permissions', async () => {
+    const permissionsData = [
+      { action: 'read', subject: 'Flow', conditions: [] },
+    ];
+
+    const transaction = vi.fn();
+    const deletePermissionsSpy = vi
+      .spyOn(Role.prototype, 'deletePermissions')
+      .mockResolvedValueOnce();
+    const createPermissionsSpy = vi
+      .spyOn(Role.prototype, 'createPermissions')
+      .mockResolvedValueOnce();
+
+    const role = await createRole({ name: 'User' });
+
+    await role.overridePermissions(permissionsData, transaction);
+
+    expect(deletePermissionsSpy.mock.invocationCallOrder[0]).toBeLessThan(
+      createPermissionsSpy.mock.invocationCallOrder[0]
+    );
+
+    expect(deletePermissionsSpy).toHaveBeenNthCalledWith(1, transaction);
+    expect(createPermissionsSpy).toHaveBeenNthCalledWith(
+      1,
+      permissionsData,
+      transaction
+    );
+  });
+
+  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);
+    });
+
+    it('should throw an error while updating the admin role', async () => {
+      const role = new Role();
+      role.name = 'Admin';
+
+      await expect(() => role.updateWithPermissions()).rejects.toThrowError(
+        'The admin role cannot be altered!'
+      );
+    });
+
+    it('should use transaction', async () => {
+      const transaction = vi.fn();
+      const overridePermissionsSpy = vi
+        .spyOn(Role.prototype, 'overridePermissions')
+        .mockResolvedValue();
+
+      const querySpy = vi.spyOn(Role.prototype, '$query').mockReturnValue({
+        patch: vi.fn().mockReturnValue(Promise.resolve()),
+        leftJoinRelated: vi.fn().mockReturnThis(),
+        withGraphFetched: vi.fn().mockResolvedValue({}),
+      });
+
+      const transactionSpy = vi
+        .spyOn(Role, 'transaction')
+        .mockImplementation(async (callback) => {
+          return await callback(transaction);
+        });
+      const role = await createRole({ name: 'User' });
+
+      const newRoleData = {
+        name: 'New user',
+        description: 'Updated user role',
+        permissions: [{ action: 'read', subject: 'Flow', conditions: [] }],
+      };
+
+      await role.updateWithPermissions(newRoleData);
+
+      expect(transactionSpy).toHaveBeenCalledOnce();
+
+      expect(overridePermissionsSpy).toHaveBeenCalledWith(
+        newRoleData.permissions,
+        transaction
+      );
+
+      expect(querySpy).toHaveBeenNthCalledWith(2, transaction);
+    });
+
+    it('should revert changes when an error occurs', async () => {
+      const role = await createRole({ name: 'User' });
+      await createPermission({
+        roleId: role.id,
+        subject: 'Flow',
+        action: 'read',
+        conditions: [],
+      });
+
+      const roleWithPermissions = await role
+        .$query()
+        .leftJoinRelated({ permissions: true })
+        .withGraphFetched({ permissions: true });
+
+      await expect(() =>
+        role.updateWithPermissions({
+          name: false,
+          description: 123,
+        })
+      ).rejects.toThrowError(
+        'name: must be string, description: must be string,null'
+      );
+
+      const refetchedRoleWithPermissions = await role
+        .$query()
+        .leftJoinRelated({ permissions: true })
+        .withGraphFetched({ permissions: true });
+
+      expect(roleWithPermissions).toStrictEqual(refetchedRoleWithPermissions);
+    });
+  });
+
+  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([]);
+    });
+
+    it('should use transaction', async () => {
+      const transaction = vi.fn();
+      const deletePermissionsSpy = vi
+        .spyOn(Role.prototype, 'deletePermissions')
+        .mockResolvedValue();
+
+      const querySpy = vi.spyOn(Role.prototype, '$query').mockReturnValue({
+        delete: vi.fn().mockReturnValue(Promise.resolve()),
+      });
+
+      const transactionSpy = vi
+        .spyOn(Role, 'transaction')
+        .mockImplementation(async (callback) => {
+          return await callback(transaction);
+        });
+      const role = await createRole({ name: 'User' });
+
+      await role.deleteWithPermissions();
+
+      expect(transactionSpy).toHaveBeenCalledOnce();
+
+      expect(deletePermissionsSpy).toHaveBeenCalledWith(transaction);
+
+      expect(querySpy).toHaveBeenCalledWith(transaction);
+    });
+  });
+
+  describe('assertNoRoleUserExists', () => {
+    it('should reject with an error when the role has users', async () => {
+      const role = await createRole({ name: 'User' });
+      await createUser({ roleId: role.id });
+
+      await expect(() => role.assertNoRoleUserExists()).rejects.toThrowError(
+        `All users must be migrated away from the "User" role.`
+      );
+    });
+
+    it('should resolve when the role does not have any users', async () => {
+      const role = await createRole();
+
+      expect(await role.assertNoRoleUserExists()).toBe(undefined);
+    });
+  });
+
+  describe('assertNoConfigurationUsage', () => {
+    it('should reject with an error when the role is used in configuration', async () => {
+      const role = await createRole();
+      await createSamlAuthProvider({ defaultRoleId: role.id });
+
+      await expect(() =>
+        role.assertNoConfigurationUsage()
+      ).rejects.toThrowError(
+        'samlAuthProvider: You need to change the default role in the SAML configuration before deleting this role.'
+      );
+    });
+
+    it('should resolve when the role does not have any users', async () => {
+      const role = await createRole();
+
+      expect(await role.assertNoConfigurationUsage()).toBe(undefined);
+    });
+  });
+
+  it('assertRoleIsNotUsed should call assertNoRoleUserExists and assertNoConfigurationUsage', async () => {
+    const role = new Role();
+
+    const assertNoRoleUserExistsSpy = vi
+      .spyOn(role, 'assertNoRoleUserExists')
+      .mockResolvedValue();
+
+    const assertNoConfigurationUsageSpy = vi
+      .spyOn(role, 'assertNoConfigurationUsage')
+      .mockResolvedValue();
+
+    await role.assertRoleIsNotUsed();
+
+    expect(assertNoRoleUserExistsSpy).toHaveBeenCalledOnce();
+    expect(assertNoConfigurationUsageSpy).toHaveBeenCalledOnce();
+  });
+
+  describe('$beforeDelete', () => {
+    it('should call preventAlteringAdmin', async () => {
+      const role = await createRole({ name: 'User' });
+
+      const preventAlteringAdminSpy = vi
+        .spyOn(role, 'preventAlteringAdmin')
+        .mockResolvedValue();
+
+      await role.$query().delete();
+
+      expect(preventAlteringAdminSpy).toHaveBeenCalledOnce();
+    });
+
+    it('should call assertRoleIsNotUsed', async () => {
+      const role = await createRole({ name: 'User' });
+
+      const assertRoleIsNotUsedSpy = vi
+        .spyOn(role, 'assertRoleIsNotUsed')
+        .mockResolvedValue();
+
+      await role.$query().delete();
+
+      expect(assertRoleIsNotUsedSpy).toHaveBeenCalledOnce();
+    });
+  });
+});