diff --git a/packages/backend-core/src/security/roles.ts b/packages/backend-core/src/security/roles.ts index e549d2f3af..c14178cacb 100644 --- a/packages/backend-core/src/security/roles.ts +++ b/packages/backend-core/src/security/roles.ts @@ -290,11 +290,23 @@ export function lowerBuiltinRoleID(roleId1?: string, roleId2?: string): string { : roleId1 } -function compareRoleIds(roleId1: string, roleId2: string) { +export function compareRoleIds(roleId1: string, roleId2: string) { // make sure both role IDs are prefixed correctly return prefixRoleID(roleId1) === prefixRoleID(roleId2) } +export function externalRole(role: RoleDoc): RoleDoc { + let _id: string | undefined + if (role._id) { + _id = getExternalRoleID(role._id) + } + return { + ...role, + _id, + inherits: getExternalRoleIDs(role.inherits, role.version), + } +} + /** * Given a list of roles, this will pick the role out, accounting for built ins. */ @@ -347,6 +359,18 @@ export async function getRole( return findRole(roleId, roleList, opts) } +export async function saveRoles(roles: RoleDoc[]) { + const db = getAppDB() + await db.bulkDocs( + roles + .filter(role => role._id) + .map(role => ({ + ...role, + _id: prefixRoleID(role._id!), + })) + ) +} + /** * Simple function to get all the roles based on the top level user role ID. */ diff --git a/packages/server/src/api/controllers/role.ts b/packages/server/src/api/controllers/role.ts index 89125f566e..f26b4bae69 100644 --- a/packages/server/src/api/controllers/role.ts +++ b/packages/server/src/api/controllers/role.ts @@ -27,15 +27,27 @@ const UpdateRolesOptions = { REMOVED: "removed", } -function externalRole(role: Role): Role { - let _id: string | undefined - if (role._id) { - _id = roles.getExternalRoleID(role._id) +async function removeRoleFromOthers(roleId: string) { + const allOtherRoles = await roles.getAllRoles() + const updated: Role[] = [] + for (let role of allOtherRoles) { + let changed = false + if (Array.isArray(role.inherits)) { + const newInherits = role.inherits.filter( + id => !roles.compareRoleIds(id, roleId) + ) + changed = role.inherits.length !== newInherits.length + role.inherits = newInherits + } else if (role.inherits && roles.compareRoleIds(role.inherits, roleId)) { + role.inherits = roles.BUILTIN_ROLE_IDS.PUBLIC + changed = true + } + if (changed) { + updated.push(role) + } } - return { - ...role, - _id, - inherits: roles.getExternalRoleIDs(role.inherits, role.version), + if (updated.length) { + await roles.saveRoles(updated) } } @@ -65,7 +77,7 @@ async function updateRolesOnUserTable( } export async function fetch(ctx: UserCtx) { - ctx.body = (await roles.getAllRoles()).map(role => externalRole(role)) + ctx.body = (await roles.getAllRoles()).map(role => roles.externalRole(role)) } export async function find(ctx: UserCtx) { @@ -73,7 +85,7 @@ export async function find(ctx: UserCtx) { if (!role) { ctx.throw(404, { message: "Role not found" }) } - ctx.body = externalRole(role) + ctx.body = roles.externalRole(role) } export async function save(ctx: UserCtx) { @@ -149,7 +161,7 @@ export async function save(ctx: UserCtx) { role.version ) role._rev = result.rev - ctx.body = externalRole(role) + ctx.body = roles.externalRole(role) const devDb = context.getDevAppDB() const prodDb = context.getProdAppDB() @@ -198,6 +210,10 @@ export async function destroy(ctx: UserCtx) { UpdateRolesOptions.REMOVED, role.version ) + + // clean up inherits + await removeRoleFromOthers(roleId) + ctx.message = `Role ${ctx.params.roleId} deleted successfully` ctx.status = 200 } diff --git a/packages/server/src/api/routes/tests/role.spec.ts b/packages/server/src/api/routes/tests/role.spec.ts index a79f219311..5668295342 100644 --- a/packages/server/src/api/routes/tests/role.spec.ts +++ b/packages/server/src/api/routes/tests/role.spec.ts @@ -217,6 +217,27 @@ describe("/roles", () => { _id: dbCore.prefixRoleID(customRole._id!), }) }) + + it("should disconnection roles when deleted", async () => { + const role1 = await config.api.roles.save({ + name: "role1", + permissionId: BuiltinPermissionID.WRITE, + inherits: [BUILTIN_ROLE_IDS.BASIC], + }) + const role2 = await config.api.roles.save({ + name: "role2", + permissionId: BuiltinPermissionID.WRITE, + inherits: [BUILTIN_ROLE_IDS.BASIC, role1._id!], + }) + const role3 = await config.api.roles.save({ + name: "role3", + permissionId: BuiltinPermissionID.WRITE, + inherits: [BUILTIN_ROLE_IDS.BASIC, role2._id!], + }) + await config.api.roles.destroy(role2, { status: 200 }) + const found = await config.api.roles.find(role3._id!, { status: 200 }) + expect(found.inherits).toEqual([BUILTIN_ROLE_IDS.BASIC]) + }) }) describe("accessible", () => {