diff --git a/packages/account-portal b/packages/account-portal index c1a53bb2f4..aca3c9b6b5 160000 --- a/packages/account-portal +++ b/packages/account-portal @@ -1 +1 @@ -Subproject commit c1a53bb2f4cafcb4c55ad7181146617b449907f2 +Subproject commit aca3c9b6b5170d35a255ceb89e57a21719f5ed29 diff --git a/packages/backend-core/src/index.ts b/packages/backend-core/src/index.ts index d04f48e5fc..7bf26f3688 100644 --- a/packages/backend-core/src/index.ts +++ b/packages/backend-core/src/index.ts @@ -33,6 +33,7 @@ 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 diff --git a/packages/backend-core/src/security/auth.ts b/packages/backend-core/src/security/auth.ts new file mode 100644 index 0000000000..c90d9df09b --- /dev/null +++ b/packages/backend-core/src/security/auth.ts @@ -0,0 +1,24 @@ +import { env } from ".." + +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 (!password || password.length < PASSWORD_MIN_LENGTH) { + return { + valid: false, + error: `Password invalid. Minimum ${PASSWORD_MIN_LENGTH} characters.`, + } + } + + if (password.length > PASSWORD_MAX_LENGTH) { + return { + valid: false, + error: `Password invalid. Maximum ${PASSWORD_MAX_LENGTH} 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" 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..b1835fdfb3 --- /dev/null +++ b/packages/backend-core/src/security/tests/auth.spec.ts @@ -0,0 +1,45 @@ +import { generator } from "../../../tests" +import { PASSWORD_MAX_LENGTH, 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 8 characters.", + }) + }) + + it.each([ + generator.word({ length: PASSWORD_MAX_LENGTH }), + generator.paragraph().substring(0, PASSWORD_MAX_LENGTH), + ])(`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 }), + generator + .paragraph({ sentences: 50 }) + .substring(0, PASSWORD_MAX_LENGTH + 1), + ])( + `passwords cannot have more than ${PASSWORD_MAX_LENGTH} characters`, + password => { + expect(validatePassword(password)).toEqual({ + valid: false, + error: "Password invalid. Maximum 512 characters.", + }) + } + ) + }) +}) diff --git a/packages/backend-core/src/users/db.ts b/packages/backend-core/src/users/db.ts index 01fa4899d1..3214b3ab63 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 { validatePassword } from "../security" 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 = 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/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/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/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") } } 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 } 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.") } } 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..d280e3b5f1 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,12 @@ 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: "budibaseUserId", + email: user.email, + } + mocks.accounts.getAccountByTenantId.mockReturnValueOnce( Promise.resolve(account) ) 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 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(), } }