feat: add remote ip to all events (2) (#7149)

## About the changes
This aligns us with the requirement of having ip in all events. After
tackling the enterprise part we will be able to make the ip field
mandatory here:
2c66a4ace4/src/lib/types/events.ts (L362)

src/lib/db/group-store.ts

@@ -33,6 +33,8 @@ const GROUP_COLUMNS = [
     'scim_id',
 ];
 
+export const SSO_SYNC_USER = 'SSO';
+
 const rowToGroup = (row) => {
     if (!row) {
         throw new NotFoundError('No group found');
@@ -332,7 +334,7 @@ export default class GroupStore implements IGroupStore {
                     })
                     .orWhereRaw('jsonb_array_length(mappings_sso) = 0'),
             )
-            .where({ 'gu.user_id': userId, 'gu.created_by': 'SSO' });
+            .where({ 'gu.user_id': userId, 'gu.created_by': SSO_SYNC_USER });
 
         return rows.map(rowToGroupUser);
     }

src/lib/features/metrics/instance/instance-service.ts

@@ -118,6 +118,7 @@ export default class ClientInstanceService {
                     createdBy: app.createdBy || SYSTEM_USER.username!,
                     data: app,
                     createdByUserId: app.createdByUserId || SYSTEM_USER.id,
+                    ip: '', // TODO: fix this, how do we get the ip from the client? This comes from a row in the DB
                 }));
                 await this.eventStore.batchStore(events);
             }

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

