mirror of
https://github.com/better-auth/better-auth.git
synced 2026-10-19 23:53:17 -05:00
fix(organization): prevent deletion of roles assigned to members (#7736)
(cherry picked from commit 3308ecef65)
This commit is contained in:
@@ -88,4 +88,6 @@ export const ORGANIZATION_ERROR_CODES = defineErrorCodes({
|
||||
INVALID_RESOURCE: "The provided permission includes an invalid resource",
|
||||
ROLE_NAME_IS_ALREADY_TAKEN: "That role name is already taken",
|
||||
CANNOT_DELETE_A_PRE_DEFINED_ROLE: "Cannot delete a pre-defined role",
|
||||
ROLE_IS_ASSIGNED_TO_MEMBERS:
|
||||
"Cannot delete a role that is assigned to members. Please reassign the members to a different role first",
|
||||
});
|
||||
|
||||
@@ -380,6 +380,132 @@ describe("dynamic access control", async (it) => {
|
||||
expect(res).not.toBeNull();
|
||||
});
|
||||
|
||||
it("should not be allowed to delete a role that is assigned to members", async () => {
|
||||
// Create a new dynamic role
|
||||
const testRole = await authClient.organization.createRole(
|
||||
{
|
||||
role: "assigned-role",
|
||||
permission: {
|
||||
project: ["read"],
|
||||
},
|
||||
additionalFields: {
|
||||
color: "#000000",
|
||||
},
|
||||
},
|
||||
{ headers },
|
||||
);
|
||||
if (!testRole.data) throw testRole.error;
|
||||
|
||||
// Create a member with the role
|
||||
const memberDetails = {
|
||||
email: `delete-role-test-${crypto.randomUUID()}@email.com`,
|
||||
name: "test-member",
|
||||
password: `password-${crypto.randomUUID()}`,
|
||||
};
|
||||
const memberUser = await auth.api.signUpEmail({ body: memberDetails });
|
||||
await auth.api.addMember({
|
||||
body: {
|
||||
// @ts-expect-error - for testing purposes
|
||||
role: "assigned-role",
|
||||
userId: memberUser.user.id,
|
||||
organizationId: org.data?.id,
|
||||
},
|
||||
headers,
|
||||
});
|
||||
|
||||
// Try to delete the role - should fail
|
||||
await expect(
|
||||
auth.api.deleteOrgRole({
|
||||
body: { roleName: "assigned-role" },
|
||||
headers,
|
||||
}),
|
||||
).rejects.toThrow(ORGANIZATION_ERROR_CODES.ROLE_IS_ASSIGNED_TO_MEMBERS);
|
||||
|
||||
// Clean up: change member's role and then delete the dynamic role
|
||||
await auth.api.updateMemberRole({
|
||||
body: {
|
||||
memberId: (
|
||||
await auth.api.listMembers({
|
||||
query: { organizationId: org.data?.id },
|
||||
headers,
|
||||
})
|
||||
).members?.find((m) => m.userId === memberUser.user.id)?.id!,
|
||||
role: "member",
|
||||
},
|
||||
headers,
|
||||
});
|
||||
await auth.api.deleteOrgRole({
|
||||
body: { roleName: "assigned-role" },
|
||||
headers,
|
||||
});
|
||||
});
|
||||
|
||||
it("should not be allowed to delete a role when member has multiple roles including that role", async () => {
|
||||
// Create two dynamic roles
|
||||
const role1 = await authClient.organization.createRole(
|
||||
{
|
||||
role: "multi-role-1",
|
||||
permission: { project: ["read"] },
|
||||
additionalFields: { color: "#000000" },
|
||||
},
|
||||
{ headers },
|
||||
);
|
||||
const role2 = await authClient.organization.createRole(
|
||||
{
|
||||
role: "multi-role-2",
|
||||
permission: { project: ["create"] },
|
||||
additionalFields: { color: "#111111" },
|
||||
},
|
||||
{ headers },
|
||||
);
|
||||
if (!role1.data || !role2.data) throw new Error("Roles not created");
|
||||
|
||||
// Create a member with multiple roles (comma-separated)
|
||||
const memberDetails = {
|
||||
email: `multi-role-test-${crypto.randomUUID()}@email.com`,
|
||||
name: "test-member",
|
||||
password: `password-${crypto.randomUUID()}`,
|
||||
};
|
||||
const memberUser = await auth.api.signUpEmail({ body: memberDetails });
|
||||
await auth.api.addMember({
|
||||
body: {
|
||||
// @ts-expect-error - for testing purposes
|
||||
role: ["multi-role-1", "multi-role-2"],
|
||||
userId: memberUser.user.id,
|
||||
organizationId: org.data?.id,
|
||||
},
|
||||
headers,
|
||||
});
|
||||
|
||||
// Try to delete one of the roles - should fail
|
||||
await expect(
|
||||
auth.api.deleteOrgRole({
|
||||
body: { roleName: "multi-role-1" },
|
||||
headers,
|
||||
}),
|
||||
).rejects.toThrow(ORGANIZATION_ERROR_CODES.ROLE_IS_ASSIGNED_TO_MEMBERS);
|
||||
|
||||
// Clean up
|
||||
const memberId = (
|
||||
await auth.api.listMembers({
|
||||
query: { organizationId: org.data?.id },
|
||||
headers,
|
||||
})
|
||||
).members?.find((m) => m.userId === memberUser.user.id)?.id!;
|
||||
await auth.api.updateMemberRole({
|
||||
body: { memberId, role: "member" },
|
||||
headers,
|
||||
});
|
||||
await auth.api.deleteOrgRole({
|
||||
body: { roleName: "multi-role-1" },
|
||||
headers,
|
||||
});
|
||||
await auth.api.deleteOrgRole({
|
||||
body: { roleName: "multi-role-2" },
|
||||
headers,
|
||||
});
|
||||
});
|
||||
|
||||
it("should not be allowed to delete a role without necessary permissions", async () => {
|
||||
const testRole = await authClient.organization.createRole(
|
||||
{
|
||||
|
||||
@@ -466,6 +466,41 @@ export const deleteOrgRole = <O extends OrganizationOptions>(options: O) => {
|
||||
existingRoleInDB.permission as never as string,
|
||||
);
|
||||
|
||||
// Check if any members are assigned to this role
|
||||
const roleToDelete = existingRoleInDB.role;
|
||||
const members = await ctx.context.adapter.findMany<Member>({
|
||||
model: "member",
|
||||
where: [
|
||||
{
|
||||
field: "organizationId",
|
||||
value: organizationId,
|
||||
operator: "eq",
|
||||
connector: "AND",
|
||||
},
|
||||
{
|
||||
field: "role",
|
||||
value: roleToDelete,
|
||||
operator: "contains",
|
||||
},
|
||||
],
|
||||
});
|
||||
const memberWithRole = members.find((member) => {
|
||||
const memberRoles = member.role.split(",").map((r) => r.trim());
|
||||
return memberRoles.includes(roleToDelete);
|
||||
});
|
||||
if (memberWithRole) {
|
||||
ctx.context.logger.error(
|
||||
`[Dynamic Access Control] Cannot delete a role that is assigned to members.`,
|
||||
{
|
||||
role: existingRoleInDB.role,
|
||||
organizationId,
|
||||
},
|
||||
);
|
||||
throw new APIError("BAD_REQUEST", {
|
||||
message: ORGANIZATION_ERROR_CODES.ROLE_IS_ASSIGNED_TO_MEMBERS,
|
||||
});
|
||||
}
|
||||
|
||||
await ctx.context.adapter.delete({
|
||||
model: "organizationRole",
|
||||
where: [
|
||||
|
||||
Reference in New Issue
Block a user