Adding test cases.

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

@@ -213,6 +213,22 @@ export function getBuiltinRole(roleId: string): Role | undefined {
   return cloneDeep(role)
 }
 
+export function validInherits(
+  allRoles: RoleDoc[],
+  inherits?: string | string[]
+): boolean {
+  if (!inherits) {
+    return false
+  }
+  const find = (id: string) => allRoles.find(r => compareRoleIds(r._id!, id))
+  if (Array.isArray(inherits)) {
+    const filtered = inherits.filter(roleId => find(roleId))
+    return inherits.length !== 0 && filtered.length === inherits.length
+  } else {
+    return !!find(inherits)
+  }
+}
+
 /**
  * Works through the inheritance ranks to see how far up the builtin stack this ID is.
  */

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

@@ -18,10 +18,12 @@ import {
   UserCtx,
   UserMetadata,
   DocumentType,
+  PermissionLevel,
 } from "@budibase/types"
 import { RoleColor, sdk as sharedSdk, helpers } from "@budibase/shared-core"
 import sdk from "../../sdk"
 import { builderSocket } from "../../websockets"
+import { validInherits } from "@budibase/backend-core/src/security/roles"
 
 const UpdateRolesOptions = {
   CREATED: "created",
@@ -125,6 +127,17 @@ export async function save(ctx: UserCtx<SaveRoleRequest, SaveRoleResponse>) {
     ctx.throw(400, "Cannot change custom role name")
   }
 
+  // custom roles should always inherit basic - if they don't inherit anything else
+  if (!inherits && roles.validInherits(allRoles, dbRole?.inherits)) {
+    inherits = dbRole?.inherits
+  } else if (!roles.validInherits(allRoles, inherits)) {
+    inherits = [roles.BUILTIN_ROLE_IDS.BASIC]
+  }
+  // assume write permission level for newly created roles
+  if (isCreate && !permissionId) {
+    permissionId = PermissionLevel.WRITE
+  }
+
   const role = new roles.Role(_id, name, permissionId, {
     displayName: uiMetadata?.displayName || name,
     description: uiMetadata?.description || "Custom role",

packages/server/src/api/routes/tests/role.spec.ts

@@ -38,6 +38,16 @@ describe("/roles", () => {
         _id: dbCore.prefixRoleID(res._id!),
       })
     })
+
+    it("handle a role with no inherits", async () => {
+      const role = basicRole()
+      role.inherits = []
+
+      const res = await config.api.roles.save(role, {
+        status: 200,
+      })
+      expect(res.inherits).toEqual([BUILTIN_ROLE_IDS.BASIC])
+    })
   })
 
   describe("update", () => {
@@ -149,6 +159,19 @@ describe("/roles", () => {
         { status: 400, body: { message: LOOP_ERROR } }
       )
     })
+
+    it("handle updating a role, without its inherits", async () => {
+      const res = await config.api.roles.save({
+        ...basicRole(),
+        inherits: [BUILTIN_ROLE_IDS.ADMIN],
+      })
+      // save again
+      const updatedRes = await config.api.roles.save({
+        ...res,
+        inherits: undefined,
+      })
+      expect(updatedRes.inherits).toEqual([BUILTIN_ROLE_IDS.ADMIN])
+    })
   })
 
   describe("fetch", () => {