diff --git a/src/lib/services/maintenance-service.ts b/src/lib/services/maintenance-service.ts index 78c2ad7d1e..015639c09d 100644 --- a/src/lib/services/maintenance-service.ts +++ b/src/lib/services/maintenance-service.ts @@ -40,6 +40,7 @@ export default class MaintenanceService { maintenanceSettingsKey, setting, user, + false, ); } } diff --git a/src/lib/services/proxy-service.ts b/src/lib/services/proxy-service.ts index 40b8aaa5a7..7bcb64862d 100644 --- a/src/lib/services/proxy-service.ts +++ b/src/lib/services/proxy-service.ts @@ -171,6 +171,7 @@ export class ProxyService { frontendSettingsKey, value, createdBy, + false, ); } diff --git a/src/lib/services/setting-service.ts b/src/lib/services/setting-service.ts index 237badc070..04711d859e 100644 --- a/src/lib/services/setting-service.ts +++ b/src/lib/services/setting-service.ts @@ -42,22 +42,39 @@ export default class SettingService { return value || defaultValue; } - async insert(id: string, value: object, createdBy: string): Promise { - const exists = await this.settingStore.exists(id); - if (exists) { + async insert( + id: string, + value: object, + createdBy: string, + hideEventDetails: boolean = true, + ): Promise { + const existingSettings = await this.settingStore.get(id); + + let data: object = { id, ...value }; + let preData = existingSettings; + + if (hideEventDetails) { + preData = { hideEventDetails: true }; + data = { id, hideEventDetails: true }; + } + + if (existingSettings) { await this.settingStore.updateRow(id, value); await this.eventService.storeEvent( - new SettingUpdatedEvent({ - createdBy, - data: { id }, - }), + new SettingUpdatedEvent( + { + createdBy, + data, + }, + preData, + ), ); } else { await this.settingStore.insert(id, value); await this.eventService.storeEvent( new SettingCreatedEvent({ createdBy, - data: { id }, + data, }), ); } diff --git a/src/lib/types/events.ts b/src/lib/types/events.ts index 4b8ac69dfc..841a69b9c1 100644 --- a/src/lib/types/events.ts +++ b/src/lib/types/events.ts @@ -983,13 +983,18 @@ export class SettingDeletedEvent extends BaseEvent { export class SettingUpdatedEvent extends BaseEvent { readonly data: any; + readonly preData: any; /** * @param createdBy accepts a string for backward compatibility. Prefer using IUser for standardization */ - constructor(eventData: { createdBy: string | IUser; data: any }) { + constructor( + eventData: { createdBy: string | IUser; data: any }, + preData: any, + ) { super(SETTING_UPDATED, eventData.createdBy); this.data = eventData.data; + this.preData = preData; } } diff --git a/src/test/e2e/services/setting-service.test.ts b/src/test/e2e/services/setting-service.test.ts index a72ea7deb5..e39e3b9ac6 100644 --- a/src/test/e2e/services/setting-service.test.ts +++ b/src/test/e2e/services/setting-service.test.ts @@ -8,6 +8,7 @@ import { SETTING_UPDATED, } from '../../../lib/types/events'; import { EventService } from '../../../lib/services'; +import { property } from 'fast-check'; let stores: IUnleashStores; let db; @@ -29,7 +30,7 @@ afterAll(async () => { test('Can create new setting', async () => { const someData = { some: 'blob' }; - await service.insert('some-setting', someData, 'test-user'); + await service.insert('some-setting', someData, 'test-user', false); const actual = await service.get('some-setting'); expect(actual).toStrictEqual(someData); @@ -38,6 +39,7 @@ test('Can create new setting', async () => { type: SETTING_CREATED, }); expect(createdEvents).toHaveLength(1); + expect(createdEvents[0].data).toEqual({ id: 'some-setting', some: 'blob' }); }); test('Can delete setting', async () => { @@ -54,17 +56,39 @@ test('Can delete setting', async () => { expect(createdEvents).toHaveLength(1); }); +test('Sentitive SSO settings are redacted in event log', async () => { + const someData = { password: 'mySecretPassword' }; + const property = 'unleash.enterprise.auth.oidc'; + await service.insert(property, someData, 'a-user-in-places'); + + await service.insert(property, { password: 'changed' }, 'a-user-in-places'); + const actual = await service.get(property); + const { eventStore } = stores; + + const updatedEvents = await eventStore.searchEvents({ + type: SETTING_UPDATED, + }); + expect(updatedEvents[0].preData).toEqual({ hideEventDetails: true }); + await service.delete(property, 'test-user'); +}); + test('Can update setting', async () => { const { eventStore } = stores; const someData = { some: 'blob' }; - await service.insert('updated-setting', someData, 'test-user'); + await service.insert('updated-setting', someData, 'test-user', false); await service.insert( 'updated-setting', { ...someData, test: 'fun' }, 'test-user', + false, ); const updatedEvents = await eventStore.searchEvents({ type: SETTING_UPDATED, }); expect(updatedEvents).toHaveLength(1); + expect(updatedEvents[0].data).toEqual({ + id: 'updated-setting', + some: 'blob', + test: 'fun', + }); }); diff --git a/src/test/e2e/services/user-service.e2e.test.ts b/src/test/e2e/services/user-service.e2e.test.ts index b699934333..f96db6b8af 100644 --- a/src/test/e2e/services/user-service.e2e.test.ts +++ b/src/test/e2e/services/user-service.e2e.test.ts @@ -211,6 +211,7 @@ test('should not login user if simple auth is disabled', async () => { simpleAuthSettingsKey, { disabled: true }, randomId(), + true, ); await userService.createUser({