From d54377ee524e7238f35d1bf3aa3151291876921d Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 27 Sep 2024 16:48:44 +0200 Subject: [PATCH 1/8] 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 2/8] 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 3/8] 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 4/8] 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 5/8] 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 6/8] 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 abb3a8fe85b3ed3ecb07696f856a23a263c76fd2 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 30 Sep 2024 16:10:44 +0200 Subject: [PATCH 7/8] 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 f31c7c3487d2b2cd91454f1e89a9be12d2ddae7e Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 1 Oct 2024 10:55:25 +0200 Subject: [PATCH 8/8] 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)