@@ -201,7 +201,7 @@ test('user with custom root role should get a user root role', async () => {
     expect(role.name).toBe('custom-root-role');
     const events = await eventStore.getEvents();
     expect(events).toHaveLength(1);
-    expect(events[0]).toEqual({
+    expect(events[0]).toMatchObject({
         type: ROLE_CREATED,
         createdBy: SYSTEM_USER_AUDIT.username,
         createdByUserId: SYSTEM_USER.id,

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

@@ -117,6 +117,7 @@ test('should trigger simple-addon eventHandler', async () => {
             enabled: false,
             strategies: [{ name: 'default' }],
         },
+        ip: '127.0.0.1',
     });
 
     const simpleProvider = addonService.addonProviders.simple;
@@ -144,7 +145,7 @@ test('should not trigger event handler if project of event is different from add
     await addonService.createAddon(config, TEST_AUDIT_USER);
     await eventService.storeEvent({
         type: FEATURE_CREATED,
-        createdBy: SYSTEM_USER.username,
+        createdBy: SYSTEM_USER.username!,
         createdByUserId: SYSTEM_USER.id,
         project: 'someotherproject',
         data: {
@@ -152,6 +153,7 @@ test('should not trigger event handler if project of event is different from add
             enabled: false,
             strategies: [{ name: 'default' }],
         },
+        ip: '127.0.0.1',
     });
     const simpleProvider = addonService.addonProviders.simple;
     // @ts-expect-error
@@ -178,7 +180,7 @@ test('should trigger event handler if project for event is one of the desired pr
     await addonService.createAddon(config, TEST_AUDIT_USER);
     await eventService.storeEvent({
         type: FEATURE_CREATED,
-        createdBy: SYSTEM_USER.username,
+        createdBy: SYSTEM_USER.username!,
         createdByUserId: SYSTEM_USER.id,
         project: desiredProject,
         data: {
@@ -186,10 +188,11 @@ test('should trigger event handler if project for event is one of the desired pr
             enabled: false,
             strategies: [{ name: 'default' }],
         },
+        ip: '127.0.0.1',
     });
     await eventService.storeEvent({
         type: FEATURE_CREATED,
-        createdBy: SYSTEM_USER.username,
+        createdBy: SYSTEM_USER.username!,
         createdByUserId: SYSTEM_USER.id,
         project: otherProject,
         data: {
@@ -197,6 +200,7 @@ test('should trigger event handler if project for event is one of the desired pr
             enabled: false,
             strategies: [{ name: 'default' }],
         },
+        ip: '127.0.0.1',
     });
     const simpleProvider = addonService.addonProviders.simple;
     // @ts-expect-error
@@ -225,7 +229,7 @@ test('should trigger events for multiple projects if addon is setup to filter mu
     await addonService.createAddon(config, TEST_AUDIT_USER);
     await eventService.storeEvent({
         type: FEATURE_CREATED,
-        createdBy: SYSTEM_USER.username,
+        createdBy: SYSTEM_USER.username!,
         createdByUserId: SYSTEM_USER.id,
         project: desiredProjects[0],
         data: {
@@ -233,10 +237,11 @@ test('should trigger events for multiple projects if addon is setup to filter mu
             enabled: false,
             strategies: [{ name: 'default' }],
         },
+        ip: '127.0.0.1',
     });
     await eventService.storeEvent({
         type: FEATURE_CREATED,
-        createdBy: SYSTEM_USER.username,
+        createdBy: SYSTEM_USER.username!,
         createdByUserId: SYSTEM_USER.id,
         project: otherProject,
         data: {
@@ -244,10 +249,11 @@ test('should trigger events for multiple projects if addon is setup to filter mu
             enabled: false,
             strategies: [{ name: 'default' }],
         },
+        ip: '127.0.0.1',
     });
     await eventService.storeEvent({
         type: FEATURE_CREATED,
-        createdBy: SYSTEM_USER.username,
+        createdBy: SYSTEM_USER.username!,
         createdByUserId: SYSTEM_USER.id,
         project: desiredProjects[1],
         data: {
@@ -255,6 +261,7 @@ test('should trigger events for multiple projects if addon is setup to filter mu
             enabled: false,
             strategies: [{ name: 'default' }],
         },
+        ip: '127.0.0.1',
     });
     const simpleProvider = addonService.addonProviders.simple;
     // @ts-expect-error
@@ -286,7 +293,7 @@ test('should filter events on environment if addon is setup to filter for it', a
     await addonService.createAddon(config, TEST_AUDIT_USER);
     await eventService.storeEvent({
         type: FEATURE_CREATED,
-        createdBy: SYSTEM_USER.username,
+        createdBy: SYSTEM_USER.username!,
         createdByUserId: SYSTEM_USER.id,
         project: desiredEnvironment,
         environment: desiredEnvironment,
@@ -295,10 +302,11 @@ test('should filter events on environment if addon is setup to filter for it', a
             enabled: false,
             strategies: [{ name: 'default' }],
         },
+        ip: '127.0.0.1',
     });
     await eventService.storeEvent({
         type: FEATURE_CREATED,
-        createdBy: SYSTEM_USER.username,
+        createdBy: SYSTEM_USER.username!,
         createdByUserId: SYSTEM_USER.id,
         environment: otherEnvironment,
         data: {
@@ -306,6 +314,7 @@ test('should filter events on environment if addon is setup to filter for it', a
             enabled: false,
             strategies: [{ name: 'default' }],
         },
+        ip: '127.0.0.1',
     });
     const simpleProvider = addonService.addonProviders.simple;
     // @ts-expect-error
@@ -333,7 +342,7 @@ test('should not filter out global events (no specific environment) even if addo
 
     const globalEventWithNoEnvironment = {
         type: FEATURE_CREATED,
-        createdBy: SYSTEM_USER.username,
+        createdBy: SYSTEM_USER.username!,
         createdByUserId: SYSTEM_USER.id,
         project: 'some-project',
         data: {
@@ -341,6 +350,7 @@ test('should not filter out global events (no specific environment) even if addo
             enabled: false,
             strategies: [{ name: 'default' }],
         },
+        ip: '127.0.0.1',
     };
 
     await addonService.createAddon(config, TEST_AUDIT_USER);
@@ -371,13 +381,14 @@ test('should not filter out global events (no specific project) even if addon is
 
     const globalEventWithNoProject = {
         type: FEATURE_CREATED,
-        createdBy: SYSTEM_USER.username,
+        createdBy: SYSTEM_USER.username!,
         createdByUserId: SYSTEM_USER.id,
         data: {
             name: 'some-toggle',
             enabled: false,
             strategies: [{ name: 'default' }],
         },
+        ip: '127.0.0.1',
     };
 
     await addonService.createAddon(config, TEST_AUDIT_USER);
@@ -409,7 +420,7 @@ test('should support wildcard option for filtering addons', async () => {
     await addonService.createAddon(config, TEST_AUDIT_USER);
     await eventService.storeEvent({
         type: FEATURE_CREATED,
-        createdBy: SYSTEM_USER.username,
+        createdBy: SYSTEM_USER.username!,
         createdByUserId: SYSTEM_USER.id,
         project: desiredProjects[0],
         data: {
@@ -417,10 +428,11 @@ test('should support wildcard option for filtering addons', async () => {
             enabled: false,
             strategies: [{ name: 'default' }],
         },
+        ip: '127.0.0.1',
     });
     await eventService.storeEvent({
         type: FEATURE_CREATED,
-        createdBy: SYSTEM_USER.username,
+        createdBy: SYSTEM_USER.username!,
         createdByUserId: SYSTEM_USER.id,
         project: otherProject,
         data: {
@@ -428,10 +440,11 @@ test('should support wildcard option for filtering addons', async () => {
             enabled: false,
             strategies: [{ name: 'default' }],
         },
+        ip: '127.0.0.1',
     });
     await eventService.storeEvent({
         type: FEATURE_CREATED,
-        createdBy: SYSTEM_USER.username,
+        createdBy: SYSTEM_USER.username!,
         createdByUserId: SYSTEM_USER.id,
         project: desiredProjects[1],
         data: {
@@ -439,6 +452,7 @@ test('should support wildcard option for filtering addons', async () => {
             enabled: false,
             strategies: [{ name: 'default' }],
         },
+        ip: '127.0.0.1',
     });
     const simpleProvider = addonService.addonProviders.simple;
     // @ts-expect-error
@@ -476,7 +490,7 @@ test('Should support filtering by both project and environment', async () => {
     await addonService.createAddon(config, TEST_AUDIT_USER);
     await eventService.storeEvent({
         type: FEATURE_CREATED,
-        createdBy: SYSTEM_USER.username,
+        createdBy: SYSTEM_USER.username!,
         createdByUserId: SYSTEM_USER.id,
         project: desiredProjects[0],
         environment: desiredEnvironments[0],
@@ -485,10 +499,11 @@ test('Should support filtering by both project and environment', async () => {
             enabled: false,
             strategies: [{ name: 'default' }],
         },
+        ip: '127.0.0.1',
     });
     await eventService.storeEvent({
         type: FEATURE_CREATED,
-        createdBy: SYSTEM_USER.username,
+        createdBy: SYSTEM_USER.username!,
         createdByUserId: SYSTEM_USER.id,
         project: desiredProjects[0],
         environment: 'wrongenvironment',
@@ -497,10 +512,11 @@ test('Should support filtering by both project and environment', async () => {
             enabled: false,
             strategies: [{ name: 'default' }],
         },
+        ip: '127.0.0.1',
     });
     await eventService.storeEvent({
         type: FEATURE_CREATED,
-        createdBy: SYSTEM_USER.username,
+        createdBy: SYSTEM_USER.username!,
         createdByUserId: SYSTEM_USER.id,
         project: desiredProjects[2],
         environment: desiredEnvironments[1],
@@ -509,10 +525,11 @@ test('Should support filtering by both project and environment', async () => {
             enabled: false,
             strategies: [{ name: 'default' }],
         },
+        ip: '127.0.0.1',
     });
     await eventService.storeEvent({
         type: FEATURE_CREATED,
-        createdBy: SYSTEM_USER.username,
+        createdBy: SYSTEM_USER.username!,
         createdByUserId: SYSTEM_USER.id,
         project: desiredProjects[2],
         environment: desiredEnvironments[2],
@@ -521,10 +538,11 @@ test('Should support filtering by both project and environment', async () => {
             enabled: false,
             strategies: [{ name: 'default' }],
         },
+        ip: '127.0.0.1',
     });
     await eventService.storeEvent({
         type: FEATURE_CREATED,
-        createdBy: SYSTEM_USER.username,
+        createdBy: SYSTEM_USER.username!,
         createdByUserId: SYSTEM_USER.id,
         project: 'wrongproject',
         environment: desiredEnvironments[0],
@@ -533,6 +551,7 @@ test('Should support filtering by both project and environment', async () => {
             enabled: false,
             strategies: [{ name: 'default' }],
         },
+        ip: '127.0.0.1',
     });
 
     const simpleProvider = addonService.addonProviders.simple;

src/lib/services/feature-service-potentially-stale.test.ts

@@ -1,6 +1,6 @@
 import {
     FEATURE_POTENTIALLY_STALE_ON,
-    type IEvent,
+    type IBaseEvent,
     type IUnleashConfig,
     type IUnleashStores,
 } from '../types';
@@ -28,7 +28,7 @@ test('Should only store events for potentially stale on', async () => {
         {
             // @ts-expect-error
             eventStore: {
-                batchStore: async (events: IEvent[]) => {
+                batchStore: async (events: IBaseEvent[]) => {
                     expect(events.length).toBe(1);
                     const [event1] = events;
 

src/lib/services/group-service.ts

@@ -29,6 +29,7 @@ import NameExistsError from '../error/name-exists-error';
 import type { IAccountStore } from '../types/stores/account-store';
 import type { IUser } from '../types/user';
 import type EventService from '../features/events/event-service';
+import { SSO_SYNC_USER } from '../db/group-store';
 
 export class GroupService {
     private groupStore: IGroupStore;
@@ -302,7 +303,7 @@ export class GroupService {
             await this.groupStore.addUserToGroups(
                 userId,
                 newGroups.map((g) => g.id),
-                auditUser.username,
+                SSO_SYNC_USER,
             );
             const oldGroups = await this.groupStore.getOldGroupsForExternalUser(
                 userId,

src/test/e2e/api/admin/event.e2e.test.ts

@@ -61,6 +61,7 @@ test('Can filter by project', async () => {
         tags: [],
         createdBy: 'test-user',
         createdByUserId: TEST_USER_ID,
+        ip: '127.0.0.1',
     });
     await eventService.storeEvent({
         type: FEATURE_CREATED,
@@ -70,6 +71,7 @@ test('Can filter by project', async () => {
         createdBy: 'test-user',
         environment: 'test',
         createdByUserId: TEST_USER_ID,
+        ip: '127.0.0.1',
     });
     await app.request
         .get('/api/admin/events?project=default')
@@ -89,6 +91,7 @@ test('can search for events', async () => {
             tags: [],
             createdBy: randomId(),
             createdByUserId: TEST_USER_ID,
+            ip: '127.0.0.1',
         },
         {
             type: FEATURE_CREATED,
@@ -98,6 +101,7 @@ test('can search for events', async () => {
             tags: [{ type: 'simple', value: randomId() }],
             createdBy: randomId(),
             createdByUserId: TEST_USER_ID,
+            ip: '127.0.0.1',
         },
     ];
 

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

@@ -8,6 +8,7 @@ import {
     type IUnleashStores,
     type IUser,
     TEST_AUDIT_USER,
+    SYSTEM_USER_AUDIT,
 } from '../../../lib/types';
 
 let stores: IUnleashStores;
@@ -69,10 +70,10 @@ test('should have three group', async () => {
 });
 
 test('should add person to 2 groups', async () => {
-    await groupService.syncExternalGroups(
+    await groupService.syncExternalGroupsWithAudit(
         user.id,
         ['dev', 'maintainer'],
-        'SSO',
+        SYSTEM_USER_AUDIT,
     );
     const groups = await groupService.getGroupsForUser(user.id);
     expect(groups.length).toBe(2);
@@ -98,7 +99,11 @@ 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');
+    await groupService.syncExternalGroupsWithAudit(
+        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');
@@ -119,7 +124,11 @@ 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');
+    await groupService.syncExternalGroupsWithAudit(
+        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');
@@ -144,7 +153,11 @@ 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.syncExternalGroups(user.id, 'Everyone' as any, 'SSO');
+    await groupService.syncExternalGroupsWithAudit(
+        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');
@@ -155,7 +168,11 @@ 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.syncExternalGroups(user.id, [], 'SSO');
+    await groupService.syncExternalGroupsWithAudit(
+        user.id,
+        [],
+        SYSTEM_USER_AUDIT,
+    );
     const groups = await groupService.getGroupsForUser(user.id);
     expect(groups.length).toBe(0);
     expect(removedGroups).toHaveLength(1);
@@ -176,7 +193,11 @@ test('should not remove user from no SSO definition group', async () => {
         description: 'no_mapping_group',
     });
     await groupStore.addUserToGroups(user.id, [group.id]);
-    await groupService.syncExternalGroups(user.id, [], 'SSO');
+    await groupService.syncExternalGroupsWithAudit(
+        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');

src/test/e2e/stores/event-store.e2e.test.ts

@@ -100,6 +100,7 @@ test('Should be able to store multiple events at once', async () => {
             clientIp: '127.0.0.1',
             appName: 'test1',
         },
+        ip: '127.0.0.1',
     };
     const event2 = {
         type: APPLICATION_CREATED,
@@ -109,6 +110,7 @@ test('Should be able to store multiple events at once', async () => {
             clientIp: '127.0.0.1',
             appName: 'test2',
         },
+        ip: '127.0.0.1',
     };
     const event3 = {
         type: APPLICATION_CREATED,
@@ -119,6 +121,7 @@ test('Should be able to store multiple events at once', async () => {
             appName: 'test3',
         },
         tags: [{ type: 'simple', value: 'mytest' }],
+        ip: '127.0.0.1',
     };
     const seen: IEvent[] = [];
     eventStore.on(APPLICATION_CREATED, (e) => seen.push(e));
@@ -142,6 +145,7 @@ test('Should get all stored events', async () => {
             enabled: true,
             strategies: [{ name: 'default' }],
         },
+        ip: '127.0.0.1',
     };
     await eventStore.store(event);
     const events = await eventStore.getAll();
@@ -161,6 +165,7 @@ test('Should delete stored event', async () => {
             enabled: true,
             strategies: [{ name: 'default' }],
         },
+        ip: '127.0.0.1',
     };
     await eventStore.store(event);
     await eventStore.store(event);
@@ -185,6 +190,7 @@ test('Should get stored event by id', async () => {
             enabled: true,
             strategies: [{ name: 'default' }],
         },
+        ip: '127.0.0.1',
     };
     await eventStore.store(event);
     const events = await eventStore.getAll();

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

@@ -1,5 +1,5 @@
 import type { IEventStore } from '../../lib/types/stores/event-store';
-import type { IEvent } from '../../lib/types/events';
+import type { IBaseEvent, IEvent } from '../../lib/types/events';
 import { sharedEventEmitter } from '../../lib/util/anyEventEmitter';
 import type { IQueryOperations } from '../../lib/features/events/event-store';
 import type { SearchEventsSchema } from '../../lib/openapi';
@@ -19,15 +19,23 @@ class FakeEventStore implements IEventStore {
         return Promise.resolve(1);
     }
 
-    store(event: IEvent): Promise<void> {
+    store(event: IBaseEvent): Promise<void> {
-        this.events.push(event);
+        this.events.push({
+            ...event,
+            id: this.events.length,
+            createdAt: new Date(),
+        });
         this.eventEmitter.emit(event.type, event);
         return Promise.resolve();
     }
 
-    batchStore(events: IEvent[]): Promise<void> {
+    batchStore(events: IBaseEvent[]): Promise<void> {
         events.forEach((event) => {
-            this.events.push(event);
+            this.events.push({
+                ...event,
+                id: this.events.length,
+                createdAt: new Date(),
+            });
             this.eventEmitter.emit(event.type, event);
         });
         return Promise.resolve();