Skip to content

Commit

Permalink
fix: filter out service and system users from inactive users list (#6134
Browse files Browse the repository at this point in the history
)
  • Loading branch information
Christopher Kolstad authored Feb 5, 2024
1 parent 1da59ab commit 3bea6bb
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 4 deletions.
29 changes: 25 additions & 4 deletions src/lib/users/inactive/inactive-users-controller.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -8,19 +13,23 @@ import {
emptyResponse,
getStandardResponses,
IdsSchema,
InactiveUserSchema,
inactiveUsersSchema,
InactiveUsersSchema,
} from '../../openapi';
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,
{
Expand All @@ -34,6 +43,7 @@ export class InactiveUsersController extends Controller {
);
this.inactiveUsersService = inactiveUsersService;
this.openApiService = openApiService;
this.flagResolver = config.flagResolver;

this.route({
method: 'get',
Expand Down Expand Up @@ -78,16 +88,27 @@ export class InactiveUsersController extends Controller {
res: Response<InactiveUsersSchema>,
): Promise<void> {
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,
inactiveUsersSchema.$id,
{ 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<undefined, undefined, IdsSchema>,
res: Response<void>,
Expand Down
2 changes: 2 additions & 0 deletions src/lib/users/inactive/inactive-users-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
11 changes: 11 additions & 0 deletions src/test/e2e/users/inactive/inactive-users-service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down

0 comments on commit 3bea6bb

Please sign in to comment.