Mirrors/unleash.unleash
1
0
Fork 0
mirror of https://github.com/Unleash/unleash.git synced 2025-03-18 00:19:49 +01:00
Code Issues Projects Releases Wiki Activity

chore: remove project at least one owner constraint (#9517)

Browse Source 
https://linear.app/unleash/issue/2-3393/remove-project-at-least-one-owner-constraint

Removes our constraint that the project must have at least one owner.
This commit is contained in:
Nuno Góis 2025-03-12 10:37:20 +00:00 committed by GitHub
parent 242b0de592
commit 6b888abe10
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 2 additions and 367 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
frontend/src/component/project/ProjectAccess/ProjectAccessTable/ProjectAccessTable.tsx
View File

@ -232,12 +232,8 @@ export const ProjectAccessTable: VFC = () => {
? 'group'
: 'user'
}/${row.entity.id}`}
disabled={access?.rows.length === 1}
tooltipProps={{
title:
access?.rows.length === 1
? 'Cannot edit access. A project must have at least one owner'
: 'Edit access',
title: 'Edit access',
}}
>
<Edit />
@ -253,12 +249,8 @@ export const ProjectAccessTable: VFC = () => {
setSelectedRow(row);
setRemoveOpen(true);
}}
disabled={access?.rows.length === 1}
tooltipProps={{
title:
access?.rows.length === 1
? 'Cannot remove access. A project must have at least one owner'
: 'Remove access',
title: 'Remove access',
}}
>
<Delete />

2
src/lib/error/index.ts
View File

@ -10,7 +10,6 @@ import PermissionError from './permission-error';
import { OperationDeniedError } from './operation-denied-error';
import UserTokenError from './used-token-error';
import RoleInUseError from './role-in-use-error';
import ProjectWithoutOwnerError from './project-without-owner-error';
import PasswordUndefinedError from './password-undefined';
import PasswordMismatchError from './password-mismatch';
import PatternError from './pattern-error';
@ -32,7 +31,6 @@ export {
OperationDeniedError,
UserTokenError,
RoleInUseError,
ProjectWithoutOwnerError,
PasswordUndefinedError,
PatternError,
PasswordMismatchError,

21
src/lib/error/project-without-owner-error.ts
View File

@ -1,21 +0,0 @@
import { type ApiErrorSchema, UnleashError } from './unleash-error';
export default class ProjectWithoutOwnerError extends UnleashError {
statusCode = 409;
constructor() {
super('A project must have at least one owner');
}
toJSON(): ApiErrorSchema {
return {
...super.toJSON(),
details: [
{
message: this.message,
validationErrors: [],
},
],
};
}
}

15
src/lib/error/unleash-error.test.ts
View File

@ -10,7 +10,6 @@ import PermissionError from './permission-error';
import OwaspValidationError from './owasp-validation-error';
import IncompatibleProjectError from './incompatible-project-error';
import PasswordUndefinedError from './password-undefined';
import ProjectWithoutOwnerError from './project-without-owner-error';
import NotFoundError from './notfound-error';
import { validateString } from '../util/validators/constraint-types';
import { fromLegacyError } from './from-legacy-error';
@ -671,20 +670,6 @@ describe('Error serialization special cases', () => {
],
});
});
it('ProjectWithoutOwnerError: adds `validationErrors: []` to the `details` list', () => {
const error = new ProjectWithoutOwnerError();
const json = error.toJSON();
expect(json).toMatchObject({
details: [
{
validationErrors: [],
message: json.message,
},
],
});
});
});
describe('Stack traces', () => {

219
src/lib/features/project/project-service.e2e.test.ts
View File

@ -1579,225 +1579,6 @@ test('should able to assign role without existing members', async () => {
expect(testUsers).toHaveLength(1);
});
describe('ensure project has at least one owner', () => {
test('should not remove user from the project', async () => {
const project = {
id: 'remove-users-not-allowed',
name: 'New project',
description: 'Blah',
mode: 'open' as const,
defaultStickiness: 'clientId',
};
await projectService.createProject(project, user, auditUser);
const roles = await stores.roleStore.getRolesForProject(project.id);
const ownerRole = roles.find((r) => r.name === RoleName.OWNER)!;
await expect(async () => {
await projectService.removeUser(
project.id,
ownerRole.id,
user.id,
auditUser,
);
}).rejects.toThrowError(
new Error('A project must have at least one owner'),
);
await expect(async () => {
await projectService.removeUserAccess(
project.id,
user.id,
auditUser,
);
}).rejects.toThrowError(
new Error('A project must have 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, auditUser);
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],
auditUser,
);
const usersBefore = await projectService.getProjectUsers(project.id);
await projectService.removeUserAccess(
project.id,
memberUser.id,
auditUser,
);
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',
name: 'New project',
description: 'Blah',
mode: 'open' as const,
defaultStickiness: 'clientId',
};
await projectService.createProject(project, user, auditUser);
const projectMember1 = await stores.userStore.insert({
name: 'Some Member',
email: 'update991@getunleash.io',
});
const memberRole = await stores.roleStore.getRoleByName(
RoleName.MEMBER,
);
await projectService.addAccess(
project.id,
[memberRole.id],
[], // no groups
[projectMember1.id],
auditUser,
);
await expect(async () => {
await projectService.changeRole(
project.id,
memberRole.id,
user.id,
auditUser,
);
}).rejects.toThrowError(
new Error('A project must have at least one owner'),
);
await expect(async () => {
await projectService.setRolesForUser(
project.id,
user.id,
[memberRole.id],
auditUser,
);
}).rejects.toThrowError(
new Error('A project must have at least one owner'),
);
});
async function projectWithGroupOwner(projectId: string) {
const project = {
id: projectId,
name: 'New project',
description: 'Blah',
mode: 'open' as const,
defaultStickiness: 'clientId',
};
await projectService.createProject(project, user, auditUser);
const roles = await stores.roleStore.getRolesForProject(project.id);
const ownerRole = roles.find((r) => r.name === RoleName.OWNER)!;
await projectService.addGroup(
project.id,
ownerRole.id,
group.id,
auditUser,
);
// this should be fine, leaving the group as the only owner
// note group has zero members, but it still acts as an owner
await projectService.removeUser(
project.id,
ownerRole.id,
user.id,
auditUser,
);
return {
project,
group,
ownerRole,
};
}
test('should not remove group from the project', async () => {
const { project, group, ownerRole } = await projectWithGroupOwner(
'remove-group-not-allowed',
);
await expect(async () => {
await projectService.removeGroup(
project.id,
ownerRole.id,
group.id,
auditUser,
);
}).rejects.toThrowError(
new Error('A project must have at least one owner'),
);
await expect(async () => {
await projectService.removeGroupAccess(
project.id,
group.id,
auditUser,
);
}).rejects.toThrowError(
new Error('A project must have at least one owner'),
);
});
test('should not update role for group on project when she is the owner', async () => {
const { project, group } = await projectWithGroupOwner(
'update-group-not-allowed',
);
const memberRole = await stores.roleStore.getRoleByName(
RoleName.MEMBER,
);
await expect(async () => {
await projectService.changeGroupRole(
project.id,
memberRole.id,
group.id,
auditUser,
);
}).rejects.toThrowError(
new Error('A project must have at least one owner'),
);
await expect(async () => {
await projectService.setRolesForGroup(
project.id,
group.id,
[memberRole.id],
auditUser,
);
}).rejects.toThrowError(
new Error('A project must have at least one owner'),
);
});
});
test('Should allow bulk update of group permissions', async () => {
const project = {
id: 'bulk-update-project',

41
src/lib/features/project/project-service.test.ts
View File

@ -2,7 +2,6 @@ import { createTestConfig } from '../../../test/config/test-config';
import { BadDataError } from '../../error';
import { type IBaseEvent, RoleName, TEST_AUDIT_USER } from '../../types';
import { createFakeProjectService } from './createProjectService';
import ProjectService from './project-service';
describe('enterprise extension: enable change requests', () => {
const createService = () => {
@ -301,44 +300,4 @@ describe('enterprise extension: enable change requests', () => {
),
).resolves.toBeTruthy();
});
test('has at least one owner after deletion when group has the role and a project user for role exists', async () => {
const config = createTestConfig();
const projectId = 'fake-project-id';
const service = new ProjectService(
{
projectStore: {} as any,
projectOwnersReadModel: {} as any,
projectFlagCreatorsReadModel: {} as any,
eventStore: {} as any,
featureToggleStore: {} as any,
environmentStore: {} as any,
featureEnvironmentStore: {} as any,
accountStore: {} as any,
projectStatsStore: {} as any,
projectReadModel: {} as any,
onboardingReadModel: {} as any,
},
config,
{
getProjectUsersForRole: async () =>
Promise.resolve([{ id: 1 } as any]),
} as any,
{} as any,
{
getProjectGroups: async () =>
Promise.resolve([{ roles: [2, 5] } as any]),
} as any,
{} as any,
{} as any,
{} as any,
{} as any,
);
await service.validateAtLeastOneOwner(projectId, {
id: 5,
name: 'Owner',
type: 'Owner',
});
});
});

59
src/lib/features/project/project-service.ts
View File

@ -62,7 +62,6 @@ import type {
} from '../../types/stores/access-store';
import type FeatureToggleService from '../feature-toggle/feature-toggle-service';
import IncompatibleProjectError from '../../error/incompatible-project-error';
import ProjectWithoutOwnerError from '../../error/project-without-owner-error';
import { arraysHaveSameItems } from '../../util';
import type { GroupService } from '../../services/group-service';
import type { IGroupRole } from '../../types/group';
@ -665,8 +664,6 @@ export default class ProjectService {
): Promise<void> {
const role = await this.findProjectRole(projectId, roleId);
await this.validateAtLeastOneOwner(projectId, role);
await this.accessService.removeUserFromRole(userId, role.id, projectId);
const user = await this.accountStore.get(userId);
@ -695,14 +692,6 @@ export default class ProjectService {
userId,
);
const ownerRole = await this.accessService.getRoleByName(
RoleName.OWNER,
);
if (existingRoles.includes(ownerRole.id)) {
await this.validateAtLeastOneOwner(projectId, ownerRole);
}
await this.accessService.removeUserAccess(projectId, userId);
await this.eventService.storeEvent(
@ -727,14 +716,6 @@ export default class ProjectService {
groupId,
);
const ownerRole = await this.accessService.getRoleByName(
RoleName.OWNER,
);
if (existingRoles.includes(ownerRole.id)) {
await this.validateAtLeastOneOwner(projectId, ownerRole);
}
await this.accessService.removeGroupAccess(projectId, groupId);
await this.eventService.storeEvent(
@ -804,8 +785,6 @@ export default class ProjectService {
undefined,
);
await this.validateAtLeastOneOwner(projectId, role);
await this.accessService.removeGroupFromRole(
group.id,
role.id,
@ -967,15 +946,6 @@ export default class ProjectService {
projectId,
userId,
);
const ownerRole = await this.accessService.getRoleByName(
RoleName.OWNER,
);
const hasOwnerRole = includes(currentRoles, ownerRole);
const isRemovingOwnerRole = !includes(newRoles, ownerRole);
if (hasOwnerRole && isRemovingOwnerRole) {
await this.validateAtLeastOneOwner(projectId, ownerRole);
}
const isAllowedToAssignRoles = await this.isAllowedToAddAccess(
auditUser,
projectId,
@ -1019,14 +989,6 @@ export default class ProjectService {
groupId,
);
const ownerRole = await this.accessService.getRoleByName(
RoleName.OWNER,
);
const hasOwnerRole = includes(currentRoles, ownerRole);
const isRemovingOwnerRole = !includes(newRoles, ownerRole);
if (hasOwnerRole && isRemovingOwnerRole) {
await this.validateAtLeastOneOwner(projectId, ownerRole);
}
const isAllowedToAssignRoles = await this.isAllowedToAddAccess(
auditUser,
projectId,
@ -1088,25 +1050,6 @@ export default class ProjectService {
return role;
}
async validateAtLeastOneOwner(
projectId: string,
currentRole: IRoleDescriptor,
): Promise<void> {
if (currentRole.name === RoleName.OWNER) {
const users = await this.accessService.getProjectUsersForRole(
currentRole.id,
projectId,
);
const groups = await this.groupService.getProjectGroups(projectId);
const roleGroups = groups.filter((g) =>
g.roles?.includes(currentRole.id),
);
if (users.length + roleGroups.length < 2) {
throw new ProjectWithoutOwnerError();
}
}
}
/** @deprecated use projectInsightsService instead */
async getDoraMetrics(projectId: string): Promise<ProjectDoraMetricsSchema> {
const activeFeatureFlags = (
@ -1180,7 +1123,6 @@ export default class ProjectService {
// Nothing to do....
return;
}
await this.validateAtLeastOneOwner(projectId, currentRole);
await this.accessService.updateUserProjectRole(
userId,
@ -1233,7 +1175,6 @@ export default class ProjectService {
// Nothing to do....
return;
}
await this.validateAtLeastOneOwner(projectId, currentRole);
await this.accessService.updateGroupProjectRole(
userId,