Skip to content

Commit

Permalink
Permission guards in existing endpoints interacting with feature togg…
Browse files Browse the repository at this point in the history
…le configuration (#2418)

This PR adds permission guards for operations.

1. Toggling feature flag
2. Adding a strategy
3. Updating a strategy
4. Deleting a strategy
  • Loading branch information
sjaanus committed Nov 14, 2022
1 parent 3624cdc commit 131ebb9
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 1 deletion.
14 changes: 14 additions & 0 deletions src/lib/db/access-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const T = {
ROLE_PERMISSION: 'role_permission',
PERMISSIONS: 'permissions',
PERMISSION_TYPES: 'permission_types',
CHANGE_REQUEST_SETTINGS: 'change_request_settings',
};

interface IPermissionRow {
Expand Down Expand Up @@ -448,4 +449,17 @@ 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;
}
}
7 changes: 7 additions & 0 deletions src/lib/services/access-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -542,4 +542,11 @@ 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);
}
}
43 changes: 42 additions & 1 deletion src/lib/services/feature-toggle-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,11 @@ class FeatureToggleService {
const { featureName, projectId, environment } = context;
await this.validateFeatureContext(context);

if (await this.changeRequestsEnabled(projectId, environment)) {
throw new Error(
`Strategies can only be created through change requests for ${environment} environment`,
);
}
if (strategyConfig.constraints?.length > 0) {
strategyConfig.constraints = await this.validateConstraints(
strategyConfig.constraints,
Expand Down Expand Up @@ -382,6 +387,12 @@ class FeatureToggleService {
const existingStrategy = await this.featureStrategiesStore.get(id);
this.validateFeatureStrategyContext(existingStrategy, context);

if (await this.changeRequestsEnabled(projectId, environment)) {
throw new Error(
`Strategies can only be updated through change requests for ${environment} environment`,
);
}

if (existingStrategy.id === id) {
if (updates.constraints?.length > 0) {
updates.constraints = await this.validateConstraints(
Expand Down Expand Up @@ -430,6 +441,12 @@ class FeatureToggleService {
): Promise<Saved<IStrategyConfig>> {
const { projectId, environment, featureName } = context;

if (await this.changeRequestsEnabled(projectId, environment)) {
throw new Error(
`Strategies can only be updated through change requests for ${environment} environment`,
);
}

const existingStrategy = await this.featureStrategiesStore.get(id);
this.validateFeatureStrategyContext(existingStrategy, context);

Expand Down Expand Up @@ -482,6 +499,12 @@ class FeatureToggleService {
const { featureName, projectId, environment } = context;
this.validateFeatureStrategyContext(existingStrategy, context);

if (await this.changeRequestsEnabled(projectId, environment)) {
throw new Error(
`Strategies can only deleted updated through change requests for ${environment} environment`,
);
}

await this.featureStrategiesStore.delete(id);

const tags = await this.tagStore.getAllTagsForFeature(featureName);
Expand Down Expand Up @@ -903,6 +926,12 @@ class FeatureToggleService {
createdBy: string,
user?: User,
): Promise<FeatureToggle> {
if (await this.changeRequestsEnabled(project, environment)) {
throw new Error(
`Features can only be updated through change requests for ${environment} environment`,
);
}

const hasEnvironment =
await this.featureEnvironmentStore.featureHasEnvironment(
environment,
Expand All @@ -928,7 +957,11 @@ class FeatureToggleService {
if (canAddStrategies) {
await this.createStrategy(
getDefaultStrategy(featureName),
{ environment, projectId: project, featureName },
{
environment,
projectId: project,
featureName,
},
createdBy,
);
} else {
Expand Down Expand Up @@ -961,6 +994,7 @@ class FeatureToggleService {
}
return feature;
}

throw new NotFoundError(
`Could not find environment ${environment} for feature: ${featureName}`,
);
Expand Down Expand Up @@ -1186,6 +1220,13 @@ class FeatureToggleService {
});
return variableVariants.concat(fixedVariants);
}

changeRequestsEnabled(
project: string,
environment: string,
): Promise<boolean> {
return this.accessService.isChangeRequestsEnabled(project, environment);
}
}

export default FeatureToggleService;
5 changes: 5 additions & 0 deletions src/lib/types/stores/access-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,4 +132,9 @@ export interface IAccessStore extends Store<IRole, number> {
sourceEnvironment: string,
destinationEnvironment: string,
): Promise<void>;

isChangeRequestsEnabled(
project: string,
environment: string,
): Promise<boolean>;
}
7 changes: 7 additions & 0 deletions src/test/fixtures/fake-access-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@ 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[],
Expand Down

0 comments on commit 131ebb9

Please sign in to comment.