From efbec719dea11a688f729850184e05f4d850da46 Mon Sep 17 00:00:00 2001
From: Simon Hornby <liquidwicked64@gmail.com>
Date: Tue, 16 May 2023 12:11:32 +0200
Subject: [PATCH] fix: block adding a root role to a group with a project role
 (#3775)

---
 src/lib/db/group-store.ts                     |  9 +++++++
 src/lib/services/group-service.ts             | 18 ++++++++++---
 src/lib/types/stores/group-store.ts           |  2 ++
 .../e2e/services/group-service.e2e.test.ts    | 27 +++++++++++++++++++
 src/test/fixtures/fake-group-store.ts         |  4 +++
 5 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/src/lib/db/group-store.ts b/src/lib/db/group-store.ts
index b7e2d203d2..78913bd62c 100644
--- a/src/lib/db/group-store.ts
+++ b/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;
+    }
 }
diff --git a/src/lib/services/group-service.ts b/src/lib/services/group-service.ts
index d1e2600b5a..d7c646f5eb 100644
--- a/src/lib/services/group-service.ts
+++ b/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[]> {
diff --git a/src/lib/types/stores/group-store.ts b/src/lib/types/stores/group-store.ts
index 5fdf51089a..2c955221e6 100644
--- a/src/lib/types/stores/group-store.ts
+++ b/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>;
diff --git a/src/test/e2e/services/group-service.e2e.test.ts b/src/test/e2e/services/group-service.e2e.test.ts
index 0b23591bff..5048df20da 100644
--- a/src/test/e2e/services/group-service.e2e.test.ts
+++ b/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);
+});
diff --git a/src/test/fixtures/fake-group-store.ts b/src/test/fixtures/fake-group-store.ts
index 47473187eb..59bc9f2b1e 100644
--- a/src/test/fixtures/fake-group-store.ts
+++ b/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.');
+    }
 }