Skip to content

Commit

Permalink
fix: add guard for deleting role if the role is in use
Browse files Browse the repository at this point in the history
  • Loading branch information
FredrikOseberg authored and ivarconr committed Jan 11, 2022
1 parent dda9cba commit 1e46628
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 1 deletion.
24 changes: 24 additions & 0 deletions src/lib/error/role-in-use-error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
class RoleInUseError extends Error {
constructor(message: string) {
super();
Error.captureStackTrace(this, this.constructor);

this.name = this.constructor.name;
this.message = message;
}

toJSON(): object {
return {
isJoi: true,
name: this.constructor.name,
details: [
{
message: this.message,
},
],
};
}
}

export default RoleInUseError;
module.exports = RoleInUseError;
2 changes: 2 additions & 0 deletions src/lib/routes/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ export const handleErrors: (
return res.status(409).json(error).end();
case 'FeatureHasTagError':
return res.status(409).json(error).end();
case 'RoleInUseError':
return res.status(400).json(error).end();
default:
logger.error('Server failed executing request', error);
return res.status(500).end();
Expand Down
9 changes: 9 additions & 0 deletions src/lib/services/access-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
import { IRoleStore } from 'lib/types/stores/role-store';
import NameExistsError from '../error/name-exists-error';
import { IEnvironmentStore } from 'lib/types/stores/environment-store';
import RoleInUseError from '../error/role-in-use-error';

export const ALL_PROJECTS = '*';
export const ALL_ENVS = '*';
Expand Down Expand Up @@ -427,6 +428,14 @@ export class AccessService {
}

async deleteRole(id: number): Promise<void> {
const roleUsers = await this.getUsersForRole(id);

if (roleUsers.length > 0) {
throw new RoleInUseError(
'Role is in use by more than one user. You cannot delete a role that is in use without first removing the role from the users.',
);
}

return this.roleStore.delete(id);
}

Expand Down
65 changes: 64 additions & 1 deletion src/test/e2e/services/access-service.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,15 @@ import {
import * as permissions from '../../../lib/types/permissions';
import { RoleName } from '../../../lib/types/model';
import { IUnleashStores } from '../../../lib/types';
import FeatureToggleService from '../../../lib/services/feature-toggle-service';
import ProjectService from '../../../lib/services/project-service';
import { createTestConfig } from '../../config/test-config';

let db: ITestDb;
let stores: IUnleashStores;
let accessService;

let featureToggleService;
let projectService;
let editorUser;
let superUser;
let editorRole;
Expand Down Expand Up @@ -182,11 +186,23 @@ beforeAll(async () => {
db = await dbInit('access_service_serial', getLogger);
stores = db.stores;
// projectStore = stores.projectStore;
const config = createTestConfig({
getLogger,
// @ts-ignore
experimental: { environments: { enabled: true } },
});
accessService = new AccessService(stores, { getLogger });
const roles = await accessService.getRootRoles();
editorRole = roles.find((r) => r.name === RoleName.EDITOR);
adminRole = roles.find((r) => r.name === RoleName.ADMIN);
readRole = roles.find((r) => r.name === RoleName.VIEWER);
featureToggleService = new FeatureToggleService(stores, config);
projectService = new ProjectService(
stores,
config,
accessService,
featureToggleService,
);

editorUser = await createUserEditorAccess('Bob Test', 'bob@getunleash.io');
superUser = await createSuperUser();
Expand Down Expand Up @@ -604,3 +620,50 @@ test('Should be denied access to delete a strategy in an environment the user do
),
).toBe(false);
});

test('Should be denied access to delete a role that is in use', async () => {
const user = editorUser;

const project = {
id: 'projectToUseRole',
name: 'New project',
description: 'Blah',
};
await projectService.createProject(project, user.id);

const projectMember = await stores.userStore.insert({
name: 'CustomProjectMember',
email: 'custom@getunleash.io',
});

const customRole = await accessService.createRole({
name: 'RoleInUse',
description: '',
permissions: [
{
id: 2,
name: 'CREATE_FEATURE',
environment: null,
displayName: 'Create Feature Toggles',
type: 'project',
},
{
id: 8,
name: 'DELETE_FEATURE',
environment: null,
displayName: 'Delete Feature Toggles',
type: 'project',
},
],
});

await projectService.addUser(project.id, customRole.id, projectMember.id);

try {
await accessService.deleteRole(customRole.id);
} catch (e) {
expect(e.toString()).toBe(
'RoleInUseError: Role is in use by more than one user. You cannot delete a role that is in use without first removing the role from the users.',
);
}
});

0 comments on commit 1e46628

Please sign in to comment.