feat: report users on group change (#5445)

## About the changes
Add user ids to group changes. This also modifies the payload of group created to include only the user id and creates events for SSO sync functionality

src/lib/services/group-service.ts

@@ -11,7 +11,14 @@ import { IUnleashConfig, IUnleashStores } from '../types';
 import { IGroupStore } from '../types/stores/group-store';
 import { Logger } from '../logger';
 import BadDataError from '../error/bad-data-error';
-import { GROUP_CREATED, GROUP_DELETED, GROUP_UPDATED } from '../types/events';
+import {
+    GROUP_CREATED,
+    GROUP_DELETED,
+    GROUP_UPDATED,
+    GROUP_USER_ADDED,
+    GROUP_USER_REMOVED,
+    IBaseEvent,
+} from '../types/events';
 import NameExistsError from '../error/name-exists-error';
 import { IAccountStore } from '../types/stores/account-store';
 import { IUser } from '../types/user';
@@ -92,25 +99,28 @@ export class GroupService {
 
         const newGroup = await this.groupStore.create(group);
 
-        await this.groupStore.addUsersToGroup(
-            newGroup.id,
-            group.users,
-            userName,
-        );
+        if (group.users) {
+            await this.groupStore.addUsersToGroup(
+                newGroup.id,
+                group.users,
+                userName,
+            );
+        }
 
+        const newUserIds = group.users?.map((g) => g.user.id);
         await this.eventService.storeEvent({
             type: GROUP_CREATED,
             createdBy: userName,
-            data: group,
+            data: { ...group, users: newUserIds },
         });
 
         return newGroup;
     }
 
     async updateGroup(group: IGroupModel, userName: string): Promise<IGroup> {
-        const preData = await this.groupStore.get(group.id);
+        const existingGroup = await this.groupStore.get(group.id);
 
-        await this.validateGroup(group, preData);
+        await this.validateGroup(group, existingGroup);
 
         const newGroup = await this.groupStore.update(group);
 
@@ -135,11 +145,12 @@ export class GroupService {
             userName,
         );
 
+        const newUserIds = group.users.map((g) => g.user.id);
         await this.eventService.storeEvent({
             type: GROUP_UPDATED,
             createdBy: userName,
-            data: newGroup,
-            preData,
+            data: { ...newGroup, users: newUserIds },
+            preData: { ...existingGroup, users: existingUserIds },
         });
 
         return newGroup;
@@ -175,12 +186,17 @@ export class GroupService {
     async deleteGroup(id: number, userName: string): Promise<void> {
         const group = await this.groupStore.get(id);
 
+        const existingUsers = await this.groupStore.getAllUsersByGroups([
+            group.id,
+        ]);
+        const existingUserIds = existingUsers.map((g) => g.userId);
+
         await this.groupStore.delete(id);
 
         await this.eventService.storeEvent({
             type: GROUP_DELETED,
             createdBy: userName,
-            preData: group,
+            preData: { ...group, users: existingUserIds },
         });
     }
 
@@ -246,6 +262,31 @@ export class GroupService {
                 externalGroups,
             );
             await this.groupStore.deleteUsersFromGroup(oldGroups);
+
+            const events: IBaseEvent[] = [];
+            for (const group of newGroups) {
+                events.push({
+                    type: GROUP_USER_ADDED,
+                    createdBy: createdBy ?? 'unknown',
+                    data: {
+                        groupId: group.id,
+                        userId,
+                    },
+                });
+            }
+
+            for (const group of oldGroups) {
+                events.push({
+                    type: GROUP_USER_REMOVED,
+                    createdBy: createdBy ?? 'unknown',
+                    preData: {
+                        groupId: group.groupId,
+                        userId,
+                    },
+                });
+            }
+
+            await this.eventService.storeEvents(events);
         }
     }
 

src/lib/types/events.ts

@@ -105,6 +105,8 @@ export const SEGMENT_DELETED = 'segment-deleted' as const;
 export const GROUP_CREATED = 'group-created' as const;
 export const GROUP_UPDATED = 'group-updated' as const;
 export const GROUP_DELETED = 'group-deleted' as const;
+export const GROUP_USER_ADDED = 'group-user-added' as const;
+export const GROUP_USER_REMOVED = 'group-user-removed' as const;
 export const SETTING_CREATED = 'setting-created' as const;
 export const SETTING_UPDATED = 'setting-updated' as const;
 export const SETTING_DELETED = 'setting-deleted' as const;
@@ -252,6 +254,8 @@ export const IEventTypes = [
     GROUP_CREATED,
     GROUP_UPDATED,
     GROUP_DELETED,
+    GROUP_USER_ADDED,
+    GROUP_USER_REMOVED,
     SETTING_CREATED,
     SETTING_UPDATED,
     SETTING_DELETED,

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

@@ -51,6 +51,13 @@ afterAll(async () => {
 
 afterEach(async () => {});
 
+// Note: events come in reverse order, the first in the list is the last pushed
+const getTestEvents = async () => {
+    return (await eventService.getEvents()).events.filter(
+        (e) => e.createdBy !== 'migration',
+    );
+};
+
 test('should have three group', async () => {
     const project = await groupService.getAll();
     expect(project.length).toBe(3);
@@ -64,36 +71,101 @@ test('should add person to 2 groups', async () => {
     );
     const groups = await groupService.getGroupsForUser(user.id);
     expect(groups.length).toBe(2);
+    const events = await getTestEvents();
+    expect(events[0]).toMatchObject({
+        type: 'group-user-added',
+        data: {
+            userId: user.id,
+            groupId: groups[1].id,
+        },
+    });
+    expect(events[1]).toMatchObject({
+        type: 'group-user-added',
+        data: {
+            userId: user.id,
+            groupId: groups[0].id,
+        },
+    });
 });
 
+// this test depends on the other tests being executed
 test('should remove person from one group', async () => {
+    const removedGroups = (await groupService.getGroupsForUser(user.id)).filter(
+        (g) => !g.mappingsSSO?.includes('maintainer'),
+    );
     await groupService.syncExternalGroups(user.id, ['maintainer'], 'SSO');
     const groups = await groupService.getGroupsForUser(user.id);
     expect(groups.length).toBe(1);
     expect(groups[0].name).toEqual('maintainer_group');
+
+    expect(removedGroups).toHaveLength(1);
+    const events = await getTestEvents();
+    expect(events[0]).toMatchObject({
+        type: 'group-user-removed',
+        preData: {
+            userId: user.id,
+            groupId: removedGroups[0].id,
+        },
+    });
 });
 
+// this test depends on the other tests being executed
 test('should add person to completely new group with new name', async () => {
+    const removedGroups = (await groupService.getGroupsForUser(user.id)).filter(
+        (g) => !g.mappingsSSO?.includes('dev'),
+    );
     await groupService.syncExternalGroups(user.id, ['dev'], 'SSO');
     const groups = await groupService.getGroupsForUser(user.id);
     expect(groups.length).toBe(1);
     expect(groups[0].name).toEqual('dev_group');
+
+    const events = await getTestEvents();
+    expect(removedGroups).toHaveLength(1);
+    expect(events[0]).toMatchObject({
+        type: 'group-user-removed',
+        preData: {
+            userId: user.id,
+            groupId: removedGroups[0].id,
+        },
+    });
+    expect(events[1]).toMatchObject({
+        type: 'group-user-added',
+        data: {
+            userId: user.id,
+            groupId: groups[0].id,
+        },
+    });
 });
 
 test('should not update groups when not string array ', async () => {
+    const beforeEvents = await getTestEvents();
     await groupService.syncExternalGroups(user.id, 'Everyone' as any, 'SSO');
     const groups = await groupService.getGroupsForUser(user.id);
     expect(groups.length).toBe(1);
     expect(groups[0].name).toEqual('dev_group');
+    const afterEvents = await getTestEvents();
+    expect(beforeEvents).toHaveLength(afterEvents.length);
 });
 
+// this test depends on the other tests being executed
 test('should clear groups when empty array ', async () => {
+    const removedGroups = await groupService.getGroupsForUser(user.id);
     await groupService.syncExternalGroups(user.id, [], 'SSO');
     const groups = await groupService.getGroupsForUser(user.id);
     expect(groups.length).toBe(0);
+    expect(removedGroups).toHaveLength(1);
+    const events = await getTestEvents();
+    expect(events[0]).toMatchObject({
+        type: 'group-user-removed',
+        preData: {
+            userId: user.id,
+            groupId: removedGroups[0].id,
+        },
+    });
 });
 
 test('should not remove user from no SSO definition group', async () => {
+    const beforeEvents = await getTestEvents();
     const group = await groupStore.create({
         name: 'no_mapping_group',
         description: 'no_mapping_group',
@@ -103,13 +175,18 @@ test('should not remove user from no SSO definition group', async () => {
     const groups = await groupService.getGroupsForUser(user.id);
     expect(groups.length).toBe(1);
     expect(groups[0].name).toEqual('no_mapping_group');
+    const afterEvents = await getTestEvents();
+    expect(beforeEvents).toHaveLength(afterEvents.length);
 });
 
 test('adding a root role to a group with a project role should not fail', async () => {
-    const group = await groupStore.create({
-        name: 'root_group',
-        description: 'root_group',
-    });
+    const group = await groupService.createGroup(
+        {
+            name: 'root_group',
+            description: 'root_group',
+        },
+        'test',
+    );
 
     await stores.accessStore.addGroupToRole(group.id, 1, 'test', 'default');
 
@@ -133,7 +210,33 @@ test('adding a root role to a group with a project role should not fail', async
         rootRole: 1,
     });
 
-    expect.assertions(1);
+    const events = await getTestEvents();
+    expect(events[1]).toMatchObject({
+        type: 'group-created',
+        data: {
+            description: group.description,
+            name: group.name,
+        },
+    });
+    expect(events[0]).toMatchObject({
+        type: 'group-updated',
+        data: {
+            description: group.description,
+            id: group.id,
+            mappingsSSO: [],
+            name: group.name,
+            rootRole: 1,
+        },
+        preData: {
+            description: group.description,
+            id: group.id,
+            mappingsSSO: [],
+            name: group.name,
+            rootRole: null,
+        },
+    });
+
+    expect.assertions(3);
 });
 
 test('adding a nonexistent role to a group should fail', async () => {