From 8d98bcc0f06f8e2326d3c60593e9999aa23bbd88 Mon Sep 17 00:00:00 2001 From: Taesu <166604494+bytaesu@users.noreply.github.com> Date: Wed, 4 Feb 2026 02:34:34 +0900 Subject: [PATCH] fix(organization): prevent deletion of roles assigned to members (#7736) (cherry picked from commit 3308ecef659df7d3a24913d0c4e5e2997b7b850b) --- .../src/plugins/organization/error-codes.ts | 2 + .../routes/crud-access-control.test.ts | 126 ++++++++++++++++++ .../routes/crud-access-control.ts | 35 +++++ 3 files changed, 163 insertions(+) diff --git a/packages/better-auth/src/plugins/organization/error-codes.ts b/packages/better-auth/src/plugins/organization/error-codes.ts index 3c1bafe61d..7ddef48e68 100644 --- a/packages/better-auth/src/plugins/organization/error-codes.ts +++ b/packages/better-auth/src/plugins/organization/error-codes.ts @@ -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", }); diff --git a/packages/better-auth/src/plugins/organization/routes/crud-access-control.test.ts b/packages/better-auth/src/plugins/organization/routes/crud-access-control.test.ts index 4880c7e266..19f9026285 100644 --- a/packages/better-auth/src/plugins/organization/routes/crud-access-control.test.ts +++ b/packages/better-auth/src/plugins/organization/routes/crud-access-control.test.ts @@ -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( { diff --git a/packages/better-auth/src/plugins/organization/routes/crud-access-control.ts b/packages/better-auth/src/plugins/organization/routes/crud-access-control.ts index 8735dd7509..70d2c43c07 100644 --- a/packages/better-auth/src/plugins/organization/routes/crud-access-control.ts +++ b/packages/better-auth/src/plugins/organization/routes/crud-access-control.ts @@ -466,6 +466,41 @@ export const deleteOrgRole = (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({ + 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: [