Destroy test case.

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.
  */

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
     }
-  return {
-    ...role,
-    _id,
-    inherits: roles.getExternalRoleIDs(role.inherits, role.version),
+    if (changed) {
+      updated.push(role)
+    }
+  }
+  if (updated.length) {
+    await roles.saveRoles(updated)
   }
 }
 
@@ -65,7 +77,7 @@ async function updateRolesOnUserTable(
 }
 
 export async function fetch(ctx: UserCtx<void, FetchRolesResponse>) {
-  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<void, FindRoleResponse>) {
@@ -73,7 +85,7 @@ export async function find(ctx: UserCtx<void, FindRoleResponse>) {
   if (!role) {
     ctx.throw(404, { message: "Role not found" })
   }
-  ctx.body = externalRole(role)
+  ctx.body = roles.externalRole(role)
 }
 
 export async function save(ctx: UserCtx<SaveRoleRequest, SaveRoleResponse>) {
@@ -149,7 +161,7 @@ export async function save(ctx: UserCtx<SaveRoleRequest, SaveRoleResponse>) {
     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<void, DestroyRoleResponse>) {
     UpdateRolesOptions.REMOVED,
     role.version
   )
+
+  // clean up inherits
+  await removeRoleFromOthers(roleId)
+
   ctx.message = `Role ${ctx.params.roleId} deleted successfully`
   ctx.status = 200
 }

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", () => {