diff --git a/src/lib/db/group-store.ts b/src/lib/db/group-store.ts index 77107c1624..f353be131a 100644 --- a/src/lib/db/group-store.ts +++ b/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); } diff --git a/src/lib/features/metrics/instance/instance-service.ts b/src/lib/features/metrics/instance/instance-service.ts index fca6c96ee5..05b4c136c7 100644 --- a/src/lib/features/metrics/instance/instance-service.ts +++ b/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); } diff --git a/src/lib/services/access-service.test.ts b/src/lib/services/access-service.test.ts index 5324dcf742..c7ff960f25 100644 --- a/src/lib/services/access-service.test.ts +++ b/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, diff --git a/src/lib/services/addon-service.test.ts b/src/lib/services/addon-service.test.ts index ab630d487a..50d8aba708 100644 --- a/src/lib/services/addon-service.test.ts +++ b/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; diff --git a/src/lib/services/feature-service-potentially-stale.test.ts b/src/lib/services/feature-service-potentially-stale.test.ts index 960adb5588..2108950ef1 100644 --- a/src/lib/services/feature-service-potentially-stale.test.ts +++ b/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; diff --git a/src/lib/services/group-service.ts b/src/lib/services/group-service.ts index 1394cbf471..1a5da221fd 100644 --- a/src/lib/services/group-service.ts +++ b/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, diff --git a/src/test/e2e/api/admin/event.e2e.test.ts b/src/test/e2e/api/admin/event.e2e.test.ts index 04c64f6a1a..bd9ff120a9 100644 --- a/src/test/e2e/api/admin/event.e2e.test.ts +++ b/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', }, ]; diff --git a/src/test/e2e/services/group-service.e2e.test.ts b/src/test/e2e/services/group-service.e2e.test.ts index 85ed9ae335..08c77f0faf 100644 --- a/src/test/e2e/services/group-service.e2e.test.ts +++ b/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'); diff --git a/src/test/e2e/stores/event-store.e2e.test.ts b/src/test/e2e/stores/event-store.e2e.test.ts index f66d38f664..bedbfaf7e1 100644 --- a/src/test/e2e/stores/event-store.e2e.test.ts +++ b/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(); diff --git a/src/test/fixtures/fake-event-store.ts b/src/test/fixtures/fake-event-store.ts index c1c9abbfd7..153f885f04 100644 --- a/src/test/fixtures/fake-event-store.ts +++ b/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 { - this.events.push(event); + store(event: IBaseEvent): Promise { + this.events.push({ + ...event, + id: this.events.length, + createdAt: new Date(), + }); this.eventEmitter.emit(event.type, event); return Promise.resolve(); } - batchStore(events: IEvent[]): Promise { + batchStore(events: IBaseEvent[]): Promise { 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();