1
0
mirror of https://github.com/Unleash/unleash.git synced 2024-12-22 19:07:54 +01:00

refactor: read model for change request access checking (#3380)

This commit is contained in:
Mateusz Kwasniewski 2023-03-24 14:31:43 +01:00 committed by GitHub
parent 9cdd870365
commit ab913228ca
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
18 changed files with 184 additions and 47 deletions

View File

@ -475,19 +475,4 @@ export class AccessStore implements IAccessStore {
[destinationEnvironment, sourceEnvironment],
);
}
async isChangeRequestsEnabled(
project: string,
environment: string,
): Promise<boolean> {
const result = await this.db.raw(
`SELECT EXISTS(SELECT 1
FROM ${T.CHANGE_REQUEST_SETTINGS}
WHERE environment = ?
and project = ?) AS present`,
[environment, project],
);
const { present } = result.rows[0];
return present;
}
}

View File

@ -0,0 +1,13 @@
import User from '../../types/user';
export interface IChangeRequestAccessReadModel {
canBypassChangeRequest(
project: string,
environment: string,
user?: User,
): Promise<boolean>;
isChangeRequestsEnabled(
project: string,
environment: string,
): Promise<boolean>;
}

View File

@ -0,0 +1,19 @@
import { Db, IUnleashConfig } from 'lib/server-impl';
import { ChangeRequestAccessReadModel } from './sql-change-request-access-read-model';
import { createAccessService } from '../access/createAccessService';
import { FakeChangeRequestAccessReadModel } from './fake-change-request-access-read-model';
import { IChangeRequestAccessReadModel } from './change-request-access-read-model';
export const createChangeRequestAccessReadModel = (
db: Db,
config: IUnleashConfig,
): IChangeRequestAccessReadModel => {
const accessService = createAccessService(db, config);
return new ChangeRequestAccessReadModel(db, accessService);
};
export const createFakeChangeRequestAccessService =
(): IChangeRequestAccessReadModel => {
return new FakeChangeRequestAccessReadModel();
};

View File

@ -0,0 +1,22 @@
import { IChangeRequestAccessReadModel } from './change-request-access-read-model';
export class FakeChangeRequestAccessReadModel
implements IChangeRequestAccessReadModel
{
private canBypass: boolean;
private isChangeRequestEnabled: boolean;
constructor(canBypass = true, isChangeRequestEnabled = true) {
this.canBypass = canBypass;
this.isChangeRequestEnabled = isChangeRequestEnabled;
}
public async canBypassChangeRequest(): Promise<boolean> {
return this.canBypass;
}
public async isChangeRequestsEnabled(): Promise<boolean> {
return this.isChangeRequestEnabled;
}
}

View File

@ -0,0 +1,52 @@
import { SKIP_CHANGE_REQUEST } from '../../types';
import { Db } from '../../db/db';
import { AccessService } from '../../services';
import User from '../../types/user';
import { IChangeRequestAccessReadModel } from './change-request-access-read-model';
export class ChangeRequestAccessReadModel
implements IChangeRequestAccessReadModel
{
private db: Db;
private accessService: AccessService;
constructor(db: Db, accessService: AccessService) {
this.db = db;
this.accessService = accessService;
}
public async canBypassChangeRequest(
project: string,
environment: string,
user?: User,
): Promise<boolean> {
const [canSkipChangeRequest, changeRequestEnabled] = await Promise.all([
user
? this.accessService.hasPermission(
user,
SKIP_CHANGE_REQUEST,
project,
environment,
)
: Promise.resolve(false),
this.isChangeRequestsEnabled(project, environment),
]);
return !(changeRequestEnabled && !canSkipChangeRequest);
}
public async isChangeRequestsEnabled(
project: string,
environment: string,
): Promise<boolean> {
const result = await this.db.raw(
`SELECT EXISTS(SELECT 1
FROM change_request_settings
WHERE environment = ?
and project = ?) AS present`,
[environment, project],
);
const { present } = result.rows[0];
return present;
}
}

View File

