Merge branch 'master' into BUDI-8630/view-security-allow-filters-specified-on-view-to-use-any

lerna.json

@@ -1,6 +1,6 @@
 {
   "$schema": "node_modules/lerna/schemas/lerna-schema.json",
-  "version": "2.32.8",
+  "version": "2.32.9",
   "npmClient": "yarn",
   "packages": [
     "packages/*",

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/builder/src/components/automation/SetupPanel/AutomationBlockSetup.svelte

@@ -641,6 +641,8 @@
       let hasUserDefinedName = automation.stepNames?.[allSteps[idx]?.id]
       if (isLoopBlock) {
         runtimeName = `loop.${name}`
+      } else if (idx === 0) {
+        runtimeName = `trigger.${name}`
       } else if (block.name.startsWith("JS")) {
         runtimeName = hasUserDefinedName
           ? `stepsByName["${bindingName}"].${name}`
@@ -650,7 +652,7 @@
           ? `stepsByName.${bindingName}.${name}`
           : `steps.${idx - loopBlockCount}.${name}`
       }
-      return idx === 0 ? `trigger.${name}` : runtimeName
+      return runtimeName
     }
 
     const determineCategoryName = (idx, isLoopBlock, bindingName) => {
@@ -677,7 +679,7 @@
       )
       return {
         readableBinding:
-          bindingName && !isLoopBlock
+          bindingName && !isLoopBlock && idx !== 0
             ? `steps.${bindingName}.${name}`
             : runtimeBinding,
         runtimeBinding,

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"
-
-const enum PermissionUpdateType {
-  REMOVE = "remove",
-  ADD = "add",
-}
+import { PermissionUpdateType } from "../../sdk/app/permissions"
 
 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/search.spec.ts

@@ -408,7 +408,6 @@ describe.each([
     })
   })
 
-  // We've decided not to try and support binding for in-memory search just now.
   !isInMemory &&
     describe("bindings", () => {
       let globalUsers: any = []
@@ -528,6 +527,20 @@ describe.each([
         ])
       })
 
