Merge branch 'master' into better-types-on-removeInvalidFilters

packages/backend-core/src/users/users.ts

@@ -17,11 +17,8 @@ import {
   ContextUser,
   CouchFindOptions,
   DatabaseQueryOpts,
-  SearchFilters,
   SearchUsersRequest,
   User,
-  BasicOperator,
-  ArrayOperator,
 } from "@budibase/types"
 import * as context from "../context"
 import { getGlobalDB } from "../context"
@@ -45,32 +42,6 @@ function removeUserPassword(users: User | User[]) {
   return users
 }
 
-export function isSupportedUserSearch(query: SearchFilters) {
-  const allowed = [
-    { op: BasicOperator.STRING, key: "email" },
-    { op: BasicOperator.EQUAL, key: "_id" },
-    { op: ArrayOperator.ONE_OF, key: "_id" },
-  ]
-  for (let [key, operation] of Object.entries(query)) {
-    if (typeof operation !== "object") {
-      return false
-    }
-    const fields = Object.keys(operation || {})
-    // this filter doesn't contain options - ignore
-    if (fields.length === 0) {
-      continue
-    }
-    const allowedOperation = allowed.find(
-      allow =>
-        allow.op === key && fields.length === 1 && fields[0] === allow.key
-    )
-    if (!allowedOperation) {
-      return false
-    }
-  }
-  return true
-}
-
 export async function bulkGetGlobalUsersById(
   userIds: string[],
   opts?: GetOpts

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

@@ -1,9 +1,7 @@
 import { permissions, roles, context } from "@budibase/backend-core"
 import {
   UserCtx,
-  Database,
   Role,
-  PermissionLevel,
   GetResourcePermsResponse,
   ResourcePermissionInfo,
   GetDependantResourcesResponse,
@@ -12,107 +10,15 @@ import {
   RemovePermissionRequest,
   RemovePermissionResponse,
 } from "@budibase/types"
-import { getRoleParams } from "../../db/utils"
 import {
   CURRENTLY_SUPPORTED_LEVELS,
   getBasePermissions,
 } from "../../utilities/security"
-import { removeFromArray } from "../../utilities"
 import sdk from "../../sdk"
-
+import { PermissionUpdateType } from "../../sdk/app/permissions"
-const enum PermissionUpdateType {
-  REMOVE = "remove",
-  ADD = "add",
-}
 
 const SUPPORTED_LEVELS = CURRENTLY_SUPPORTED_LEVELS
 
-// utility function to stop this repetition - permissions always stored under roles
-async function getAllDBRoles(db: Database) {
-  const body = await db.allDocs<Role>(
-    getRoleParams(null, {
-      include_docs: true,
-    })
-  )
-  return body.rows.map(row => row.doc!)
-}
-
-async function updatePermissionOnRole(
-  {
-    roleId,
-    resourceId,
-    level,
-  }: { roleId: string; resourceId: string; level: PermissionLevel },
-  updateType: PermissionUpdateType
-) {
-  const db = context.getAppDB()
-  const remove = updateType === PermissionUpdateType.REMOVE
-  const isABuiltin = roles.isBuiltin(roleId)
-  const dbRoleId = roles.getDBRoleID(roleId)
-  const dbRoles = await getAllDBRoles(db)
-  const docUpdates: Role[] = []
-
-  // the permission is for a built in, make sure it exists
-  if (isABuiltin && !dbRoles.some(role => role._id === dbRoleId)) {
-    const builtin = roles.getBuiltinRoles()[roleId]
-    builtin._id = roles.getDBRoleID(builtin._id!)
-    dbRoles.push(builtin)
-  }
-
-  // now try to find any roles which need updated, e.g. removing the
-  // resource from another role and then adding to the new role
-  for (let role of dbRoles) {
-    let updated = false
-    const rolePermissions: Record<string, PermissionLevel[]> = role.permissions
-      ? role.permissions
-      : {}
-    // make sure its an array, also handle migrating
-    if (
-      !rolePermissions[resourceId] ||
-      !Array.isArray(rolePermissions[resourceId])
-    ) {
-      rolePermissions[resourceId] =
-        typeof rolePermissions[resourceId] === "string"
-          ? [rolePermissions[resourceId] as unknown as PermissionLevel]
-          : []
-    }
-    // handle the removal/updating the role which has this permission first
-    // the updating (role._id !== dbRoleId) is required because a resource/level can
-    // only be permitted in a single role (this reduces hierarchy confusion and simplifies
-    // the general UI for this, rather than needing to show everywhere it is used)
-    if (
-      (role._id !== dbRoleId || remove) &&
-      rolePermissions[resourceId].indexOf(level) !== -1
-    ) {
-      removeFromArray(rolePermissions[resourceId], level)
-      updated = true
-    }
-    // handle the adding, we're on the correct role, at it to this
-    if (!remove && role._id === dbRoleId) {
-      const set = new Set(rolePermissions[resourceId])
-      rolePermissions[resourceId] = [...set.add(level)]
-      updated = true
-    }
-    // handle the update, add it to bulk docs to perform at end
-    if (updated) {
-      role.permissions = rolePermissions
-      docUpdates.push(role)
-    }
-  }
-
-  const response = await db.bulkDocs(docUpdates)
-  return response.map(resp => {
-    const version = docUpdates.find(role => role._id === resp.id)?.version
-    const _id = roles.getExternalRoleID(resp.id, version)
-    return {
-      _id,
-      rev: resp.rev,
-      error: resp.error,
-      reason: resp.reason,
-    }
-  })
-}
-
 export function fetchBuiltin(ctx: UserCtx) {
   ctx.body = Object.values(permissions.getBuiltinPermissions())
 }
@@ -124,7 +30,7 @@ export function fetchLevels(ctx: UserCtx) {
 
 export async function fetch(ctx: UserCtx) {
   const db = context.getAppDB()
-  const dbRoles: Role[] = await getAllDBRoles(db)
+  const dbRoles: Role[] = await sdk.permissions.getAllDBRoles(db)
   let permissions: any = {}
   // create an object with structure role ID -> resource ID -> level
   for (let role of dbRoles) {
@@ -186,12 +92,18 @@ export async function getDependantResources(
 
 export async function addPermission(ctx: UserCtx<void, AddPermissionResponse>) {
   const params: AddPermissionRequest = ctx.params
-  ctx.body = await updatePermissionOnRole(params, PermissionUpdateType.ADD)
+  ctx.body = await sdk.permissions.updatePermissionOnRole(
+    params,
+    PermissionUpdateType.ADD
+  )
 }
 
 export async function removePermission(
   ctx: UserCtx<void, RemovePermissionResponse>
 ) {
   const params: RemovePermissionRequest = ctx.params
-  ctx.body = await updatePermissionOnRole(params, PermissionUpdateType.REMOVE)
+  ctx.body = await sdk.permissions.updatePermissionOnRole(
+    params,
+    PermissionUpdateType.REMOVE
+  )
 }

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

@@ -125,6 +125,13 @@ describe("/permission", () => {
     })
 
     it("should be able to access the view data when the table is set to public and with no view permissions overrides", async () => {
+      // Make view inherit table permissions. Needed for backwards compatibility with existing views.
+      await config.api.permission.revoke({
+        roleId: STD_ROLE_ID,
+        resourceId: view.id,
+        level: PermissionLevel.READ,
+      })
+
       // replicate changes before checking permissions
       await config.publish()
 
@@ -138,6 +145,14 @@ describe("/permission", () => {
         resourceId: table._id,
         level: PermissionLevel.READ,
       })
+
+      // Make view inherit table permissions. Needed for backwards compatibility with existing views.
+      await config.api.permission.revoke({
+        roleId: STD_ROLE_ID,
+        resourceId: view.id,
+        level: PermissionLevel.READ,
+      })
+
       // replicate changes before checking permissions
       await config.publish()
 

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

@@ -826,11 +826,20 @@ describe("/rowsActions", () => {
               )
             ).id
 
+            // Allow row action on view
             await config.api.rowAction.setViewPermission(
               tableId,
               viewId,
               rowAction.id
             )
+
+            // Delete explicit view permissions so they inherit table permissions
+            await config.api.permission.revoke({
+              roleId: roles.BUILTIN_ROLE_IDS.PUBLIC, // Don't think this matters since we are revoking the permission
+              level: PermissionLevel.READ,
+              resourceId: viewId,
+            })
+
             return { permissionResource: tableId, triggerResouce: viewId }
           },
         ],

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

@@ -2460,6 +2460,11 @@ describe.each([
           level: PermissionLevel.READ,
           resourceId: table._id!,
         })
+        await config.api.permission.revoke({
+          roleId: roles.BUILTIN_ROLE_IDS.PUBLIC, // Don't think this matters since we are revoking the permission
+          level: PermissionLevel.READ,
+          resourceId: view.id,
+        })
         await config.publish()
 
         const response = await config.api.viewV2.publicSearch(view.id)

packages/server/src/sdk/app/permissions/index.ts

@@ -1,22 +1,30 @@
-import { db, docIds, roles } from "@budibase/backend-core"
+import { db, roles, context, docIds } from "@budibase/backend-core"
 import {
   PermissionLevel,
   PermissionSource,
   VirtualDocumentType,
+  Role,
+  Database,
 } from "@budibase/types"
-import { extractViewInfoFromID } from "../../../db/utils"
+import { extractViewInfoFromID, getRoleParams } from "../../../db/utils"
 import {
   CURRENTLY_SUPPORTED_LEVELS,
   getBasePermissions,
 } from "../../../utilities/security"
 import sdk from "../../../sdk"
 import { isV2 } from "../views"
+import { removeFromArray } from "../../../utilities"
 
 type ResourcePermissions = Record<
   string,
   { role: string; type: PermissionSource }
 >
 
+export const enum PermissionUpdateType {
+  REMOVE = "remove",
+  ADD = "add",
+}
+
 export async function getInheritablePermissions(
   resourceId: string
 ): Promise<ResourcePermissions | undefined> {
@@ -100,3 +108,89 @@ export async function getDependantResources(
 
   return
 }
+
+export async function updatePermissionOnRole(
+  {
+    roleId,
+    resourceId,
+    level,
+  }: { roleId: string; resourceId: string; level: PermissionLevel },
+  updateType: PermissionUpdateType
+) {
+  const db = context.getAppDB()
+  const remove = updateType === PermissionUpdateType.REMOVE
+  const isABuiltin = roles.isBuiltin(roleId)
+  const dbRoleId = roles.getDBRoleID(roleId)
+  const dbRoles = await getAllDBRoles(db)
+  const docUpdates: Role[] = []
+
+  // the permission is for a built in, make sure it exists
+  if (isABuiltin && !dbRoles.some(role => role._id === dbRoleId)) {
+    const builtin = roles.getBuiltinRoles()[roleId]
+    builtin._id = roles.getDBRoleID(builtin._id!)
+    dbRoles.push(builtin)
+  }
+
+  // now try to find any roles which need updated, e.g. removing the
+  // resource from another role and then adding to the new role
+  for (let role of dbRoles) {
+    let updated = false
+    const rolePermissions: Record<string, PermissionLevel[]> = role.permissions
+      ? role.permissions
+      : {}
+    // make sure its an array, also handle migrating
+    if (
+      !rolePermissions[resourceId] ||
+      !Array.isArray(rolePermissions[resourceId])
+    ) {
+      rolePermissions[resourceId] =
+        typeof rolePermissions[resourceId] === "string"
+          ? [rolePermissions[resourceId] as unknown as PermissionLevel]
+          : []
+    }
+    // handle the removal/updating the role which has this permission first
+    // the updating (role._id !== dbRoleId) is required because a resource/level can
+    // only be permitted in a single role (this reduces hierarchy confusion and simplifies
+    // the general UI for this, rather than needing to show everywhere it is used)
+    if (
+      (role._id !== dbRoleId || remove) &&
+      rolePermissions[resourceId].indexOf(level) !== -1
+    ) {
+      removeFromArray(rolePermissions[resourceId], level)
+      updated = true
+    }
+    // handle the adding, we're on the correct role, at it to this
+    if (!remove && role._id === dbRoleId) {
+      const set = new Set(rolePermissions[resourceId])
+      rolePermissions[resourceId] = [...set.add(level)]
+      updated = true
+    }
+    // handle the update, add it to bulk docs to perform at end
+    if (updated) {
+      role.permissions = rolePermissions
+      docUpdates.push(role)
+    }
+  }
+
+  const response = await db.bulkDocs(docUpdates)
+  return response.map(resp => {
+    const version = docUpdates.find(role => role._id === resp.id)?.version
+    const _id = roles.getExternalRoleID(resp.id, version)
+    return {
+      _id,
+      rev: resp.rev,
+      error: resp.error,
+      reason: resp.reason,
+    }
+  })
+}
+
+// utility function to stop this repetition - permissions always stored under roles
+export async function getAllDBRoles(db: Database) {
+  const body = await db.allDocs<Role>(
+    getRoleParams(null, {
+      include_docs: true,
+    })
+  )
+  return body.rows.map(row => row.doc!)
+}

packages/server/src/sdk/app/views/index.ts

@@ -1,5 +1,6 @@
 import {
   FieldType,
+  PermissionLevel,
   RelationSchemaField,
   RenameColumn,
   Table,
@@ -9,7 +10,7 @@ import {
   ViewV2ColumnEnriched,
   ViewV2Enriched,
 } from "@budibase/types"
-import { context, docIds, HTTPError } from "@budibase/backend-core"
+import { context, docIds, HTTPError, roles } from "@budibase/backend-core"
 import {
   helpers,
   PROTECTED_EXTERNAL_COLUMNS,
@@ -22,6 +23,7 @@ import { isExternalTableID } from "../../../integrations/utils"
 import * as internal from "./internal"
 import * as external from "./external"
 import sdk from "../../../sdk"
+import { PermissionUpdateType, updatePermissionOnRole } from "../permissions"
 
 function pickApi(tableId: any) {
   if (isExternalTableID(tableId)) {
@@ -191,7 +193,30 @@ export async function create(
 ): Promise<ViewV2> {
   await guardViewSchema(tableId, viewRequest)
 
-  return pickApi(tableId).create(tableId, viewRequest)
+  const view = await pickApi(tableId).create(tableId, viewRequest)
+
+  // Set permissions to be the same as the table
+  const tablePerms = await sdk.permissions.getResourcePerms(tableId)
+  const readRole = tablePerms[PermissionLevel.READ]?.role
+  const writeRole = tablePerms[PermissionLevel.WRITE]?.role
+  await updatePermissionOnRole(
+    {
+      roleId: readRole || roles.BUILTIN_ROLE_IDS.BASIC,
+      resourceId: view.id,
+      level: PermissionLevel.READ,
+    },
+    PermissionUpdateType.ADD
+  )
+  await updatePermissionOnRole(
+    {
+      roleId: writeRole || roles.BUILTIN_ROLE_IDS.BASIC,
+      resourceId: view.id,
+      level: PermissionLevel.WRITE,
+    },
+    PermissionUpdateType.ADD
+  )
+
+  return view
 }
 
 export async function update(tableId: string, view: ViewV2): Promise<ViewV2> {

packages/shared-core/src/utils.ts

@@ -1,3 +1,4 @@
+import { ArrayOperator, BasicOperator, SearchFilters } from "@budibase/types"
 import * as Constants from "./constants"
 
 export function unreachable(
@@ -77,3 +78,29 @@ export function trimOtherProps(object: any, allowedProps: string[]) {
     )
   return result
 }
+
+export function isSupportedUserSearch(query: SearchFilters) {
+  const allowed = [
+    { op: BasicOperator.STRING, key: "email" },
+    { op: BasicOperator.EQUAL, key: "_id" },
+    { op: ArrayOperator.ONE_OF, key: "_id" },
+  ]
+  for (let [key, operation] of Object.entries(query)) {
+    if (typeof operation !== "object") {
+      return false
+    }
+    const fields = Object.keys(operation || {})
+    // this filter doesn't contain options - ignore
+    if (fields.length === 0) {
+      continue
+    }
+    const allowedOperation = allowed.find(
+      allow =>
+        allow.op === key && fields.length === 1 && fields[0] === allow.key
+    )
+    if (!allowedOperation) {
+      return false
+    }
+  }
+  return true
+}

packages/worker/src/api/controllers/global/users.ts

@@ -37,7 +37,7 @@ import {
 } from "@budibase/backend-core"
 import { checkAnyUserExists } from "../../../utilities/users"
 import { isEmailConfigured } from "../../../utilities/email"
-import { BpmStatusKey, BpmStatusValue } from "@budibase/shared-core"
+import { BpmStatusKey, BpmStatusValue, utils } from "@budibase/shared-core"
 
 const MAX_USERS_UPLOAD_LIMIT = 1000
 
@@ -256,7 +256,7 @@ export const search = async (ctx: Ctx<SearchUsersRequest>) => {
       }
     }
     // Validate we aren't trying to search on any illegal fields
-    if (!userSdk.core.isSupportedUserSearch(body.query)) {
+    if (!utils.isSupportedUserSearch(body.query)) {
       ctx.throw(400, "Can only search by string.email, equal._id or oneOf._id")
     }
   }