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

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

@@ -223,6 +223,11 @@ export function lowerBuiltinRoleID(roleId1?: string, roleId2?: string): string {
     : roleId1
 }
 
+function compareRoleIds(roleId1: string, roleId2: string) {
+  // make sure both role IDs are prefixed correctly
+  return prefixRoleID(roleId1) === prefixRoleID(roleId2)
+}
+
 /**
  * Given a list of roles, this will pick the role out, accounting for built ins.
  */
@@ -239,7 +244,7 @@ export function findRole(
     roleId = prefixRoleID(roleId)
   }
   const dbRole = roles.find(
-    role => role._id && role._id === getExternalRoleID(roleId, role.version)
+    role => role._id && compareRoleIds(role._id, roleId)
   )
   if (!dbRole && !isBuiltin(roleId) && opts?.defaultPublic) {
     return cloneDeep(BUILTIN_ROLES.PUBLIC)
@@ -474,7 +479,10 @@ export class AccessController {
       this.userHierarchies[userRoleId] = roleIds
     }
 
-    return roleIds?.indexOf(tryingRoleId) !== -1
+    return (
+      roleIds?.find(roleId => compareRoleIds(roleId, tryingRoleId)) !==
+      undefined
+    )
   }
 
   async checkScreensAccess(screens: Screen[], userRoleId: string) {

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

@@ -82,7 +82,10 @@ export async function save(ctx: UserCtx<SaveRoleRequest, SaveRoleResponse>) {
     _id = dbCore.prefixRoleID(_id)
   }
 
-  const allRoles = await roles.getAllRoles()
+  const allRoles = (await roles.getAllRoles()).map(role => ({
+    ...role,
+    _id: dbCore.prefixRoleID(role._id!),
+  }))
   let dbRole: Role | undefined
   if (!isCreate && _id?.startsWith(DocumentType.ROLE)) {
     dbRole = allRoles.find(role => role._id === _id)

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

@@ -47,6 +47,25 @@ describe("/roles", () => {
       expect(events.role.updated).toHaveBeenCalledTimes(1)
       expect(events.role.updated).toHaveBeenCalledWith(res)
     })
+
+    it("disallow loops", async () => {
+      let role1 = basicRole()
+      role1 = await config.api.roles.save(role1, {
+        status: 200,
+      })
+      let role2 = basicRole()
+      role2.inherits = [role1._id!]
+      role2 = await config.api.roles.save(role2, {
+        status: 200,
+      })
+      role1.inherits = [role2._id!]
+      await config.api.roles.save(role1, {
+        status: 400,
+        body: {
+          message: "Role inheritance contains a loop, this is not supported",
+        },
+      })
+    })
   })
 
   describe("fetch", () => {

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

@@ -1,103 +0,0 @@
-const { checkBuilderEndpoint } = require("./utilities/TestFunctions")
-const setup = require("./utilities")
-const { basicScreen } = setup.structures
-const { events } = require("@budibase/backend-core")
-
-describe("/screens", () => {
-  let request = setup.getRequest()
-  let config = setup.getConfig()
-  let screen
-
-  afterAll(setup.afterAll)
-
-  beforeAll(async () => {
-    await config.init()
-    screen = await config.createScreen()
-  })
-
-  describe("fetch", () => {
-    it("should be able to create a layout", async () => {
-      const res = await request
-        .get(`/api/screens`)
-        .set(config.defaultHeaders())
-        .expect("Content-Type", /json/)
-        .expect(200)
-      expect(res.body.length).toEqual(1)
-      expect(res.body.some(s => s._id === screen._id)).toEqual(true)
-    })
-
-    it("should apply authorization to endpoint", async () => {
-      await checkBuilderEndpoint({
-        config,
-        method: "GET",
-        url: `/api/screens`,
-      })
-    })
-  })
-
-  describe("save", () => {
-    const saveScreen = async screen => {
-      const res = await request
-        .post(`/api/screens`)
-        .send(screen)
-        .set(config.defaultHeaders())
-        .expect("Content-Type", /json/)
-        .expect(200)
-      return res
-    }
-
-    it("should be able to create a screen", async () => {
-      jest.clearAllMocks()
-
-      const screen = basicScreen()
-      const res = await saveScreen(screen)
-
-      expect(res.body._rev).toBeDefined()
-      expect(res.body.name).toEqual(screen.name)
-      expect(events.screen.created).toBeCalledTimes(1)
-    })
-
-    it("should be able to update a screen", async () => {
-      const screen = basicScreen()
-      let res = await saveScreen(screen)
-      screen._id = res.body._id
-      screen._rev = res.body._rev
-      screen.name = "edit"
-      jest.clearAllMocks()
-
-      res = await saveScreen(screen)
-
-      expect(res.body._rev).toBeDefined()
-      expect(res.body.name).toEqual(screen.name)
-      expect(events.screen.created).not.toBeCalled()
-    })
-
-    it("should apply authorization to endpoint", async () => {
-      await checkBuilderEndpoint({
-        config,
-        method: "POST",
-        url: `/api/screens`,
-      })
-    })
-  })
-
-  describe("destroy", () => {
-    it("should be able to delete the screen", async () => {
-      const res = await request
-        .delete(`/api/screens/${screen._id}/${screen._rev}`)
-        .set(config.defaultHeaders())
-        .expect("Content-Type", /json/)
-        .expect(200)
-      expect(res.body.message).toBeDefined()
-      expect(events.screen.deleted).toBeCalledTimes(1)
-    })
-
-    it("should apply authorization to endpoint", async () => {
-      await checkBuilderEndpoint({
-        config,
-        method: "DELETE",
-        url: `/api/screens/${screen._id}/${screen._rev}`,
-      })
-    })
-  })
-})

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

@@ -0,0 +1,166 @@
+import { checkBuilderEndpoint } from "./utilities/TestFunctions"
+import * as setup from "./utilities"
+import { events, roles } from "@budibase/backend-core"
+import { Screen, PermissionLevel, Role } from "@budibase/types"
+
+const { basicScreen } = setup.structures
+
+describe("/screens", () => {
+  let config = setup.getConfig()
+  let screen: Screen
+
+  afterAll(setup.afterAll)
+
+  beforeAll(async () => {
+    await config.init()
+    screen = await config.createScreen()
+  })
+
+  describe("fetch", () => {
+    it("should be able to create a layout", async () => {
+      const screens = await config.api.screen.list({ status: 200 })
+      expect(screens.length).toEqual(1)
+      expect(screens.some(s => s._id === screen._id)).toEqual(true)
+    })
+
+    it("should apply authorization to endpoint", async () => {
+      await checkBuilderEndpoint({
+        config,
+        method: "GET",
+        url: `/api/screens`,
+      })
+    })
+  })
+
+  describe("permissions", () => {
+    let screen1: Screen, screen2: Screen
+    let role1: Role, role2: Role, multiRole: Role
+
+    beforeAll(async () => {
+      role1 = await config.api.roles.save({
+        name: "role1",
+        inherits: roles.BUILTIN_ROLE_IDS.BASIC,
+        permissionId: PermissionLevel.WRITE,
+      })
+      role2 = await config.api.roles.save({
+        name: "role2",
+        inherits: roles.BUILTIN_ROLE_IDS.BASIC,
+        permissionId: PermissionLevel.WRITE,
+      })
+      multiRole = await config.api.roles.save({
+        name: "multiRole",
+        inherits: [role1._id!, role2._id!],
+        permissionId: PermissionLevel.WRITE,
+      })
+      screen1 = await config.api.screen.save(
+        {
+          ...basicScreen(),
+          routing: {
+            roleId: role1._id!,
+            route: "/foo",
+            homeScreen: false,
+          },
+        },
+        { status: 200 }
+      )
+      screen2 = await config.api.screen.save(
+        {
+          ...basicScreen(),
+          routing: {
+            roleId: role2._id!,
+            route: "/bar",
+            homeScreen: false,
+          },
+        },
+        { status: 200 }
+      )
+      // get into prod app
+      await config.publish()
+    })
+
+    async function checkScreens(roleId: string, screenIds: string[]) {
+      await config.setRole(roleId, async () => {
+        const res = await config.api.application.getDefinition(
+          config.prodAppId!,
+          {
+            status: 200,
+          }
+        )
+        // basic and role1 screen
+        expect(res.screens.length).toEqual(screenIds.length)
+        expect(res.screens.map(s => s._id).sort()).toEqual(screenIds.sort())
+      })
+    }
+
+    it("should be able to fetch basic and screen1 with role1", async () => {
+      await checkScreens(role1._id!, [screen._id!, screen1._id!])
+    })
+
+    it("should be able to fetch basic and screen2 with role2", async () => {
+      await checkScreens(role2._id!, [screen._id!, screen2._id!])
+    })
+
+    it("should be able to fetch basic, screen1 and screen2 with multi-inheritance role", async () => {
+      await checkScreens(multiRole._id!, [
+        screen._id!,
+        screen1._id!,
+        screen2._id!,
+      ])
+    })
+  })
+
+  describe("save", () => {
+    it("should be able to create a screen", async () => {
+      const screen = basicScreen()
+      const responseScreen = await config.api.screen.save(screen, {
+        status: 200,
+      })
+
+      expect(responseScreen._rev).toBeDefined()
+      expect(responseScreen.name).toEqual(screen.name)
+      expect(events.screen.created).toHaveBeenCalledTimes(1)
+    })
+
+    it("should be able to update a screen", async () => {
+      const screen = basicScreen()
+      let responseScreen = await config.api.screen.save(screen, { status: 200 })
+      screen._id = responseScreen._id
+      screen._rev = responseScreen._rev
+      screen.name = "edit"
+
+      responseScreen = await config.api.screen.save(screen, { status: 200 })
+
+      expect(responseScreen._rev).toBeDefined()
+      expect(responseScreen.name).toEqual(screen.name)
+      expect(events.screen.created).not.toHaveBeenCalled()
+    })
+
+    it("should apply authorization to endpoint", async () => {
+      await checkBuilderEndpoint({
+        config,
+        method: "POST",
+        url: `/api/screens`,
+      })
+    })
+  })
+
+  describe("destroy", () => {
+    it("should be able to delete the screen", async () => {
+      const response = await config.api.screen.destroy(
+        screen._id!,
+        screen._rev!,
+        { status: 200 }
+      )
+      expect(response.message).toBeDefined()
+      expect(events.screen.deleted).toHaveBeenCalledTimes(1)
+    })
+
+    it("should apply authorization to endpoint", async () => {
+      await checkBuilderEndpoint({
+        config,
+        method: "DELETE",
+        url: `/api/screens/${screen._id}/${screen._rev}`,
+      })
+    })
+  })
+})

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

@@ -5,4 +5,27 @@ export class ScreenAPI extends TestAPI {
   list = async (expectations?: Expectations): Promise<Screen[]> => {
     return await this._get<Screen[]>(`/api/screens`, { expectations })
   }
+
+  save = async (
+    screen: Screen,
+    expectations?: Expectations
+  ): Promise<Screen> => {
+    return await this._post<Screen>(`/api/screens`, {
+      expectations,
+      body: screen,
+    })
+  }
+
+  destroy = async (
+    screenId: string,
+    screenRev: string,
+    expectations?: Expectations
+  ): Promise<{ message: string }> => {
+    return this._delete<{ message: string }>(
+      `/api/screens/${screenId}/${screenRev}`,
+      {
+        expectations,
+      }
+    )
+  }
 }