From 3bea6bbd204098ec04cecd8000dfa060a9e5314f Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Mon, 5 Feb 2024 16:41:51 +0100 Subject: [PATCH] fix: filter out service and system users from inactive users list (#6134) --- .../inactive/inactive-users-controller.ts | 29 ++++++++++++++++--- .../users/inactive/inactive-users-store.ts | 2 ++ .../inactive/inactive-users-service.test.ts | 11 +++++++ 3 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/lib/users/inactive/inactive-users-controller.ts b/src/lib/users/inactive/inactive-users-controller.ts index 93e9b4dd41aa..9e9acae1acc2 100644 --- a/src/lib/users/inactive/inactive-users-controller.ts +++ b/src/lib/users/inactive/inactive-users-controller.ts @@ -1,5 +1,10 @@ import Controller from '../../routes/controller'; -import { ADMIN, IUnleashConfig, IUnleashServices } from '../../types'; +import { + ADMIN, + IFlagResolver, + IUnleashConfig, + IUnleashServices, +} from '../../types'; import { Logger } from '../../logger'; import { InactiveUsersService } from './inactive-users-service'; import { @@ -8,6 +13,7 @@ import { emptyResponse, getStandardResponses, IdsSchema, + InactiveUserSchema, inactiveUsersSchema, InactiveUsersSchema, } from '../../openapi'; @@ -15,12 +21,15 @@ import { IAuthRequest } from '../../routes/unleash-types'; import { Response } from 'express'; import { OpenApiService } from '../../services'; import { DAYS_TO_BE_COUNTED_AS_INACTIVE } from './createInactiveUsersService'; +import { anonymise } from '../../util'; export class InactiveUsersController extends Controller { private readonly logger: Logger; private inactiveUsersService: InactiveUsersService; private openApiService: OpenApiService; + + private flagResolver: IFlagResolver; constructor( config: IUnleashConfig, { @@ -34,6 +43,7 @@ export class InactiveUsersController extends Controller { ); this.inactiveUsersService = inactiveUsersService; this.openApiService = openApiService; + this.flagResolver = config.flagResolver; this.route({ method: 'get', @@ -78,8 +88,10 @@ export class InactiveUsersController extends Controller { res: Response, ): Promise { this.logger.info('Hitting inactive users'); - const inactiveUsers = - await this.inactiveUsersService.getInactiveUsers(); + let inactiveUsers = await this.inactiveUsersService.getInactiveUsers(); + if (this.flagResolver.isEnabled('anonymiseEventLog')) { + inactiveUsers = this.anonymiseUsers(inactiveUsers); + } this.openApiService.respondWithValidation( 200, res, @@ -87,7 +99,16 @@ export class InactiveUsersController extends Controller { { version: 1, inactiveUsers }, ); } - + anonymiseUsers(users: InactiveUserSchema[]): InactiveUserSchema[] { + return users.map((u) => ({ + ...u, + name: anonymise(u.name || ''), + username: anonymise(u.username || ''), + email: anonymise(u.email || 'random'), + imageUrl: + 'https://gravatar.com/avatar/21232f297a57a5a743894a0e4a801fc3?size=42&default=retro', + })); + } async deleteInactiveUsers( req: IAuthRequest, res: Response, diff --git a/src/lib/users/inactive/inactive-users-store.ts b/src/lib/users/inactive/inactive-users-store.ts index f55124e7dce1..9c0d69ca7e44 100644 --- a/src/lib/users/inactive/inactive-users-store.ts +++ b/src/lib/users/inactive/inactive-users-store.ts @@ -46,6 +46,8 @@ export class InactiveUsersStore implements IInactiveUsersStore { 'users.id', ) .where('deleted_at', null) + .andWhere('is_service', false) + .andWhere('is_system', false) .andWhereRaw( `(users.seen_at IS NULL OR users.seen_at < now() - INTERVAL '?? days') AND (users.created_at IS NULL OR users.created_at < now() - INTERVAL '?? days') diff --git a/src/test/e2e/users/inactive/inactive-users-service.test.ts b/src/test/e2e/users/inactive/inactive-users-service.test.ts index 7269b9ba8ff6..7cd38ed9490e 100644 --- a/src/test/e2e/users/inactive/inactive-users-service.test.ts +++ b/src/test/e2e/users/inactive/inactive-users-service.test.ts @@ -139,6 +139,17 @@ describe('Inactive users service', () => { expect(users).toBeTruthy(); expect(users).toHaveLength(0); }); + test('System users and service users are not returned, even if not seen', async () => { + await db.rawDatabase.raw( + `INSERT INTO users(id, name, created_at, is_service) VALUES (4949, 'service_account', now() - INTERVAL '1 YEAR', true)`, + ); + await db.rawDatabase.raw( + `INSERT INTO users(id, name, created_at, is_system) VALUES (13337, 'service_account', now() - INTERVAL '1 YEAR', true)`, + ); + const users = await inactiveUserService.getInactiveUsers(); + expect(users).toBeTruthy(); + expect(users).toHaveLength(0); + }); }); describe('Deleting inactive users', () => { test('Deletes users that have never logged in but was created before our deadline', async () => {