Merge branch 'feature/role-multi-inheritance' of github.com:Budibase/budibase into new-rbac-ui

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

@@ -43,7 +43,7 @@ export class Role implements RoleDoc {
   _rev?: string
   name: string
   permissionId: string
-  inherits?: string
+  inherits?: string | string[]
   version?: string
   permissions: Record<string, PermissionLevel[]> = {}
   uiMetadata?: RoleUIMetadata
@@ -62,8 +62,10 @@ export class Role implements RoleDoc {
     this.version = RoleIDVersion.NAME
   }
 
-  addInheritance(inherits: string) {
-    this.inherits = inherits
+  addInheritance(inherits?: string | string[]) {
+    if (inherits) {
+      this.inherits = inherits
+    }
     return this
   }
 }
@@ -154,7 +156,11 @@ export function builtinRoleToNumber(id: string) {
     if (!role) {
       break
     }
-    role = builtins[role.inherits!]
+    if (Array.isArray(role.inherits)) {
+      // TODO: role inheritance
+    } else {
+      role = builtins[role.inherits!]
+    }
     count++
   } while (role !== null)
   return count
@@ -171,7 +177,12 @@ export async function roleToNumber(id: string) {
     defaultPublic: true,
   })) as RoleDoc[]
   for (let role of hierarchy) {
-    if (role?.inherits && isBuiltin(role.inherits)) {
+    if (!role.inherits) {
+      continue
+    }
+    if (Array.isArray(role.inherits)) {
+      // TODO: role inheritance
+    } else if (isBuiltin(role.inherits)) {
       return builtinRoleToNumber(role.inherits) + 1
     }
   }
@@ -243,16 +254,27 @@ async function getAllUserRoles(
   let currentRole = await getRole(userRoleId, opts)
   let roles = currentRole ? [currentRole] : []
   let roleIds = [userRoleId]
+  const rolesFound = (ids: string | string[]) => {
+    if (Array.isArray(ids)) {
+      return ids.filter(id => roleIds.includes(id)).length === ids.length
+    } else {
+      return roleIds.includes(ids)
+    }
+  }
   // get all the inherited roles
   while (
     currentRole &&
     currentRole.inherits &&
-    roleIds.indexOf(currentRole.inherits) === -1
+    !rolesFound(currentRole.inherits)
   ) {
-    roleIds.push(currentRole.inherits)
-    currentRole = await getRole(currentRole.inherits)
-    if (currentRole) {
-      roles.push(currentRole)
+    if (Array.isArray(currentRole.inherits)) {
+      // TODO: role inheritance
+    } else {
+      roleIds.push(currentRole.inherits)
+      currentRole = await getRole(currentRole.inherits)
+      if (currentRole) {
+        roles.push(currentRole)
+      }
     }
   }
   return roles

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

@@ -194,7 +194,10 @@ export async function accessible(ctx: UserCtx<void, AccessibleRolesResponse>) {
     let filteredRoles = [roleHeader]
     for (let role of orderedRoles) {
       filteredRoles = [role, ...filteredRoles]
-      if (role === inherits) {
+      if (
+        (Array.isArray(inherits) && inherits.includes(role)) ||
+        role === inherits
+      ) {
         break
       }
     }

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

@@ -1,182 +0,0 @@
-const { roles, events, permissions } = require("@budibase/backend-core")
-const setup = require("./utilities")
-const { PermissionLevel } = require("@budibase/types")
-const { basicRole } = setup.structures
-const { BUILTIN_ROLE_IDS } = roles
-const { BuiltinPermissionID } = permissions
-
-describe("/roles", () => {
-  let request = setup.getRequest()
-  let config = setup.getConfig()
-
-  afterAll(setup.afterAll)
-
-  beforeAll(async () => {
-    await config.init()
-  })
-
-  const createRole = async role => {
-    if (!role) {
-      role = basicRole()
-    }
-
-    return request
-      .post(`/api/roles`)
-      .send(role)
-      .set(config.defaultHeaders())
-      .expect("Content-Type", /json/)
-      .expect(200)
-  }
-
-  describe("create", () => {
-    it("returns a success message when role is successfully created", async () => {
-      const role = basicRole()
-      const res = await createRole(role)
-
-      expect(res.body._id).toBeDefined()
-      expect(res.body._rev).toBeDefined()
-      expect(events.role.updated).not.toBeCalled()
-      expect(events.role.created).toBeCalledTimes(1)
-      expect(events.role.created).toBeCalledWith(res.body)
-    })
-  })
-
-  describe("update", () => {
-    it("updates a role", async () => {
-      const role = basicRole()
-      let res = await createRole(role)
-      jest.clearAllMocks()
-      res = await createRole(res.body)
-
-      expect(res.body._id).toBeDefined()
-      expect(res.body._rev).toBeDefined()
-      expect(events.role.created).not.toBeCalled()
-      expect(events.role.updated).toBeCalledTimes(1)
-      expect(events.role.updated).toBeCalledWith(res.body)
-    })
-  })
-
-  describe("fetch", () => {
-    beforeAll(async () => {
-      // Recreate the app
-      await config.init()
-    })
-
-    it("should list custom roles, plus 2 default roles", async () => {
-      const customRole = await config.createRole()
-
-      const res = await request
-        .get(`/api/roles`)
-        .set(config.defaultHeaders())
-        .expect("Content-Type", /json/)
-        .expect(200)
-
-      expect(res.body.length).toBe(5)
-
-      const adminRole = res.body.find(r => r._id === BUILTIN_ROLE_IDS.ADMIN)
-      expect(adminRole).toBeDefined()
-      expect(adminRole.inherits).toEqual(BUILTIN_ROLE_IDS.POWER)
-      expect(adminRole.permissionId).toEqual(BuiltinPermissionID.ADMIN)
-
-      const powerUserRole = res.body.find(r => r._id === BUILTIN_ROLE_IDS.POWER)
-      expect(powerUserRole).toBeDefined()
-      expect(powerUserRole.inherits).toEqual(BUILTIN_ROLE_IDS.BASIC)
-      expect(powerUserRole.permissionId).toEqual(BuiltinPermissionID.POWER)
-
-      const customRoleFetched = res.body.find(r => r._id === customRole.name)
-      expect(customRoleFetched).toBeDefined()
-      expect(customRoleFetched.inherits).toEqual(BUILTIN_ROLE_IDS.BASIC)
-      expect(customRoleFetched.permissionId).toEqual(
-        BuiltinPermissionID.READ_ONLY
-      )
-    })
-
-    it("should be able to get the role with a permission added", async () => {
-      const table = await config.createTable()
-      await config.api.permission.add({
-        roleId: BUILTIN_ROLE_IDS.POWER,
-        resourceId: table._id,
-        level: PermissionLevel.READ,
-      })
-      const res = await request
-        .get(`/api/roles`)
-        .set(config.defaultHeaders())
-        .expect("Content-Type", /json/)
-        .expect(200)
-      expect(res.body.length).toBeGreaterThan(0)
-      const power = res.body.find(role => role._id === BUILTIN_ROLE_IDS.POWER)
-      expect(power.permissions[table._id]).toEqual(["read"])
-    })
-  })
-
-  describe("destroy", () => {
-    it("should delete custom roles", async () => {
-      const customRole = await config.createRole({
-        name: "user",
-        permissionId: BuiltinPermissionID.READ_ONLY,
-        inherits: BUILTIN_ROLE_IDS.BASIC,
-      })
-      delete customRole._rev_tree
-      await request
-        .delete(`/api/roles/${customRole._id}/${customRole._rev}`)
-        .set(config.defaultHeaders())
-        .expect(200)
-      await request
-        .get(`/api/roles/${customRole._id}`)
-        .set(config.defaultHeaders())
-        .expect(404)
-      expect(events.role.deleted).toBeCalledTimes(1)
-      expect(events.role.deleted).toBeCalledWith(customRole)
-    })
-  })
-
-  describe("accessible", () => {
-    it("should be able to fetch accessible roles (with builder)", async () => {
-      const res = await request
-        .get("/api/roles/accessible")
-        .set(config.defaultHeaders())
-        .expect(200)
-      expect(res.body.length).toBe(5)
-      expect(typeof res.body[0]).toBe("string")
-    })
-
-    it("should be able to fetch accessible roles (basic user)", async () => {
-      const res = await request
-        .get("/api/roles/accessible")
-        .set(await config.basicRoleHeaders())
-        .expect(200)
-      expect(res.body.length).toBe(2)
-      expect(res.body[0]).toBe("BASIC")
-      expect(res.body[1]).toBe("PUBLIC")
-    })
-
-    it("should be able to fetch accessible roles (no user)", async () => {
-      const res = await request
-        .get("/api/roles/accessible")
-        .set(config.publicHeaders())
-        .expect(200)
-      expect(res.body.length).toBe(1)
-      expect(res.body[0]).toBe("PUBLIC")
-    })
-
-    it("should not fetch higher level accessible roles when a custom role header is provided", async () => {
-      await createRole({
-        name: `CUSTOM_ROLE`,
-        inherits: roles.BUILTIN_ROLE_IDS.BASIC,
-        permissionId: permissions.BuiltinPermissionID.READ_ONLY,
-        version: "name",
-      })
-      const res = await request
-        .get("/api/roles/accessible")
-        .set({
-          ...config.defaultHeaders(),
-          "x-budibase-role": "CUSTOM_ROLE",
-        })
-        .expect(200)
-      expect(res.body.length).toBe(3)
-      expect(res.body[0]).toBe("CUSTOM_ROLE")
-      expect(res.body[1]).toBe("BASIC")
-      expect(res.body[2]).toBe("PUBLIC")
-    })
-  })
-})

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

@@ -0,0 +1,164 @@
+import { roles, events, permissions } from "@budibase/backend-core"
+import * as setup from "./utilities"
+import { PermissionLevel } from "@budibase/types"
+
+const { basicRole } = setup.structures
+const { BUILTIN_ROLE_IDS } = roles
+const { BuiltinPermissionID } = permissions
+
+describe("/roles", () => {
+  let config = setup.getConfig()
+
+  afterAll(setup.afterAll)
+
+  beforeAll(async () => {
+    await config.init()
+  })
+
+  describe("create", () => {
+    it("returns a success message when role is successfully created", async () => {
+      const role = basicRole()
+      const res = await config.api.roles.save(role, {
+        status: 200,
+      })
+
+      expect(res._id).toBeDefined()
+      expect(res._rev).toBeDefined()
+      expect(events.role.updated).not.toHaveBeenCalled()
+      expect(events.role.created).toHaveBeenCalledTimes(1)
+      expect(events.role.created).toHaveBeenCalledWith(res)
+    })
+  })
+
+  describe("update", () => {
+    it("updates a role", async () => {
+      const role = basicRole()
+      let res = await config.api.roles.save(role, {
+        status: 200,
+      })
+      jest.clearAllMocks()
+      res = await config.api.roles.save(res, {
+        status: 200,
+      })
+
+      expect(res._id).toBeDefined()
+      expect(res._rev).toBeDefined()
+      expect(events.role.created).not.toHaveBeenCalled()
+      expect(events.role.updated).toHaveBeenCalledTimes(1)
+      expect(events.role.updated).toHaveBeenCalledWith(res)
+    })
+  })
+
+  describe("fetch", () => {
+    beforeAll(async () => {
+      // Recreate the app
+      await config.init()
+    })
+
+    it("should list custom roles, plus 2 default roles", async () => {
+      const customRole = await config.createRole()
+
+      const res = await config.api.roles.fetch({
+        status: 200,
+      })
+
+      expect(res.length).toBe(5)
+
+      const adminRole = res.find(r => r._id === BUILTIN_ROLE_IDS.ADMIN)
+      expect(adminRole).toBeDefined()
+      expect(adminRole!.inherits).toEqual(BUILTIN_ROLE_IDS.POWER)
+      expect(adminRole!.permissionId).toEqual(BuiltinPermissionID.ADMIN)
+
+      const powerUserRole = res.find(r => r._id === BUILTIN_ROLE_IDS.POWER)
+      expect(powerUserRole).toBeDefined()
+      expect(powerUserRole!.inherits).toEqual(BUILTIN_ROLE_IDS.BASIC)
+      expect(powerUserRole!.permissionId).toEqual(BuiltinPermissionID.POWER)
+
+      const customRoleFetched = res.find(r => r._id === customRole.name)
+      expect(customRoleFetched).toBeDefined()
+      expect(customRoleFetched!.inherits).toEqual(BUILTIN_ROLE_IDS.BASIC)
+      expect(customRoleFetched!.permissionId).toEqual(
+        BuiltinPermissionID.READ_ONLY
+      )
+    })
+
+    it("should be able to get the role with a permission added", async () => {
+      const table = await config.createTable()
+      await config.api.permission.add({
+        roleId: BUILTIN_ROLE_IDS.POWER,
+        resourceId: table._id!,
+        level: PermissionLevel.READ,
+      })
+      const res = await config.api.roles.fetch()
+      expect(res.length).toBeGreaterThan(0)
+      const power = res.find(role => role._id === BUILTIN_ROLE_IDS.POWER)
+      expect(power?.permissions[table._id!]).toEqual(["read"])
+    })
+  })
+
+  describe("destroy", () => {
+    it("should delete custom roles", async () => {
+      const customRole = await config.createRole({
+        name: "user",
+        permissionId: BuiltinPermissionID.READ_ONLY,
+        inherits: BUILTIN_ROLE_IDS.BASIC,
+      })
+      await config.api.roles.destroy(customRole, {
+        status: 200,
+      })
+      await config.api.roles.find(customRole._id!, {
+        status: 404,
+      })
+      expect(events.role.deleted).toHaveBeenCalledTimes(1)
+      expect(events.role.deleted).toHaveBeenCalledWith(customRole)
+    })
+  })
+
+  describe("accessible", () => {
+    it("should be able to fetch accessible roles (with builder)", async () => {
+      const res = await config.api.roles.accessible(config.defaultHeaders(), {
+        status: 200,
+      })
+      expect(res.length).toBe(5)
+      expect(typeof res[0]).toBe("string")
+    })
+
+    it("should be able to fetch accessible roles (basic user)", async () => {
+      const headers = await config.basicRoleHeaders()
+      const res = await config.api.roles.accessible(headers, {
+        status: 200,
+      })
+      expect(res.length).toBe(2)
+      expect(res[0]).toBe("BASIC")
+      expect(res[1]).toBe("PUBLIC")
+    })
+
+    it("should be able to fetch accessible roles (no user)", async () => {
+      const res = await config.api.roles.accessible(config.publicHeaders(), {
+        status: 200,
+      })
+      expect(res.length).toBe(1)
+      expect(res[0]).toBe("PUBLIC")
+    })
+
+    it("should not fetch higher level accessible roles when a custom role header is provided", async () => {
+      const customRoleName = "CUSTOM_ROLE"
+      await config.api.roles.save({
+        name: customRoleName,
+        inherits: roles.BUILTIN_ROLE_IDS.BASIC,
+        permissionId: permissions.BuiltinPermissionID.READ_ONLY,
+        version: "name",
+      })
+      const res = await config.api.roles.accessible(
+        { "x-budibase-role": customRoleName },
+        {
+          status: 200,
+        }
+      )
+      expect(res.length).toBe(3)
+      expect(res[0]).toBe(customRoleName)
+      expect(res[1]).toBe("BASIC")
+      expect(res[2]).toBe("PUBLIC")
+    })
+  })
+})

packages/server/src/api/routes/utils/validators.ts

@@ -226,7 +226,10 @@ export function roleValidator() {
           )
         )
         .optional(),
-      inherits: OPTIONAL_STRING,
+      inherits: Joi.alternatives().try(
+        OPTIONAL_STRING,
+        Joi.array().items(OPTIONAL_STRING)
+      ),
     }).unknown(true)
   )
 }

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