+      !isLucene &&
+        it("should return all rows matching the session user firstname when logical operator used", async () => {
+          await expectQuery({
+            $and: {
+              conditions: [{ equal: { name: "{{ [user].firstName }}" } }],
+            },
+          }).toContainExactly([
+            {
+              name: config.getUser().firstName,
+              appointment: future.toISOString(),
+            },
+          ])
+        })
+
       it("should parse the date binding and return all rows after the resolved value", async () => {
         await tk.withFreeze(serverTime, async () => {
           await expectQuery({

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/rows/queryUtils.ts

@@ -16,11 +16,11 @@ export const removeInvalidFilters = (
 
   validFields = validFields.map(f => f.toLowerCase())
   for (const filterKey of Object.keys(result) as (keyof SearchFilters)[]) {
-    const filter = result[filterKey]
-    if (!filter || typeof filter !== "object") {
-      continue
-    }
     if (isLogicalSearchOperator(filterKey)) {
+      const filter = result[filterKey]
+      if (!filter || typeof filter !== "object") {
+        continue
+      }
       const resultingConditions: SearchFilters[] = []
       for (const condition of filter.conditions) {
         const resultingCondition = removeInvalidFilters(condition, validFields)
@@ -36,6 +36,11 @@ export const removeInvalidFilters = (
       continue
     }
 
+    const filter = result[filterKey]
+    if (!filter || typeof filter !== "object") {
+      continue
+    }
+
     for (const columnKey of Object.keys(filter)) {
       const possibleKeys = [columnKey, db.removeKeyNumbering(columnKey)].map(
         c => c.toLowerCase()

packages/server/src/sdk/app/rows/search/utils.ts

@@ -11,7 +11,7 @@ import {
   RowSearchParams,
 } from "@budibase/types"
 import { db as dbCore, context } from "@budibase/backend-core"
-import { utils } from "@budibase/shared-core"
+import { utils, dataFilters } from "@budibase/shared-core"
 
 export async function paginatedSearch(
   query: SearchFilters,
@@ -31,13 +31,13 @@ export async function fullSearch(
 
 function findColumnInQueries(
   column: string,
-  options: RowSearchParams,
+  filters: SearchFilters,
   callback: (filter: any) => any
 ) {
-  if (!options.query) {
+  if (!filters) {
     return
   }
-  for (let filterBlock of Object.values(options.query)) {
+  for (let filterBlock of Object.values(filters)) {
     if (typeof filterBlock !== "object") {
       continue
     }
@@ -49,8 +49,8 @@ function findColumnInQueries(
   }
 }
 
-function userColumnMapping(column: string, options: RowSearchParams) {
-  findColumnInQueries(column, options, (filterValue: any): any => {
+function userColumnMapping(column: string, filters: SearchFilters) {
+  findColumnInQueries(column, filters, (filterValue: any): any => {
     const isArray = Array.isArray(filterValue),
       isString = typeof filterValue === "string"
     if (!isString && !isArray) {
@@ -83,26 +83,31 @@ function userColumnMapping(column: string, options: RowSearchParams) {
 // maps through the search parameters to check if any of the inputs are invalid
 // based on the table schema, converts them to something that is valid.
 export function searchInputMapping(table: Table, options: RowSearchParams) {
-  for (let [key, column] of Object.entries(table.schema || {})) {
-    switch (column.type) {
-      case FieldType.BB_REFERENCE_SINGLE: {
-        const subtype = column.subtype
-        switch (subtype) {
-          case BBReferenceFieldSubType.USER:
-            userColumnMapping(key, options)
-            break
+  // need an internal function to loop over filters, because this takes the full options
+  function checkFilters(filters: SearchFilters) {
+    for (let [key, column] of Object.entries(table.schema || {})) {
+      switch (column.type) {
+        case FieldType.BB_REFERENCE_SINGLE: {
+          const subtype = column.subtype
+          switch (subtype) {
+            case BBReferenceFieldSubType.USER:
+              userColumnMapping(key, filters)
+              break
 
-          default:
-            utils.unreachable(subtype)
+            default:
+              utils.unreachable(subtype)
+          }
+          break
+        }
+        case FieldType.BB_REFERENCE: {
+          userColumnMapping(key, filters)
+          break
         }
-        break
-      }
-      case FieldType.BB_REFERENCE: {
-        userColumnMapping(key, options)
-        break
       }
     }
+    return dataFilters.recurseLogicalOperators(filters, checkFilters)
   }
+  options.query = checkFilters(options.query)
   return options
 }
 

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/filters.ts

@@ -124,7 +124,7 @@ export function recurseLogicalOperators(
   fn: (f: SearchFilters) => SearchFilters
 ) {
   for (const logical of LOGICAL_OPERATORS) {
-    if (filters[logical]) {
+    if (filters?.[logical]) {
       filters[logical]!.conditions = filters[logical]!.conditions.map(
         condition => fn(condition)
       )

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/types/src/sdk/search.ts

@@ -68,6 +68,8 @@ type RangeFilter = Record<
   [InternalSearchFilterOperator.COMPLEX_ID_OPERATOR]?: never
 }
 
+type LogicalFilter = { conditions: SearchFilters[] }
+
 export type AnySearchFilter = BasicFilter | ArrayFilter | RangeFilter
 
 export interface SearchFilters {
@@ -92,12 +94,8 @@ export interface SearchFilters {
   // specific document type (such as just rows)
   documentType?: DocumentType
 
-  [LogicalOperator.AND]?: {
-    conditions: SearchFilters[]
-  }
-  [LogicalOperator.OR]?: {
-    conditions: SearchFilters[]
-  }
+  [LogicalOperator.AND]?: LogicalFilter
+  [LogicalOperator.OR]?: LogicalFilter
 }
 
 export type SearchFilterKey = keyof Omit<

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")
     }
   }