From c15a917e0049f3e8b858b4cf3644306390c83edd Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 2 Jan 2024 12:29:27 +0100 Subject: [PATCH 01/26] Validate password in backend-core --- packages/account-portal | 2 +- packages/backend-core/src/index.ts | 1 + packages/backend-core/src/security/auth.ts | 11 +++++++++++ packages/backend-core/src/security/index.ts | 1 + 4 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 packages/backend-core/src/security/auth.ts create mode 100644 packages/backend-core/src/security/index.ts diff --git a/packages/account-portal b/packages/account-portal index e46a352a63..c764a88325 160000 --- a/packages/account-portal +++ b/packages/account-portal @@ -1 +1 @@ -Subproject commit e46a352a6326a838faa00f912de069aee95d7300 +Subproject commit c764a88325b7ecd55aeff2140eab08fe9b32dac7 diff --git a/packages/backend-core/src/index.ts b/packages/backend-core/src/index.ts index d04f48e5fc..32213a9310 100644 --- a/packages/backend-core/src/index.ts +++ b/packages/backend-core/src/index.ts @@ -38,6 +38,7 @@ export * as docIds from "./docIds" // circular dependencies import * as context from "./context" import * as _tenancy from "./tenancy" +export * as security from "./security" export const tenancy = { ..._tenancy, diff --git a/packages/backend-core/src/security/auth.ts b/packages/backend-core/src/security/auth.ts new file mode 100644 index 0000000000..60fe72947b --- /dev/null +++ b/packages/backend-core/src/security/auth.ts @@ -0,0 +1,11 @@ +const MIN_LENGTH = 8 + +export function validatePassword( + password: string +): { valid: true } | { valid: false; error: string } { + if (password?.length < MIN_LENGTH) { + return { valid: false, error: "Password invalid. Minimum eight characters" } + } + + return { valid: true } +} diff --git a/packages/backend-core/src/security/index.ts b/packages/backend-core/src/security/index.ts new file mode 100644 index 0000000000..306751af96 --- /dev/null +++ b/packages/backend-core/src/security/index.ts @@ -0,0 +1 @@ +export * from "./auth" From d9c921e101f92cbe90c46f9ff25674225690c0a5 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 2 Jan 2024 12:56:08 +0100 Subject: [PATCH 02/26] Add tests --- packages/backend-core/src/security/auth.ts | 2 +- .../src/security/tests/auth.spec.ts | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 packages/backend-core/src/security/tests/auth.spec.ts diff --git a/packages/backend-core/src/security/auth.ts b/packages/backend-core/src/security/auth.ts index 60fe72947b..218cc87467 100644 --- a/packages/backend-core/src/security/auth.ts +++ b/packages/backend-core/src/security/auth.ts @@ -3,7 +3,7 @@ const MIN_LENGTH = 8 export function validatePassword( password: string ): { valid: true } | { valid: false; error: string } { - if (password?.length < MIN_LENGTH) { + if (!password || password.length < MIN_LENGTH) { return { valid: false, error: "Password invalid. Minimum eight characters" } } diff --git a/packages/backend-core/src/security/tests/auth.spec.ts b/packages/backend-core/src/security/tests/auth.spec.ts new file mode 100644 index 0000000000..f2d3d08579 --- /dev/null +++ b/packages/backend-core/src/security/tests/auth.spec.ts @@ -0,0 +1,20 @@ +import { validatePassword } from "../auth" + +describe("auth", () => { + describe("validatePassword", () => { + it("a valid password returns successful", () => { + expect(validatePassword("password")).toEqual({ valid: true }) + }) + + it.each([ + ["undefined", undefined], + ["null", null], + ["empty", ""], + ])("%s returns unsuccessful", (_, password) => { + expect(validatePassword(password as string)).toEqual({ + valid: false, + error: "Password invalid. Minimum eight characters", + }) + }) + }) +}) From 1633284f9d5812dbbff9e49389e7dfd3c7888c97 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 2 Jan 2024 13:02:24 +0100 Subject: [PATCH 03/26] Bubble up error --- packages/account-portal | 2 +- packages/worker/src/api/controllers/global/auth.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/account-portal b/packages/account-portal index c764a88325..5664817459 160000 --- a/packages/account-portal +++ b/packages/account-portal @@ -1 +1 @@ -Subproject commit c764a88325b7ecd55aeff2140eab08fe9b32dac7 +Subproject commit 56648174593e9531fbc27c4b841f06866f11e759 diff --git a/packages/worker/src/api/controllers/global/auth.ts b/packages/worker/src/api/controllers/global/auth.ts index a94ed082f7..cfee8c00dc 100644 --- a/packages/worker/src/api/controllers/global/auth.ts +++ b/packages/worker/src/api/controllers/global/auth.ts @@ -122,10 +122,10 @@ export const resetUpdate = async (ctx: Ctx) => { ctx.body = { message: "password reset successfully.", } - } catch (err) { + } catch (err: any) { console.warn(err) // hide any details of the error for security - ctx.throw(400, "Cannot reset password.") + ctx.throw(400, err.message || "Cannot reset password.") } } From 7b9fadc3badc6c21e18b4b07644d86e366fa39d4 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 2 Jan 2024 13:05:48 +0100 Subject: [PATCH 04/26] Validate password on reset --- packages/worker/src/sdk/auth/auth.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/worker/src/sdk/auth/auth.ts b/packages/worker/src/sdk/auth/auth.ts index 1f9da8a260..be5de649da 100644 --- a/packages/worker/src/sdk/auth/auth.ts +++ b/packages/worker/src/sdk/auth/auth.ts @@ -7,6 +7,7 @@ import { tenancy, utils as coreUtils, cache, + security, } from "@budibase/backend-core" import { PlatformLogoutOpts, User } from "@budibase/types" import jwt from "jsonwebtoken" @@ -73,6 +74,11 @@ export const reset = async (email: string) => { * Perform the user password update if the provided reset code is valid. */ export const resetUpdate = async (resetCode: string, password: string) => { + const validation = security.validatePassword(password) + if (!validation.valid) { + throw new HTTPError(validation.error, 400) + } + const { userId } = await cache.passwordReset.getCode(resetCode) let user = await userSdk.db.getUser(userId) From 7bf7ca327f8bf16aa161ae636173875121eebc65 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 2 Jan 2024 13:10:47 +0100 Subject: [PATCH 05/26] Bubble up error --- packages/account-portal | 2 +- packages/builder/src/pages/builder/auth/reset.svelte | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/account-portal b/packages/account-portal index 5664817459..7f2c6cd62b 160000 --- a/packages/account-portal +++ b/packages/account-portal @@ -1 +1 @@ -Subproject commit 56648174593e9531fbc27c4b841f06866f11e759 +Subproject commit 7f2c6cd62b09f7f891d54702248532514c42d7d4 diff --git a/packages/builder/src/pages/builder/auth/reset.svelte b/packages/builder/src/pages/builder/auth/reset.svelte index ac7414c5ca..06d5feaa53 100644 --- a/packages/builder/src/pages/builder/auth/reset.svelte +++ b/packages/builder/src/pages/builder/auth/reset.svelte @@ -45,7 +45,7 @@ } } catch (err) { submitted = false - notifications.error("Unable to reset password") + notifications.error(err.message || "Unable to reset password") } } From 3b50d1a9880035ffe07b59441791300063b30dd0 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 2 Jan 2024 13:22:38 +0100 Subject: [PATCH 06/26] Copy change --- packages/backend-core/src/security/auth.ts | 5 ++++- packages/backend-core/src/security/tests/auth.spec.ts | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/backend-core/src/security/auth.ts b/packages/backend-core/src/security/auth.ts index 218cc87467..2488d67654 100644 --- a/packages/backend-core/src/security/auth.ts +++ b/packages/backend-core/src/security/auth.ts @@ -4,7 +4,10 @@ export function validatePassword( password: string ): { valid: true } | { valid: false; error: string } { if (!password || password.length < MIN_LENGTH) { - return { valid: false, error: "Password invalid. Minimum eight characters" } + return { + valid: false, + error: "Password invalid. Minimum eight characters.", + } } return { valid: true } diff --git a/packages/backend-core/src/security/tests/auth.spec.ts b/packages/backend-core/src/security/tests/auth.spec.ts index f2d3d08579..c72310da24 100644 --- a/packages/backend-core/src/security/tests/auth.spec.ts +++ b/packages/backend-core/src/security/tests/auth.spec.ts @@ -13,7 +13,7 @@ describe("auth", () => { ])("%s returns unsuccessful", (_, password) => { expect(validatePassword(password as string)).toEqual({ valid: false, - error: "Password invalid. Minimum eight characters", + error: "Password invalid. Minimum eight characters.", }) }) }) From e50cc35140aff4c1a9b5d6a867a75041d8d5910b Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 2 Jan 2024 13:23:08 +0100 Subject: [PATCH 07/26] Validate password on admin creation --- packages/builder/src/pages/builder/admin/index.svelte | 2 +- packages/worker/src/api/controllers/global/users.ts | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/builder/src/pages/builder/admin/index.svelte b/packages/builder/src/pages/builder/admin/index.svelte index 9723c6b621..a9c9748216 100644 --- a/packages/builder/src/pages/builder/admin/index.svelte +++ b/packages/builder/src/pages/builder/admin/index.svelte @@ -38,7 +38,7 @@ $goto("../portal") } catch (error) { submitted = false - notifications.error("Failed to create admin user") + notifications.error(error.message || "Failed to create admin user") } } diff --git a/packages/worker/src/api/controllers/global/users.ts b/packages/worker/src/api/controllers/global/users.ts index b0e3219656..257d2b9a89 100644 --- a/packages/worker/src/api/controllers/global/users.ts +++ b/packages/worker/src/api/controllers/global/users.ts @@ -27,6 +27,7 @@ import { platform, tenancy, db, + security, } from "@budibase/backend-core" import { checkAnyUserExists } from "../../../utilities/users" import { isEmailConfigured } from "../../../utilities/email" @@ -98,6 +99,11 @@ export const adminUser = async ( ctx.throw(403, "Organisation already exists.") } + const passwordValidation = security.validatePassword(password) + if (!passwordValidation.valid) { + ctx.throw(400, passwordValidation.error) + } + if (env.MULTI_TENANCY) { // store the new tenant record in the platform db await platform.tenants.addTenant(tenantId) From 5609db3545cadaf01299d96b7a95c6b92b939a1f Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 2 Jan 2024 15:16:09 +0100 Subject: [PATCH 08/26] Add max limit --- packages/backend-core/src/security/auth.ts | 8 ++++++++ packages/backend-core/src/security/tests/auth.spec.ts | 11 +++++++++++ 2 files changed, 19 insertions(+) diff --git a/packages/backend-core/src/security/auth.ts b/packages/backend-core/src/security/auth.ts index 2488d67654..3bcecd9e11 100644 --- a/packages/backend-core/src/security/auth.ts +++ b/packages/backend-core/src/security/auth.ts @@ -1,4 +1,5 @@ const MIN_LENGTH = 8 +const MAX_LENGTH = 100 export function validatePassword( password: string @@ -10,5 +11,12 @@ export function validatePassword( } } + if (password.length > MAX_LENGTH) { + return { + valid: false, + error: "Password invalid. Maximum hundred characters.", + } + } + return { valid: true } } diff --git a/packages/backend-core/src/security/tests/auth.spec.ts b/packages/backend-core/src/security/tests/auth.spec.ts index c72310da24..7be049ae1a 100644 --- a/packages/backend-core/src/security/tests/auth.spec.ts +++ b/packages/backend-core/src/security/tests/auth.spec.ts @@ -1,3 +1,4 @@ +import { generator } from "../../../tests" import { validatePassword } from "../auth" describe("auth", () => { @@ -16,5 +17,15 @@ describe("auth", () => { error: "Password invalid. Minimum eight characters.", }) }) + + it.each([ + generator.word({ length: 101 }), + generator.paragraph().substring(0, 101), + ])("limit password length", password => { + expect(validatePassword(password as string)).toEqual({ + valid: false, + error: "Password invalid. Maximum hundred characters.", + }) + }) }) }) From fe83800a1ae9e079d1d68fd406fac67bbef0f273 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 2 Jan 2024 15:18:04 +0100 Subject: [PATCH 09/26] Update account-portal ref --- packages/account-portal | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/account-portal b/packages/account-portal index 7f2c6cd62b..e937137ff9 160000 --- a/packages/account-portal +++ b/packages/account-portal @@ -1 +1 @@ -Subproject commit 7f2c6cd62b09f7f891d54702248532514c42d7d4 +Subproject commit e937137ff91965000ae8e30d3d0c6b957206c17f From 59fba524bcdf242be23994a055135453245ac02f Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 2 Jan 2024 16:33:00 +0100 Subject: [PATCH 10/26] Lint --- packages/backend-core/src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend-core/src/index.ts b/packages/backend-core/src/index.ts index 32213a9310..7bf26f3688 100644 --- a/packages/backend-core/src/index.ts +++ b/packages/backend-core/src/index.ts @@ -33,12 +33,12 @@ export * as docUpdates from "./docUpdates" export * from "./utils/Duration" export { SearchParams } from "./db" export * as docIds from "./docIds" +export * as security from "./security" // Add context to tenancy for backwards compatibility // only do this for external usages to prevent internal // circular dependencies import * as context from "./context" import * as _tenancy from "./tenancy" -export * as security from "./security" export const tenancy = { ..._tenancy, From 780a0ee68795ccfb67604489ba27caa97b6dd908 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 2 Jan 2024 16:38:58 +0100 Subject: [PATCH 11/26] Increase limits --- packages/backend-core/src/security/auth.ts | 10 +++++----- .../src/security/tests/auth.spec.ts | 20 +++++++++++++++---- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/packages/backend-core/src/security/auth.ts b/packages/backend-core/src/security/auth.ts index 3bcecd9e11..c44a17a54f 100644 --- a/packages/backend-core/src/security/auth.ts +++ b/packages/backend-core/src/security/auth.ts @@ -1,20 +1,20 @@ -const MIN_LENGTH = 8 -const MAX_LENGTH = 100 +export const PASSWORD_MIN_LENGTH = 8 +export const PASSWORD_MAX_LENGTH = 512 export function validatePassword( password: string ): { valid: true } | { valid: false; error: string } { - if (!password || password.length < MIN_LENGTH) { + if (!password || password.length < PASSWORD_MIN_LENGTH) { return { valid: false, error: "Password invalid. Minimum eight characters.", } } - if (password.length > MAX_LENGTH) { + if (password.length > PASSWORD_MAX_LENGTH) { return { valid: false, - error: "Password invalid. Maximum hundred characters.", + error: `Password invalid. Maximum ${PASSWORD_MAX_LENGTH} characters.`, } } diff --git a/packages/backend-core/src/security/tests/auth.spec.ts b/packages/backend-core/src/security/tests/auth.spec.ts index 7be049ae1a..46ebfae655 100644 --- a/packages/backend-core/src/security/tests/auth.spec.ts +++ b/packages/backend-core/src/security/tests/auth.spec.ts @@ -1,5 +1,5 @@ import { generator } from "../../../tests" -import { validatePassword } from "../auth" +import { PASSWORD_MAX_LENGTH, validatePassword } from "../auth" describe("auth", () => { describe("validatePassword", () => { @@ -19,12 +19,24 @@ describe("auth", () => { }) it.each([ - generator.word({ length: 101 }), - generator.paragraph().substring(0, 101), + generator.word({ length: PASSWORD_MAX_LENGTH }), + generator.paragraph().substring(0, PASSWORD_MAX_LENGTH), + ])( + `can use passwords up to ${PASSWORD_MAX_LENGTH} characters in length`, + password => { + expect(validatePassword(password as string)).toEqual({ + valid: true, + }) + } + ) + + it.each([ + generator.word({ length: PASSWORD_MAX_LENGTH + 1 }), + generator.paragraph().substring(0, PASSWORD_MAX_LENGTH + 1), ])("limit password length", password => { expect(validatePassword(password as string)).toEqual({ valid: false, - error: "Password invalid. Maximum hundred characters.", + error: "Password invalid. Maximum 512 characters.", }) }) }) From b0a12e034d80dcc88a50c68483671d5e20e951dd Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 2 Jan 2024 16:58:47 +0100 Subject: [PATCH 12/26] Remove limits from password in dev --- packages/backend-core/src/security/auth.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/backend-core/src/security/auth.ts b/packages/backend-core/src/security/auth.ts index c44a17a54f..a5d4138724 100644 --- a/packages/backend-core/src/security/auth.ts +++ b/packages/backend-core/src/security/auth.ts @@ -1,9 +1,16 @@ +import { env } from ".." + export const PASSWORD_MIN_LENGTH = 8 export const PASSWORD_MAX_LENGTH = 512 export function validatePassword( password: string ): { valid: true } | { valid: false; error: string } { + if (env.isDev() && !env.isTest() && password) { + // We accept any password while on development + return { valid: true } + } + if (!password || password.length < PASSWORD_MIN_LENGTH) { return { valid: false, From fe0ac2885160324116e15d1f4cdef060433f3c28 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 2 Jan 2024 17:03:13 +0100 Subject: [PATCH 13/26] Fix test --- .../src/security/tests/auth.spec.ts | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/packages/backend-core/src/security/tests/auth.spec.ts b/packages/backend-core/src/security/tests/auth.spec.ts index 46ebfae655..8a77646885 100644 --- a/packages/backend-core/src/security/tests/auth.spec.ts +++ b/packages/backend-core/src/security/tests/auth.spec.ts @@ -24,7 +24,7 @@ describe("auth", () => { ])( `can use passwords up to ${PASSWORD_MAX_LENGTH} characters in length`, password => { - expect(validatePassword(password as string)).toEqual({ + expect(validatePassword(password)).toEqual({ valid: true, }) } @@ -32,12 +32,18 @@ describe("auth", () => { it.each([ generator.word({ length: PASSWORD_MAX_LENGTH + 1 }), - generator.paragraph().substring(0, PASSWORD_MAX_LENGTH + 1), - ])("limit password length", password => { - expect(validatePassword(password as string)).toEqual({ - valid: false, - error: "Password invalid. Maximum 512 characters.", - }) - }) + generator + .paragraph({ sentences: 50 }) + .substring(0, PASSWORD_MAX_LENGTH + 1), + ])( + `passwords cannot have more than ${PASSWORD_MAX_LENGTH} characters`, + password => { + console.error(password) + expect(validatePassword(password)).toEqual({ + valid: false, + error: "Password invalid. Maximum 512 characters.", + }) + } + ) }) }) From 1de80742609ed1cb1b14324b5ff47f89bb20d80d Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 2 Jan 2024 17:09:15 +0100 Subject: [PATCH 14/26] Update ref --- packages/account-portal | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/account-portal b/packages/account-portal index e937137ff9..e3352972fa 160000 --- a/packages/account-portal +++ b/packages/account-portal @@ -1 +1 @@ -Subproject commit e937137ff91965000ae8e30d3d0c6b957206c17f +Subproject commit e3352972fa69b5e8aee15e846809e0c4950c1c1c From 82abdd5f0971a13acf515c2f572c8d3c6d06c79b Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 2 Jan 2024 17:12:04 +0100 Subject: [PATCH 15/26] Update ref --- packages/account-portal | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/account-portal b/packages/account-portal index e3352972fa..a3dfaaeaef 160000 --- a/packages/account-portal +++ b/packages/account-portal @@ -1 +1 @@ -Subproject commit e3352972fa69b5e8aee15e846809e0c4950c1c1c +Subproject commit a3dfaaeaef622961fb7e87e9e981545fe3f70b98 From ba2b54f842f4cb7f0506a23a50b048f3aa2b0af7 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 3 Jan 2024 10:12:22 +0100 Subject: [PATCH 16/26] Use env variables instead of checking if isdev --- packages/backend-core/src/security/auth.ts | 11 +++-------- .../backend-core/src/security/tests/auth.spec.ts | 15 ++++++--------- packages/server/scripts/dev/manage.js | 1 + packages/worker/scripts/dev/manage.js | 1 + 4 files changed, 11 insertions(+), 17 deletions(-) diff --git a/packages/backend-core/src/security/auth.ts b/packages/backend-core/src/security/auth.ts index a5d4138724..c90d9df09b 100644 --- a/packages/backend-core/src/security/auth.ts +++ b/packages/backend-core/src/security/auth.ts @@ -1,20 +1,15 @@ import { env } from ".." -export const PASSWORD_MIN_LENGTH = 8 -export const PASSWORD_MAX_LENGTH = 512 +export const PASSWORD_MIN_LENGTH = +(process.env.PASSWORD_MIN_LENGTH || 8) +export const PASSWORD_MAX_LENGTH = +(process.env.PASSWORD_MAX_LENGTH || 512) export function validatePassword( password: string ): { valid: true } | { valid: false; error: string } { - if (env.isDev() && !env.isTest() && password) { - // We accept any password while on development - return { valid: true } - } - if (!password || password.length < PASSWORD_MIN_LENGTH) { return { valid: false, - error: "Password invalid. Minimum eight characters.", + error: `Password invalid. Minimum ${PASSWORD_MIN_LENGTH} characters.`, } } diff --git a/packages/backend-core/src/security/tests/auth.spec.ts b/packages/backend-core/src/security/tests/auth.spec.ts index 8a77646885..795268bb66 100644 --- a/packages/backend-core/src/security/tests/auth.spec.ts +++ b/packages/backend-core/src/security/tests/auth.spec.ts @@ -14,21 +14,18 @@ describe("auth", () => { ])("%s returns unsuccessful", (_, password) => { expect(validatePassword(password as string)).toEqual({ valid: false, - error: "Password invalid. Minimum eight characters.", + error: "Password invalid. Minimum 8 characters.", }) }) it.each([ generator.word({ length: PASSWORD_MAX_LENGTH }), generator.paragraph().substring(0, PASSWORD_MAX_LENGTH), - ])( - `can use passwords up to ${PASSWORD_MAX_LENGTH} characters in length`, - password => { - expect(validatePassword(password)).toEqual({ - valid: true, - }) - } - ) + ])(`can use passwords up to 512 characters in length`, password => { + expect(validatePassword(password)).toEqual({ + valid: true, + }) + }) it.each([ generator.word({ length: PASSWORD_MAX_LENGTH + 1 }), diff --git a/packages/server/scripts/dev/manage.js b/packages/server/scripts/dev/manage.js index 6dc0966f78..1c7a70931b 100644 --- a/packages/server/scripts/dev/manage.js +++ b/packages/server/scripts/dev/manage.js @@ -48,6 +48,7 @@ async function init() { HTTP_MIGRATIONS: "0", HTTP_LOGGING: "0", VERSION: "0.0.0+local", + PASSWORD_MIN_LENGTH: "1", } config = { ...config, ...existingConfig } diff --git a/packages/worker/scripts/dev/manage.js b/packages/worker/scripts/dev/manage.js index 1b7c6f0ddd..acab87eb5e 100644 --- a/packages/worker/scripts/dev/manage.js +++ b/packages/worker/scripts/dev/manage.js @@ -30,6 +30,7 @@ async function init() { ENABLE_EMAIL_TEST_MODE: "1", HTTP_LOGGING: "0", VERSION: "0.0.0+local", + PASSWORD_MIN_LENGTH: "1", } config = { ...config, ...existingConfig } From 1a0361e33fbdeba019e34166cd1bc8c609cfc19e Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 3 Jan 2024 10:43:17 +0100 Subject: [PATCH 17/26] Update --- packages/account-portal | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/account-portal b/packages/account-portal index a3dfaaeaef..0b71817304 160000 --- a/packages/account-portal +++ b/packages/account-portal @@ -1 +1 @@ -Subproject commit a3dfaaeaef622961fb7e87e9e981545fe3f70b98 +Subproject commit 0b71817304e5019bcd44f0b336e8822b4a49f2de From 874c3f9dd4ea3dec1a222cc99a8be2ea2d0b0acc Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 3 Jan 2024 11:41:57 +0100 Subject: [PATCH 18/26] Fix tests --- .../worker/src/api/routes/global/tests/auth.spec.ts | 11 ++++++++--- packages/worker/src/sdk/auth/auth.ts | 6 +++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/worker/src/api/routes/global/tests/auth.spec.ts b/packages/worker/src/api/routes/global/tests/auth.spec.ts index 59b6fe5869..e984649507 100644 --- a/packages/worker/src/api/routes/global/tests/auth.spec.ts +++ b/packages/worker/src/api/routes/global/tests/auth.spec.ts @@ -229,7 +229,7 @@ describe("/api/global/auth", () => { ) expect(res.body).toEqual({ - message: "Cannot reset password.", + message: "Password change is disabled for this user", status: 400, }) } @@ -261,8 +261,13 @@ describe("/api/global/auth", () => { ) // convert to account owner now that password has been requested - const account = structures.accounts.ssoAccount() as CloudAccount - mocks.accounts.getAccount.mockReturnValueOnce( + const account: CloudAccount = { + ...structures.accounts.ssoAccount(), + budibaseUserId: user._id!, + email: user.email, + tenantId: config.getTenantId(), + } + mocks.accounts.getAccountByTenantId.mockReturnValueOnce( Promise.resolve(account) ) diff --git a/packages/worker/src/sdk/auth/auth.ts b/packages/worker/src/sdk/auth/auth.ts index e670a7d091..5ddccdd2b1 100644 --- a/packages/worker/src/sdk/auth/auth.ts +++ b/packages/worker/src/sdk/auth/auth.ts @@ -74,14 +74,14 @@ export const reset = async (email: string) => { * Perform the user password update if the provided reset code is valid. */ export const resetUpdate = async (resetCode: string, password: string) => { + const { userId } = await cache.passwordReset.getCode(resetCode) + let user = await userSdk.db.getUser(userId) + const validation = security.validatePassword(password) if (!validation.valid) { throw new HTTPError(validation.error, 400) } - const { userId } = await cache.passwordReset.getCode(resetCode) - - let user = await userSdk.db.getUser(userId) user.password = password user = await userSdk.db.save(user) From 66fd8b936f682531194d46805bb74d94d85ce2a2 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 3 Jan 2024 11:45:25 +0100 Subject: [PATCH 19/26] Clean log --- packages/backend-core/src/security/tests/auth.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/backend-core/src/security/tests/auth.spec.ts b/packages/backend-core/src/security/tests/auth.spec.ts index 795268bb66..b1835fdfb3 100644 --- a/packages/backend-core/src/security/tests/auth.spec.ts +++ b/packages/backend-core/src/security/tests/auth.spec.ts @@ -35,7 +35,6 @@ describe("auth", () => { ])( `passwords cannot have more than ${PASSWORD_MAX_LENGTH} characters`, password => { - console.error(password) expect(validatePassword(password)).toEqual({ valid: false, error: "Password invalid. Maximum 512 characters.", From b45717a1e10f72d3e8f64201b31b9f259c086b87 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 3 Jan 2024 12:00:25 +0100 Subject: [PATCH 20/26] Move password checks to db --- packages/backend-core/src/users/db.ts | 7 +++++++ packages/worker/src/api/controllers/global/users.ts | 6 ------ packages/worker/src/sdk/auth/auth.ts | 6 ------ 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/packages/backend-core/src/users/db.ts b/packages/backend-core/src/users/db.ts index 01fa4899d1..6cc5d2de5a 100644 --- a/packages/backend-core/src/users/db.ts +++ b/packages/backend-core/src/users/db.ts @@ -27,6 +27,7 @@ import { } from "./utils" import { searchExistingEmails } from "./lookup" import { hash } from "../utils" +import { security } from ".." type QuotaUpdateFn = ( change: number, @@ -110,6 +111,12 @@ export class UserDB { if (await UserDB.isPreventPasswordActions(user, account)) { throw new HTTPError("Password change is disabled for this user", 400) } + + const passwordValidation = security.validatePassword(password) + if (!passwordValidation.valid) { + throw new HTTPError(passwordValidation.error, 400) + } + hashedPassword = opts.hashPassword ? await hash(password) : password } else if (dbUser) { hashedPassword = dbUser.password diff --git a/packages/worker/src/api/controllers/global/users.ts b/packages/worker/src/api/controllers/global/users.ts index 257d2b9a89..b0e3219656 100644 --- a/packages/worker/src/api/controllers/global/users.ts +++ b/packages/worker/src/api/controllers/global/users.ts @@ -27,7 +27,6 @@ import { platform, tenancy, db, - security, } from "@budibase/backend-core" import { checkAnyUserExists } from "../../../utilities/users" import { isEmailConfigured } from "../../../utilities/email" @@ -99,11 +98,6 @@ export const adminUser = async ( ctx.throw(403, "Organisation already exists.") } - const passwordValidation = security.validatePassword(password) - if (!passwordValidation.valid) { - ctx.throw(400, passwordValidation.error) - } - if (env.MULTI_TENANCY) { // store the new tenant record in the platform db await platform.tenants.addTenant(tenantId) diff --git a/packages/worker/src/sdk/auth/auth.ts b/packages/worker/src/sdk/auth/auth.ts index 5ddccdd2b1..f8796f0bed 100644 --- a/packages/worker/src/sdk/auth/auth.ts +++ b/packages/worker/src/sdk/auth/auth.ts @@ -7,7 +7,6 @@ import { tenancy, utils as coreUtils, cache, - security, } from "@budibase/backend-core" import { PlatformLogoutOpts, User } from "@budibase/types" import jwt from "jsonwebtoken" @@ -77,11 +76,6 @@ export const resetUpdate = async (resetCode: string, password: string) => { const { userId } = await cache.passwordReset.getCode(resetCode) let user = await userSdk.db.getUser(userId) - const validation = security.validatePassword(password) - if (!validation.valid) { - throw new HTTPError(validation.error, 400) - } - user.password = password user = await userSdk.db.save(user) From b09f941027e47e152f992194b06b82e89497f37a Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 3 Jan 2024 12:06:23 +0100 Subject: [PATCH 21/26] Fix tests --- packages/backend-core/tests/core/utilities/structures/users.ts | 2 +- packages/worker/src/tests/TestConfiguration.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/backend-core/tests/core/utilities/structures/users.ts b/packages/backend-core/tests/core/utilities/structures/users.ts index 68ee29686c..8f4096d401 100644 --- a/packages/backend-core/tests/core/utilities/structures/users.ts +++ b/packages/backend-core/tests/core/utilities/structures/users.ts @@ -21,7 +21,7 @@ export const user = (userProps?: Partial>): User => { _id: userId, userId, email: newEmail(), - password: "test", + password: "password", roles: { app_test: "admin" }, firstName: generator.first(), lastName: generator.last(), diff --git a/packages/worker/src/tests/TestConfiguration.ts b/packages/worker/src/tests/TestConfiguration.ts index 8e163f0373..df6726eed1 100644 --- a/packages/worker/src/tests/TestConfiguration.ts +++ b/packages/worker/src/tests/TestConfiguration.ts @@ -45,7 +45,7 @@ class TestConfiguration { tenantId: string user?: User apiKey?: string - userPassword = "test" + userPassword = "password" constructor(opts: { openServer: boolean } = { openServer: true }) { // default to cloud hosting From 4db5d9dab239650c773e06081a285691c23c7bfa Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 3 Jan 2024 12:08:18 +0100 Subject: [PATCH 22/26] Lint --- packages/worker/src/sdk/auth/auth.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/worker/src/sdk/auth/auth.ts b/packages/worker/src/sdk/auth/auth.ts index f8796f0bed..3f24de440a 100644 --- a/packages/worker/src/sdk/auth/auth.ts +++ b/packages/worker/src/sdk/auth/auth.ts @@ -74,8 +74,8 @@ export const reset = async (email: string) => { */ export const resetUpdate = async (resetCode: string, password: string) => { const { userId } = await cache.passwordReset.getCode(resetCode) - let user = await userSdk.db.getUser(userId) + let user = await userSdk.db.getUser(userId) user.password = password user = await userSdk.db.save(user) From 8b866a53c8c39885ef1a419f6de6f3f7416020ab Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 3 Jan 2024 12:11:25 +0100 Subject: [PATCH 23/26] Clean --- packages/worker/src/api/routes/global/tests/auth.spec.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/worker/src/api/routes/global/tests/auth.spec.ts b/packages/worker/src/api/routes/global/tests/auth.spec.ts index e984649507..d280e3b5f1 100644 --- a/packages/worker/src/api/routes/global/tests/auth.spec.ts +++ b/packages/worker/src/api/routes/global/tests/auth.spec.ts @@ -263,9 +263,8 @@ describe("/api/global/auth", () => { // convert to account owner now that password has been requested const account: CloudAccount = { ...structures.accounts.ssoAccount(), - budibaseUserId: user._id!, + budibaseUserId: "budibaseUserId", email: user.email, - tenantId: config.getTenantId(), } mocks.accounts.getAccountByTenantId.mockReturnValueOnce( Promise.resolve(account) From 699555c04b62675071bb789a4d728d9451d5a1af Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 3 Jan 2024 12:35:52 +0100 Subject: [PATCH 24/26] Fix hosting --- packages/backend-core/src/users/db.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/backend-core/src/users/db.ts b/packages/backend-core/src/users/db.ts index 6cc5d2de5a..3214b3ab63 100644 --- a/packages/backend-core/src/users/db.ts +++ b/packages/backend-core/src/users/db.ts @@ -27,7 +27,7 @@ import { } from "./utils" import { searchExistingEmails } from "./lookup" import { hash } from "../utils" -import { security } from ".." +import { validatePassword } from "../security" type QuotaUpdateFn = ( change: number, @@ -112,7 +112,7 @@ export class UserDB { throw new HTTPError("Password change is disabled for this user", 400) } - const passwordValidation = security.validatePassword(password) + const passwordValidation = validatePassword(password) if (!passwordValidation.valid) { throw new HTTPError(passwordValidation.error, 400) } From 3ce3f6b5acdcd33b21b3a7ef10b236c4d5936b41 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 3 Jan 2024 12:42:52 +0100 Subject: [PATCH 25/26] Fix flakiness test --- packages/worker/src/tests/api/users.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/worker/src/tests/api/users.ts b/packages/worker/src/tests/api/users.ts index 5ecd1447ca..45105c99da 100644 --- a/packages/worker/src/tests/api/users.ts +++ b/packages/worker/src/tests/api/users.ts @@ -101,7 +101,7 @@ export class UserAPI extends TestAPI { if (!request) { request = { email: structures.email(), - password: generator.string(), + password: generator.string({ length: 8 }), tenantId: structures.tenant.id(), } } From f8d03a918d9ad81d13d4d6c126d0ad5dcf49479b Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 3 Jan 2024 12:44:53 +0100 Subject: [PATCH 26/26] Update ref --- packages/account-portal | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/account-portal b/packages/account-portal index 0b71817304..aca3c9b6b5 160000 --- a/packages/account-portal +++ b/packages/account-portal @@ -1 +1 @@ -Subproject commit 0b71817304e5019bcd44f0b336e8822b4a49f2de +Subproject commit aca3c9b6b5170d35a255ceb89e57a21719f5ed29