@ -34,6 +34,10 @@ import FakeAccessStore from '../../../test/fixtures/fake-access-store';
import FakeRoleStore from '../../../test/fixtures/fake-role-store';
import FakeEnvironmentStore from '../../../test/fixtures/fake-environment-store';
import EventStore from '../../db/event-store';
import {
createChangeRequestAccessReadModel,
createFakeChangeRequestAccessService,
} from '../change-request-access-service/createChangeRequestAccessReadModel';
export const createFeatureToggleService = (
db: Db,
@ -85,6 +89,10 @@ export const createFeatureToggleService = (
{ segmentStore, featureStrategiesStore, eventStore },
config,
);
const changeRequestAccessReadMode = createChangeRequestAccessReadModel(
db,
config,
);
const featureToggleService = new FeatureToggleService(
{
featureStrategiesStore,
@ -99,6 +107,7 @@ export const createFeatureToggleService = (
{ getLogger, flagResolver },
segmentService,
accessService,
changeRequestAccessReadMode,
);
return featureToggleService;
};
@ -134,6 +143,7 @@ export const createFakeFeatureToggleService = (
{ segmentStore, featureStrategiesStore, eventStore },
config,
);
const changeRequestAccessReadMode = createFakeChangeRequestAccessService();
const featureToggleService = new FeatureToggleService(
{
featureStrategiesStore,
@ -148,6 +158,7 @@ export const createFakeFeatureToggleService = (
{ getLogger, flagResolver },
segmentService,
accessService,
changeRequestAccessReadMode,
);
return featureToggleService;
};

View File

@ -2,3 +2,4 @@ export * from './access/createAccessService';
export * from './export-import-toggles/createExportImportService';
export * from './feature-toggle/createFeatureToggleService';
export * from './project/createProjectService';
export * from './change-request-access-service/createChangeRequestAccessReadModel';

View File

@ -550,11 +550,4 @@ export class AccessService {
await this.validateRoleIsUnique(role.name, existingId);
return cleanedRole;
}
async isChangeRequestsEnabled(
project: string,
environment: string,
): Promise<boolean> {
return this.store.isChangeRequestsEnabled(project, environment);
}
}

View File

@ -83,6 +83,7 @@ import NoAccessError from '../error/no-access-error';
import { IFeatureProjectUserParams } from '../routes/admin-api/project/project-features';
import { unique } from '../util/unique';
import { ISegmentService } from 'lib/segments/segment-service-interface';
import { IChangeRequestAccessReadModel } from '../features/change-request-access-service/change-request-access-read-model';
interface IFeatureContext {
featureName: string;
@ -130,6 +131,8 @@ class FeatureToggleService {
private flagResolver: IFlagResolver;
private changeRequestAccessReadModel: IChangeRequestAccessReadModel;
constructor(
{
featureStrategiesStore,
@ -157,6 +160,7 @@ class FeatureToggleService {
}: Pick<IUnleashConfig, 'getLogger' | 'flagResolver'>,
segmentService: ISegmentService,
accessService: AccessService,
changeRequestAccessReadModel: IChangeRequestAccessReadModel,
) {
this.logger = getLogger('services/feature-toggle-service.ts');
this.featureStrategiesStore = featureStrategiesStore;
@ -170,6 +174,7 @@ class FeatureToggleService {
this.segmentService = segmentService;
this.accessService = accessService;
this.flagResolver = flagResolver;
this.changeRequestAccessReadModel = changeRequestAccessReadModel;
}
async validateFeaturesContext(
@ -1734,18 +1739,13 @@ class FeatureToggleService {
environment: string,
user?: User,
) {
const [canSkipChangeRequest, changeRequestEnabled] = await Promise.all([
user
? this.accessService.hasPermission(
user,
SKIP_CHANGE_REQUEST,
const canBypass =
await this.changeRequestAccessReadModel.canBypassChangeRequest(
project,
environment,
)
: Promise.resolve(false),
this.accessService.isChangeRequestsEnabled(project, environment),
]);
if (changeRequestEnabled && !canSkipChangeRequest) {
user,
);
if (!canBypass) {
throw new NoAccessError(SKIP_CHANGE_REQUEST);
}
}

View File

@ -52,6 +52,10 @@ import {
createFakeExportImportTogglesService,
} from '../features/export-import-toggles/createExportImportService';
import { Db } from '../db/db';
import {
createChangeRequestAccessReadModel,
createFakeChangeRequestAccessService,
} from '../features/change-request-access-service/createChangeRequestAccessReadModel';
// TODO: will be moved to scheduler feature directory
export const scheduleServices = (
@ -149,11 +153,15 @@ export const createServices = (
const healthService = new HealthService(stores, config);
const userFeedbackService = new UserFeedbackService(stores, config);
const segmentService = new SegmentService(stores, config);
const changeRequestAccessReadModel = db
? createChangeRequestAccessReadModel(db, config)
: createFakeChangeRequestAccessService();
const featureToggleServiceV2 = new FeatureToggleService(
stores,
config,
segmentService,
accessService,
changeRequestAccessReadModel,
);
const environmentService = new EnvironmentService(stores, config);
const featureTagService = new FeatureTagService(stores, config);

View File

@ -138,9 +138,4 @@ export interface IAccessStore extends Store<IRole, number> {
sourceEnvironment: string,
destinationEnvironment: string,
): Promise<void>;
isChangeRequestsEnabled(
project: string,
environment: string,
): Promise<boolean>;
}

View File

@ -15,6 +15,7 @@ import { ALL_PROJECTS } from '../../../lib/util/constants';
import { SegmentService } from '../../../lib/services/segment-service';
import { GroupService } from '../../../lib/services/group-service';
import { FavoritesService } from '../../../lib/services';
import { ChangeRequestAccessReadModel } from '../../../lib/features/change-request-access-service/sql-change-request-access-read-model';
let db: ITestDb;
let stores: IUnleashStores;
@ -216,11 +217,16 @@ beforeAll(async () => {
editorRole = roles.find((r) => r.name === RoleName.EDITOR);
adminRole = roles.find((r) => r.name === RoleName.ADMIN);
readRole = roles.find((r) => r.name === RoleName.VIEWER);
const changeRequestAccessReadModel = new ChangeRequestAccessReadModel(
db.rawDatabase,
accessService,
);
featureToggleService = new FeatureToggleService(
stores,
config,
new SegmentService(stores, config),
accessService,
changeRequestAccessReadModel,
);
favoritesService = new FavoritesService(stores, config);
projectService = new ProjectService(

View File

@ -11,6 +11,7 @@ import { AccessService } from '../../../lib/services/access-service';
import { SegmentService } from '../../../lib/services/segment-service';
import { GroupService } from '../../../lib/services/group-service';
import { FavoritesService } from '../../../lib/services';
import { ChangeRequestAccessReadModel } from '../../../lib/features/change-request-access-service/sql-change-request-access-read-model';
let db;
let stores;
@ -26,11 +27,16 @@ beforeAll(async () => {
stores = db.stores;
const groupService = new GroupService(stores, config);
const accessService = new AccessService(stores, config, groupService);
const changeRequestAccessReadModel = new ChangeRequestAccessReadModel(
db.rawDatabase,
accessService,
);
const featureToggleService = new FeatureToggleService(
stores,
config,
new SegmentService(stores, config),
accessService,
changeRequestAccessReadModel,
);
const project = {
id: 'test-project',

View File

@ -12,6 +12,7 @@ import EnvironmentService from '../../../lib/services/environment-service';
import { NoAccessError } from '../../../lib/error';
import { SKIP_CHANGE_REQUEST } from '../../../lib/types';
import { ISegmentService } from '../../../lib/segments/segment-service-interface';
import { ChangeRequestAccessReadModel } from '../../../lib/features/change-request-access-service/sql-change-request-access-read-model';
let stores;
let db;
@ -39,11 +40,16 @@ beforeAll(async () => {
segmentService = new SegmentService(stores, config);
const groupService = new GroupService(stores, config);
const accessService = new AccessService(stores, config, groupService);
const changeRequestAccessReadModel = new ChangeRequestAccessReadModel(
db.rawDatabase,
accessService,
);
service = new FeatureToggleService(
stores,
config,
segmentService,
accessService,
changeRequestAccessReadModel,
);
});
@ -384,6 +390,10 @@ test('If change requests are enabled, cannot change variants without going via C
unleashConfig,
groupService,
);
const changeRequestAccessReadModel = new ChangeRequestAccessReadModel(
db.rawDatabase,
accessService,
);
// Force all feature flags on to make sure we have Change requests on
const customFeatureService = new FeatureToggleService(
stores,
@ -395,6 +405,7 @@ test('If change requests are enabled, cannot change variants without going via C
},
segmentService,
accessService,
changeRequestAccessReadModel,
);
const newVariant: IVariant = {
@ -462,6 +473,10 @@ test('If CRs are protected for any environment in the project stops bulk update
unleashConfig,
groupService,
);
const changeRequestAccessReadModel = new ChangeRequestAccessReadModel(
db.rawDatabase,
accessService,
);
// Force all feature flags on to make sure we have Change requests on
const customFeatureService = new FeatureToggleService(
stores,
@ -473,6 +488,7 @@ test('If CRs are protected for any environment in the project stops bulk update
},
segmentService,
accessService,
changeRequestAccessReadModel,
);
const toggle = await service.createFeatureToggle(

View File

@ -21,6 +21,7 @@ import { PlaygroundSegmentSchema } from 'lib/openapi/spec/playground-segment-sch
import { GroupService } from '../../../lib/services/group-service';
import { AccessService } from '../../../lib/services/access-service';
import { ISegmentService } from '../../../lib/segments/segment-service-interface';
import { ChangeRequestAccessReadModel } from '../../../lib/features/change-request-access-service/sql-change-request-access-read-model';
let stores: IUnleashStores;
let db: ITestDb;
@ -35,11 +36,16 @@ beforeAll(async () => {
segmentService = new SegmentService(stores, config);
const groupService = new GroupService(stores, config);
const accessService = new AccessService(stores, config, groupService);
const changeRequestAccessReadModel = new ChangeRequestAccessReadModel(
db.rawDatabase,
accessService,
);
featureToggleService = new FeatureToggleService(
stores,
config,
segmentService,
accessService,
changeRequestAccessReadModel,
);
service = new PlaygroundService(config, {
featureToggleServiceV2: featureToggleService,

View File

@ -10,6 +10,7 @@ import { IUser } from '../../../lib/server-impl';
import { SegmentService } from '../../../lib/services/segment-service';
import { GroupService } from '../../../lib/services/group-service';
import { FavoritesService } from '../../../lib/services';
import { ChangeRequestAccessReadModel } from '../../../lib/features/change-request-access-service/sql-change-request-access-read-model';
let stores: IUnleashStores;
let db: ITestDb;
@ -31,11 +32,16 @@ beforeAll(async () => {
});
groupService = new GroupService(stores, config);
accessService = new AccessService(stores, config, groupService);
const changeRequestAccessReadModel = new ChangeRequestAccessReadModel(
db.rawDatabase,
accessService,
);
featureToggleService = new FeatureToggleService(
stores,
config,
new SegmentService(stores, config),
accessService,
changeRequestAccessReadModel,
);
favoritesService = new FavoritesService(stores, config);

View File

@ -14,6 +14,7 @@ import { GroupService } from '../../../lib/services/group-service';
import { FavoritesService } from '../../../lib/services';
import { FeatureEnvironmentEvent } from '../../../lib/types/events';
import { subDays } from 'date-fns';
import { ChangeRequestAccessReadModel } from '../../../lib/features/change-request-access-service/sql-change-request-access-read-model';
let stores;
let db: ITestDb;
@ -50,11 +51,16 @@ beforeAll(async () => {
});
groupService = new GroupService(stores, config);
accessService = new AccessService(stores, config, groupService);
const changeRequestAccessReadModel = new ChangeRequestAccessReadModel(
db.rawDatabase,
accessService,
);
featureToggleService = new FeatureToggleService(
stores,
config,
new SegmentService(stores, config),
accessService,
changeRequestAccessReadModel,
);
favoritesService = new FavoritesService(stores, config);

View File

@ -1,5 +1,4 @@
/* eslint-disable @typescript-eslint/no-unused-vars */
import noLoggerProvider from './no-logger';
import {
IAccessInfo,
IAccessStore,
@ -11,13 +10,6 @@ import {
import { IPermission } from 'lib/types/model';
class AccessStoreMock implements IAccessStore {
isChangeRequestsEnabled(
project: string,
environment: string,
): Promise<boolean> {
throw new Error('Method not implemented.');
}
addAccessToProject(
users: IAccessInfo[],
groups: IAccessInfo[],