fix: block adding a root role to a group with a project role (#3775)

src/lib/db/group-store.ts

@@ -287,4 +287,13 @@ export default class GroupStore implements IGroupStore {
             .where('user_id', userId);
         return rows.map(rowToGroup);
     }
+
+    async hasProjectRole(groupId: number): Promise<boolean> {
+        const result = await this.db.raw(
+            `SELECT EXISTS(SELECT 1 FROM ${T.GROUP_ROLE} WHERE group_id = ?) AS present`,
+            [groupId],
+        );
+        const { present } = result.rows[0];
+        return present;
+    }
 }

src/lib/services/group-service.ts

@@ -179,18 +179,28 @@ export class GroupService {
     }
 
     async validateGroup(
-        { name }: IGroupModel,
+        group: IGroupModel,
         existingGroup?: IGroup,
     ): Promise<void> {
-        if (!name) {
+        if (!group.name) {
             throw new BadDataError('Group name cannot be empty');
         }
 
-        if (!existingGroup || existingGroup.name != name) {
-            if (await this.groupStore.existsWithName(name)) {
+        if (!existingGroup || existingGroup.name != group.name) {
+            if (await this.groupStore.existsWithName(group.name)) {
                 throw new NameExistsError('Group name already exists');
             }
         }
+
+        if (
+            group.id &&
+            group.rootRole &&
+            (await this.groupStore.hasProjectRole(group.id))
+        ) {
+            throw new BadDataError(
+                'This group already has a project role and cannot also be given a root role',
+            );
+        }
     }
 
     async getRolesForProject(projectId: string): Promise<IGroupRole[]> {

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

@@ -58,6 +58,8 @@ export interface IGroupStore extends Store<IGroup, number> {
 
     existsWithName(name: string): Promise<boolean>;
 
+    hasProjectRole(groupId: number): Promise<boolean>;
+
     create(group: IStoreGroup): Promise<IGroup>;
 
     count(): Promise<number>;

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

@@ -97,3 +97,30 @@ test('should not remove user from no SSO definition group', async () => {
     expect(groups.length).toBe(1);
     expect(groups[0].name).toEqual('no_mapping_group');
 });
+
+test('adding a root role to a group with a project role should fail', async () => {
+    const group = await groupStore.create({
+        name: 'root_group',
+        description: 'root_group',
+    });
+
+    stores.accessStore.addGroupToRole(group.id, 1, 'test', 'default');
+
+    await expect(() => {
+        return groupService.updateGroup(
+            {
+                id: group.id,
+                name: group.name,
+                users: [],
+                rootRole: 1,
+                createdAt: new Date(),
+                createdBy: 'test',
+            },
+            'test',
+        );
+    }).rejects.toThrow(
+        'This group already has a project role and cannot also be given a root role',
+    );
+
+    expect.assertions(1);
+});

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

@@ -113,4 +113,8 @@ export default class FakeGroupStore implements IGroupStore {
     getGroupsForUser(userId: number): Promise<Group[]> {
         throw new Error('Method not implemented.');
     }
+
+    hasProjectRole(groupId: number): Promise<boolean> {
+        throw new Error('Method not implemented.');
+    }
 }