Merge pull request #11585 from Budibase/BUDI-7393/allow-view-permissions

Allow setting view permissions

packages/server/src/api/routes/row.ts

@@ -1,10 +1,11 @@
 import Router from "@koa/router"
 import * as rowController from "../controllers/row"
-import authorized from "../../middleware/authorized"
+import authorized, { authorizedResource } from "../../middleware/authorized"
 import { paramResource, paramSubResource } from "../../middleware/resourceId"
 import { permissions } from "@budibase/backend-core"
 import { internalSearchValidator } from "./utils/validators"
 import trimViewRowInfo from "../../middleware/trimViewRowInfo"
+
 const { PermissionType, PermissionLevel } = permissions
 
 const router: Router = new Router()
@@ -269,7 +270,7 @@ router
 
 router.post(
   "/api/v2/views/:viewId/search",
-  authorized(PermissionType.TABLE, PermissionLevel.READ),
+  authorizedResource(PermissionType.VIEW, PermissionLevel.READ, "viewId"),
   rowController.views.searchView
 )
 

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

@@ -12,8 +12,10 @@ import {
   PermissionLevel,
   Row,
   Table,
+  ViewV2,
 } from "@budibase/types"
 import * as setup from "./utilities"
+import { mocks } from "@budibase/backend-core/tests"
 
 const { basicRow } = setup.structures
 const { BUILTIN_ROLE_IDS } = roles
