Skip to content

Commit

Permalink
feat: filter projectless events for normal users (#7914)
Browse files Browse the repository at this point in the history
Now events that do not have project ( for example user creation, segment
creation etc), will not be displayed to non root admins.
  • Loading branch information
sjaanus authored Aug 21, 2024
1 parent 6c5ce52 commit df73c65
Show file tree
Hide file tree
Showing 15 changed files with 246 additions and 44 deletions.
3 changes: 3 additions & 0 deletions src/lib/features/access/access-read-model-type.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export interface IAccessReadModel {
isRootAdmin(userId: number): Promise<boolean>;
}
28 changes: 28 additions & 0 deletions src/lib/features/access/access-read-model.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import {
ADMIN_TOKEN_USER,
type IAccessStore,
type IUnleashStores,
SYSTEM_USER_ID,
} from '../../types';
import type { IAccessReadModel } from './access-read-model-type';
import * as permissions from '../../types/permissions';

const { ADMIN } = permissions;

export class AccessReadModel implements IAccessReadModel {
private store: IAccessStore;

constructor({ accessStore }: Pick<IUnleashStores, 'accessStore'>) {
this.store = accessStore;
}

async isRootAdmin(userId: number): Promise<boolean> {
if (userId === SYSTEM_USER_ID || userId === ADMIN_TOKEN_USER.id) {
return true;
}
const roles = await this.store.getRolesForUserId(userId);
return roles.some(
(role) => role.name.toLowerCase() === ADMIN.toLowerCase(),
);
}
}
24 changes: 24 additions & 0 deletions src/lib/features/access/createAccessReadModel.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import type { Db, IUnleashConfig } from '../../server-impl';
import type { IAccessReadModel } from './access-read-model-type';
import { AccessReadModel } from './access-read-model';
import { AccessStore } from '../../db/access-store';
import FakeRoleStore from '../../../test/fixtures/fake-role-store';
import FakeAccessStore from '../../../test/fixtures/fake-access-store';
import type { IAccessStore } from '../../types';

export const createAccessReadModel = (
db: Db,
config: IUnleashConfig,
): IAccessReadModel => {
const { eventBus, getLogger } = config;
const accessStore = new AccessStore(db, eventBus, getLogger);
return new AccessReadModel({ accessStore });
};

export const createFakeAccessReadModel = (
accessStore?: IAccessStore,
): IAccessReadModel => {
const roleStore = new FakeRoleStore();
const finalAccessStore = accessStore ?? new FakeAccessStore(roleStore);
return new AccessReadModel({ accessStore: finalAccessStore });
};
1 change: 0 additions & 1 deletion src/lib/features/access/createAccessService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ export const createAccessService = (
{ getLogger },
eventService,
);

return new AccessService(
{ accessStore, accountStore, roleStore, environmentStore },
{ getLogger },
Expand Down
8 changes: 8 additions & 0 deletions src/lib/features/events/createEventsService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ import {
createFakePrivateProjectChecker,
createPrivateProjectChecker,
} from '../private-project/createPrivateProjectChecker';
import {
createAccessReadModel,
createFakeAccessReadModel,
} from '../access/createAccessReadModel';

export const createEventsService: (
db: Db,
Expand All @@ -25,10 +29,12 @@ export const createEventsService: (
config.getLogger,
);
const privateProjectChecker = createPrivateProjectChecker(db, config);
const accessReadModel = createAccessReadModel(db, config);
return new EventService(
{ eventStore, featureTagStore },
config,
privateProjectChecker,
accessReadModel,
);
};

Expand All @@ -43,9 +49,11 @@ export const createFakeEventsService: (
const featureTagStore =
stores?.featureTagStore || new FakeFeatureTagStore();
const fakePrivateProjectChecker = createFakePrivateProjectChecker();
const fakeAccessReadModel = createFakeAccessReadModel();
return new EventService(
{ eventStore, featureTagStore },
config,
fakePrivateProjectChecker,
fakeAccessReadModel,
);
};
11 changes: 4 additions & 7 deletions src/lib/features/events/event-created-by-migration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { EventEmitter } from 'stream';
import { EVENTS_CREATED_BY_PROCESSED } from '../../metric-events';
import type { IUnleashConfig } from '../../types';
import { createTestConfig } from '../../../test/config/test-config';
import EventService from './event-service';
import { createEventsService } from './createEventsService';

let db: ITestDb;

Expand Down Expand Up @@ -126,14 +126,11 @@ test('emits events with details on amount of updated rows', async () => {
const store = new EventStore(db.rawDatabase, getLogger);

const eventBus = new EventEmitter();
const service = new EventService(
{ eventStore: store, featureTagStore: db.stores.featureTagStore },
{ getLogger, eventBus },
{} as any,
);
const config = createTestConfig();
const service = createEventsService(db.rawDatabase, config);
let triggered = false;

eventBus.on(EVENTS_CREATED_BY_PROCESSED, ({ updated }) => {
config.eventBus.on(EVENTS_CREATED_BY_PROCESSED, ({ updated }) => {
expect(updated).toBe(2);
triggered = true;
});
Expand Down
15 changes: 15 additions & 0 deletions src/lib/features/events/event-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { parseSearchOperatorValue } from '../feature-search/search-utils';
import { endOfDay, formatISO } from 'date-fns';
import type { IPrivateProjectChecker } from '../private-project/privateProjectCheckerType';
import type { ProjectAccess } from '../private-project/privateProjectStore';
import type { IAccessReadModel } from '../access/access-read-model-type';

export default class EventService {
private logger: Logger;
Expand All @@ -24,6 +25,8 @@ export default class EventService {

private featureTagStore: IFeatureTagStore;

private accessReadModel: IAccessReadModel;

private privateProjectChecker: IPrivateProjectChecker;

private eventBus: EventEmitter;
Expand All @@ -35,12 +38,14 @@ export default class EventService {
}: Pick<IUnleashStores, 'eventStore' | 'featureTagStore'>,
{ getLogger, eventBus }: Pick<IUnleashConfig, 'getLogger' | 'eventBus'>,
privateProjectChecker: IPrivateProjectChecker,
accessReadModel: IAccessReadModel,
) {
this.logger = getLogger('services/event-service.ts');
this.eventStore = eventStore;
this.privateProjectChecker = privateProjectChecker;
this.featureTagStore = featureTagStore;
this.eventBus = eventBus;
this.accessReadModel = accessReadModel;
}

async getEvents(): Promise<IEventList> {
Expand Down Expand Up @@ -77,6 +82,8 @@ export default class EventService {
);

const queryParams = this.convertToDbParams(search);
const projectFilter = await this.getProjectFilterForNonAdmins(userId);
queryParams.push(...projectFilter);

const totalEvents = await this.eventStore.searchEventsCount(
{
Expand Down Expand Up @@ -222,6 +229,14 @@ export default class EventService {
async getEventCreators() {
return this.eventStore.getEventCreators();
}

async getProjectFilterForNonAdmins(userId: number): Promise<IQueryParam[]> {
const isRootAdmin = await this.accessReadModel.isRootAdmin(userId);
if (!isRootAdmin) {
return [{ field: 'project', operator: 'IS_NOT', values: [null] }];
}
return [];
}
}

export const filterAccessibleProjects = (
Expand Down
7 changes: 6 additions & 1 deletion src/lib/features/feature-search/search-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,19 @@ export const applyGenericQueryParams = (
queryParams: IQueryParam[],
): void => {
queryParams.forEach((param) => {
const isSingleParam = param.values.length === 1;
switch (param.operator) {
case 'IS':
case 'IS_ANY_OF':
query.whereIn(param.field, param.values);
break;
case 'IS_NOT':
case 'IS_NONE_OF':
query.whereNotIn(param.field, param.values);
if (isSingleParam) {
query.whereNot(param.field, param.values[0]);
} else {
query.whereNotIn(param.field, param.values);
}
break;
case 'IS_BEFORE':
query.where(param.field, '<', param.values[0]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export type IQueryOperator =
export interface IQueryParam {
field: string;
operator: IQueryOperator;
values: string[];
values: (string | null)[];
}

export interface IFeatureStrategiesStore
Expand Down
2 changes: 1 addition & 1 deletion src/lib/features/segment/segment-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ export class SegmentService implements ISegmentService {
await this.eventService.storeEvent(
new SegmentCreatedEvent({
data: segment,
project: segment.project || 'no-project',
project: segment.project,
auditUser,
}),
);
Expand Down
19 changes: 14 additions & 5 deletions src/lib/services/access-service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,22 @@ import {
} from '../../lib/types';
import BadDataError from '../../lib/error/bad-data-error';
import { createFakeEventsService } from '../../lib/features/events/createEventsService';
import { createFakeAccessReadModel } from '../features/access/createAccessReadModel';

function getSetup() {
const config = createTestConfig({
getLogger,
});

return createFakeAccessService(config);
const { accessService, eventStore, accessStore } =
createFakeAccessService(config);

return {
accessService,
eventStore,
accessStore,
accessReadModel: createFakeAccessReadModel(accessStore),
};
}

test('should fail when name exists', async () => {
Expand Down Expand Up @@ -287,28 +296,28 @@ describe('addAccessToProject', () => {
});

test('should return true if user has admin role', async () => {
const { accessService, accessStore } = getSetup();
const { accessReadModel, accessStore } = getSetup();

const userId = 1;
accessStore.getRolesForUserId = jest
.fn()
.mockResolvedValue([{ id: 1, name: 'ADMIN', type: 'custom' }]);

const result = await accessService.isRootAdmin(userId);
const result = await accessReadModel.isRootAdmin(userId);

expect(result).toBe(true);
expect(accessStore.getRolesForUserId).toHaveBeenCalledWith(userId);
});

test('should return false if user does not have admin role', async () => {
const { accessService, accessStore } = getSetup();
const { accessReadModel, accessStore } = getSetup();

const userId = 2;
accessStore.getRolesForUserId = jest
.fn()
.mockResolvedValue([{ id: 2, name: 'user', type: 'custom' }]);

const result = await accessService.isRootAdmin(userId);
const result = await accessReadModel.isRootAdmin(userId);

expect(result).toBe(false);
expect(accessStore.getRolesForUserId).toHaveBeenCalledWith(userId);
Expand Down
12 changes: 0 additions & 12 deletions src/lib/services/access-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,11 @@ import BadDataError from '../error/bad-data-error';
import type { IGroup } from '../types/group';
import type { GroupService } from './group-service';
import {
ADMIN_TOKEN_USER,
type IUnleashConfig,
type IUserAccessOverview,
RoleCreatedEvent,
RoleDeletedEvent,
RoleUpdatedEvent,
SYSTEM_USER_ID,
} from '../types';
import type EventService from '../features/events/event-service';

Expand Down Expand Up @@ -889,14 +887,4 @@ export class AccessService {
async getUserAccessOverview(): Promise<IUserAccessOverview[]> {
return this.store.getUserAccessOverview();
}

async isRootAdmin(userId: number): Promise<boolean> {
if (userId === SYSTEM_USER_ID || userId === ADMIN_TOKEN_USER.id) {
return true;
}
const roles = await this.store.getRolesForUserId(userId);
return roles.some(
(role) => role.name.toLowerCase() === ADMIN.toLowerCase(),
);
}
}
1 change: 1 addition & 0 deletions src/lib/services/feature-service-potentially-stale.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ test('Should only store events for potentially stale on', async () => {
},
config,
{},
{},
);

const featureToggleService = new FeatureToggleService(
Expand Down
4 changes: 2 additions & 2 deletions src/lib/types/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1930,12 +1930,12 @@ export class AddonConfigDeletedEvent extends BaseEvent {
}

export class SegmentCreatedEvent extends BaseEvent {
readonly project: string;
readonly project: string | undefined;
readonly data: any;

constructor(eventData: {
auditUser: IAuditUser;
project: string;
project: string | undefined;
data: any;
}) {
super(SEGMENT_CREATED, eventData.auditUser);
Expand Down
Loading

0 comments on commit df73c65

Please sign in to comment.