From b2545a30e1850a981394c627fd6520ef1d05a03a Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 30 Sep 2024 11:51:28 +0100 Subject: [PATCH 01/19] Update account-portal submodule to latest master. --- packages/account-portal | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/account-portal b/packages/account-portal index ef8690c955..3e24f6293f 160000 --- a/packages/account-portal +++ b/packages/account-portal @@ -1 +1 @@ -Subproject commit ef8690c955cb83768f21770ece72d68cccddc8a9 +Subproject commit 3e24f6293ff5ee5f9b42822e001504e3bbf19cc0 From d54377ee524e7238f35d1bf3aa3151291876921d Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 27 Sep 2024 16:48:44 +0200 Subject: [PATCH 02/19] Update tool-versions --- .tool-versions | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.tool-versions b/.tool-versions index 946d5198ce..cf78481d93 100644 --- a/.tool-versions +++ b/.tool-versions @@ -1,3 +1,3 @@ nodejs 20.10.0 python 3.10.0 -yarn 1.22.19 +yarn 1.22.22 From 26638ace0aa3cddec63bbabbdd8eac5863f6a822 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 30 Sep 2024 15:18:27 +0200 Subject: [PATCH 03/19] Add globalId and userId to userContextBindings --- packages/server/src/sdk/users/utils.ts | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/packages/server/src/sdk/users/utils.ts b/packages/server/src/sdk/users/utils.ts index 0194e900fe..74389a1444 100644 --- a/packages/server/src/sdk/users/utils.ts +++ b/packages/server/src/sdk/users/utils.ts @@ -130,6 +130,26 @@ export function getUserContextBindings(user: ContextUser) { return {} } // Current user context for bindable search - const { _id, _rev, firstName, lastName, email, status, roleId } = user - return { _id, _rev, firstName, lastName, email, status, roleId } + const { + _id, + _rev, + firstName, + lastName, + email, + status, + roleId, + globalId, + userId, + } = user + return { + _id, + _rev, + firstName, + lastName, + email, + status, + roleId, + globalId, + userId, + } } From 6e1cd6eb01dba0d28e0da3ca5b209275a885ffe0 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 30 Sep 2024 15:18:42 +0200 Subject: [PATCH 04/19] Move query logic to sdk --- .../server/src/api/controllers/row/views.ts | 72 ++++---------- packages/server/src/sdk/app/rows/search.ts | 95 ++++++++++++++----- 2 files changed, 87 insertions(+), 80 deletions(-) diff --git a/packages/server/src/api/controllers/row/views.ts b/packages/server/src/api/controllers/row/views.ts index 68958da8e7..66c755b881 100644 --- a/packages/server/src/api/controllers/row/views.ts +++ b/packages/server/src/api/controllers/row/views.ts @@ -3,14 +3,9 @@ import { ViewV2, SearchRowResponse, SearchViewRowRequest, - SearchFilterKey, - LogicalOperator, } from "@budibase/types" -import { dataFilters } from "@budibase/shared-core" import sdk from "../../../sdk" -import { db, context, features } from "@budibase/backend-core" -import { enrichSearchContext } from "./utils" -import { isExternalTableID } from "../../../integrations/utils" +import { context } from "@budibase/backend-core" export async function searchView( ctx: UserCtx @@ -27,58 +22,23 @@ export async function searchView( const { body } = ctx.request - // Enrich saved query with ephemeral query params. - // We prevent searching on any fields that are saved as part of the query, as - // that could let users find rows they should not be allowed to access. - let query = dataFilters.buildQuery(view.query || []) - if (body.query) { - // Delete extraneous search params that cannot be overridden - delete body.query.onEmptyFilter - - if ( - !isExternalTableID(view.tableId) && - !(await features.flags.isEnabled("SQS")) - ) { - // Extract existing fields - const existingFields = - view.query - ?.filter(filter => filter.field) - .map(filter => db.removeKeyNumbering(filter.field)) || [] - - // Carry over filters for unused fields - Object.keys(body.query).forEach(key => { - const operator = key as Exclude - Object.keys(body.query[operator] || {}).forEach(field => { - if (!existingFields.includes(db.removeKeyNumbering(field))) { - query[operator]![field] = body.query[operator]![field] - } - }) - }) - } else { - query = { - $and: { - conditions: [query, body.query], - }, - } - } - } - await context.ensureSnippetContext(true) - const enrichedQuery = await enrichSearchContext(query, { - user: sdk.users.getUserContextBindings(ctx.user), - }) - - const result = await sdk.rows.search({ - viewId: view.id, - tableId: view.tableId, - query: enrichedQuery, - ...getSortOptions(body, view), - limit: body.limit, - bookmark: body.bookmark, - paginate: body.paginate, - countRows: body.countRows, - }) + const result = await sdk.rows.search( + { + viewId: view.id, + tableId: view.tableId, + query: body.query, + ...getSortOptions(body, view), + limit: body.limit, + bookmark: body.bookmark, + paginate: body.paginate, + countRows: body.countRows, + }, + { + user: sdk.users.getUserContextBindings(ctx.user), + } + ) result.rows.forEach(r => (r._viewId = view.id)) ctx.body = result diff --git a/packages/server/src/sdk/app/rows/search.ts b/packages/server/src/sdk/app/rows/search.ts index 809bd73d1f..c09c625085 100644 --- a/packages/server/src/sdk/app/rows/search.ts +++ b/packages/server/src/sdk/app/rows/search.ts @@ -1,7 +1,10 @@ import { EmptyFilterOption, + LogicalOperator, Row, RowSearchParams, + SearchFilterKey, + SearchFilters, SearchResponse, SortOrder, Table, @@ -14,9 +17,10 @@ import { ExportRowsParams, ExportRowsResult } from "./search/types" import { dataFilters } from "@budibase/shared-core" import sdk from "../../index" import { searchInputMapping } from "./search/utils" -import { features } from "@budibase/backend-core" +import { db, features } from "@budibase/backend-core" import tracer from "dd-trace" import { getQueryableFields, removeInvalidFilters } from "./queryUtils" +import { enrichSearchContext } from "../../../api/controllers/row/utils" export { isValidFilter } from "../../../integrations/utils" @@ -34,7 +38,8 @@ function pickApi(tableId: any) { } export async function search( - options: RowSearchParams + options: RowSearchParams, + context?: Record ): Promise> { return await tracer.trace("search", async span => { span?.addTags({ @@ -51,6 +56,69 @@ export async function search( countRows: options.countRows, }) + let source: Table | ViewV2 + let table: Table + if (options.viewId) { + source = await sdk.views.get(options.viewId) + table = await sdk.views.getTable(source) + options = searchInputMapping(table, options) + } else if (options.tableId) { + source = await sdk.tables.getTable(options.tableId) + table = source + } else { + throw new Error(`Must supply either a view ID or a table ID`) + } + + if (options.query) { + const visibleFields = ( + options.fields || Object.keys(table.schema) + ).filter(field => table.schema[field].visible !== false) + + const queryableFields = await getQueryableFields(table, visibleFields) + options.query = removeInvalidFilters(options.query, queryableFields) + } + + if (options.viewId) { + const view = await sdk.views.get(options.viewId) + // Enrich saved query with ephemeral query params. + // We prevent searching on any fields that are saved as part of the query, as + // that could let users find rows they should not be allowed to access. + let viewQuery = dataFilters.buildQuery(view.query || []) + + if (!isExternalTable && !(await features.flags.isEnabled("SQS"))) { + // Lucene does not accept conditional filters, so we need to keep the old logic + const query: SearchFilters = {} + + // Extract existing fields + const existingFields = + view.query + ?.filter(filter => filter.field) + .map(filter => db.removeKeyNumbering(filter.field)) || [] + + // Carry over filters for unused fields + Object.keys(options.query).forEach(key => { + const operator = key as Exclude + Object.keys(options.query[operator] || {}).forEach(field => { + if (existingFields.includes(db.removeKeyNumbering(field))) { + query[operator]![field] = viewQuery[operator]![field] + } else { + query[operator]![field] = options.query[operator]![field] + } + }) + }) + } else { + options.query = { + $and: { + conditions: [viewQuery, options.query], + }, + } + } + } + + if (context) { + options.query = await enrichSearchContext(options.query, context) + } + options.query = dataFilters.cleanupQuery(options.query || {}) options.query = dataFilters.fixupFilterArrays(options.query) @@ -72,28 +140,7 @@ export async function search( options.sortOrder = options.sortOrder.toLowerCase() as SortOrder } - let source: Table | ViewV2 - let table: Table - if (options.viewId) { - source = await sdk.views.get(options.viewId) - table = await sdk.views.getTable(source) - options = searchInputMapping(table, options) - } else if (options.tableId) { - source = await sdk.tables.getTable(options.tableId) - table = source - options = searchInputMapping(table, options) - } else { - throw new Error(`Must supply either a view ID or a table ID`) - } - - if (options.query) { - const visibleFields = ( - options.fields || Object.keys(table.schema) - ).filter(field => table.schema[field].visible !== false) - - const queryableFields = await getQueryableFields(table, visibleFields) - options.query = removeInvalidFilters(options.query, queryableFields) - } + options = searchInputMapping(table, options) const isExternalTable = isExternalTableID(table._id!) let result: SearchResponse From 7d8238ec9830b76017239e6bf82d871794a3df98 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 30 Sep 2024 15:40:21 +0200 Subject: [PATCH 05/19] Fix --- packages/server/src/sdk/app/rows/search.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/server/src/sdk/app/rows/search.ts b/packages/server/src/sdk/app/rows/search.ts index c09c625085..f5a4957d54 100644 --- a/packages/server/src/sdk/app/rows/search.ts +++ b/packages/server/src/sdk/app/rows/search.ts @@ -69,6 +69,8 @@ export async function search( throw new Error(`Must supply either a view ID or a table ID`) } + const isExternalTable = isExternalTableID(table._id!) + if (options.query) { const visibleFields = ( options.fields || Object.keys(table.schema) @@ -142,7 +144,6 @@ export async function search( options = searchInputMapping(table, options) - const isExternalTable = isExternalTableID(table._id!) let result: SearchResponse if (isExternalTable) { span?.addTags({ searchType: "external" }) From 53620907bb8a32e53ca2eb54a7becc2b368a4f07 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 30 Sep 2024 15:55:34 +0200 Subject: [PATCH 06/19] Fix lucene views --- packages/server/src/sdk/app/rows/search.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/server/src/sdk/app/rows/search.ts b/packages/server/src/sdk/app/rows/search.ts index f5a4957d54..94ca2182b4 100644 --- a/packages/server/src/sdk/app/rows/search.ts +++ b/packages/server/src/sdk/app/rows/search.ts @@ -89,7 +89,7 @@ export async function search( if (!isExternalTable && !(await features.flags.isEnabled("SQS"))) { // Lucene does not accept conditional filters, so we need to keep the old logic - const query: SearchFilters = {} + const query: SearchFilters = viewQuery // Extract existing fields const existingFields = @@ -101,13 +101,12 @@ export async function search( Object.keys(options.query).forEach(key => { const operator = key as Exclude Object.keys(options.query[operator] || {}).forEach(field => { - if (existingFields.includes(db.removeKeyNumbering(field))) { - query[operator]![field] = viewQuery[operator]![field] - } else { + if (!existingFields.includes(db.removeKeyNumbering(field))) { query[operator]![field] = options.query[operator]![field] } }) }) + options.query = query } else { options.query = { $and: { From be70692cfd842e40e817d64ba294c84640084d03 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 30 Sep 2024 15:57:49 +0200 Subject: [PATCH 07/19] Fix --- packages/server/src/sdk/app/rows/search.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/sdk/app/rows/search.ts b/packages/server/src/sdk/app/rows/search.ts index 94ca2182b4..8a0fa3b2f8 100644 --- a/packages/server/src/sdk/app/rows/search.ts +++ b/packages/server/src/sdk/app/rows/search.ts @@ -98,7 +98,7 @@ export async function search( .map(filter => db.removeKeyNumbering(filter.field)) || [] // Carry over filters for unused fields - Object.keys(options.query).forEach(key => { + Object.keys(options.query || {}).forEach(key => { const operator = key as Exclude Object.keys(options.query[operator] || {}).forEach(field => { if (!existingFields.includes(db.removeKeyNumbering(field))) { From 3ecd86b2afdab6d54fa62801a66ac6132608a05f Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 30 Sep 2024 14:58:27 +0100 Subject: [PATCH 08/19] Fixing an issue with user columns getting correct user ID, the logical operators were not being recursed correctly. --- .../src/api/routes/tests/search.spec.ts | 15 +++++- .../server/src/sdk/app/rows/search/utils.ts | 49 +++++++++++-------- 2 files changed, 42 insertions(+), 22 deletions(-) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 090514250d..1ec5ca792a 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/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({ diff --git a/packages/server/src/sdk/app/rows/search/utils.ts b/packages/server/src/sdk/app/rows/search/utils.ts index 6548f963b8..fc0064b52c 100644 --- a/packages/server/src/sdk/app/rows/search/utils.ts +++ b/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,33 @@ 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 + 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 filters } + options.query = dataFilters.recurseLogicalOperators( + options.query, + checkFilters + ) return options } From dc23977619b8e937bb46ec68d55165e8e9465d68 Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Mon, 30 Sep 2024 15:02:55 +0100 Subject: [PATCH 09/19] fix broken trigger binding --- .../automation/SetupPanel/AutomationBlockSetup.svelte | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/builder/src/components/automation/SetupPanel/AutomationBlockSetup.svelte b/packages/builder/src/components/automation/SetupPanel/AutomationBlockSetup.svelte index aceb980786..66fb8d9cba 100644 --- a/packages/builder/src/components/automation/SetupPanel/AutomationBlockSetup.svelte +++ b/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, From abb3a8fe85b3ed3ecb07696f856a23a263c76fd2 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 30 Sep 2024 16:10:44 +0200 Subject: [PATCH 10/19] Fix --- packages/server/src/sdk/app/rows/search.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/server/src/sdk/app/rows/search.ts b/packages/server/src/sdk/app/rows/search.ts index 8a0fa3b2f8..8de5818805 100644 --- a/packages/server/src/sdk/app/rows/search.ts +++ b/packages/server/src/sdk/app/rows/search.ts @@ -78,6 +78,8 @@ export async function search( const queryableFields = await getQueryableFields(table, visibleFields) options.query = removeInvalidFilters(options.query, queryableFields) + } else { + options.query = {} } if (options.viewId) { @@ -120,7 +122,7 @@ export async function search( options.query = await enrichSearchContext(options.query, context) } - options.query = dataFilters.cleanupQuery(options.query || {}) + options.query = dataFilters.cleanupQuery(options.query) options.query = dataFilters.fixupFilterArrays(options.query) span.addTags({ From c0cc2a9e3dfcbae3ab624f045166871a61bd1daa Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 30 Sep 2024 15:16:24 +0100 Subject: [PATCH 11/19] Move isSupportedUserSearch from backend-core to shared-core. --- packages/backend-core/src/users/users.ts | 29 ------------------- packages/shared-core/src/utils.ts | 27 +++++++++++++++++ .../src/api/controllers/global/users.ts | 4 +-- 3 files changed, 29 insertions(+), 31 deletions(-) diff --git a/packages/backend-core/src/users/users.ts b/packages/backend-core/src/users/users.ts index 0c994d8287..d8546afa8b 100644 --- a/packages/backend-core/src/users/users.ts +++ b/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 diff --git a/packages/shared-core/src/utils.ts b/packages/shared-core/src/utils.ts index b69a059745..81fab659c6 100644 --- a/packages/shared-core/src/utils.ts +++ b/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 +} diff --git a/packages/worker/src/api/controllers/global/users.ts b/packages/worker/src/api/controllers/global/users.ts index 6ce0eef5a0..921e0324d1 100644 --- a/packages/worker/src/api/controllers/global/users.ts +++ b/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) => { } } // 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") } } From 7888c22015b5300bf45f9278fdf915da46a1432c Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 30 Sep 2024 15:17:03 +0100 Subject: [PATCH 12/19] Quick fix. --- packages/server/src/sdk/app/rows/search/utils.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/server/src/sdk/app/rows/search/utils.ts b/packages/server/src/sdk/app/rows/search/utils.ts index fc0064b52c..8d96c2f8f9 100644 --- a/packages/server/src/sdk/app/rows/search/utils.ts +++ b/packages/server/src/sdk/app/rows/search/utils.ts @@ -83,6 +83,7 @@ function userColumnMapping(column: string, filters: SearchFilters) { // 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) { + // 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) { @@ -106,9 +107,8 @@ export function searchInputMapping(table: Table, options: RowSearchParams) { } return filters } - options.query = dataFilters.recurseLogicalOperators( - options.query, - checkFilters + options.query = checkFilters( + dataFilters.recurseLogicalOperators(options.query, checkFilters) ) return options } From f28cb1badbb503e548d4faf871bb789bba9f0d05 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 30 Sep 2024 15:18:15 +0100 Subject: [PATCH 13/19] Another slight change. --- packages/server/src/sdk/app/rows/search/utils.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/server/src/sdk/app/rows/search/utils.ts b/packages/server/src/sdk/app/rows/search/utils.ts index 8d96c2f8f9..1dba420a28 100644 --- a/packages/server/src/sdk/app/rows/search/utils.ts +++ b/packages/server/src/sdk/app/rows/search/utils.ts @@ -105,11 +105,9 @@ export function searchInputMapping(table: Table, options: RowSearchParams) { } } } - return filters + return dataFilters.recurseLogicalOperators(filters, checkFilters) } - options.query = checkFilters( - dataFilters.recurseLogicalOperators(options.query, checkFilters) - ) + options.query = checkFilters(options.query) return options } From 28bb3215115aada6a10c50202eae175916c1343d Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 30 Sep 2024 15:36:49 +0100 Subject: [PATCH 14/19] Set view permissions to explicit roles from the parent table --- .../server/src/api/controllers/permission.ts | 108 ++---------------- .../src/api/routes/tests/permissions.spec.ts | 15 +++ .../src/api/routes/tests/rowAction.spec.ts | 9 ++ .../src/api/routes/tests/viewV2.spec.ts | 5 + .../server/src/sdk/app/permissions/index.ts | 98 +++++++++++++++- packages/server/src/sdk/app/views/index.ts | 29 ++++- 6 files changed, 162 insertions(+), 102 deletions(-) diff --git a/packages/server/src/api/controllers/permission.ts b/packages/server/src/api/controllers/permission.ts index 66a3254348..c7afb6a351 100644 --- a/packages/server/src/api/controllers/permission.ts +++ b/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( - 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 = 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) { 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 ) { const params: RemovePermissionRequest = ctx.params - ctx.body = await updatePermissionOnRole(params, PermissionUpdateType.REMOVE) + ctx.body = await sdk.permissions.updatePermissionOnRole( + params, + PermissionUpdateType.REMOVE + ) } diff --git a/packages/server/src/api/routes/tests/permissions.spec.ts b/packages/server/src/api/routes/tests/permissions.spec.ts index 0f059998ae..b148d6fde1 100644 --- a/packages/server/src/api/routes/tests/permissions.spec.ts +++ b/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() diff --git a/packages/server/src/api/routes/tests/rowAction.spec.ts b/packages/server/src/api/routes/tests/rowAction.spec.ts index ef7d2afbba..4fe248984a 100644 --- a/packages/server/src/api/routes/tests/rowAction.spec.ts +++ b/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 } }, ], diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 3712efa5a4..aab846e704 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/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) diff --git a/packages/server/src/sdk/app/permissions/index.ts b/packages/server/src/sdk/app/permissions/index.ts index d5e4aefe3a..2c3c0af95b 100644 --- a/packages/server/src/sdk/app/permissions/index.ts +++ b/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 { @@ -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 = 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( + getRoleParams(null, { + include_docs: true, + }) + ) + return body.rows.map(row => row.doc!) +} diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index cd0fdf077e..24e4da3172 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/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 { 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 { From 01415fb06673e3834d464d35226820ea22789352 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 30 Sep 2024 15:43:58 +0100 Subject: [PATCH 15/19] Quick fix. --- packages/shared-core/src/filters.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index 45e9a7c6d0..ef0500b01a 100644 --- a/packages/shared-core/src/filters.ts +++ b/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) ) From f6649b294b1b62e54009a280d1ccaa0bdcb6b6ce Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 30 Sep 2024 15:54:01 +0100 Subject: [PATCH 16/19] Remove an implicit any from removeInvalidFilters. --- packages/server/src/sdk/app/rows/queryUtils.ts | 13 +++++++++---- packages/types/src/sdk/search.ts | 10 ++++------ 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/packages/server/src/sdk/app/rows/queryUtils.ts b/packages/server/src/sdk/app/rows/queryUtils.ts index 7ef776a989..c4f4a1eb2c 100644 --- a/packages/server/src/sdk/app/rows/queryUtils.ts +++ b/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() diff --git a/packages/types/src/sdk/search.ts b/packages/types/src/sdk/search.ts index 1d5b36031c..647a9e7d00 100644 --- a/packages/types/src/sdk/search.ts +++ b/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< From 8462b4e839dde7f138f97a5361ca9eb45510eda1 Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Mon, 30 Sep 2024 16:10:39 +0000 Subject: [PATCH 17/19] Bump version to 2.32.9 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index 10d36c9eaf..5a279b3e44 100644 --- a/lerna.json +++ b/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/*", From f31c7c3487d2b2cd91454f1e89a9be12d2ddae7e Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 1 Oct 2024 10:55:25 +0200 Subject: [PATCH 18/19] Add test --- .../src/api/routes/tests/viewV2.spec.ts | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index aab846e704..09273abdce 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -1738,6 +1738,40 @@ describe.each([ }) }) + it("views filters are respected even if the column is hidden", async () => { + await config.api.row.save(table._id!, { + one: "foo", + two: "bar", + }) + const two = await config.api.row.save(table._id!, { + one: "foo2", + two: "bar2", + }) + + const view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + query: [ + { + operator: BasicOperator.EQUAL, + field: "two", + value: "bar2", + }, + ], + schema: { + id: { visible: true }, + one: { visible: false }, + two: { visible: false }, + }, + }) + + const response = await config.api.viewV2.search(view.id) + expect(response.rows).toHaveLength(1) + expect(response.rows).toEqual([ + expect.objectContaining({ _id: two._id }), + ]) + }) + it("views without data can be returned", async () => { const response = await config.api.viewV2.search(view.id) expect(response.rows).toHaveLength(0) From 4b65ce4f8b51e55c9dc33de08337c8b8e0cbd7dd Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Tue, 1 Oct 2024 09:31:57 +0000 Subject: [PATCH 19/19] Bump version to 2.32.10 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index 5a279b3e44..092e9a133e 100644 --- a/lerna.json +++ b/lerna.json @@ -1,6 +1,6 @@ { "$schema": "node_modules/lerna/schemas/lerna-schema.json", - "version": "2.32.9", + "version": "2.32.10", "npmClient": "yarn", "packages": [ "packages/*",