chore: sync user groups is a system action (#7214)

## About the changes
After an internal conversation, we concluded that syncExternalGroups is
an action that Unleash performs as a system, not something triggered by
the user. We keep the method and just write the event log that the
action was performed by the system user.

src/lib/services/group-service.ts

@@ -10,6 +10,7 @@ import type {
 import {
     GroupDeletedEvent,
     GroupUpdatedEvent,
+    SYSTEM_USER_AUDIT,
     type IAuditUser,
     type IUnleashConfig,
     type IUnleashStores,
@@ -19,8 +20,6 @@ import type { Logger } from '../logger';
 import BadDataError from '../error/bad-data-error';
 import {
     GROUP_CREATED,
-    GROUP_USER_ADDED,
-    GROUP_USER_REMOVED,
     GroupUserAdded,
     GroupUserRemoved,
     type IBaseEvent,
@@ -238,62 +237,11 @@ export class GroupService {
         return this.groupStore.getProjectGroupRoles(projectId);
     }
 
-    /** @deprecated use syncExternalGroupsWithAudit */
     async syncExternalGroups(
         userId: number,
         externalGroups: string[],
-        createdBy?: string,
+        createdBy?: string, // deprecated
-        createdByUserId?: number,
+        createdByUserId?: number, // deprecated
-    ): Promise<void> {
-        if (Array.isArray(externalGroups)) {
-            const newGroups = await this.groupStore.getNewGroupsForExternalUser(
-                userId,
-                externalGroups,
-            );
-            await this.groupStore.addUserToGroups(
-                userId,
-                newGroups.map((g) => g.id),
-                createdBy,
-            );
-            const oldGroups = await this.groupStore.getOldGroupsForExternalUser(
-                userId,
-                externalGroups,
-            );
-            await this.groupStore.deleteUsersFromGroup(oldGroups);
-
-            const events: IBaseEvent[] = [];
-            for (const group of newGroups) {
-                events.push({
-                    type: GROUP_USER_ADDED,
-                    createdBy: createdBy ?? 'unknown',
-                    createdByUserId: createdByUserId ?? -9999,
-                    data: {
-                        groupId: group.id,
-                        userId,
-                    },
-                });
-            }
-
-            for (const group of oldGroups) {
-                events.push({
-                    type: GROUP_USER_REMOVED,
-                    createdBy: createdBy ?? 'unknown',
-                    createdByUserId: createdByUserId ?? -9999,
-                    preData: {
-                        groupId: group.groupId,
-                        userId,
-                    },
-                });
-            }
-
-            await this.eventService.storeEvents(events);
-        }
-    }
-
-    async syncExternalGroupsWithAudit(
-        userId: number,
-        externalGroups: string[],
-        auditUser: IAuditUser,
     ): Promise<void> {
         if (Array.isArray(externalGroups)) {
             const newGroups = await this.groupStore.getNewGroupsForExternalUser(
@@ -317,7 +265,7 @@ export class GroupService {
                     new GroupUserAdded({
                         userId,
                         groupId: group.id,
-                        auditUser,
+                        auditUser: SYSTEM_USER_AUDIT,
                     }),
                 );
             }
@@ -327,7 +275,7 @@ export class GroupService {
                     new GroupUserRemoved({
                         userId,
                         groupId: group.groupId,
-                        auditUser,
+                        auditUser: SYSTEM_USER_AUDIT,
                     }),
                 );
             }

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

@@ -8,7 +8,6 @@ import {
     type IUnleashStores,
     type IUser,
     TEST_AUDIT_USER,
-    SYSTEM_USER_AUDIT,
 } from '../../../lib/types';
 
 let stores: IUnleashStores;
@@ -70,11 +69,7 @@ test('should have three group', async () => {
 });
 
 test('should add person to 2 groups', async () => {
-    await groupService.syncExternalGroupsWithAudit(
+    await groupService.syncExternalGroups(user.id, ['dev', 'maintainer']);
-        user.id,
-        ['dev', 'maintainer'],
-        SYSTEM_USER_AUDIT,
-    );
     const groups = await groupService.getGroupsForUser(user.id);
     expect(groups.length).toBe(2);
     const events = await getTestEvents();
@@ -99,11 +94,7 @@ test('should remove person from one group', async () => {
     const removedGroups = (await groupService.getGroupsForUser(user.id)).filter(
         (g) => !g.mappingsSSO?.includes('maintainer'),
     );
-    await groupService.syncExternalGroupsWithAudit(
+    await groupService.syncExternalGroups(user.id, ['maintainer']);
-        user.id,
-        ['maintainer'],
-        SYSTEM_USER_AUDIT,
-    );
     const groups = await groupService.getGroupsForUser(user.id);
     expect(groups.length).toBe(1);
     expect(groups[0].name).toEqual('maintainer_group');
@@ -124,11 +115,7 @@ 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.syncExternalGroupsWithAudit(
+    await groupService.syncExternalGroups(user.id, ['dev']);
-        user.id,
-        ['dev'],
-        SYSTEM_USER_AUDIT,
-    );
     const groups = await groupService.getGroupsForUser(user.id);
     expect(groups.length).toBe(1);
     expect(groups[0].name).toEqual('dev_group');
@@ -153,11 +140,7 @@ test('should add person to completely new group with new name', async () => {
 
 test('should not update groups when not string array ', async () => {
     const beforeEvents = await getTestEvents();
-    await groupService.syncExternalGroupsWithAudit(
+    await groupService.syncExternalGroups(user.id, 'Everyone' as any);
-        user.id,
-        'Everyone' as any,
-        SYSTEM_USER_AUDIT,
-    );
     const groups = await groupService.getGroupsForUser(user.id);
     expect(groups.length).toBe(1);
     expect(groups[0].name).toEqual('dev_group');
@@ -168,11 +151,7 @@ test('should not update groups when not string array ', async () => {
 // 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.syncExternalGroupsWithAudit(
+    await groupService.syncExternalGroups(user.id, []);
-        user.id,
-        [],
-        SYSTEM_USER_AUDIT,
-    );
     const groups = await groupService.getGroupsForUser(user.id);
     expect(groups.length).toBe(0);
     expect(removedGroups).toHaveLength(1);
@@ -193,11 +172,7 @@ test('should not remove user from no SSO definition group', async () => {
         description: 'no_mapping_group',
     });
     await groupStore.addUserToGroups(user.id, [group.id]);
-    await groupService.syncExternalGroupsWithAudit(
+    await groupService.syncExternalGroups(user.id, []);
-        user.id,
-        [],
-        SYSTEM_USER_AUDIT,
-    );
     const groups = await groupService.getGroupsForUser(user.id);
     expect(groups.length).toBe(1);
     expect(groups[0].name).toEqual('no_mapping_group');