fix: deletion validation didnt account for groups (#4441)

<!-- Thanks for creating a PR! To make it easier for reviewers and
everyone else to understand what your changes relate to, please add some
relevant content to the headings below. Feel free to ignore or delete
sections that you don't think are relevant. Thank you! ❤️ -->

## About the changes
<!-- Describe the changes introduced. What are they and why are they
being introduced? Feel free to also add screenshots or steps to view the
changes if they're visual. -->

Fixes an issue where project role deletion validation didn't validate
against project roles being connected to groups

src/lib/db/access-store.ts

@@ -296,6 +296,14 @@ export class AccessStore implements IAccessStore {
         return rows.map((r) => r.user_id);
     }
 
+    async getGroupIdsForRole(roleId: number): Promise<number[]> {
+        const rows = await this.db
+            .select(['group_id'])
+            .from<IRole>(T.GROUP_ROLE)
+            .where('role_id', roleId);
+        return rows.map((r) => r.group_id);
+    }
+
     async addUserToRole(
         userId: number,
         roleId: number,

src/lib/features/access/createAccessService.ts

@@ -30,7 +30,7 @@ export const createAccessService = (
     );
 
     return new AccessService(
-        { accessStore, accountStore, roleStore, environmentStore },
+        { accessStore, accountStore, roleStore, environmentStore, groupStore },
         { getLogger, flagResolver },
         groupService,
     );
@@ -52,7 +52,7 @@ export const createFakeAccessService = (
     );
 
     return new AccessService(
-        { accessStore, accountStore, roleStore, environmentStore },
+        { accessStore, accountStore, roleStore, environmentStore, groupStore },
         { getLogger, flagResolver },
         groupService,
     );

src/lib/features/feature-toggle/createFeatureToggleService.ts

@@ -86,7 +86,7 @@ export const createFeatureToggleService = (
         { getLogger },
     );
     const accessService = new AccessService(
-        { accessStore, accountStore, roleStore, environmentStore },
+        { accessStore, accountStore, roleStore, environmentStore, groupStore },
         { getLogger, flagResolver },
         groupService,
     );
@@ -136,7 +136,7 @@ export const createFakeFeatureToggleService = (
         { getLogger },
     );
     const accessService = new AccessService(
-        { accessStore, accountStore, roleStore, environmentStore },
+        { accessStore, accountStore, roleStore, environmentStore, groupStore },
         { getLogger, flagResolver },
         groupService,
     );

src/lib/services/access-service.test.ts

@@ -1,9 +1,18 @@
 import NameExistsError from '../error/name-exists-error';
 import getLogger from '../../test/fixtures/no-logger';
 import { createFakeAccessService } from '../features/access/createAccessService';
-import { IRoleValidation } from './access-service';
+import { AccessService, IRoleValidation } from './access-service';
 import { createTestConfig } from '../../test/config/test-config';
 import { CUSTOM_ROOT_ROLE_TYPE } from '../util/constants';
+import FakeGroupStore from '../../test/fixtures/fake-group-store';
+import { FakeAccountStore } from '../../test/fixtures/fake-account-store';
+import FakeRoleStore from '../../test/fixtures/fake-role-store';
+import FakeEnvironmentStore from '../../test/fixtures/fake-environment-store';
+import AccessStoreMock from '../../test/fixtures/fake-access-store';
+import { GroupService } from '../services/group-service';
+import FakeEventStore from '../../test/fixtures/fake-event-store';
+import { IRole } from 'lib/types/stores/access-store';
+import { IGroup } from 'lib/types';
 
 function getSetup(customRootRoles: boolean = false) {
     const config = createTestConfig({
@@ -169,3 +178,58 @@ test('user with custom root role should get a user root role', async () => {
     expect(roles).toHaveLength(1);
     expect(roles[0].name).toBe('custom-root-role');
 });
+
+test('throws error when trying to delete a project role in use by group', async () => {
+    const groupIdResultOverride = async (): Promise<number[]> => {
+        return [1];
+    };
+    const config = createTestConfig({
+        getLogger,
+        experimental: {
+            flags: {
+                customRootRoles: false,
+            },
+        },
+    });
+
+    const eventStore = new FakeEventStore();
+    const groupStore = new FakeGroupStore();
+    groupStore.getAllWithId = async (): Promise<IGroup[]> => {
+        return [{ name: 'group' }];
+    };
+    const accountStore = new FakeAccountStore();
+    const roleStore = new FakeRoleStore();
+    const environmentStore = new FakeEnvironmentStore();
+    const accessStore = new AccessStoreMock();
+    accessStore.getGroupIdsForRole = groupIdResultOverride;
+    accessStore.getUserIdsForRole = async (): Promise<number[]> => {
+        return [];
+    };
+    accessStore.get = async (): Promise<IRole> => {
+        return { id: 1, type: 'custom', name: 'project role' };
+    };
+    const groupService = new GroupService(
+        { groupStore, eventStore, accountStore },
+        { getLogger },
+    );
+
+    const accessService = new AccessService(
+        {
+            accessStore,
+            accountStore,
+            roleStore,
+            environmentStore,
+            groupStore,
+        },
+        config,
+        groupService,
+    );
+
+    try {
+        await accessService.deleteRole(1);
+    } catch (e) {
+        expect(e.toString()).toBe(
+            'RoleInUseError: Role is in use by users(0) or groups(1). You cannot delete a role that is in use without first removing the role from the users and groups.',
+        );
+    }
+});

src/lib/services/access-service.ts

@@ -10,7 +10,7 @@ import {
     IUserRole,
 } from '../types/stores/access-store';
 import { Logger } from '../logger';
-import { IAccountStore, IUnleashStores } from '../types/stores';
+import { IAccountStore, IGroupStore, IUnleashStores } from '../types/stores';
 import {
     IAvailablePermissions,
     ICustomRole,
@@ -34,7 +34,7 @@ import {
 import { DEFAULT_PROJECT } from '../types/project';
 import InvalidOperationError from '../error/invalid-operation-error';
 import BadDataError from '../error/bad-data-error';
-import { IGroupModelWithProjectRole } from '../types/group';
+import { IGroup, IGroupModelWithProjectRole } from '../types/group';
 import { GroupService } from './group-service';
 import { IFlagResolver, IUnleashConfig } from 'lib/types';
 
@@ -80,6 +80,8 @@ export class AccessService {
 
     private groupService: GroupService;
 
+    private groupStore: IGroupStore;
+
     private environmentStore: IEnvironmentStore;
 
     private logger: Logger;
@@ -92,9 +94,14 @@ export class AccessService {
             accountStore,
             roleStore,
             environmentStore,
+            groupStore,
         }: Pick<
             IUnleashStores,
-            'accessStore' | 'accountStore' | 'roleStore' | 'environmentStore'
+            | 'accessStore'
+            | 'accountStore'
+            | 'roleStore'
+            | 'environmentStore'
+            | 'groupStore'
         >,
         {
             getLogger,
@@ -109,6 +116,7 @@ export class AccessService {
         this.environmentStore = environmentStore;
         this.logger = getLogger('/services/access-service.ts');
         this.flagResolver = flagResolver;
+        this.groupStore = groupStore;
     }
 
     /**
@@ -397,6 +405,14 @@ export class AccessService {
         return [];
     }
 
+    async getGroupsForRole(roleId: number): Promise<IGroup[]> {
+        const groupdIdList = await this.store.getGroupIdsForRole(roleId);
+        if (groupdIdList.length > 0) {
+            return this.groupStore.getAllWithId(groupdIdList);
+        }
+        return [];
+    }
+
     async getProjectUsersForRole(
         roleId: number,
         projectId?: string,
@@ -578,10 +594,11 @@ export class AccessService {
         await this.validateRoleIsNotBuiltIn(id);
 
         const roleUsers = await this.getUsersForRole(id);
+        const roleGroups = await this.getGroupsForRole(id);
 
-        if (roleUsers.length > 0) {
+        if (roleUsers.length > 0 || roleGroups.length > 0) {
             throw new RoleInUseError(
-                'Role is in use by more than one user. You cannot delete a role that is in use without first removing the role from the users.',
+                `Role is in use by users(${roleUsers.length}) or groups(${roleGroups.length}). You cannot delete a role that is in use without first removing the role from the users and groups.`,
             );
         }
 

src/lib/types/stores/access-store.ts

@@ -68,6 +68,8 @@ export interface IAccessStore extends Store<IRole, number> {
 
     getUserIdsForRole(roleId: number, projectId?: string): Promise<number[]>;
 
+    getGroupIdsForRole(roleId: number, projectId?: string): Promise<number[]>;
+
     wipePermissionsFromRole(role_id: number): Promise<void>;
 
     addEnvironmentPermissionsToRole(

src/test/e2e/services/access-service.e2e.test.ts

@@ -735,7 +735,7 @@ test('Should be denied access to delete a role that is in use', async () => {
         await accessService.deleteRole(customRole.id);
     } catch (e) {
         expect(e.toString()).toBe(
-            'RoleInUseError: Role is in use by more than one user. You cannot delete a role that is in use without first removing the role from the users.',
+            'RoleInUseError: Role is in use by users(1) or groups(0). You cannot delete a role that is in use without first removing the role from the users and groups.',
         );
     }
 });

src/test/fixtures/access-service-mock.ts

@@ -19,6 +19,7 @@ class AccessServiceMock extends AccessService {
                 accountStore: undefined,
                 roleStore: undefined,
                 environmentStore: undefined,
+                groupStore: undefined,
             },
             { getLogger: noLoggerProvider, flagResolver: undefined },
             undefined,

src/test/fixtures/fake-access-store.ts

@@ -146,6 +146,10 @@ class AccessStoreMock implements IAccessStore {
         throw new Error('Method not implemented.');
     }
 
+    getGroupIdsForRole(roleId: number, projectId?: string): Promise<number[]> {
+        throw new Error('Method not implemented.');
+    }
+
     addUserToRole(userId: number, roleId: number): Promise<void> {
         this.userToRoleMap.set(userId, roleId);
         return Promise.resolve(undefined);