From 6617243ce529f09fb8dcceec38012375922e6cd9 Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Mon, 11 Dec 2023 17:23:02 +0000 Subject: [PATCH 1/4] Update global users search to account for numeric prefixing --- .../src/api/controllers/global/users.ts | 22 ++++++++++++++++--- .../src/api/routes/global/tests/users.spec.ts | 18 +++++++++++++++ 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/packages/worker/src/api/controllers/global/users.ts b/packages/worker/src/api/controllers/global/users.ts index 58979ec799..1c470bdba9 100644 --- a/packages/worker/src/api/controllers/global/users.ts +++ b/packages/worker/src/api/controllers/global/users.ts @@ -14,6 +14,7 @@ import { InviteUsersResponse, MigrationType, SaveUserResponse, + SearchQueryOperators, SearchUsersRequest, User, UserCtx, @@ -29,6 +30,7 @@ import { } from "@budibase/backend-core" import { checkAnyUserExists } from "../../../utilities/users" import { isEmailConfigured } from "../../../utilities/email" +import { removeKeyNumbering } from "@budibase/backend-core/src/db" const MAX_USERS_UPLOAD_LIMIT = 1000 @@ -185,9 +187,23 @@ export const getAppUsers = async (ctx: Ctx) => { export const search = async (ctx: Ctx) => { const body = ctx.request.body - // TODO: for now only one supported search key, string.email - if (body?.query && !userSdk.core.isSupportedUserSearch(body.query)) { - ctx.throw(501, "Can only search by string.email or equal._id") + // TODO: for now only two supported search keys; string.email and equal._id + if (body?.query) { + // Clean numeric prefixing. This will overwrite duplicate search fields, + // but this is fine because we only support a single custom search on + // email and id + for (let filters of Object.values(body.query)) { + if (filters && typeof filters === "object") { + for (let [field, value] of Object.entries(filters)) { + delete filters[field] + filters[removeKeyNumbering(field)] = value + } + } + } + // Validate we aren't trying to search on any illegal fields + if (!userSdk.core.isSupportedUserSearch(body.query)) { + ctx.throw(501, "Can only search by string.email or equal._id") + } } if (body.paginate === false) { diff --git a/packages/worker/src/api/routes/global/tests/users.spec.ts b/packages/worker/src/api/routes/global/tests/users.spec.ts index a85933255a..cb534a770a 100644 --- a/packages/worker/src/api/routes/global/tests/users.spec.ts +++ b/packages/worker/src/api/routes/global/tests/users.spec.ts @@ -590,6 +590,15 @@ describe("/api/global/users", () => { expect(response.body.data[0].email).toBe(user.email) }) + it("should be able to search by email with numeric prefixing", async () => { + const user = await config.createUser() + const response = await config.api.users.searchUsers({ + query: { string: { ["999:email"]: user.email } }, + }) + expect(response.body.data.length).toBe(1) + expect(response.body.data[0].email).toBe(user.email) + }) + it("should be able to search by _id", async () => { const user = await config.createUser() const response = await config.api.users.searchUsers({ @@ -599,6 +608,15 @@ describe("/api/global/users", () => { expect(response.body.data[0]._id).toBe(user._id) }) + it("should be able to search by _id with numeric prefixing", async () => { + const user = await config.createUser() + const response = await config.api.users.searchUsers({ + query: { equal: { ["1:_id"]: user._id } }, + }) + expect(response.body.data.length).toBe(1) + expect(response.body.data[0]._id).toBe(user._id) + }) + it("should throw an error when unimplemented options used", async () => { const user = await config.createUser() await config.api.users.searchUsers( From e16cc267815314c1780e506a706cad6cc8dafab5 Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Mon, 11 Dec 2023 17:25:42 +0000 Subject: [PATCH 2/4] Lint --- packages/worker/src/api/controllers/global/users.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/worker/src/api/controllers/global/users.ts b/packages/worker/src/api/controllers/global/users.ts index 1c470bdba9..2f9c78bf93 100644 --- a/packages/worker/src/api/controllers/global/users.ts +++ b/packages/worker/src/api/controllers/global/users.ts @@ -14,7 +14,6 @@ import { InviteUsersResponse, MigrationType, SaveUserResponse, - SearchQueryOperators, SearchUsersRequest, User, UserCtx, From 66f219d7787118cfeb17ccc730a3100606c91e79 Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Tue, 12 Dec 2023 10:26:48 +0000 Subject: [PATCH 3/4] Lint and change status code to 400 when searching on invalid fields --- packages/worker/src/api/controllers/global/users.ts | 6 +++--- packages/worker/src/api/routes/global/tests/users.spec.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/worker/src/api/controllers/global/users.ts b/packages/worker/src/api/controllers/global/users.ts index 2f9c78bf93..0520990f2f 100644 --- a/packages/worker/src/api/controllers/global/users.ts +++ b/packages/worker/src/api/controllers/global/users.ts @@ -26,10 +26,10 @@ import { migrations, platform, tenancy, + db, } from "@budibase/backend-core" import { checkAnyUserExists } from "../../../utilities/users" import { isEmailConfigured } from "../../../utilities/email" -import { removeKeyNumbering } from "@budibase/backend-core/src/db" const MAX_USERS_UPLOAD_LIMIT = 1000 @@ -195,13 +195,13 @@ export const search = async (ctx: Ctx) => { if (filters && typeof filters === "object") { for (let [field, value] of Object.entries(filters)) { delete filters[field] - filters[removeKeyNumbering(field)] = value + filters[db.removeKeyNumbering(field)] = value } } } // Validate we aren't trying to search on any illegal fields if (!userSdk.core.isSupportedUserSearch(body.query)) { - ctx.throw(501, "Can only search by string.email or equal._id") + ctx.throw(400, "Can only search by string.email or equal._id") } } diff --git a/packages/worker/src/api/routes/global/tests/users.spec.ts b/packages/worker/src/api/routes/global/tests/users.spec.ts index cb534a770a..1365173b21 100644 --- a/packages/worker/src/api/routes/global/tests/users.spec.ts +++ b/packages/worker/src/api/routes/global/tests/users.spec.ts @@ -623,7 +623,7 @@ describe("/api/global/users", () => { { query: { equal: { firstName: user.firstName } }, }, - { status: 501 } + { status: 400 } ) }) From f7b7f3efdee911b7b162a8a12236c0e7b2288ace Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Tue, 12 Dec 2023 11:15:29 +0000 Subject: [PATCH 4/4] Error when searching global users using more than one filter per field --- .../src/api/controllers/global/users.ts | 6 +++- .../src/api/routes/global/tests/users.spec.ts | 30 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/packages/worker/src/api/controllers/global/users.ts b/packages/worker/src/api/controllers/global/users.ts index 0520990f2f..b0e3219656 100644 --- a/packages/worker/src/api/controllers/global/users.ts +++ b/packages/worker/src/api/controllers/global/users.ts @@ -195,7 +195,11 @@ export const search = async (ctx: Ctx) => { if (filters && typeof filters === "object") { for (let [field, value] of Object.entries(filters)) { delete filters[field] - filters[db.removeKeyNumbering(field)] = value + const cleanedField = db.removeKeyNumbering(field) + if (filters[cleanedField] !== undefined) { + ctx.throw(400, "Only 1 filter per field is supported") + } + filters[cleanedField] = value } } } diff --git a/packages/worker/src/api/routes/global/tests/users.spec.ts b/packages/worker/src/api/routes/global/tests/users.spec.ts index 1365173b21..c792de70a9 100644 --- a/packages/worker/src/api/routes/global/tests/users.spec.ts +++ b/packages/worker/src/api/routes/global/tests/users.spec.ts @@ -617,6 +617,36 @@ describe("/api/global/users", () => { expect(response.body.data[0]._id).toBe(user._id) }) + it("should throw an error when using multiple filters on the same field", async () => { + const user = await config.createUser() + await config.api.users.searchUsers( + { + query: { + string: { + ["1:email"]: user.email, + ["2:email"]: "something else", + }, + }, + }, + { status: 400 } + ) + }) + + it("should throw an error when using multiple filters on the same field without prefixes", async () => { + const user = await config.createUser() + await config.api.users.searchUsers( + { + query: { + string: { + ["_id"]: user.email, + ["999:_id"]: "something else", + }, + }, + }, + { status: 400 } + ) + }) + it("should throw an error when unimplemented options used", async () => { const user = await config.createUser() await config.api.users.searchUsers(