Merge pull request #12684 from Budibase/vulnerability/budi-7793-ddos

Set password limits

packages/account-portal

@@ -1 +1 @@
-Subproject commit c1a53bb2f4cafcb4c55ad7181146617b449907f2
+Subproject commit aca3c9b6b5170d35a255ceb89e57a21719f5ed29

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

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 }
+}

packages/backend-core/src/security/index.ts

@@ -0,0 +1 @@
+export * from "./auth"

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.",
+        })
+      }
+    )
+  })
+})

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

packages/backend-core/tests/core/utilities/structures/users.ts

@@ -21,7 +21,7 @@ export const user = (userProps?: Partial<Omit<User, "userId">>): User => {
     _id: userId,
     userId,
     email: newEmail(),
-    password: "test",
+    password: "password",
     roles: { app_test: "admin" },
     firstName: generator.first(),
     lastName: generator.last(),

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")
     }
   }
 </script>

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")
     }
   }
 

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 }

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 }

packages/worker/src/api/controllers/global/auth.ts

@@ -122,10 +122,10 @@ export const resetUpdate = async (ctx: Ctx<PasswordResetUpdateRequest>) => {
     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.")
   }
 }
 

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)
             )
 

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

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(),
       }
     }