Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: user loging event emitting with login order #8021

Merged
merged 2 commits into from
Aug 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 23 additions & 7 deletions src/lib/db/user-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,20 +231,36 @@ class UserStore implements IUserStore {
return this.buildSelectUser(user).increment('login_attempts', 1);
}

async successfullyLogin(user: User): Promise<void> {
async successfullyLogin(user: User): Promise<number> {
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) {
sellinjaanus marked this conversation as resolved.
Show resolved Hide resolved
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<void> {
Expand Down
3 changes: 3 additions & 0 deletions src/lib/metric-events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -70,6 +72,7 @@ export {
PROXY_REPOSITORY_CREATED,
PROXY_FEATURES_FOR_TOKEN_TIME,
STAGE_ENTERED,
USER_LOGIN,
EXCEEDS_LIMIT,
REQUEST_ORIGIN,
ADDON_EVENTS_HANDLED,
Expand Down
20 changes: 16 additions & 4 deletions src/lib/services/user-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -76,6 +78,8 @@ class UserService {

private eventService: EventService;

private eventBus: EventEmitter;

private accessService: AccessService;

private resetTokenService: ResetTokenService;
Expand All @@ -98,7 +102,11 @@ class UserService {
server,
getLogger,
authentication,
}: Pick<IUnleashConfig, 'getLogger' | 'authentication' | 'server'>,
eventBus,
}: Pick<
IUnleashConfig,
'getLogger' | 'authentication' | 'server' | 'eventBus'
>,
services: {
accessService: AccessService;
resetTokenService: ResetTokenService;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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<IUser> {
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;
}

Expand Down
2 changes: 1 addition & 1 deletion src/lib/types/stores/user-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export interface IUserStore extends Store<IUser, number> {
): Promise<void>;
getPasswordsPreviouslyUsed(userId: number): Promise<string[]>;
incLoginAttempts(user: IUser): Promise<void>;
successfullyLogin(user: IUser): Promise<void>;
successfullyLogin(user: IUser): Promise<number>;
count(): Promise<number>;
countServiceAccounts(): Promise<number>;
}
14 changes: 14 additions & 0 deletions src/test/e2e/services/user-service.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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(
Expand Down Expand Up @@ -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',
Expand All @@ -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 () => {
Expand Down Expand Up @@ -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 = '[email protected]';
const user = await userService.loginUserSSO({
email,
Expand All @@ -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 () => {
Expand Down
21 changes: 20 additions & 1 deletion src/test/e2e/stores/user-store.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,20 @@ 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;
});

afterAll(async () => {
await db.destroy();
});

beforeEach(async () => {
await stores.userStore.deleteAll();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removing shared state between tests

});

test('should have no users', async () => {
const users = await stores.userStore.getAll();
expect(users).toEqual([]);
Expand Down Expand Up @@ -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: '[email protected]' });
const user2 = await store.insert({ email: '[email protected]' });
const user3 = await store.insert({ email: '[email protected]' });

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 = '[email protected]';
Expand Down
3 changes: 2 additions & 1 deletion src/test/fixtures/fake-user-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,11 @@ class UserStoreMock implements IUserStore {
return Promise.resolve();
}

async successfullyLogin(user: User): Promise<void> {
async successfullyLogin(user: User): Promise<number> {
if (!this.exists(user.id)) {
throw new Error('No such user');
}
return 0;
}

buildSelectUser(): any {
Expand Down