Skip to content

Commit

Permalink
fix: remove user from project (#5383)
Browse files Browse the repository at this point in the history
Removing a user from a project was impossible if you only had 1 owner.
It worked fine when having more than an owner. This should fix it and
we'll add tests later
  • Loading branch information
gastonfournier committed Nov 21, 2023
1 parent 1429b54 commit 7ddccee
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 6 deletions.
23 changes: 17 additions & 6 deletions src/lib/services/project-service.ts
Expand Up @@ -430,7 +430,9 @@ export default class ProjectService {
return this.accessService.getProjectRoleAccess(projectId);
}

// Deprecated: See addAccess instead.
/**
* @deprecated see addAccess instead.
*/
async addUser(
projectId: string,
roleId: number,
Expand Down Expand Up @@ -470,6 +472,9 @@ export default class ProjectService {
);
}

/**
* @deprecated use removeUserAccess
*/
async removeUser(
projectId: string,
roleId: number,
Expand Down Expand Up @@ -511,7 +516,10 @@ export default class ProjectService {
const ownerRole = await this.accessService.getRoleByName(
RoleName.OWNER,
);
await this.validateAtLeastOneOwner(projectId, ownerRole);

if (existingRoles.includes(ownerRole.id)) {
await this.validateAtLeastOneOwner(projectId, ownerRole);
}

await this.accessService.removeUserAccess(projectId, userId);

Expand Down Expand Up @@ -540,7 +548,10 @@ export default class ProjectService {
const ownerRole = await this.accessService.getRoleByName(
RoleName.OWNER,
);
await this.validateAtLeastOneOwner(projectId, ownerRole);

if (existingRoles.includes(ownerRole.id)) {
await this.validateAtLeastOneOwner(projectId, ownerRole);
}

await this.accessService.removeGroupAccess(projectId, groupId);

Expand Down Expand Up @@ -592,6 +603,9 @@ export default class ProjectService {
);
}

/**
* @deprecated use removeGroupAccess
*/
async removeGroup(
projectId: string,
roleId: number,
Expand Down Expand Up @@ -745,7 +759,6 @@ export default class ProjectService {
if (hasOwnerRole && isRemovingOwnerRole) {
await this.validateAtLeastOneOwner(projectId, ownerRole);
}
await this.validateAtLeastOneOwner(projectId, ownerRole);

await this.accessService.setProjectRolesForGroup(
projectId,
Expand Down Expand Up @@ -871,7 +884,6 @@ export default class ProjectService {
// Nothing to do....
return;
}

await this.validateAtLeastOneOwner(projectId, currentRole);

await this.accessService.updateUserProjectRole(
Expand Down Expand Up @@ -925,7 +937,6 @@ export default class ProjectService {
// Nothing to do....
return;
}

await this.validateAtLeastOneOwner(projectId, currentRole);

await this.accessService.updateGroupProjectRole(
Expand Down
38 changes: 38 additions & 0 deletions src/test/e2e/services/project-service.e2e.test.ts
Expand Up @@ -1048,6 +1048,44 @@ describe('ensure project has at least one owner', () => {
);
});

test('should be able to remove member user from the project when another is owner', async () => {
const project = {
id: 'remove-users-members-allowed',
name: 'New project',
description: 'Blah',
mode: 'open' as const,
defaultStickiness: 'clientId',
};
await projectService.createProject(project, user);

const memberRole = await stores.roleStore.getRoleByName(
RoleName.MEMBER,
);

const memberUser = await stores.userStore.insert({
name: 'Some Name',
email: 'member@getunleash.io',
});

await projectService.addAccess(
project.id,
[memberRole.id],
[],
[memberUser.id],
'test',
);

const usersBefore = await projectService.getProjectUsers(project.id);
await projectService.removeUserAccess(
project.id,
memberUser.id,
'test',
);
const usersAfter = await projectService.getProjectUsers(project.id);
expect(usersBefore).toHaveLength(2);
expect(usersAfter).toHaveLength(1);
});

test('should not update role for user on project when she is the owner', async () => {
const project = {
id: 'update-users-not-allowed',
Expand Down

0 comments on commit 7ddccee

Please sign in to comment.