feat: audit roles (#5408)

## About the changes Audit changes to roles both root and project roles.
2024-12-22 19:07:54 +01:00 · 2023-11-24 14:22:31 +01:00 · 2023-11-24 14:22:31 +01:00 · d680e50055
commit d680e50055
parent 295b0c073e
17 changed files with 197 additions and 53 deletions
--- a/src/lib/features/access/createAccessService.ts
+++ b/src/lib/features/access/createAccessService.ts
@ -14,6 +14,7 @@ import FakeEnvironmentStore from '../../../test/fixtures/fake-environment-store'
 import FakeAccessStore from '../../../test/fixtures/fake-access-store';
 import FeatureTagStore from '../../db/feature-tag-store';
 import FakeFeatureTagStore from '../../../test/fixtures/fake-feature-tag-store';
+import { IEventStore } from '../../types';

 export const createAccessService = (
    db: Db,
@ -38,15 +39,16 @@ export const createAccessService = (
    );

    return new AccessService(
-        { accessStore, accountStore, roleStore, environmentStore, groupStore },
+        { accessStore, accountStore, roleStore, environmentStore },
        { getLogger, flagResolver },
        groupService,
+        eventService,
    );
 };

 export const createFakeAccessService = (
    config: IUnleashConfig,
-): AccessService => {
+): { accessService: AccessService; eventStore: IEventStore } => {
    const { getLogger, flagResolver } = config;
    const eventStore = new FakeEventStore();
    const groupStore = new FakeGroupStore();
@ -65,9 +67,15 @@ export const createFakeAccessService = (
        eventService,
    );

-    return new AccessService(
+    const accessService = new AccessService(
        { accessStore, accountStore, roleStore, environmentStore, groupStore },
        { getLogger, flagResolver },
        groupService,
+        eventService,
    );
+
+    return {
+        accessService,
+        eventStore,
+    };
 };
--- a/src/lib/features/export-import-toggles/createExportImportService.ts
+++ b/src/lib/features/export-import-toggles/createExportImportService.ts
@ -71,7 +71,7 @@ export const createFakeExportImportTogglesService = (
    const eventStore = new FakeEventStore();
    const featureStrategiesStore = new FakeFeatureStrategiesStore();
    const featureEnvironmentStore = new FakeFeatureEnvironmentStore();
-    const accessService = createFakeAccessService(config);
+    const { accessService } = createFakeAccessService(config);
    const featureToggleService = createFakeFeatureToggleService(config);
    const privateProjectChecker = createFakePrivateProjectChecker();

--- a/src/lib/features/feature-toggle/createFeatureToggleService.ts
+++ b/src/lib/features/feature-toggle/createFeatureToggleService.ts
@ -110,9 +110,10 @@ export const createFeatureToggleService = (
        eventService,
    );
    const accessService = new AccessService(
-        { accessStore, accountStore, roleStore, environmentStore, groupStore },
+        { accessStore, accountStore, roleStore, environmentStore },
        { getLogger, flagResolver },
        groupService,
+        eventService,
    );
    const segmentService = createSegmentService(db, config);
    const changeRequestAccessReadModel = createChangeRequestAccessReadModel(
@ -180,6 +181,7 @@ export const createFakeFeatureToggleService = (
        { accessStore, accountStore, roleStore, environmentStore, groupStore },
        { getLogger, flagResolver },
        groupService,
+        eventService,
    );
    const segmentService = createFakeSegmentService(config);
    const changeRequestAccessReadModel = createFakeChangeRequestAccessService();
--- a/src/lib/features/project/createProjectService.ts
+++ b/src/lib/features/project/createProjectService.ts
@ -134,7 +134,7 @@ export const createFakeProjectService = (
    const environmentStore = new FakeEnvironmentStore();
    const featureEnvironmentStore = new FakeFeatureEnvironmentStore();
    const projectStatsStore = new FakeProjectStatsStore();
-    const accessService = createFakeAccessService(config);
+    const { accessService } = createFakeAccessService(config);
    const featureToggleService = createFakeFeatureToggleService(config);
    const favoriteFeaturesStore = new FakeFavoriteFeaturesStore();
    const favoriteProjectsStore = new FakeFavoriteProjectsStore();
--- a/src/lib/services/access-service.test.ts
+++ b/src/lib/services/access-service.test.ts
@ -1,7 +1,11 @@
 import NameExistsError from '../error/name-exists-error';
 import getLogger from '../../test/fixtures/no-logger';
 import { createFakeAccessService } from '../features/access/createAccessService';
-import { AccessService, IRoleValidation } from './access-service';
+import {
+    AccessService,
+    IRoleCreation,
+    IRoleValidation,
+} from './access-service';
 import { createTestConfig } from '../../test/config/test-config';
 import { CUSTOM_ROOT_ROLE_TYPE } from '../util/constants';
 import FakeGroupStore from '../../test/fixtures/fake-group-store';
@ -11,8 +15,8 @@ import FakeEnvironmentStore from '../../test/fixtures/fake-environment-store';
 import AccessStoreMock from '../../test/fixtures/fake-access-store';
 import { GroupService } from '../services/group-service';
 import FakeEventStore from '../../test/fixtures/fake-event-store';
-import { IRole } from 'lib/types/stores/access-store';
-import { IGroup } from 'lib/types';
+import { IRole } from '../../lib/types/stores/access-store';
+import { IGroup, ROLE_CREATED } from '../../lib/types';
 import EventService from './event-service';
 import FakeFeatureTagStore from '../../test/fixtures/fake-feature-tag-store';

@ -26,9 +30,7 @@ function getSetup(customRootRolesKillSwitch: boolean = true) {
        },
    });

-    return {
-        accessService: createFakeAccessService(config),
-    };
+    return createFakeAccessService(config);
 }

 test('should fail when name exists', async () => {
@ -164,13 +166,24 @@ test('should be able to validate and cleanup with additional properties', async
 });

 test('user with custom root role should get a user root role', async () => {
-    const { accessService } = getSetup(false);
-    const customRootRole = await accessService.createRole({
+    const { accessService, eventStore } = getSetup(false);
+    const createRoleInput: IRoleCreation = {
        name: 'custom-root-role',
        description: 'test custom root role',
        type: CUSTOM_ROOT_ROLE_TYPE,
-        permissions: [],
-    });
+        permissions: [
+            {
+                id: 1,
+                environment: 'development',
+                name: 'fake',
+            },
+            {
+                name: 'root-fake-permission',
+            },
+        ],
+    };
+
+    const customRootRole = await accessService.createRole(createRoleInput);
    const user = {
        id: 1,
        rootRole: customRootRole.id,
@ -180,6 +193,23 @@ test('user with custom root role should get a user root role', async () => {
    const roles = await accessService.getUserRootRoles(user.id);
    expect(roles).toHaveLength(1);
    expect(roles[0].name).toBe('custom-root-role');
+    const events = await eventStore.getEvents();
+    expect(events).toHaveLength(1);
+    expect(events[0]).toEqual({
+        type: ROLE_CREATED,
+        createdBy: 'unknown',
+        data: {
+            id: 0,
+            name: 'custom-root-role',
+            description: 'test custom root role',
+            type: CUSTOM_ROOT_ROLE_TYPE,
+            // make sure we have a cleaned up version of permissions in the event
+            permissions: [
+                { environment: 'development', name: 'fake' },
+                { name: 'root-fake-permission' },
+            ],
+        },
+    });
 });

 test('throws error when trying to delete a project role in use by group', async () => {
@ -222,10 +252,10 @@ test('throws error when trying to delete a project role in use by group', async
            accountStore,
            roleStore,
            environmentStore,
-            groupStore,
        },
        config,
        groupService,
+        eventService,
    );

    try {
--- a/src/lib/services/access-service.ts
+++ b/src/lib/services/access-service.ts
@ -1,5 +1,5 @@
 import * as permissions from '../types/permissions';
-import User, { IUser } from '../types/user';
+import { IUser } from '../types/user';
 import {
    IAccessInfo,
    IAccessStore,
@ -14,7 +14,7 @@ import {
    IUserWithProjectRoles,
 } from '../types/stores/access-store';
 import { Logger } from '../logger';
-import { IAccountStore, IGroupStore, IUnleashStores } from '../types/stores';
+import { IAccountStore, IUnleashStores } from '../types/stores';
 import {
    IAvailablePermissions,
    ICustomRole,
@ -23,9 +23,9 @@ import {
    IUserWithRole,
    RoleName,
 } from '../types/model';
-import { IRoleStore } from 'lib/types/stores/role-store';
+import { IRoleStore } from '../types/stores/role-store';
 import NameExistsError from '../error/name-exists-error';
-import { IEnvironmentStore } from 'lib/types/stores/environment-store';
+import { IEnvironmentStore } from '../types/stores/environment-store';
 import RoleInUseError from '../error/role-in-use-error';
 import { roleSchema } from '../schema/role-schema';
 import {
@ -40,7 +40,15 @@ import InvalidOperationError from '../error/invalid-operation-error';
 import BadDataError from '../error/bad-data-error';
 import { IGroup } from '../types/group';
 import { GroupService } from './group-service';
-import { IFlagResolver, IUnleashConfig, IUserAccessOverview } from 'lib/types';
+import {
+    IFlagResolver,
+    IUnleashConfig,
+    IUserAccessOverview,
+    ROLE_CREATED,
+    ROLE_DELETED,
+    ROLE_UPDATED,
+} from '../types';
+import EventService from './event-service';

 const { ADMIN } = permissions;

@ -57,11 +65,12 @@ export type IdPermissionRef = Pick<IPermission, 'id' | 'environment'>;
 export type NamePermissionRef = Pick<IPermission, 'name' | 'environment'>;
 export type PermissionRef = IdPermissionRef | NamePermissionRef;

-interface IRoleCreation {
+export interface IRoleCreation {
    name: string;
    description: string;
    type?: 'root-custom' | 'custom';
    permissions?: PermissionRef[];
+    createdBy?: string;
 }

 export interface IRoleValidation {
@ -76,6 +85,7 @@ export interface IRoleUpdate {
    description: string;
    type?: 'root-custom' | 'custom';
    permissions?: PermissionRef[];
+    createdBy?: string;
 }

 export interface AccessWithRoles {
@ -95,34 +105,30 @@ export class AccessService {

    private groupService: GroupService;

-    private groupStore: IGroupStore;
-
    private environmentStore: IEnvironmentStore;

    private logger: Logger;

    private flagResolver: IFlagResolver;

+    private eventService: EventService;
+
    constructor(
        {
            accessStore,
            accountStore,
            roleStore,
            environmentStore,
-            groupStore,
        }: Pick<
            IUnleashStores,
-            | 'accessStore'
-            | 'accountStore'
-            | 'roleStore'
-            | 'environmentStore'
-            | 'groupStore'
-        >,
+            'accessStore' | 'accountStore' | 'roleStore' | 'environmentStore'
+        > & { groupStore?: any }, // TODO remove groupStore later, kept for backward compatibility with enterprise
        {
            getLogger,
            flagResolver,
        }: Pick<IUnleashConfig, 'getLogger' | 'flagResolver'>,
        groupService: GroupService,
+        eventService: EventService,
    ) {
        this.store = accessStore;
        this.accountStore = accountStore;
@ -131,7 +137,7 @@ export class AccessService {
        this.environmentStore = environmentStore;
        this.logger = getLogger('/services/access-service.ts');
        this.flagResolver = flagResolver;
-        this.groupStore = groupStore;
+        this.eventService = eventService;
    }

    /**
@ -483,7 +489,7 @@ export class AccessService {
    async getGroupsForRole(roleId: number): Promise<IGroup[]> {
        const groupdIdList = await this.store.getGroupIdsForRole(roleId);
        if (groupdIdList.length > 0) {
-            return this.groupStore.getAllWithId(groupdIdList);
+            return this.groupService.getAllWithId(groupdIdList);
        }
        return [];
    }
@ -643,6 +649,17 @@ export class AccessService {
                );
            }
        }
+        const addedPermissions = await this.store.getPermissionsForRole(
+            newRole.id,
+        );
+        this.eventService.storeEvent({
+            type: ROLE_CREATED,
+            createdBy: role.createdBy || 'unknown',
+            data: {
+                ...newRole,
+                permissions: this.sanitizePermissions(addedPermissions),
+            },
+        });
        return newRole;
    }

@ -662,6 +679,7 @@ export class AccessService {
        }

        await this.validateRole(role, role.id);
+        const existingRole = await this.roleStore.get(role.id);
        const baseRole = {
            id: role.id,
            name: role.name,
@ -669,25 +687,55 @@ export class AccessService {
            roleType,
        };
        const rolePermissions = role.permissions;
-        const newRole = await this.roleStore.update(baseRole);
+        const updatedRole = await this.roleStore.update(baseRole);
+        const existingPermissions = await this.store.getPermissionsForRole(
+            role.id,
+        );
        if (rolePermissions) {
-            await this.store.wipePermissionsFromRole(newRole.id);
+            await this.store.wipePermissionsFromRole(updatedRole.id);
            if (roleType === CUSTOM_ROOT_ROLE_TYPE) {
                await this.store.addPermissionsToRole(
-                    newRole.id,
+                    updatedRole.id,
                    rolePermissions,
                );
            } else {
                await this.store.addEnvironmentPermissionsToRole(
-                    newRole.id,
+                    updatedRole.id,
                    rolePermissions,
                );
            }
        }
-        return newRole;
+        const updatedPermissions = await this.store.getPermissionsForRole(
+            role.id,
+        );
+        this.eventService.storeEvent({
+            type: ROLE_UPDATED,
+            createdBy: role.createdBy || 'unknown',
+            data: {
+                ...updatedRole,
+                permissions: this.sanitizePermissions(updatedPermissions),
+            },
+            preData: {
+                ...existingRole,
+                permissions: this.sanitizePermissions(existingPermissions),
+            },
+        });
+        return updatedRole;
    }

-    async deleteRole(id: number): Promise<void> {
+    sanitizePermissions(
+        permissions: IPermission[],
+    ): { name: string; environment?: string }[] {
+        return permissions.map(({ name, environment }) => {
+            const sanitizedEnvironment =
+                environment && environment !== null && environment !== ''
+                    ? environment
+                    : undefined;
+            return { name, environment: sanitizedEnvironment };
+        });
+    }
+
+    async deleteRole(id: number, deletedBy = 'unknown'): Promise<void> {
        await this.validateRoleIsNotBuiltIn(id);

        const roleUsers = await this.getUsersForRole(id);
@ -699,7 +747,18 @@ export class AccessService {
            );
        }

-        return this.roleStore.delete(id);
+        const existingRole = await this.roleStore.get(id);
+        const existingPermissions = await this.store.getPermissionsForRole(id);
+        await this.roleStore.delete(id);
+        this.eventService.storeEvent({
+            type: ROLE_DELETED,
+            createdBy: deletedBy,
+            preData: {
+                ...existingRole,
+                permissions: this.sanitizePermissions(existingPermissions),
+            },
+        });
+        return;
    }

    async validateRoleIsUnique(
--- a/src/lib/services/email-service.ts
+++ b/src/lib/services/email-service.ts
@ -41,7 +41,7 @@ export class EmailService {

    private readonly sender: string;

-    constructor(email: IEmailOption, getLogger: LogProvider) {
+    constructor(email: IEmailOption | undefined, getLogger: LogProvider) {
        this.logger = getLogger('services/email-service.ts');
        if (email?.host) {
            this.sender = email.sender;
@ -101,7 +101,7 @@ export class EmailService {
                text: bodyText,
            };
            process.nextTick(() => {
-                this.mailer.sendMail(email).then(
+                this.mailer!.sendMail(email).then(
                    () =>
                        this.logger.info(
                            'Successfully sent reset-password email',
@ -162,7 +162,7 @@ export class EmailService {
                text: bodyText,
            };
            process.nextTick(() => {
-                this.mailer.sendMail(email).then(
+                this.mailer!.sendMail(email).then(
                    () =>
                        this.logger.info(
                            'Successfully sent getting started email',
--- a/src/lib/services/group-service.ts
+++ b/src/lib/services/group-service.ts
@ -59,6 +59,10 @@ export class GroupService {
        });
    }

+    async getAllWithId(ids: number[]) {
+        return this.groupStore.getAllWithId(ids);
+    }
+
    mapGroupWithProjects(
        groupProjects: IGroupProject[],
        group: IGroupModel,
--- a/src/lib/services/index.ts
+++ b/src/lib/services/index.ts
@ -106,7 +106,12 @@ export const createServices = (
 ): IUnleashServices => {
    const eventService = new EventService(stores, config);
    const groupService = new GroupService(stores, config, eventService);
-    const accessService = new AccessService(stores, config, groupService);
+    const accessService = new AccessService(
+        stores,
+        config,
+        groupService,
+        eventService,
+    );
    const apiTokenService = new ApiTokenService(stores, config, eventService);
    const lastSeenService = db
        ? createLastSeenService(db, config)
--- a/src/lib/types/events.ts
+++ b/src/lib/types/events.ts
@ -62,6 +62,10 @@ export const PROJECT_ACCESS_USER_ROLES_DELETED =
 export const PROJECT_ACCESS_GROUP_ROLES_DELETED =
    'project-access-group-roles-deleted';

+export const ROLE_CREATED = 'role-created';
+export const ROLE_UPDATED = 'role-updated';
+export const ROLE_DELETED = 'role-deleted';
+
 export const PROJECT_CREATED = 'project-created' as const;
 export const PROJECT_UPDATED = 'project-updated' as const;
 export const PROJECT_DELETED = 'project-deleted' as const;
@ -208,6 +212,9 @@ export const IEventTypes = [
    PROJECT_GROUP_ROLE_CHANGED,
    PROJECT_GROUP_ADDED,
    PROJECT_GROUP_REMOVED,
+    ROLE_CREATED,
+    ROLE_UPDATED,
+    ROLE_DELETED,
    DROP_PROJECTS,
    TAG_CREATED,
    TAG_DELETED,
--- a/src/test/e2e/api/auth/reset-password-controller.e2e.test.ts
+++ b/src/test/e2e/api/auth/reset-password-controller.e2e.test.ts
@ -51,7 +51,12 @@ beforeAll(async () => {
    app = await setupApp(stores);
    const eventService = new EventService(stores, config);
    const groupService = new GroupService(stores, config, eventService);
-    accessService = new AccessService(stores, config, groupService);
+    accessService = new AccessService(
+        stores,
+        config,
+        groupService,
+        eventService,
+    );
    const emailService = new EmailService(config.email, config.getLogger);
    const sessionStore = new SessionStore(
        db,
--- a/src/test/e2e/api/auth/simple-password-provider.e2e.test.ts
+++ b/src/test/e2e/api/auth/simple-password-provider.e2e.test.ts
@ -37,9 +37,13 @@ beforeAll(async () => {
    app = await setupApp(stores);
    const eventService = new EventService(stores, config);
    const groupService = new GroupService(stores, config, eventService);
-    const accessService = new AccessService(stores, config, groupService);
+    const accessService = new AccessService(
+        stores,
+        config,
+        groupService,
+        eventService,
+    );
    const resetTokenService = new ResetTokenService(stores, config);
-    // @ts-ignore
    const emailService = new EmailService(undefined, config.getLogger);
    const sessionService = new SessionService(stores, config);
    const settingService = new SettingService(stores, config, eventService);
--- a/src/test/e2e/services/reset-token-service.e2e.test.ts
+++ b/src/test/e2e/services/reset-token-service.e2e.test.ts
@ -30,7 +30,12 @@ beforeAll(async () => {
    stores = db.stores;
    const eventService = new EventService(stores, config);
    const groupService = new GroupService(stores, config, eventService);
-    accessService = new AccessService(stores, config, groupService);
+    accessService = new AccessService(
+        stores,
+        config,
+        groupService,
+        eventService,
+    );
    resetTokenService = new ResetTokenService(stores, config);
    sessionService = new SessionService(stores, config);
    const emailService = new EmailService(undefined, config.getLogger);
--- a/src/test/e2e/services/user-service.e2e.test.ts
+++ b/src/test/e2e/services/user-service.e2e.test.ts
@ -34,7 +34,12 @@ beforeAll(async () => {
    const config = createTestConfig();
    const eventService = new EventService(stores, config);
    const groupService = new GroupService(stores, config, eventService);
-    const accessService = new AccessService(stores, config, groupService);
+    const accessService = new AccessService(
+        stores,
+        config,
+        groupService,
+        eventService,
+    );
    const resetTokenService = new ResetTokenService(stores, config);
    const emailService = new EmailService(undefined, config.getLogger);
    sessionService = new SessionService(stores, config);
--- a/src/test/fixtures/access-service-mock.ts
+++ b/src/test/fixtures/access-service-mock.ts
@ -17,10 +17,10 @@ class AccessServiceMock extends AccessService {
                accountStore: undefined,
                roleStore: undefined,
                environmentStore: undefined,
-                groupStore: undefined,
            },
            { getLogger: noLoggerProvider, flagResolver: undefined },
            undefined,
+            undefined,
        );
    }

--- a/src/test/fixtures/fake-access-store.ts
+++ b/src/test/fixtures/fake-access-store.ts
@ -13,12 +13,15 @@ import { IPermission } from 'lib/types/model';
 import { IRoleStore, IUserAccessOverview } from 'lib/types';
 import FakeRoleStore from './fake-role-store';
 import { PermissionRef } from 'lib/services/access-service';
+import { P } from 'ts-toolbelt/out/Object/_api';

 class AccessStoreMock implements IAccessStore {
    fakeRolesStore: IRoleStore;

    userToRoleMap: Map<number, number> = new Map();

+    rolePermissions: Map<number, IPermission[]> = new Map();
+
    constructor(roleStore?: IRoleStore) {
        this.fakeRolesStore = roleStore ?? new FakeRoleStore();
    }
@ -133,7 +136,8 @@ class AccessStoreMock implements IAccessStore {
    }

    getPermissionsForRole(roleId: number): Promise<IPermission[]> {
-        throw new Error('Method not implemented.');
+        const found = this.rolePermissions.get(roleId) ?? [];
+        return Promise.resolve(found);
    }

    getRoles(): Promise<IRole[]> {
@ -183,7 +187,12 @@ class AccessStoreMock implements IAccessStore {
        permissions: PermissionRef[],
        environment?: string,
    ): Promise<void> {
-        // do nothing for now
+        this.rolePermissions.set(
+            role_id,
+            (environment
+                ? permissions.map((p) => ({ ...p, environment }))
+                : permissions) as IPermission[],
+        );
        return Promise.resolve(undefined);
    }

--- a/src/test/fixtures/fake-role-store.ts
+++ b/src/test/fixtures/fake-role-store.ts
@ -42,6 +42,7 @@ export default class FakeRoleStore implements IRoleStore {
            ...role,
            type: role.roleType,
            id: this.roles.length,
+            roleType: undefined, // roleType is not part of ICustomRole and simulates what the DB responds
        };
        this.roles.push(roleCreated);
        return Promise.resolve(roleCreated);