@@ -27,6 +29,7 @@ describe("/permission", () => {
   let table: Table & { _id: string }
   let perms: Document[]
   let row: Row
+  let view: ViewV2
 
   afterAll(setup.afterAll)
 
@@ -35,10 +38,12 @@ describe("/permission", () => {
   })
 
   beforeEach(async () => {
+    mocks.licenses.useCloudFree()
     mockedSdk.resourceActionAllowed.mockResolvedValue({ allowed: true })
 
     table = (await config.createTable()) as typeof table
     row = await config.createRow()
+    view = await config.api.viewV2.create({ tableId: table._id })
     perms = await config.api.permission.set({
       roleId: STD_ROLE_ID,
       resourceId: table._id,
@@ -162,6 +167,72 @@ describe("/permission", () => {
       expect(res.body[0]._id).toEqual(row._id)
     })
 
+    it("should be able to access the view data when the table is set to public and with no view permissions overrides", async () => {
+      // replicate changes before checking permissions
+      await config.publish()
+
+      const res = await config.api.viewV2.search(view.id, undefined, {
+        usePublicUser: true,
+      })
+      expect(res.body.rows[0]._id).toEqual(row._id)
+    })
+
+    it("should not be able to access the view data when the table is not public and there are no view permissions overrides", async () => {
+      await config.api.permission.revoke({
+        roleId: STD_ROLE_ID,
+        resourceId: table._id,
+        level: PermissionLevel.READ,
+      })
+      // replicate changes before checking permissions
+      await config.publish()
+
+      await config.api.viewV2.search(view.id, undefined, {
+        expectStatus: 403,
+        usePublicUser: true,
+      })
+    })
+
+    it("should ignore the view permissions if the flag is not on", async () => {
+      await config.api.permission.set({
+        roleId: STD_ROLE_ID,
+        resourceId: view.id,
+        level: PermissionLevel.READ,
+      })
+      await config.api.permission.revoke({
+        roleId: STD_ROLE_ID,
+        resourceId: table._id,
+        level: PermissionLevel.READ,
+      })
+      // replicate changes before checking permissions
+      await config.publish()
+
+      await config.api.viewV2.search(view.id, undefined, {
+        expectStatus: 403,
+        usePublicUser: true,
+      })
+    })
+
+    it("should use the view permissions if the flag is on", async () => {
+      mocks.licenses.useViewPermissions()
+      await config.api.permission.set({
+        roleId: STD_ROLE_ID,
+        resourceId: view.id,
+        level: PermissionLevel.READ,
+      })
+      await config.api.permission.revoke({
+        roleId: STD_ROLE_ID,
+        resourceId: table._id,
+        level: PermissionLevel.READ,
+      })
+      // replicate changes before checking permissions
+      await config.publish()
+
+      const res = await config.api.viewV2.search(view.id, undefined, {
+        usePublicUser: true,
+      })
+      expect(res.body.rows[0]._id).toEqual(row._id)
+    })
+
     it("shouldn't allow writing from a public user", async () => {
       const res = await request
         .post(`/api/${table._id}/rows`)

packages/server/src/middleware/authorized.ts

@@ -6,8 +6,11 @@ import {
   users,
 } from "@budibase/backend-core"
 import { PermissionLevel, PermissionType, Role, UserCtx } from "@budibase/types"
+import { features } from "@budibase/pro"
 import builderMiddleware from "./builder"
 import { isWebhookEndpoint } from "./utils"
+import { paramResource } from "./resourceId"
+import { extractViewInfoFromID, isViewID } from "../db/utils"
 
 function hasResource(ctx: any) {
   return ctx.resourceId != null
@@ -74,10 +77,37 @@ const checkAuthorizedResource = async (
   }
 }
 
-export default (
+const resourceIdTranformers: Partial<
+  Record<PermissionType, (ctx: UserCtx) => Promise<void>>
+> = {
+  [PermissionType.VIEW]: async ctx => {
+    const { resourceId } = ctx
+    if (!resourceId) {
+      ctx.throw(400, `Cannot obtain the view id`)
+      return
+    }
+
+    if (!isViewID(resourceId)) {
+      ctx.throw(400, `"${resourceId}" is not a valid view id`)
+      return
+    }
+
+    if (await features.isViewPermissionEnabled()) {
+      ctx.subResourceId = ctx.resourceId
+      ctx.resourceId = extractViewInfoFromID(resourceId).tableId
+    } else {
+      ctx.resourceId = extractViewInfoFromID(resourceId).tableId
+      delete ctx.subResourceId
+    }
+  },
+}
+
+const authorized =
+  (
     permType: PermissionType,
     permLevel?: PermissionLevel,
-    opts = { schema: false }
+    opts = { schema: false },
+    resourcePath?: string
   ) =>
   async (ctx: any, next: any) => {
     // webhooks don't need authentication, each webhook unique
@@ -97,11 +127,27 @@ export default (
       permLevel === PermissionLevel.READ
         ? PermissionLevel.WRITE
         : PermissionLevel.READ
-    const appId = context.getAppId()
+
-    if (appId && hasResource(ctx)) {
+    if (resourcePath) {
-      resourceRoles = await roles.getRequiredResourceRole(permLevel!, ctx)
+      // Reusing the existing middleware to extract the value
+      paramResource(resourcePath)(ctx, () => {})
+    }
+
+    if (resourceIdTranformers[permType]) {
+      await resourceIdTranformers[permType]!(ctx)
+    }
+
+    if (hasResource(ctx)) {
+      const { resourceId, subResourceId } = ctx
+      resourceRoles = await roles.getRequiredResourceRole(permLevel!, {
+        resourceId,
+        subResourceId,
+      })
       if (opts && opts.schema) {
-        otherLevelRoles = await roles.getRequiredResourceRole(otherLevel, ctx)
+        otherLevelRoles = await roles.getRequiredResourceRole(otherLevel, {
+          resourceId,
+          subResourceId,
+        })
       }
     }
 
@@ -143,3 +189,17 @@ export default (
     // csrf protection
     return csrf(ctx, next)
   }
+
+export default (
+  permType: PermissionType,
+  permLevel?: PermissionLevel,
+  opts = { schema: false }
+) => authorized(permType, permLevel, opts)
+
+export const authorizedResource = (
+  permType: PermissionType,
+  permLevel: PermissionLevel,
+  resourcePath: string
+) => {
+  return authorized(permType, permLevel, undefined, resourcePath)
+}

packages/server/src/middleware/resourceId.ts

@@ -43,6 +43,7 @@ export class ResourceIdGetter {
   }
 }
 
+/** @deprecated we should use the authorizedResource middleware instead */
 export function paramResource(main: string) {
   return new ResourceIdGetter("params").mainResource(main).build()
 }

packages/server/src/middleware/tests/authorized.spec.js

@@ -1,163 +0,0 @@
-jest.mock("../../environment", () => ({
-    prod: false,
-    isTest: () => true,
-    isProd: () => this.prod,
-    _set: function(key, value) {
-      this.prod = value === "production"
-    }
-  })
-)
-const authorizedMiddleware = require("../authorized").default
-const env = require("../../environment")
-const { PermissionType, PermissionLevel } = require("@budibase/types")
-
-const APP_ID = ""
-
-class TestConfiguration {
-  constructor(role) {
-    this.middleware = authorizedMiddleware(role)
-    this.next = jest.fn()
-    this.throw = jest.fn()
-    this.headers = {}
-    this.ctx = {
-      headers: {},
-      request: {
-        url: ""
-      },
-      appId: APP_ID,
-      auth: {},
-      next: this.next,
-      throw: this.throw,
-      get: (name) => this.headers[name],
-    }
-  }
-
-  executeMiddleware() {
-    return this.middleware(this.ctx, this.next)
-  }
-
-  setUser(user) {
-    this.ctx.user = user
-  }
-
-  setMiddlewareRequiredPermission(...perms) {
-    this.middleware = authorizedMiddleware(...perms)
-  }
-
-  setResourceId(id) {
-    this.ctx.resourceId = id
-  }
-
-  setAuthenticated(isAuthed) {
-    this.ctx.isAuthenticated = isAuthed
-  }
-
-  setRequestUrl(url) {
-    this.ctx.request.url = url
-  }
-
-  setEnvironment(isProd) {
-    env._set("NODE_ENV", isProd ? "production" : "jest")
-  }
-
-  setRequestHeaders(headers) {
-    this.ctx.headers = headers 
-  }
-
-  afterEach() {
-    jest.clearAllMocks()
-  }
-}
-
-
-describe("Authorization middleware", () => {
-  const next = jest.fn()
-  let config
-
-  afterEach(() => {
-    config.afterEach()
-  })
-
-  beforeEach(() => {
-    config = new TestConfiguration()
-  })
-
-  describe("non-webhook call", () => {
-    let config
-
-    beforeEach(() => {
-      config = new TestConfiguration()
-      config.setEnvironment(true)
-      config.setAuthenticated(true)
-    })
-
-    it("throws when no user data is present in context", async () => {
-      await config.executeMiddleware()
-
-      expect(config.throw).toHaveBeenCalledWith(403, "No user info found")
-    })
-
-    it("passes on to next() middleware if user is an admin", async () => {
-      config.setUser({
-        _id: "user",
-        role: {
-          _id: "ADMIN",
-        }
-      })
-
-      await config.executeMiddleware()
-
-      expect(config.next).toHaveBeenCalled()
-    })
-    
-    it("throws if the user does not have builder permissions", async () => {
-      config.setEnvironment(false)
-      config.setMiddlewareRequiredPermission(PermissionType.BUILDER)
-      config.setUser({
-        role: {
-          _id: ""
-        }
-      })
-      await config.executeMiddleware()
-
-      expect(config.throw).toHaveBeenCalledWith(403, "Not Authorized")
-    })
-
-    it("passes on to next() middleware if the user has resource permission", async () => {
-      config.setResourceId(PermissionType.QUERY)
-      config.setUser({
-        role: {
-          _id: ""
-        }
-      })
-      config.setMiddlewareRequiredPermission(PermissionType.QUERY)
-
-      await config.executeMiddleware()
-      expect(config.next).toHaveBeenCalled()
-    })
-
-    it("throws if the user session is not authenticated", async () => {
-      config.setUser({
-        role: {
-          _id: ""
-        },
-      })
-      config.setAuthenticated(false)
-
-      await config.executeMiddleware()
-      expect(config.throw).toHaveBeenCalledWith(403, "Session not authenticated")
-    })
-
-    it("throws if the user does not have base permissions to perform the operation", async () => {
-      config.setUser({
-        role: {
-          _id: ""
-        },
-      })
-      config.setMiddlewareRequiredPermission(PermissionType.ADMIN, PermissionLevel.BASIC)
-      
-      await config.executeMiddleware()
-      expect(config.throw).toHaveBeenCalledWith(403, "User does not have permission")
-    })
-  })
-})

packages/server/src/middleware/tests/authorized.spec.ts

@@ -0,0 +1,272 @@
+jest.mock("@budibase/backend-core", () => ({
+  ...jest.requireActual("@budibase/backend-core"),
+  roles: {
+    ...jest.requireActual("@budibase/backend-core").roles,
+    getRequiredResourceRole: jest.fn().mockResolvedValue([]),
+  },
+}))
+jest.mock("../../environment", () => ({
+  prod: false,
+  isTest: () => true,
+  // @ts-ignore
+  isProd: () => this.prod,
+  _set: function (_key: string, value: string) {
+    this.prod = value === "production"
+  },
+}))
+
+import { PermissionType, PermissionLevel } from "@budibase/types"
+
+import authorizedMiddleware from "../authorized"
+import env from "../../environment"
+import { generateTableID, generateViewID } from "../../db/utils"
+import { roles } from "@budibase/backend-core"
+import { mocks } from "@budibase/backend-core/tests"
+import { initProMocks } from "../../tests/utilities/mocks/pro"
+
+const APP_ID = ""
+
+initProMocks()
+
+class TestConfiguration {
+  middleware: (ctx: any, next: any) => Promise<void>
+  next: () => void
+  throw: () => void
+  headers: Record<string, any>
+  ctx: any
+
+  constructor() {
+    this.middleware = authorizedMiddleware(PermissionType.APP)
+    this.next = jest.fn()
+    this.throw = jest.fn()
+    this.headers = {}
+    this.ctx = {
+      headers: {},
+      request: {
+        url: "",
+      },
+      appId: APP_ID,
+      auth: {},
+      next: this.next,
+      throw: this.throw,
+      get: (name: string) => this.headers[name],
+    }
+  }
+
+  executeMiddleware() {
+    return this.middleware(this.ctx, this.next)
+  }
+
+  setUser(user: any) {
+    this.ctx.user = user
+  }
+
+  setMiddlewareRequiredPermission(...perms: any[]) {
+    // @ts-ignore
+    this.middleware = authorizedMiddleware(...perms)
+  }
+
+  setResourceId(id?: string) {
+    this.ctx.resourceId = id
+  }
+
+  setAuthenticated(isAuthed: boolean) {
+    this.ctx.isAuthenticated = isAuthed
+  }
+
+  setRequestUrl(url: string) {
+    this.ctx.request.url = url
+  }
+
+  setEnvironment(isProd: boolean) {
+    env._set("NODE_ENV", isProd ? "production" : "jest")
+  }
+
+  setRequestHeaders(headers: Record<string, any>) {
+    this.ctx.headers = headers
+  }
+
+  afterEach() {
+    jest.clearAllMocks()
+  }
+}
+
+describe("Authorization middleware", () => {
+  let config: TestConfiguration
+
+  afterEach(() => {
+    config.afterEach()
+  })
+
+  beforeEach(() => {
+    jest.clearAllMocks()
+    mocks.licenses.useCloudFree()
+    config = new TestConfiguration()
+  })
+
+  describe("non-webhook call", () => {
+    beforeEach(() => {
+      config = new TestConfiguration()
+      config.setEnvironment(true)
+      config.setAuthenticated(true)
+    })
+
+    it("throws when no user data is present in context", async () => {
+      await config.executeMiddleware()
+
+      expect(config.throw).toHaveBeenCalledWith(403, "No user info found")
+    })
+
+    it("passes on to next() middleware if user is an admin", async () => {
+      config.setUser({
+        _id: "user",
+        role: {
+          _id: "ADMIN",
+        },
+      })
+
+      await config.executeMiddleware()
+
+      expect(config.next).toHaveBeenCalled()
+    })
+
+    it("throws if the user does not have builder permissions", async () => {
+      config.setEnvironment(false)
+      config.setMiddlewareRequiredPermission(PermissionType.BUILDER)
+      config.setUser({
+        role: {
+          _id: "",
+        },
+      })
+      await config.executeMiddleware()
+
+      expect(config.throw).toHaveBeenCalledWith(403, "Not Authorized")
+    })
+
+    it("passes on to next() middleware if the user has resource permission", async () => {
+      config.setResourceId(PermissionType.QUERY)
+      config.setUser({
+        role: {
+          _id: "",
+        },
+      })
+      config.setMiddlewareRequiredPermission(PermissionType.QUERY)
+
+      await config.executeMiddleware()
+      expect(config.next).toHaveBeenCalled()
+    })
+
+    it("throws if the user session is not authenticated", async () => {
+      config.setUser({
+        role: {
+          _id: "",
+        },
+      })
+      config.setAuthenticated(false)
+
+      await config.executeMiddleware()
+      expect(config.throw).toHaveBeenCalledWith(
+        403,
+        "Session not authenticated"
+      )
+    })
+
+    it("throws if the user does not have base permissions to perform the operation", async () => {
+      config.setUser({
+        role: {
+          _id: "",
+        },
+      })
+      config.setMiddlewareRequiredPermission(
+        PermissionType.APP,
+        PermissionLevel.READ
+      )
+
+      await config.executeMiddleware()
+      expect(config.throw).toHaveBeenCalledWith(
+        403,
+        "User does not have permission"
+      )
+    })
+
+    describe("view type", () => {
+      const tableId = generateTableID()
+      const viewId = generateViewID(tableId)
+
+      const mockedGetRequiredResourceRole =
+        roles.getRequiredResourceRole as jest.MockedFunction<
+          typeof roles.getRequiredResourceRole
+        >
+
+      beforeEach(() => {
+        config.setMiddlewareRequiredPermission(
+          PermissionType.VIEW,
+          PermissionLevel.READ
+        )
+        config.setResourceId(viewId)
+
+        mockedGetRequiredResourceRole.mockResolvedValue(["PUBLIC"])
+
+        config.setUser({
+          _id: "user",
+          role: {
+            _id: "PUBLIC",
+          },
+        })
+      })
+
+      it("will ignore view permissions if flag is off", async () => {
+        await config.executeMiddleware()
+
+        expect(config.throw).not.toBeCalled()
+        expect(config.next).toHaveBeenCalled()
+
+        expect(mockedGetRequiredResourceRole).toBeCalledTimes(1)
+        expect(mockedGetRequiredResourceRole).toBeCalledWith(
+          PermissionLevel.READ,
+          expect.objectContaining({
+            resourceId: tableId,
+            subResourceId: undefined,
+          })
+        )
+      })
+
+      it("will use view permissions if flag is on", async () => {
+        mocks.licenses.useViewPermissions()
+        await config.executeMiddleware()
+
+        expect(config.throw).not.toBeCalled()
+        expect(config.next).toHaveBeenCalled()
+
+        expect(mockedGetRequiredResourceRole).toBeCalledTimes(1)
+        expect(mockedGetRequiredResourceRole).toBeCalledWith(
+          PermissionLevel.READ,
+          expect.objectContaining({
+            resourceId: tableId,
+            subResourceId: viewId,
+          })
+        )
+      })
+
+      it("throw an exception if the resource id is not provided", async () => {
+        config.setResourceId(undefined)
+        await config.executeMiddleware()
+        expect(config.throw).toHaveBeenNthCalledWith(
+          1,
+          400,
+          "Cannot obtain the view id"
+        )
+      })
+
+      it("throw an exception if the resource id is not a valid view id", async () => {
+        config.setResourceId(tableId)
+        await config.executeMiddleware()
+        expect(config.throw).toHaveBeenNthCalledWith(
+          1,
+          400,
+          `"${tableId}" is not a valid view id`
+        )
+      })
+    })
+  })
+})

packages/server/src/sdk/app/permissions/tests/permissions.spec.ts

@@ -1,12 +1,13 @@
-import TestConfiguration from "../../../../tests/utilities/TestConfiguration"
 import { PermissionLevel } from "@budibase/types"
 import { mocks, structures } from "@budibase/backend-core/tests"
 import { resourceActionAllowed } from ".."
 import { generateViewID } from "../../../../db/utils"
+import { initProMocks } from "../../../../tests/utilities/mocks/pro"
+
+initProMocks()
 
 describe("permissions sdk", () => {
   beforeEach(() => {
-    new TestConfiguration()
     mocks.licenses.useCloudFree()
   })
 

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

@@ -77,12 +77,16 @@ export class ViewV2API extends TestAPI {
   search = async (
     viewId: string,
     params?: SearchViewRowRequest,
-    { expectStatus } = { expectStatus: 200 }
+    { expectStatus = 200, usePublicUser = false } = {}
   ) => {
     return this.request
       .post(`/api/v2/views/${viewId}/search`)
       .send(params)
-      .set(this.config.defaultHeaders())
+      .set(
+        usePublicUser
+          ? this.config.publicHeaders()
+          : this.config.defaultHeaders()
+      )
       .expect("Content-Type", /json/)
       .expect(expectStatus)
   }

packages/server/src/tests/utilities/mocks/pro.ts

@@ -0,0 +1,10 @@
+// init the licensing mock
+import { mocks } from "@budibase/backend-core/tests"
+import * as pro from "@budibase/pro"
+
+export const initProMocks = () => {
+  mocks.licenses.init(pro)
+
+  // use unlimited license by default
+  mocks.licenses.useUnlimited()
+}

packages/types/src/sdk/permissions.ts

@@ -15,4 +15,5 @@ export enum PermissionType {
   BUILDER = "builder",
   GLOBAL_BUILDER = "globalBuilder",
   QUERY = "query",
+  VIEW = "view",
 }