PR comments.

packages/backend-core/src/security/roles.ts

@@ -76,11 +76,9 @@ export class Role implements RoleDoc {
     if (inherits && typeof inherits === "string") {
       inherits = prefixRoleIDNoBuiltin(inherits)
     } else if (inherits && Array.isArray(inherits)) {
-      inherits = inherits.map(inherit => prefixRoleIDNoBuiltin(inherit))
+      inherits = inherits.map(prefixRoleIDNoBuiltin)
     }
-    if (inherits) {
     this.inherits = inherits
-    }
     return this
   }
 }
@@ -264,7 +262,7 @@ export async function roleToNumber(id: string) {
             return findNumber(foundRole) + 1
           }
         })
-        .filter(number => !!number)
+        .filter(number => number)
         .sort()
         .pop()
       if (highestBuiltin != undefined) {

packages/server/src/api/controllers/role.ts

@@ -78,10 +78,10 @@ export async function find(ctx: UserCtx<void, FindRoleResponse>) {
 
 export async function save(ctx: UserCtx<SaveRoleRequest, SaveRoleResponse>) {
   const db = context.getAppDB()
-  let { _id, name, inherits, permissionId, version, uiMetadata } =
+  let { _id, _rev, name, inherits, permissionId, version, uiMetadata } =
     ctx.request.body
   let isCreate = false
-  if (!ctx.request.body._rev && !version) {
+  if (!_rev && !version) {
     version = roles.RoleIDVersion.NAME
   }
   const isNewVersion = version === roles.RoleIDVersion.NAME
@@ -132,7 +132,7 @@ export async function save(ctx: UserCtx<SaveRoleRequest, SaveRoleResponse>) {
     ctx.throw(400, "Role inheritance contains a loop, this is not supported")
   }
 
-  const foundRev = ctx.request.body._rev || dbRole?._rev
+  const foundRev = _rev || dbRole?._rev
   if (foundRev) {
     role._rev = foundRev
   }

packages/server/src/tests/utilities/TestConfiguration.ts

@@ -432,7 +432,7 @@ export default class TestConfiguration {
   async loginAsRole(roleId: string, cb: () => Promise<unknown>) {
     const roleUser = await this.createUser({
       roles: {
-        [this.prodAppId!]: roleId,
+        [this.getProdAppId()]: roleId,
       },
       builder: { global: false },
       admin: { global: false },
@@ -443,18 +443,10 @@ export default class TestConfiguration {
       builder: false,
       prodApp: true,
     })
-    const temp = this.user
-    this.user = roleUser
+    await this.withUser(roleUser, async () => {
       await cb()
-    if (temp) {
-      this.user = temp
-      await this.login({
-        userId: temp._id!,
-        builder: true,
-        prodApp: false,
     })
   }
-  }
 
   defaultHeaders(extras = {}, prodApp = false) {
     const tenantId = this.getTenantId()

packages/shared-core/src/helpers/tests/roles.spec.ts

@@ -64,5 +64,10 @@ describe("role utilities", () => {
       role1.inherits = "role_2"
       check(true)
     })
+
+    it("self reference contains loop", () => {
+      role("role1", "role1")
+      check(true)
+    })
   })
 })