@@ -459,6 +459,7 @@ export default class TestConfiguration {
 
     const headers: any = {
       Accept: "application/json",
+      Cookie: "",
     }
     if (appId) {
       headers[constants.Header.APP_ID] = appId

packages/server/src/tests/utilities/api/role.ts

@@ -4,6 +4,7 @@ import {
   FindRoleResponse,
   SaveRoleRequest,
   SaveRoleResponse,
+  Role,
 } from "@budibase/types"
 import { Expectations, TestAPI } from "./base"
 
@@ -27,14 +28,18 @@ export class RoleAPI extends TestAPI {
     })
   }
 
-  destroy = async (roleId: string, expectations?: Expectations) => {
-    return await this._delete(`/api/roles/${roleId}`, {
+  destroy = async (role: Role, expectations?: Expectations) => {
+    return await this._delete(`/api/roles/${role._id}/${role._rev}`, {
       expectations,
     })
   }
 
-  accesssible = async (expectations?: Expectations) => {
+  accessible = async (
+    headers: Record<string, string | string[]>,
+    expectations?: Expectations
+  ) => {
     return await this._get<AccessibleRolesResponse>(`/api/roles/accessible`, {
+      headers,
       expectations,
     })
   }

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

@@ -30,6 +30,7 @@ import {
   BBReferenceFieldSubType,
   JsonFieldSubType,
   AutoFieldSubType,
+  Role,
   CreateViewRequest,
 } from "@budibase/types"
 import { LoopInput } from "../../definitions/automations"
@@ -510,11 +511,12 @@ export function basicLinkedRow(
   }
 }
 
-export function basicRole() {
+export function basicRole(): Role {
   return {
     name: `NewRole_${utils.newid()}`,
     inherits: roles.BUILTIN_ROLE_IDS.BASIC,
     permissionId: permissions.BuiltinPermissionID.READ_ONLY,
+    permissions: {},
     version: "name",
   }
 }

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

@@ -0,0 +1,47 @@
+import { Role } from "@budibase/types"
+
+// Function to detect loops in roles
+export function checkForRoleInheritanceLoops(roles: Role[]): boolean {
+  const roleMap = new Map<string, Role>()
+  roles.forEach(role => {
+    roleMap.set(role._id!, role)
+  })
+
+  const checked = new Set<string>()
+  const checking = new Set<string>()
+
+  function hasLoop(roleId: string): boolean {
+    if (checking.has(roleId)) {
+      return true
+    }
+    if (checked.has(roleId)) {
+      return false
+    }
+
+    checking.add(roleId)
+
+    const role = roleMap.get(roleId)
+    if (!role) {
+      // role not found - ignore
+      checking.delete(roleId)
+      return false
+    }
+
+    const inherits = Array.isArray(role.inherits)
+      ? role.inherits
+      : [role.inherits]
+    for (const inheritedId of inherits) {
+      if (inheritedId && hasLoop(inheritedId)) {
+        return true
+      }
+    }
+
+    // mark this role has been fully checked
+    checking.delete(roleId)
+    checked.add(roleId)
+
+    return false
+  }
+
+  return !!roles.find(role => hasLoop(role._id!))
+}

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

@@ -0,0 +1,61 @@
+import { checkForRoleInheritanceLoops } from "../roles"
+import { Role } from "@budibase/types"
+
+/**
+ * This unit test exists as this utility will be used in the frontend and backend, confirmation
+ * of its API and expected results is useful since the backend tests won't confirm it works
+ * exactly as the frontend needs it to - easy to add specific test cases here that the frontend
+ * might need to check/cover.
+ */
+
+interface TestRole extends Omit<Role, "_id"> {
+  _id: string
+}
+
+let allRoles: TestRole[] = []
+
+function role(id: string, inherits: string | string[]): TestRole {
+  const role = {
+    _id: id,
+    inherits: inherits,
+    name: "ROLE",
+    permissionId: "PERMISSION",
+    permissions: {}, // not needed for this test
+  }
+  allRoles.push(role)
+  return role
+}
+
+describe("role utilities", () => {
+  let role1: TestRole, role2: TestRole
+
+  beforeEach(() => {
+    role1 = role("role_1", [])
+    role2 = role("role_2", [role1._id])
+  })
+
+  afterEach(() => {
+    allRoles = []
+  })
+
+  function check(hasLoop: boolean) {
+    const result = checkForRoleInheritanceLoops(allRoles)
+    expect(result).toBe(hasLoop)
+  }
+
+  describe("checkForRoleInheritanceLoops", () => {
+    it("should confirm no loops", () => {
+      check(false)
+    })
+
+    it("should confirm there is a loop", () => {
+      const role3 = role("role_3", [role2._id])
+      const role4 = role("role_4", [role3._id, role2._id, role1._id])
+      role3.inherits = [
+        ...(Array.isArray(role3.inherits) ? role3.inherits : []),
+        role4._id,
+      ]
+      check(true)
+    })
+  })
+})

packages/types/src/api/web/role.ts

@@ -4,9 +4,9 @@ export interface SaveRoleRequest {
   _id?: string
   _rev?: string
   name: string
-  inherits: string
+  inherits?: string | string[]
   permissionId: string
-  version: string
+  version?: string
   uiMetadata?: RoleUIMetadata
 }
 

packages/types/src/documents/app/role.ts

@@ -9,7 +9,7 @@ export interface RoleUIMetadata {
 
 export interface Role extends Document {
   permissionId: string
-  inherits?: string
+  inherits?: string | string[]
   permissions: Record<string, PermissionLevel[]>
   version?: string
   name: string

packages/types/src/sdk/events/role.ts

@@ -3,19 +3,19 @@ import { BaseEvent } from "./event"
 export interface RoleCreatedEvent extends BaseEvent {
   roleId: string
   permissionId: string
-  inherits?: string
+  inherits?: string | string[]
 }
 
 export interface RoleUpdatedEvent extends BaseEvent {
   roleId: string
   permissionId: string
-  inherits?: string
+  inherits?: string | string[]
 }
 
 export interface RoleDeletedEvent extends BaseEvent {
   roleId: string
   permissionId: string
-  inherits?: string
+  inherits?: string | string[]
 }
 
 export interface RoleAssignedEvent extends BaseEvent {