From 2a35976081906f25c9e5c3fe1c80288c986a129a Mon Sep 17 00:00:00 2001 From: Mateusz Kwasniewski Date: Fri, 30 Aug 2024 11:28:19 +0200 Subject: [PATCH] feat: user loging event emitting with login order (#8021) --- src/lib/db/user-store.ts | 30 ++++++++++++++----- src/lib/metric-events.ts | 3 ++ src/lib/services/user-service.ts | 20 ++++++++++--- src/lib/types/stores/user-store.ts | 2 +- .../e2e/services/user-service.e2e.test.ts | 14 +++++++++ src/test/e2e/stores/user-store.e2e.test.ts | 21 ++++++++++++- src/test/fixtures/fake-user-store.ts | 3 +- 7 files changed, 79 insertions(+), 14 deletions(-) diff --git a/src/lib/db/user-store.ts b/src/lib/db/user-store.ts index 01c3c13903..f5a2f8acdb 100644 --- a/src/lib/db/user-store.ts +++ b/src/lib/db/user-store.ts @@ -231,20 +231,36 @@ class UserStore implements IUserStore { return this.buildSelectUser(user).increment('login_attempts', 1); } - async successfullyLogin(user: User): Promise { + async successfullyLogin(user: User): Promise { const currentDate = new Date(); const updateQuery = this.buildSelectUser(user).update({ login_attempts: 0, seen_at: currentDate, }); + + let firstLoginOrder = 0; + if (this.flagResolver.isEnabled('onboardingMetrics')) { - updateQuery.update({ - first_seen_at: this.db.raw('COALESCE(first_seen_at, ?)', [ - currentDate, - ]), - }); + const existingUser = + await this.buildSelectUser(user).first('first_seen_at'); + + if (!existingUser.first_seen_at) { + const countEarlierUsers = await this.db(TABLE) + .whereNotNull('first_seen_at') + .andWhere('first_seen_at', '<', currentDate) + .count('*') + .then((res) => Number(res[0].count)); + + firstLoginOrder = countEarlierUsers; + + await updateQuery.update({ + first_seen_at: currentDate, + }); + } } - return updateQuery; + + await updateQuery; + return firstLoginOrder; } async deleteAll(): Promise { diff --git a/src/lib/metric-events.ts b/src/lib/metric-events.ts index 5dd781b402..261e76728d 100644 --- a/src/lib/metric-events.ts +++ b/src/lib/metric-events.ts @@ -10,6 +10,7 @@ const FRONTEND_API_REPOSITORY_CREATED = 'frontend_api_repository_created'; const PROXY_REPOSITORY_CREATED = 'proxy_repository_created'; const PROXY_FEATURES_FOR_TOKEN_TIME = 'proxy_features_for_token_time'; const STAGE_ENTERED = 'stage-entered' as const; +const USER_LOGIN = 'user-login' as const; const EXCEEDS_LIMIT = 'exceeds-limit' as const; const REQUEST_ORIGIN = 'request_origin' as const; const ADDON_EVENTS_HANDLED = 'addon-event-handled' as const; @@ -25,6 +26,7 @@ type MetricEvent = | typeof PROXY_REPOSITORY_CREATED | typeof PROXY_FEATURES_FOR_TOKEN_TIME | typeof STAGE_ENTERED + | typeof USER_LOGIN | typeof EXCEEDS_LIMIT | typeof REQUEST_ORIGIN; @@ -70,6 +72,7 @@ export { PROXY_REPOSITORY_CREATED, PROXY_FEATURES_FOR_TOKEN_TIME, STAGE_ENTERED, + USER_LOGIN, EXCEEDS_LIMIT, REQUEST_ORIGIN, ADDON_EVENTS_HANDLED, diff --git a/src/lib/services/user-service.ts b/src/lib/services/user-service.ts index a1c5bab9a7..1febd79f16 100644 --- a/src/lib/services/user-service.ts +++ b/src/lib/services/user-service.ts @@ -43,6 +43,8 @@ import type EventService from '../features/events/event-service'; import { SYSTEM_USER, SYSTEM_USER_AUDIT } from '../types'; import { PasswordPreviouslyUsedError } from '../error/password-previously-used'; import { RateLimitError } from '../error/rate-limit-error'; +import type EventEmitter from 'events'; +import { USER_LOGIN } from '../metric-events'; export interface ICreateUser { name?: string; @@ -76,6 +78,8 @@ class UserService { private eventService: EventService; + private eventBus: EventEmitter; + private accessService: AccessService; private resetTokenService: ResetTokenService; @@ -98,7 +102,11 @@ class UserService { server, getLogger, authentication, - }: Pick, + eventBus, + }: Pick< + IUnleashConfig, + 'getLogger' | 'authentication' | 'server' | 'eventBus' + >, services: { accessService: AccessService; resetTokenService: ResetTokenService; @@ -110,6 +118,7 @@ class UserService { ) { this.logger = getLogger('service/user-service.js'); this.store = stores.userStore; + this.eventBus = eventBus; this.eventService = services.eventService; this.accessService = services.accessService; this.resetTokenService = services.resetTokenService; @@ -390,7 +399,8 @@ class UserService { if (user && passwordHash) { const match = await bcrypt.compare(password, passwordHash); if (match) { - await this.store.successfullyLogin(user); + const loginOrder = await this.store.successfullyLogin(user); + this.eventBus.emit(USER_LOGIN, { loginOrder }); return user; } } @@ -443,13 +453,15 @@ class UserService { throw e; } } - await this.store.successfullyLogin(user); + const loginOrder = await this.store.successfullyLogin(user); + this.eventBus.emit(USER_LOGIN, { loginOrder }); return user; } async loginDemoAuthDefaultAdmin(): Promise { const user = await this.store.getByQuery({ id: 1 }); - await this.store.successfullyLogin(user); + const loginOrder = await this.store.successfullyLogin(user); + this.eventBus.emit(USER_LOGIN, { loginOrder }); return user; } diff --git a/src/lib/types/stores/user-store.ts b/src/lib/types/stores/user-store.ts index 5cdb907c49..73f5484d6c 100644 --- a/src/lib/types/stores/user-store.ts +++ b/src/lib/types/stores/user-store.ts @@ -35,7 +35,7 @@ export interface IUserStore extends Store { ): Promise; getPasswordsPreviouslyUsed(userId: number): Promise; incLoginAttempts(user: IUser): Promise; - successfullyLogin(user: IUser): Promise; + successfullyLogin(user: IUser): Promise; count(): Promise; countServiceAccounts(): Promise; } diff --git a/src/test/e2e/services/user-service.e2e.test.ts b/src/test/e2e/services/user-service.e2e.test.ts index 5439db489a..31d878c1bf 100644 --- a/src/test/e2e/services/user-service.e2e.test.ts +++ b/src/test/e2e/services/user-service.e2e.test.ts @@ -29,6 +29,8 @@ import { import { CUSTOM_ROOT_ROLE_TYPE } from '../../../lib/util'; import { PasswordPreviouslyUsedError } from '../../../lib/error/password-previously-used'; import { createEventsService } from '../../../lib/features'; +import type EventEmitter from 'events'; +import { USER_LOGIN } from '../../../lib/metric-events'; let db: ITestDb; let stores: IUnleashStores; @@ -41,11 +43,13 @@ let sessionService: SessionService; let settingService: SettingService; let eventService: EventService; let accessService: AccessService; +let eventBus: EventEmitter; beforeAll(async () => { db = await dbInit('user_service_serial', getLogger); stores = db.stores; const config = createTestConfig(); + eventBus = config.eventBus; eventService = createEventsService(db.rawDatabase, config); const groupService = new GroupService(stores, config, eventService); accessService = new AccessService( @@ -138,6 +142,10 @@ test('should not be allowed to create existing user', async () => { }); test('should create user with password', async () => { + const recordedEvents: Array<{ loginOrder: number }> = []; + eventBus.on(USER_LOGIN, (data) => { + recordedEvents.push(data); + }); await userService.createUser( { username: 'test', @@ -151,6 +159,7 @@ test('should create user with password', async () => { 'A very strange P4ssw0rd_', ); expect(user.username).toBe('test'); + expect(recordedEvents).toEqual([{ loginOrder: 0 }]); }); test('should create user with rootRole in audit-log', async () => { @@ -377,6 +386,10 @@ test('updating a user without an email should not strip the email', async () => }); test('should login and create user via SSO', async () => { + const recordedEvents: Array<{ loginOrder: number }> = []; + eventBus.on(USER_LOGIN, (data) => { + recordedEvents.push(data); + }); const email = 'some@test.com'; const user = await userService.loginUserSSO({ email, @@ -390,6 +403,7 @@ test('should login and create user via SSO', async () => { expect(user.name).toBe('some'); expect(userWithRole.name).toBe('some'); expect(userWithRole.rootRole).toBe(viewerRole.id); + expect(recordedEvents).toEqual([{ loginOrder: 0 }]); }); test('should throw if rootRole is wrong via SSO', async () => { diff --git a/src/test/e2e/stores/user-store.e2e.test.ts b/src/test/e2e/stores/user-store.e2e.test.ts index 5ba6dae575..dda39b36e4 100644 --- a/src/test/e2e/stores/user-store.e2e.test.ts +++ b/src/test/e2e/stores/user-store.e2e.test.ts @@ -7,7 +7,9 @@ let stores: IUnleashStores; let db: ITestDb; beforeAll(async () => { - db = await dbInit('user_store_serial', getLogger); + db = await dbInit('user_store_serial', getLogger, { + experimental: { flags: { onboardingMetrics: true } }, + }); stores = db.stores; }); @@ -15,6 +17,10 @@ afterAll(async () => { await db.destroy(); }); +beforeEach(async () => { + await stores.userStore.deleteAll(); +}); + test('should have no users', async () => { const users = await stores.userStore.getAll(); expect(users).toEqual([]); @@ -108,6 +114,19 @@ test('should reset user after successful login', async () => { expect(storedUser.seenAt! >= user.seenAt!).toBe(true); }); +test('should return first login order for every new user', async () => { + const store = stores.userStore; + const user1 = await store.insert({ email: 'user1@mail.com' }); + const user2 = await store.insert({ email: 'user2@mail.com' }); + const user3 = await store.insert({ email: 'user3@mail.com' }); + + expect(await store.successfullyLogin(user1)).toBe(0); + expect(await store.successfullyLogin(user1)).toBe(0); + expect(await store.successfullyLogin(user2)).toBe(1); + expect(await store.successfullyLogin(user1)).toBe(0); + expect(await store.successfullyLogin(user3)).toBe(2); +}); + test('should only update specified fields on user', async () => { const store = stores.userStore; const email = 'usertobeupdated@mail.com'; diff --git a/src/test/fixtures/fake-user-store.ts b/src/test/fixtures/fake-user-store.ts index 01a2d16155..0dda7cbe81 100644 --- a/src/test/fixtures/fake-user-store.ts +++ b/src/test/fixtures/fake-user-store.ts @@ -108,10 +108,11 @@ class UserStoreMock implements IUserStore { return Promise.resolve(); } - async successfullyLogin(user: User): Promise { + async successfullyLogin(user: User): Promise { if (!this.exists(user.id)) { throw new Error('No such user'); } + return 0; } buildSelectUser(): any {