diff --git a/.env.example b/.env.example index a2819beb2a4f..aa0e30df677e 100644 --- a/.env.example +++ b/.env.example @@ -209,6 +209,12 @@ PASSWORD_RESET_DISABLED=1 # Development-only helper: log the password reset link to the server console instead of sending reset emails. # DEBUG_SHOW_RESET_LINK=1 +# Disable the Have-I-Been-Pwned breach check performed when a password is set (signup / reset). +# The check is an outbound HTTPS call to api.pwnedpasswords.com; if it can't be reached it fails +# open (password allowed, warning logged). Set this to 1 on air-gapped / closed-network +# deployments to skip the outbound call entirely. +# PASSWORD_HIBP_CHECK_DISABLED=1 + # Email login. Disable the ability for users to login with email. # EMAIL_AUTH_DISABLED=1 diff --git a/apps/web/i18n.lock b/apps/web/i18n.lock index 5c3598dee853..81732b47edd4 100644 --- a/apps/web/i18n.lock +++ b/apps/web/i18n.lock @@ -83,6 +83,7 @@ checksums: auth/oauth/scopes/surveys_read: 8280264457fe7c3b759c82422052aed2 auth/oauth/scopes/surveys_write: fb128cebe62dfd5685d086d706c432c2 auth/oauth/unknown_client: 561bc5afdb90606fbc1ed97f20698ead + auth/password_compromised: d4ab0f66aaed039f956c2515328ed1c9 auth/saml_connection_error: 03c69c534e7eaafcb2c22b7daf9f3efc auth/signup/captcha_failed: 4e1ed87800585b8c1da1514fa86ab943 auth/signup/company_email_required: 997bf11a31512cedf4ab8acb7b4f56c3 diff --git a/apps/web/integration/setup.ts b/apps/web/integration/setup.ts index c5350467844b..a673dfe2c531 100644 --- a/apps/web/integration/setup.ts +++ b/apps/web/integration/setup.ts @@ -19,6 +19,10 @@ process.env.REDIS_URL = process.env.TEST_REDIS_URL ?? "redis://localhost:6379/15 process.env.BETTER_AUTH_SECRET ??= "integration-test-better-auth-secret-0123456789abcdef"; // Disable rate limiting in the harness — counters would accumulate in the shared Redis db across tests. process.env.RATE_LIMITING_DISABLED ??= "1"; +// Disable the HIBP breach check by default — otherwise every signup/reset in the suite would make a +// live call to api.pwnedpasswords.com (ENG-1587). The dedicated better-auth-hibp.integration.test.ts +// re-enables it for itself (with betterFetch mocked). +process.env.PASSWORD_HIBP_CHECK_DISABLED ??= "1"; // server-only is a Next.js build guard; no-op it under vitest. vi.mock("server-only", () => ({})); diff --git a/apps/web/lib/constants.ts b/apps/web/lib/constants.ts index 49f9b153f46d..ef722aef092d 100644 --- a/apps/web/lib/constants.ts +++ b/apps/web/lib/constants.ts @@ -167,6 +167,11 @@ export const REDIS_URL = env.REDIS_URL; export const RATE_LIMITING_DISABLED = env.RATE_LIMITING_DISABLED === "1"; export const TELEMETRY_DISABLED = env.TELEMETRY_DISABLED === "1"; +// Opt-out for the Have-I-Been-Pwned breach check (ENG-1587). Set to "1" on air-gapped / +// closed-network deployments that can't reach api.pwnedpasswords.com and want no outbound +// attempt at all. When unset the check is active but fails open on network errors. +export const PASSWORD_HIBP_CHECK_DISABLED = env.PASSWORD_HIBP_CHECK_DISABLED === "1"; + export const BREVO_API_KEY = env.BREVO_API_KEY; export const BREVO_LIST_ID = env.BREVO_LIST_ID; diff --git a/apps/web/lib/env.ts b/apps/web/lib/env.ts index c434305ce593..2805f5256b78 100644 --- a/apps/web/lib/env.ts +++ b/apps/web/lib/env.ts @@ -307,6 +307,7 @@ const parsedEnv = createEnv({ process.env.NODE_ENV === "test" ? z.string().optional() : z.url("REDIS_URL is required for caching, rate limiting, and audit logging"), + PASSWORD_HIBP_CHECK_DISABLED: z.enum(["1", "0"]).optional(), PASSWORD_RESET_DISABLED: z.enum(["1", "0"]).optional(), PASSWORD_RESET_TOKEN_LIFETIME_MINUTES: z.coerce.number().int().min(5).max(120).optional().default(30), PRIVACY_URL: z @@ -475,6 +476,7 @@ const parsedEnv = createEnv({ OIDC_ISSUER: process.env.OIDC_ISSUER, OIDC_SIGNING_ALGORITHM: process.env.OIDC_SIGNING_ALGORITHM, REDIS_URL: process.env.REDIS_URL, + PASSWORD_HIBP_CHECK_DISABLED: process.env.PASSWORD_HIBP_CHECK_DISABLED, PASSWORD_RESET_DISABLED: process.env.PASSWORD_RESET_DISABLED, PASSWORD_RESET_TOKEN_LIFETIME_MINUTES: process.env.PASSWORD_RESET_TOKEN_LIFETIME_MINUTES, PRIVACY_URL: process.env.PRIVACY_URL, diff --git a/apps/web/lib/user/password.test.ts b/apps/web/lib/user/password.test.ts index 74c1884efdad..145c5df0379a 100644 --- a/apps/web/lib/user/password.test.ts +++ b/apps/web/lib/user/password.test.ts @@ -1,12 +1,12 @@ import { beforeEach, describe, expect, test, vi } from "vitest"; import { prisma } from "@formbricks/database"; -import { InvalidInputError, ResourceNotFoundError } from "@formbricks/types/errors"; +import { InvalidInputError } from "@formbricks/types/errors"; import { verifyPassword } from "@/modules/auth/lib/utils"; -import { getUserAuthenticationData, verifyUserPassword } from "./password"; +import { getCredentialPasswordHash, verifyUserPassword } from "./password"; vi.mock("@formbricks/database", () => ({ prisma: { - user: { + account: { findUnique: vi.fn(), }, }, @@ -16,7 +16,7 @@ vi.mock("@/modules/auth/lib/utils", () => ({ verifyPassword: vi.fn(), })); -const mockPrismaUserFindUnique = vi.mocked(prisma.user.findUnique); +const mockPrismaAccountFindUnique = vi.mocked(prisma.account.findUnique); const mockVerifyPassword = vi.mocked(verifyPassword); describe("user password helpers", () => { @@ -24,43 +24,33 @@ describe("user password helpers", () => { vi.resetAllMocks(); }); - test("returns authentication data for an existing user", async () => { - const user = { - email: "user@example.com", - identityProvider: "email", - identityProviderAccountId: null, - password: "hashed-password", - }; - mockPrismaUserFindUnique.mockResolvedValue(user as any); + test("getCredentialPasswordHash reads the credential Account scoped to the user", async () => { + mockPrismaAccountFindUnique.mockResolvedValue({ password: "hashed-password" } as any); - await expect(getUserAuthenticationData("user-with-password")).resolves.toEqual(user); + await expect(getCredentialPasswordHash("user-1")).resolves.toBe("hashed-password"); - expect(mockPrismaUserFindUnique).toHaveBeenCalledWith({ + expect(mockPrismaAccountFindUnique).toHaveBeenCalledWith({ where: { - id: "user-with-password", - }, - select: { - email: true, - password: true, - identityProvider: true, - identityProviderAccountId: true, + provider_providerAccountId: { provider: "credential", providerAccountId: "user-1" }, }, + select: { password: true }, }); }); - test("throws when authentication data is missing", async () => { - mockPrismaUserFindUnique.mockResolvedValue(null); + test("getCredentialPasswordHash returns null when there is no credential Account", async () => { + mockPrismaAccountFindUnique.mockResolvedValue(null); + + await expect(getCredentialPasswordHash("sso-user")).resolves.toBeNull(); + }); + + test("getCredentialPasswordHash returns null when the credential Account has no password", async () => { + mockPrismaAccountFindUnique.mockResolvedValue({ password: null } as any); - await expect(getUserAuthenticationData("missing-user")).rejects.toThrow(ResourceNotFoundError); + await expect(getCredentialPasswordHash("user-without-hash")).resolves.toBeNull(); }); - test("verifies a password against the stored hash", async () => { - mockPrismaUserFindUnique.mockResolvedValue({ - email: "password-user@example.com", - identityProvider: "email", - identityProviderAccountId: null, - password: "hashed-password", - } as any); + test("verifies a password against the credential Account hash", async () => { + mockPrismaAccountFindUnique.mockResolvedValue({ password: "hashed-password" } as any); mockVerifyPassword.mockResolvedValue(true); await expect(verifyUserPassword("password-user", "plain-password")).resolves.toBe(true); @@ -68,13 +58,8 @@ describe("user password helpers", () => { expect(mockVerifyPassword).toHaveBeenCalledWith("plain-password", "hashed-password"); }); - test("returns false when the password does not match the stored hash", async () => { - mockPrismaUserFindUnique.mockResolvedValue({ - email: "password-user@example.com", - identityProvider: "email", - identityProviderAccountId: null, - password: "hashed-password", - } as any); + test("returns false when the password does not match the credential Account hash", async () => { + mockPrismaAccountFindUnique.mockResolvedValue({ password: "hashed-password" } as any); mockVerifyPassword.mockResolvedValue(false); await expect(verifyUserPassword("password-user", "wrong-password")).resolves.toBe(false); @@ -82,13 +67,8 @@ describe("user password helpers", () => { expect(mockVerifyPassword).toHaveBeenCalledWith("wrong-password", "hashed-password"); }); - test("rejects password verification for users without a password", async () => { - mockPrismaUserFindUnique.mockResolvedValue({ - email: "sso-user@example.com", - identityProvider: "google", - identityProviderAccountId: "google-account-id", - password: null, - } as any); + test("fails closed when the user has no credential Account (no password to verify)", async () => { + mockPrismaAccountFindUnique.mockResolvedValue(null); await expect(verifyUserPassword("sso-user", "plain-password")).rejects.toThrow(InvalidInputError); diff --git a/apps/web/lib/user/password.ts b/apps/web/lib/user/password.ts index c08b7a328746..f255b387403c 100644 --- a/apps/web/lib/user/password.ts +++ b/apps/web/lib/user/password.ts @@ -1,40 +1,34 @@ import "server-only"; import { cache as reactCache } from "react"; import { prisma } from "@formbricks/database"; -import { User } from "@formbricks/database/prisma"; -import { InvalidInputError, ResourceNotFoundError } from "@formbricks/types/errors"; +import { InvalidInputError } from "@formbricks/types/errors"; import { verifyPassword } from "@/modules/auth/lib/utils"; -export const getUserAuthenticationData = reactCache( - async ( - userId: string - ): Promise> => { - const user = await prisma.user.findUnique({ - where: { - id: userId, - }, - select: { - email: true, - password: true, - identityProvider: true, - identityProviderAccountId: true, - }, - }); +/** + * Returns the bcrypt password hash on the user's Better Auth `credential` Account + * (provider = "credential", providerAccountId = userId), or null when the user has no credential + * account (e.g. SSO-only users). Post-ENG-1054 the password lives here, NOT on `User.password` + * (which is null for accounts created after the cutover). Scoped strictly to the given user id. + */ +export const getCredentialPasswordHash = reactCache(async (userId: string): Promise => { + const account = await prisma.account.findUnique({ + where: { + provider_providerAccountId: { provider: "credential", providerAccountId: userId }, + }, + select: { password: true }, + }); - if (!user) { - throw new ResourceNotFoundError("user", userId); - } - - return user; - } -); + return account?.password ?? null; +}); export const verifyUserPassword = async (userId: string, password: string): Promise => { - const user = await getUserAuthenticationData(userId); + const passwordHash = await getCredentialPasswordHash(userId); - if (!user.password) { + // Fail closed: no credential account (e.g. SSO-only user) means there is no password to verify + // against, so the operation must be rejected — never treated as a successful verification. + if (!passwordHash) { throw new InvalidInputError("Password is not set for this user"); } - return await verifyPassword(password, user.password); + return await verifyPassword(password, passwordHash); }; diff --git a/apps/web/locales/de-DE.json b/apps/web/locales/de-DE.json index a28374f81a74..b7714c94990d 100644 --- a/apps/web/locales/de-DE.json +++ b/apps/web/locales/de-DE.json @@ -100,6 +100,7 @@ }, "unknown_client": "Unknown client" }, + "password_compromised": "Dieses Passwort ist in einem Datenleck aufgetaucht. Bitte wähle ein anderes.", "saml_connection_error": "Etwas ist schiefgelaufen. Bitte überprüfe die App-Konsole für weitere Details.", "signup": { "captcha_failed": "Captcha fehlgeschlagen", diff --git a/apps/web/locales/en-US.json b/apps/web/locales/en-US.json index c2bad846def1..ee59fae87668 100644 --- a/apps/web/locales/en-US.json +++ b/apps/web/locales/en-US.json @@ -100,6 +100,7 @@ }, "unknown_client": "Unknown client" }, + "password_compromised": "This password has appeared in a data breach. Please choose a different one.", "saml_connection_error": "Something went wrong. Please check your app console for more details.", "signup": { "captcha_failed": "Captcha failed", diff --git a/apps/web/locales/es-ES.json b/apps/web/locales/es-ES.json index f4e96c330d5d..0c2e409e3b01 100644 --- a/apps/web/locales/es-ES.json +++ b/apps/web/locales/es-ES.json @@ -100,6 +100,7 @@ }, "unknown_client": "Unknown client" }, + "password_compromised": "Esta contraseña ha aparecido en una filtración de datos. Elige otra.", "saml_connection_error": "Algo salió mal. Por favor, consulta la consola de tu aplicación para más detalles.", "signup": { "captcha_failed": "Captcha fallido", diff --git a/apps/web/locales/fr-FR.json b/apps/web/locales/fr-FR.json index e8e9a208fccc..d09330c41022 100644 --- a/apps/web/locales/fr-FR.json +++ b/apps/web/locales/fr-FR.json @@ -100,6 +100,7 @@ }, "unknown_client": "Unknown client" }, + "password_compromised": "Ce mot de passe est apparu dans une fuite de données. Veuillez en choisir un autre.", "saml_connection_error": "Quelque chose s'est mal passé. Veuillez vérifier la console de votre application pour plus de détails.", "signup": { "captcha_failed": "Captcha échoué", diff --git a/apps/web/locales/hu-HU.json b/apps/web/locales/hu-HU.json index 50b25c392ca8..678e3c3519b7 100644 --- a/apps/web/locales/hu-HU.json +++ b/apps/web/locales/hu-HU.json @@ -100,6 +100,7 @@ }, "unknown_client": "Unknown client" }, + "password_compromised": "Ez a jelszó megjelent egy adatszivárgásban. Kérjük, válassz másikat.", "saml_connection_error": "Valami probléma történt. A további részletekért nézze meg az alkalmazás konzolját.", "signup": { "captcha_failed": "A captcha sikertelen", diff --git a/apps/web/locales/ja-JP.json b/apps/web/locales/ja-JP.json index 64f13ac99e9f..24581a5ff1d8 100644 --- a/apps/web/locales/ja-JP.json +++ b/apps/web/locales/ja-JP.json @@ -100,6 +100,7 @@ }, "unknown_client": "Unknown client" }, + "password_compromised": "このパスワードはデータ漏洩で見つかりました。別のパスワードを選択してください。", "saml_connection_error": "問題が発生しました。詳細については、アプリのコンソールを確認してください。", "signup": { "captcha_failed": "CAPTCHAに失敗しました", diff --git a/apps/web/locales/nl-NL.json b/apps/web/locales/nl-NL.json index 963afea85e9f..834d48e319f8 100644 --- a/apps/web/locales/nl-NL.json +++ b/apps/web/locales/nl-NL.json @@ -100,6 +100,7 @@ }, "unknown_client": "Unknown client" }, + "password_compromised": "Dit wachtwoord is aangetroffen in een datalek. Kies een ander wachtwoord.", "saml_connection_error": "Er is iets misgegaan. Controleer uw app-console voor meer details.", "signup": { "captcha_failed": "Captcha is mislukt", diff --git a/apps/web/locales/pt-BR.json b/apps/web/locales/pt-BR.json index 6b6204d5dcb7..40546cc45d9c 100644 --- a/apps/web/locales/pt-BR.json +++ b/apps/web/locales/pt-BR.json @@ -100,6 +100,7 @@ }, "unknown_client": "Unknown client" }, + "password_compromised": "Esta senha apareceu em um vazamento de dados. Escolha outra.", "saml_connection_error": "Algo deu errado. Por favor, verifica o console do app para mais detalhes.", "signup": { "captcha_failed": "reCAPTCHA falhou", diff --git a/apps/web/locales/pt-PT.json b/apps/web/locales/pt-PT.json index 0681b7afb99c..e10cacc869a6 100644 --- a/apps/web/locales/pt-PT.json +++ b/apps/web/locales/pt-PT.json @@ -100,6 +100,7 @@ }, "unknown_client": "Unknown client" }, + "password_compromised": "Esta palavra-passe apareceu numa fuga de dados. Por favor, escolha outra.", "saml_connection_error": "Algo correu mal. Por favor, verifique a consola da aplicação para mais detalhes.", "signup": { "captcha_failed": "Captcha falhou", diff --git a/apps/web/locales/ro-RO.json b/apps/web/locales/ro-RO.json index e304a3c21734..e1bd1cb692c2 100644 --- a/apps/web/locales/ro-RO.json +++ b/apps/web/locales/ro-RO.json @@ -100,6 +100,7 @@ }, "unknown_client": "Unknown client" }, + "password_compromised": "Această parolă a apărut într-o breșă de date. Te rugăm să alegi alta.", "saml_connection_error": "Ceva nu a mers. Vă rugăm să verificați consola aplicației pentru mai multe detalii.", "signup": { "captcha_failed": "Captcha eșuat", diff --git a/apps/web/locales/ru-RU.json b/apps/web/locales/ru-RU.json index c90e1da44fd2..b11903017f52 100644 --- a/apps/web/locales/ru-RU.json +++ b/apps/web/locales/ru-RU.json @@ -100,6 +100,7 @@ }, "unknown_client": "Unknown client" }, + "password_compromised": "Этот пароль был обнаружен в утечке данных. Пожалуйста, выберите другой.", "saml_connection_error": "Что-то пошло не так. Пожалуйста, проверьте консоль приложения для получения подробностей.", "signup": { "captcha_failed": "Не удалось пройти капчу", diff --git a/apps/web/locales/sv-SE.json b/apps/web/locales/sv-SE.json index 207d64a12837..2100e11bae6c 100644 --- a/apps/web/locales/sv-SE.json +++ b/apps/web/locales/sv-SE.json @@ -100,6 +100,7 @@ }, "unknown_client": "Unknown client" }, + "password_compromised": "Det här lösenordet har förekommit i en dataläcka. Välj ett annat.", "saml_connection_error": "Något gick fel. Kontrollera din appkonsol för mer information.", "signup": { "captcha_failed": "Captcha misslyckades", diff --git a/apps/web/locales/tr-TR.json b/apps/web/locales/tr-TR.json index 18b105da4b71..cf700da3318c 100644 --- a/apps/web/locales/tr-TR.json +++ b/apps/web/locales/tr-TR.json @@ -100,6 +100,7 @@ }, "unknown_client": "Unknown client" }, + "password_compromised": "Bu parola bir veri ihlalinde görülmüş. Lütfen farklı bir parola seçin.", "saml_connection_error": "Bir sorun oluştu. Daha fazla bilgi için uygulama konsolunuzu kontrol edin.", "signup": { "captcha_failed": "Captcha başarısız oldu", diff --git a/apps/web/locales/zh-Hans-CN.json b/apps/web/locales/zh-Hans-CN.json index 04bb95f82eb6..faa41e0b691a 100644 --- a/apps/web/locales/zh-Hans-CN.json +++ b/apps/web/locales/zh-Hans-CN.json @@ -100,6 +100,7 @@ }, "unknown_client": "Unknown client" }, + "password_compromised": "该密码曾出现在数据泄露事件中,请选择其他密码。", "saml_connection_error": "出了问题。请查看应用 控制台 以获取更多详情。", "signup": { "captcha_failed": "验证码 验证 失败", diff --git a/apps/web/locales/zh-Hant-TW.json b/apps/web/locales/zh-Hant-TW.json index 283eede45696..03c3aeabc575 100644 --- a/apps/web/locales/zh-Hant-TW.json +++ b/apps/web/locales/zh-Hant-TW.json @@ -100,6 +100,7 @@ }, "unknown_client": "Unknown client" }, + "password_compromised": "此密碼曾出現在資料外洩事件中,請選擇其他密碼。", "saml_connection_error": "發生錯誤。請檢查您的 app 主控台以取得更多詳細資料。", "signup": { "captcha_failed": "驗證碼失敗", diff --git a/apps/web/modules/auth/forgot-password/reset/actions.test.ts b/apps/web/modules/auth/forgot-password/reset/actions.test.ts index b4ed52164640..834eed6d3710 100644 --- a/apps/web/modules/auth/forgot-password/reset/actions.test.ts +++ b/apps/web/modules/auth/forgot-password/reset/actions.test.ts @@ -2,8 +2,10 @@ import { APIError } from "better-auth/api"; import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import { INVALID_PASSWORD_RESET_TOKEN_ERROR_CODE, + InvalidInputError, InvalidPasswordResetTokenError, OperationNotAllowedError, + PASSWORD_COMPROMISED_ERROR_CODE, } from "@formbricks/types/errors"; import { auth } from "@/modules/auth/lib/auth"; import { resetPasswordAction } from "./actions"; @@ -75,6 +77,20 @@ describe("resetPasswordAction", () => { expect(auth.api.resetPassword).toHaveBeenCalledTimes(1); }); + test("maps a compromised-password APIError to the compromised code, not the invalid-token error", async () => { + // The HIBP plugin rejects a breached password with its own APIError. This must be surfaced as the + // stable compromised code — NOT swallowed by the catch-all that maps every APIError to a bogus + // "invalid/expired token" message. + vi.mocked(auth.api.resetPassword).mockRejectedValue( + new APIError("BAD_REQUEST", { message: "breached", code: PASSWORD_COMPROMISED_ERROR_CODE }) + ); + + const result = resetPasswordAction({ parsedInput } as any); + await expect(result).rejects.toBeInstanceOf(InvalidInputError); + await expect(result).rejects.toThrow(PASSWORD_COMPROMISED_ERROR_CODE); + expect(auth.api.resetPassword).toHaveBeenCalledTimes(1); + }); + test("propagates an unexpected (non-APIError) error unchanged", async () => { vi.mocked(auth.api.resetPassword).mockRejectedValue(new Error("Database error")); diff --git a/apps/web/modules/auth/forgot-password/reset/actions.ts b/apps/web/modules/auth/forgot-password/reset/actions.ts index 0afaf9ce2dec..16fbacb19e2e 100644 --- a/apps/web/modules/auth/forgot-password/reset/actions.ts +++ b/apps/web/modules/auth/forgot-password/reset/actions.ts @@ -5,13 +5,16 @@ import { headers } from "next/headers"; import { z } from "zod"; import { INVALID_PASSWORD_RESET_TOKEN_ERROR_CODE, + InvalidInputError, InvalidPasswordResetTokenError, OperationNotAllowedError, + PASSWORD_COMPROMISED_ERROR_CODE, } from "@formbricks/types/errors"; import { ZUserPassword } from "@formbricks/types/user"; import { PASSWORD_RESET_DISABLED } from "@/lib/constants"; import { actionClient } from "@/lib/utils/action-client"; import { auth } from "@/modules/auth/lib/auth"; +import { isPasswordCompromisedError } from "@/modules/auth/lib/better-auth-hibp"; const ZResetPasswordAction = z.object({ token: z.string().min(1), @@ -31,6 +34,12 @@ export const resetPasswordAction = actionClient headers: await headers(), }); } catch (error) { + // A breached password is rejected by the HIBP plugin with its own APIError — surface it as the + // stable compromised-password code (the reset form maps it to a localized message). This MUST + // precede the catch-all below, which would otherwise mislabel it as an invalid/expired token. + if (isPasswordCompromisedError(error)) { + throw new InvalidInputError(PASSWORD_COMPROMISED_ERROR_CODE); + } // Better Auth rejects an invalid/expired/already-used token with an APIError; surface it as the // existing invalid-reset-token error so the form shows the same message. The post-reset side // effects — session revocation (revokeSessionsOnPasswordReset), the security notification email, diff --git a/apps/web/modules/auth/forgot-password/reset/components/reset-password-form.tsx b/apps/web/modules/auth/forgot-password/reset/components/reset-password-form.tsx index 1d34abf9cd65..970d4467f2d1 100644 --- a/apps/web/modules/auth/forgot-password/reset/components/reset-password-form.tsx +++ b/apps/web/modules/auth/forgot-password/reset/components/reset-password-form.tsx @@ -6,7 +6,10 @@ import { SubmitHandler, useForm } from "react-hook-form"; import { toast } from "react-hot-toast"; import { useTranslation } from "react-i18next"; import { z } from "zod"; -import { INVALID_PASSWORD_RESET_TOKEN_ERROR_CODE } from "@formbricks/types/errors"; +import { + INVALID_PASSWORD_RESET_TOKEN_ERROR_CODE, + PASSWORD_COMPROMISED_ERROR_CODE, +} from "@formbricks/types/errors"; import { ZUserPassword } from "@formbricks/types/user"; import { getFormattedErrorMessage } from "@/lib/utils/helper"; import { resetPasswordAction } from "@/modules/auth/forgot-password/reset/actions"; @@ -58,6 +61,10 @@ export const ResetPasswordForm = () => { router.push("/auth/forgot-password/reset/success"); } else { const errorMessage = getFormattedErrorMessage(resetPasswordResponse); + if (errorMessage === PASSWORD_COMPROMISED_ERROR_CODE) { + toast.error(t("auth.password_compromised")); + return; + } toast.error( errorMessage === INVALID_PASSWORD_RESET_TOKEN_ERROR_CODE ? t("c.link_expired_description") diff --git a/apps/web/modules/auth/lib/auth.ts b/apps/web/modules/auth/lib/auth.ts index d296f33b616e..a7fc7fdcd643 100644 --- a/apps/web/modules/auth/lib/auth.ts +++ b/apps/web/modules/auth/lib/auth.ts @@ -27,6 +27,7 @@ import { ssoRecoverySignInPlugin } from "@/modules/ee/sso/lib/better-auth-recove import { runAfterAuthHooks } from "./after-auth-hooks"; import { rejectInactiveUserOnSessionCreate } from "./better-auth-active-user-gate"; import { createBrevoCustomerAfterEmailVerification } from "./better-auth-email-verification"; +import { hibpBreachCheckBeforeHandler } from "./better-auth-hibp"; import { auditPasswordReset, betterAuthLogger, signInAuditDatabaseHook } from "./better-auth-observability"; import { getMcpOauthProviderOptions } from "./mcp-oauth-provider-options"; import { getAuthIssuerUrl, getMcpResourceUrl } from "./oauth-urls"; @@ -236,6 +237,10 @@ export const auth = betterAuth({ before: createAuthMiddleware(async (ctx) => { await ssoLicenseGateBeforeHandler(ctx); await requireDeletionConfirmationBeforeHandler(ctx); + // ENG-1587: reject known-breached passwords on set (signup / reset) BEFORE the endpoint runs, so + // the reset token isn't consumed on a rejection. Fails open when api.pwnedpasswords.com is + // unreachable and honors PASSWORD_HIBP_CHECK_DISABLED. See better-auth-hibp.ts. + await hibpBreachCheckBeforeHandler(ctx); }), after: createAuthMiddleware(runAfterAuthHooks), }, diff --git a/apps/web/modules/auth/lib/better-auth-hibp.integration.test.ts b/apps/web/modules/auth/lib/better-auth-hibp.integration.test.ts new file mode 100644 index 000000000000..8c348f543d28 --- /dev/null +++ b/apps/web/modules/auth/lib/better-auth-hibp.integration.test.ts @@ -0,0 +1,132 @@ +import { createHash } from "node:crypto"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; +import { prisma } from "@formbricks/database"; +import { resetDb } from "@/integration/reset-db"; +import { auth } from "@/modules/auth/lib/auth"; +import { sendPasswordResetLinkEmail } from "@/modules/email"; + +/** + * Integration coverage for the HIBP breach check (ENG-1587) — drives the REAL Better Auth instance + * (with the hibpBreachCheckBeforeHandler wired into auth.ts `hooks.before`) against a real Postgres. + * Only the outbound range call to api.pwnedpasswords.com is intercepted; everything else (the before + * hook, BA path routing, token consumption, the bcrypt hash, user/account creation) runs for real. + * This is what proves BA actually routes /sign-up/email and /reset-password through our hook — and, + * for reset, that a breached attempt does NOT consume the token (the ENG-1587 review finding). + */ + +// integration/setup.ts disables the HIBP check suite-wide (so other tests don't hit the network); +// re-enable it for this file, which is the one that actually exercises the breach check. Global fetch +// is stubbed below, so this still makes no real outbound call. +vi.mock("@/lib/constants", async (importOriginal) => ({ + ...(await importOriginal()), + PASSWORD_HIBP_CHECK_DISABLED: false, +})); + +// Intercept ONLY the pwnedpasswords range endpoint (matched by parsed host); delegate every other +// fetch to the real implementation. Each test sets the range payload via `setRangeResponse`. +const realFetch = globalThis.fetch; +let rangeResponse: Response = new Response(""); +const setRangeResponse = (r: Response) => { + rangeResponse = r; +}; + +/** Build a range-endpoint body that DOES contain the given password's SHA-1 suffix (a breach hit). */ +const rangeHitFor = (password: string): string => { + const sha1 = createHash("sha1").update(password).digest("hex").toUpperCase(); + return `${sha1.substring(5)}:99\nAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA:1`; +}; + +beforeEach(async () => { + await resetDb(); + setRangeResponse(new Response("")); + vi.stubGlobal( + "fetch", + vi.fn((input: RequestInfo | URL, init?: RequestInit) => { + const url = typeof input === "string" ? input : input instanceof URL ? input.href : input.url; + let host = ""; + try { + host = new URL(url).hostname; + } catch { + // non-absolute URL → not the range endpoint; fall through to the real fetch + } + if (host === "api.pwnedpasswords.com") return Promise.resolve(rangeResponse.clone()); + return realFetch(input, init); + }) + ); +}); + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +describe("HIBP breach check on sign-up (real Better Auth + Postgres)", () => { + test("rejects a breached password and creates no user", async () => { + const password = "P4ssw0rd-breached"; + setRangeResponse(new Response(rangeHitFor(password))); + + await expect( + auth.api.signUpEmail({ body: { email: "breach@example.com", password, name: "Bree" } }) + ).rejects.toMatchObject({ body: { code: "password_compromised" } }); + + expect(await prisma.user.findUnique({ where: { email: "breach@example.com" } })).toBeNull(); + }); + + test("allows a clean password (suffix absent from the range response)", async () => { + // A range body whose suffixes cannot match any real SHA-1 suffix (too short) → not compromised. + setRangeResponse(new Response("0000000000:1\n1111111111:2")); + + const response = await auth.api.signUpEmail({ + body: { email: "clean@example.com", password: "Str0ng-unique-passphrase", name: "Cleo" }, + asResponse: true, + }); + expect(response.status).toBeLessThan(300); + expect(await prisma.user.findUnique({ where: { email: "clean@example.com" } })).not.toBeNull(); + }); + + test("fails open: a range-endpoint error still lets sign-up through", async () => { + setRangeResponse(new Response(null, { status: 503 })); + + const response = await auth.api.signUpEmail({ + body: { email: "failopen@example.com", password: "Another-good-one-1", name: "Fay" }, + asResponse: true, + }); + expect(response.status).toBeLessThan(300); + expect(await prisma.user.findUnique({ where: { email: "failopen@example.com" } })).not.toBeNull(); + }); +}); + +describe("HIBP breach check on password reset — token is preserved on rejection (ENG-1587 review)", () => { + // Drive a real reset token through Better Auth and capture it from the mocked reset email. + const provisionResetToken = async (email: string): Promise => { + // beforeEach leaves the range response empty (no match), so this sign-up is treated as clean. + await auth.api.signUpEmail({ body: { email, password: "Initial-passphrase-1", name: "Rae" } }); + vi.mocked(sendPasswordResetLinkEmail).mockClear(); + + // No redirectTo → avoids Better Auth's trusted-origin check. BA puts the token in the URL PATH + // (`/reset-password/?callbackURL=`), not a query param. + await auth.api.requestPasswordReset({ body: { email } }); + const verifyLink = vi.mocked(sendPasswordResetLinkEmail).mock.calls[0]?.[0]?.verifyLink; + const token = verifyLink ? new URL(verifyLink).pathname.split("/").filter(Boolean).pop() : null; + if (!token) throw new Error("failed to capture reset token from the mocked email"); + return token; + }; + + test("a breached first attempt does not consume the token; a valid retry succeeds", async () => { + const email = "reset@example.com"; + const token = await provisionResetToken(email); + + // 1) Breached password → rejected. The before hook runs BEFORE the token is consumed. + setRangeResponse(new Response(rangeHitFor("Breached-reset-1"))); + await expect( + auth.api.resetPassword({ body: { token, newPassword: "Breached-reset-1" } }) + ).rejects.toMatchObject({ body: { code: "password_compromised" } }); + + // 2) Same token, a clean password → succeeds (the token was NOT burned in step 1). + setRangeResponse(new Response("0000000000:1")); + const ok = await auth.api.resetPassword({ + body: { token, newPassword: "Fresh-unique-passphrase-2" }, + asResponse: true, + }); + expect(ok.status).toBeLessThan(300); + }); +}); diff --git a/apps/web/modules/auth/lib/better-auth-hibp.test.ts b/apps/web/modules/auth/lib/better-auth-hibp.test.ts new file mode 100644 index 000000000000..6d7c010c4e0f --- /dev/null +++ b/apps/web/modules/auth/lib/better-auth-hibp.test.ts @@ -0,0 +1,112 @@ +import { APIError } from "better-auth/api"; +import { createHash } from "node:crypto"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; +import { logger } from "@formbricks/logger"; + +// Real SHA-1 (node:crypto) so prefix/suffix are the actual values the hook computes. +const suffixOf = (password: string): string => + createHash("sha1").update(password).digest("hex").toUpperCase().substring(5); + +vi.mock("@formbricks/logger", () => ({ logger: { warn: vi.fn(), error: vi.fn(), info: vi.fn() } })); +// Minimal constants mock so the heavy real module (and env) is not loaded; default: check enabled. +vi.mock("@/lib/constants", () => ({ PASSWORD_HIBP_CHECK_DISABLED: false })); + +// Fake fetch Response for the range endpoint. +const rangeOk = (body: string) => ({ ok: true, status: 200, text: async () => body }) as unknown as Response; +const rangeErr = (status: number) => ({ ok: false, status, text: async () => "" }) as unknown as Response; + +// Minimal AuthHookContext: the handler only reads `path` and `body`. +const ctxFor = (path: string, body: Record) => ({ path, body }) as never; + +const loadHandler = async () => (await import("./better-auth-hibp")).hibpBreachCheckBeforeHandler; + +describe("hibpBreachCheckBeforeHandler", () => { + beforeEach(() => { + vi.stubGlobal("fetch", vi.fn()); + }); + afterEach(() => { + vi.clearAllMocks(); + vi.unstubAllGlobals(); + }); + + test("rejects a breached password on /sign-up/email (body.password)", async () => { + // CRLF + surrounding whitespace to prove line-trimming works. + vi.mocked(fetch).mockResolvedValue(rangeOk(` ${suffixOf("breached")}:42 \r\nDEADBEEF:1`)); + const handler = await loadHandler(); + + const result = handler(ctxFor("/sign-up/email", { password: "breached" })); + await expect(result).rejects.toBeInstanceOf(APIError); + await expect(result).rejects.toMatchObject({ body: { code: "password_compromised" } }); + }); + + test("rejects a breached password on /reset-password (body.newPassword)", async () => { + vi.mocked(fetch).mockResolvedValue(rangeOk(`${suffixOf("breached")}:9`)); + const handler = await loadHandler(); + + await expect(handler(ctxFor("/reset-password", { newPassword: "breached" }))).rejects.toMatchObject({ + body: { code: "password_compromised" }, + }); + }); + + test("allows a password absent from the corpus", async () => { + vi.mocked(fetch).mockResolvedValue(rangeOk("0000000000:1\nDEADBEEF:2")); + const handler = await loadHandler(); + + await expect(handler(ctxFor("/sign-up/email", { password: "clean" }))).resolves.toBeUndefined(); + expect(fetch).toHaveBeenCalledTimes(1); + }); + + test("fails open (allows) when the range API returns a non-OK status", async () => { + vi.mocked(fetch).mockResolvedValue(rangeErr(503)); + const handler = await loadHandler(); + + await expect(handler(ctxFor("/reset-password", { newPassword: "clean" }))).resolves.toBeUndefined(); + expect(logger.warn).toHaveBeenCalled(); + }); + + test("fails open (allows) when the fetch throws (e.g. timeout)", async () => { + vi.mocked(fetch).mockRejectedValue(new Error("The operation was aborted")); + const handler = await loadHandler(); + + await expect(handler(ctxFor("/sign-up/email", { password: "clean" }))).resolves.toBeUndefined(); + expect(logger.warn).toHaveBeenCalled(); + }); + + test("no-ops on a non-password-set path", async () => { + const handler = await loadHandler(); + await expect(handler(ctxFor("/sign-in/email", { password: "whatever" }))).resolves.toBeUndefined(); + expect(fetch).not.toHaveBeenCalled(); + }); + + test("no-ops when the body carries no password", async () => { + const handler = await loadHandler(); + await expect(handler(ctxFor("/reset-password", { token: "abc" }))).resolves.toBeUndefined(); + expect(fetch).not.toHaveBeenCalled(); + }); + + test("skips the check entirely when disabled by env flag", async () => { + vi.resetModules(); + vi.doMock("@/lib/constants", () => ({ PASSWORD_HIBP_CHECK_DISABLED: true })); + const handler = (await import("./better-auth-hibp")).hibpBreachCheckBeforeHandler; + + await expect(handler(ctxFor("/sign-up/email", { password: "breached" }))).resolves.toBeUndefined(); + expect(fetch).not.toHaveBeenCalled(); + + // Restore the default (enabled) constants mock so later re-imports don't load the real env module. + vi.doMock("@/lib/constants", () => ({ PASSWORD_HIBP_CHECK_DISABLED: false })); + vi.resetModules(); + }); +}); + +describe("isPasswordCompromisedError", () => { + test("true only for the hook's compromised APIError", async () => { + const { isPasswordCompromisedError } = await import("./better-auth-hibp"); + const compromised = new APIError("BAD_REQUEST", { message: "x", code: "password_compromised" }); + const otherApiError = new APIError("BAD_REQUEST", { message: "x", code: "SOMETHING_ELSE" }); + + expect(isPasswordCompromisedError(compromised)).toBe(true); + expect(isPasswordCompromisedError(otherApiError)).toBe(false); + expect(isPasswordCompromisedError(new Error("nope"))).toBe(false); + expect(isPasswordCompromisedError(null)).toBe(false); + }); +}); diff --git a/apps/web/modules/auth/lib/better-auth-hibp.ts b/apps/web/modules/auth/lib/better-auth-hibp.ts new file mode 100644 index 000000000000..f2bd076928fa --- /dev/null +++ b/apps/web/modules/auth/lib/better-auth-hibp.ts @@ -0,0 +1,109 @@ +import "server-only"; +import { APIError, isAPIError } from "better-auth/api"; +import { createHash } from "node:crypto"; +import { logger } from "@formbricks/logger"; +import { PASSWORD_COMPROMISED_ERROR_CODE } from "@formbricks/types/errors"; +import { PASSWORD_HIBP_CHECK_DISABLED } from "@/lib/constants"; +import type { AuthHookContext } from "@/modules/ee/sso/lib/better-auth-hooks"; + +/** + * Have-I-Been-Pwned breach check on password set (ENG-1587). + * + * A locally-owned alternative to Better Auth's stock `haveIBeenPwned` plugin, run as a Better Auth + * `before` hook (see auth.ts `hooks.before`). Two behaviours differ from the stock plugin, both + * important for self-hosting: + * + * 1. FAIL OPEN. The stock plugin rejects the password on ANY network error (throws 500), which would + * break signup + password reset entirely on air-gapped / closed-network deployments. Here a + * network error, timeout, or non-200 is logged and the password is ALLOWED — only a password that + * is *definitively found* in the breach corpus is rejected. (NIST 800-63B-4 requires checking a + * blocklist, not blocking when the blocklist is unreachable.) + * 2. OPERATOR OPT-OUT. `PASSWORD_HIBP_CHECK_DISABLED=1` skips the outbound call entirely, for + * deployments that want zero egress attempts. + * + * WHY a `before` hook rather than the stock plugin's `password.hash` wrapper: on `/reset-password`, + * Better Auth *consumes the reset token before hashing* (api/routes/password.ts). Rejecting at hash + * time would burn the token, so a valid retry would fail with "link already used" (ENG-1587 review). + * Running before the endpoint handler rejects a breached password without consuming the token (and + * without creating the user on signup), and it covers the raw `/api/auth/*` endpoints as well as the + * server actions. + */ + +// The two Better Auth paths that set a password in Formbricks. Signup carries `password`; reset +// carries `newPassword`. +const CHECKED_PATHS = new Set(["/sign-up/email", "/reset-password"]); + +// Tight timeout: this call blocks a login-critical flow, so we bound it well under the license +// check's 5s. On timeout we fail open (allow the password). +const HIBP_FETCH_TIMEOUT_MS = 3000; + +const HIBP_RANGE_URL = "https://api.pwnedpasswords.com/range"; + +/** + * Returns true only when the password is confirmed present in the breach corpus. A network failure, + * timeout, or non-OK response returns false (fail open) after logging — never throws for + * infrastructure problems. + */ +const isPasswordCompromised = async (password: string): Promise => { + // SHA-1 is mandated by the HaveIBeenPwned range (k-anonymity) API — only the first 5 hex chars of + // this digest ever leave the process, and it is NOT used for storage or authentication (bcrypt via + // hashSecret does that). The CodeQL "insufficient computational effort" and SonarQube S4790 + // weak-hash alerts here are false positives. + const sha1 = createHash("sha1").update(password).digest("hex").toUpperCase(); // NOSONAR S4790 - see above + const prefix = sha1.substring(0, 5); + const suffix = sha1.substring(5); + + try { + const res = await fetch(`${HIBP_RANGE_URL}/${prefix}`, { + // "Add-Padding" pads the response with decoy hashes so the row count can't hint at the prefix. + headers: { "Add-Padding": "true", "User-Agent": "Formbricks Password Checker" }, + signal: AbortSignal.timeout(HIBP_FETCH_TIMEOUT_MS), + // Next.js instruments global fetch with its Data Cache — force every check to hit the live corpus + // so a breach verdict is never served from a cached/stale response. + cache: "no-store", + }); + + if (!res.ok) { + logger.warn({ status: res.status }, "HIBP breach check unreachable; allowing password (fail open)"); + return false; + } + + const body = await res.text(); + // Each line is ":"; trim to tolerate CRLF endings. A match means the full hash + // is in the corpus. + return body.split("\n").some((line) => line.trim().split(":")[0].toUpperCase() === suffix); + } catch (err) { + // Timeout / DNS / connection error — fail open. + logger.warn({ err }, "HIBP breach check failed; allowing password (fail open)"); + return false; + } +}; + +/** + * Better Auth `before` hook: reject a breached password on the password-set paths before the endpoint + * handler runs (i.e. before the reset token is consumed / the user is created). No-ops on every other + * path and when the check is disabled. + */ +export const hibpBreachCheckBeforeHandler = async (ctx: AuthHookContext): Promise => { + if (PASSWORD_HIBP_CHECK_DISABLED || !CHECKED_PATHS.has(ctx.path)) return; + + const body = ctx.body as { password?: unknown; newPassword?: unknown } | undefined; + const password = body?.password ?? body?.newPassword; + if (typeof password !== "string" || password.length === 0) return; + + if (await isPasswordCompromised(password)) { + throw new APIError("BAD_REQUEST", { + message: "The password you entered has been found in a data breach.", + code: PASSWORD_COMPROMISED_ERROR_CODE, + }); + } +}; + +/** + * True when `error` is the breach-check rejection thrown by this hook (a Better Auth APIError carrying + * PASSWORD_COMPROMISED_ERROR_CODE). Sign-up / reset actions use this to re-surface it as a Formbricks + * expected error with a stable, client-mappable code. + */ +export const isPasswordCompromisedError = (error: unknown): boolean => + isAPIError(error) && + (error.body as { code?: string } | undefined)?.code === PASSWORD_COMPROMISED_ERROR_CODE; diff --git a/apps/web/modules/auth/signup/actions.ts b/apps/web/modules/auth/signup/actions.ts index 310f5d3e14f0..b3f83ad03736 100644 --- a/apps/web/modules/auth/signup/actions.ts +++ b/apps/web/modules/auth/signup/actions.ts @@ -5,6 +5,7 @@ import { z } from "zod"; import { logger } from "@formbricks/logger"; import { InvalidInputError, + PASSWORD_COMPROMISED_ERROR_CODE, SIGNUP_EMAIL_DOMAIN_BLOCKED_ERROR_CODE, UnknownError, } from "@formbricks/types/errors"; @@ -20,6 +21,7 @@ import { ActionClientCtx } from "@/lib/utils/action-client/types/context"; import { DEFAULT_WORKSPACE_NAME } from "@/lib/workspace/constants"; import { ATTRIBUTION_COOKIE_NAME, getAttributionPropertiesFromCookies } from "@/modules/auth/lib/attribution"; import { auth } from "@/modules/auth/lib/auth"; +import { isPasswordCompromisedError } from "@/modules/auth/lib/better-auth-hibp"; import { isSignupEmailDomainBlocked } from "@/modules/auth/lib/signup-email-domain"; import { markSignupDomainAllowed, @@ -96,6 +98,11 @@ async function signUpUserSafely( // legacy verification-token email. await auth.api.signUpEmail({ body: { email: normalizedEmail, password, name } }); } catch (error) { + // A breached password is rejected before any user is created — surface it as an expected error + // with a stable code the sign-up form maps to a localized message (not an enumeration signal). + if (isPasswordCompromisedError(error)) { + throw new InvalidInputError(PASSWORD_COMPROMISED_ERROR_CODE); + } // Enumeration-safe: a duplicate email resolves to "already existed", not a surfaced error. const existing = await getUserByEmail(normalizedEmail); if (existing) { diff --git a/apps/web/modules/auth/signup/components/signup-form.tsx b/apps/web/modules/auth/signup/components/signup-form.tsx index a13f7e9a2fd9..795c7ac1317a 100644 --- a/apps/web/modules/auth/signup/components/signup-form.tsx +++ b/apps/web/modules/auth/signup/components/signup-form.tsx @@ -9,7 +9,10 @@ import toast from "react-hot-toast"; import { useTranslation } from "react-i18next"; import Turnstile, { useTurnstile } from "react-turnstile"; import { z } from "zod"; -import { SIGNUP_EMAIL_DOMAIN_BLOCKED_ERROR_CODE } from "@formbricks/types/errors"; +import { + PASSWORD_COMPROMISED_ERROR_CODE, + SIGNUP_EMAIL_DOMAIN_BLOCKED_ERROR_CODE, +} from "@formbricks/types/errors"; import { TUserLocale, ZUserName, ZUserPassword } from "@formbricks/types/user"; import { getFormattedErrorMessage } from "@/lib/utils/helper"; import { buildAttributionQuerySuffix } from "@/modules/auth/lib/attribution"; @@ -152,6 +155,9 @@ export const SignupForm = ({ // Personal-email block: surface under the email field rather than as a toast. if (errorMessage === SIGNUP_EMAIL_DOMAIN_BLOCKED_ERROR_CODE) { form.setError("email", { type: "manual", message: t("auth.signup.company_email_required") }); + } else if (errorMessage === PASSWORD_COMPROMISED_ERROR_CODE) { + // Breached password: surface under the password field with a clear, actionable message. + form.setError("password", { type: "manual", message: t("auth.password_compromised") }); } else { toast.error(errorMessage); } diff --git a/apps/web/modules/ee/two-factor-auth/components/confirm-password-form.tsx b/apps/web/modules/ee/two-factor-auth/components/confirm-password-form.tsx index 04733d099442..6abe9773b8f3 100644 --- a/apps/web/modules/ee/two-factor-auth/components/confirm-password-form.tsx +++ b/apps/web/modules/ee/two-factor-auth/components/confirm-password-form.tsx @@ -50,8 +50,10 @@ export const ConfirmPasswordForm = ({ setSecret(secret); setCurrentStep("scanQRCode"); } else { + // Always surface something: getFormattedErrorMessage can resolve to an empty string for some + // action errors, which would show a blank toast (the "spinner then nothing" symptom). const errorMessage = getFormattedErrorMessage(setupTwoFactorAuthResponse); - toast.error(errorMessage); + toast.error(errorMessage || t("common.something_went_wrong_please_try_again")); } }; diff --git a/apps/web/modules/ee/two-factor-auth/lib/two-factor-auth.test.ts b/apps/web/modules/ee/two-factor-auth/lib/two-factor-auth.test.ts index 9fbc4ea80b68..9707e596b854 100644 --- a/apps/web/modules/ee/two-factor-auth/lib/two-factor-auth.test.ts +++ b/apps/web/modules/ee/two-factor-auth/lib/two-factor-auth.test.ts @@ -1,9 +1,9 @@ -import { afterEach, describe, expect, test, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import { prisma } from "@formbricks/database"; import { InvalidInputError, ResourceNotFoundError } from "@formbricks/types/errors"; import { symmetricDecrypt, symmetricEncrypt } from "@/lib/crypto"; +import { getCredentialPasswordHash, verifyUserPassword } from "@/lib/user/password"; import { totpAuthenticatorCheck } from "@/modules/auth/lib/totp"; -import { verifyPassword } from "@/modules/auth/lib/utils"; import { disableTwoFactorAuth, enableTwoFactorAuth, setupTwoFactorAuth } from "./two-factor-auth"; vi.mock("@formbricks/database", () => ({ @@ -20,8 +20,9 @@ vi.mock("@/lib/crypto", () => ({ symmetricDecrypt: vi.fn(), })); -vi.mock("@/modules/auth/lib/utils", () => ({ - verifyPassword: vi.fn(), +vi.mock("@/lib/user/password", () => ({ + getCredentialPasswordHash: vi.fn(), + verifyUserPassword: vi.fn(), })); vi.mock("@/modules/auth/lib/totp", () => ({ @@ -29,6 +30,14 @@ vi.mock("@/modules/auth/lib/totp", () => ({ })); describe("Two Factor Auth", () => { + beforeEach(() => { + // Happy-path defaults: enable checks credential presence (getCredentialPasswordHash); setup and + // disable delegate password verification to verifyUserPassword. "No password" / "incorrect + // password" tests override these. + vi.mocked(getCredentialPasswordHash).mockResolvedValue("hashedPassword"); + vi.mocked(verifyUserPassword).mockResolvedValue(true); + }); + afterEach(() => { vi.clearAllMocks(); }); @@ -42,15 +51,17 @@ describe("Two Factor Auth", () => { }); }); - test("setupTwoFactorAuth should throw InvalidInputError when user has no password", async () => { + test("setupTwoFactorAuth should reject when the user has no password (via verifyUserPassword)", async () => { vi.mocked(prisma.user.findUnique).mockResolvedValue({ id: "user123", - password: null, identityProvider: "email", } as any); + vi.mocked(verifyUserPassword).mockRejectedValue( + new InvalidInputError("Password is not set for this user") + ); await expect(setupTwoFactorAuth("user123", "password123")).rejects.toThrow( - new InvalidInputError("User does not have a password set") + new InvalidInputError("Password is not set for this user") ); }); @@ -72,7 +83,7 @@ describe("Two Factor Auth", () => { password: "hashedPassword", identityProvider: "email", } as any); - vi.mocked(verifyPassword).mockResolvedValue(false); + vi.mocked(verifyUserPassword).mockResolvedValue(false); await expect(setupTwoFactorAuth("user123", "wrongPassword")).rejects.toThrow( new InvalidInputError("Incorrect password") @@ -87,7 +98,7 @@ describe("Two Factor Auth", () => { email: "test@example.com", }; vi.mocked(prisma.user.findUnique).mockResolvedValue(mockUser as any); - vi.mocked(verifyPassword).mockResolvedValue(true); + vi.mocked(verifyUserPassword).mockResolvedValue(true); vi.mocked(symmetricEncrypt).mockImplementation((data) => `encrypted_${data}`); vi.mocked(prisma.user.update).mockResolvedValue(mockUser as any); @@ -113,9 +124,9 @@ describe("Two Factor Auth", () => { test("enableTwoFactorAuth should throw InvalidInputError when user has no password", async () => { vi.mocked(prisma.user.findUnique).mockResolvedValue({ id: "user123", - password: null, identityProvider: "email", } as any); + vi.mocked(getCredentialPasswordHash).mockResolvedValue(null); await expect(enableTwoFactorAuth("user123", "123456")).rejects.toThrow( new InvalidInputError("User does not have a password set") @@ -225,15 +236,18 @@ describe("Two Factor Auth", () => { }); }); - test("disableTwoFactorAuth should throw InvalidInputError when user has no password", async () => { + test("disableTwoFactorAuth should reject when the user has no password (via verifyUserPassword)", async () => { vi.mocked(prisma.user.findUnique).mockResolvedValue({ id: "user123", - password: null, identityProvider: "email", + twoFactorEnabled: true, } as any); + vi.mocked(verifyUserPassword).mockRejectedValue( + new InvalidInputError("Password is not set for this user") + ); await expect(disableTwoFactorAuth("user123", { password: "password123" })).rejects.toThrow( - new InvalidInputError("User does not have a password set") + new InvalidInputError("Password is not set for this user") ); }); @@ -270,7 +284,7 @@ describe("Two Factor Auth", () => { identityProvider: "email", twoFactorEnabled: true, } as any); - vi.mocked(verifyPassword).mockResolvedValue(false); + vi.mocked(verifyUserPassword).mockResolvedValue(false); await expect(disableTwoFactorAuth("user123", { password: "wrongPassword" })).rejects.toThrow( new InvalidInputError("Incorrect password") @@ -285,7 +299,7 @@ describe("Two Factor Auth", () => { twoFactorEnabled: true, backupCodes: "encrypted_backup_codes", } as any); - vi.mocked(verifyPassword).mockResolvedValue(true); + vi.mocked(verifyUserPassword).mockResolvedValue(true); vi.mocked(symmetricDecrypt).mockReturnValue(JSON.stringify(["code1", "code2"])); await expect( @@ -301,7 +315,7 @@ describe("Two Factor Auth", () => { twoFactorEnabled: true, twoFactorSecret: "encrypted_secret", } as any); - vi.mocked(verifyPassword).mockResolvedValue(true); + vi.mocked(verifyUserPassword).mockResolvedValue(true); vi.mocked(symmetricDecrypt).mockReturnValue("12345678901234567890123456789012"); vi.mocked(totpAuthenticatorCheck).mockReturnValue(false); @@ -319,7 +333,7 @@ describe("Two Factor Auth", () => { backupCodes: "encrypted_backup_codes", }; vi.mocked(prisma.user.findUnique).mockResolvedValue(mockUser as any); - vi.mocked(verifyPassword).mockResolvedValue(true); + vi.mocked(verifyUserPassword).mockResolvedValue(true); vi.mocked(symmetricDecrypt).mockReturnValue(JSON.stringify(["validcode", "code2"])); vi.mocked(prisma.user.update).mockResolvedValue(mockUser as any); @@ -348,7 +362,7 @@ describe("Two Factor Auth", () => { twoFactorSecret: "encrypted_secret", }; vi.mocked(prisma.user.findUnique).mockResolvedValue(mockUser as any); - vi.mocked(verifyPassword).mockResolvedValue(true); + vi.mocked(verifyUserPassword).mockResolvedValue(true); vi.mocked(symmetricDecrypt).mockReturnValue("12345678901234567890123456789012"); vi.mocked(totpAuthenticatorCheck).mockReturnValue(true); vi.mocked(prisma.user.update).mockResolvedValue(mockUser as any); diff --git a/apps/web/modules/ee/two-factor-auth/lib/two-factor-auth.ts b/apps/web/modules/ee/two-factor-auth/lib/two-factor-auth.ts index 8543c5c31451..13f3988c689e 100644 --- a/apps/web/modules/ee/two-factor-auth/lib/two-factor-auth.ts +++ b/apps/web/modules/ee/two-factor-auth/lib/two-factor-auth.ts @@ -5,8 +5,8 @@ import { prisma } from "@formbricks/database"; import { InvalidInputError, ResourceNotFoundError } from "@formbricks/types/errors"; import { ENCRYPTION_KEY } from "@/lib/constants"; import { symmetricDecrypt, symmetricEncrypt } from "@/lib/crypto"; +import { getCredentialPasswordHash, verifyUserPassword } from "@/lib/user/password"; import { totpAuthenticatorCheck } from "@/modules/auth/lib/totp"; -import { verifyPassword } from "@/modules/auth/lib/utils"; export const setupTwoFactorAuth = async ( userId: string, @@ -34,16 +34,13 @@ export const setupTwoFactorAuth = async ( throw new ResourceNotFoundError("user", userId); } - if (!user.password) { - throw new InvalidInputError("User does not have a password set"); - } - if (user.identityProvider !== "email") { throw new InvalidInputError("Third party login is already enabled"); } - const isCorrectPassword = await verifyPassword(password, user.password); - + // Password verification — the credential-account lookup and fail-closed "no password" handling — + // is owned by verifyUserPassword; 2FA setup just needs the yes/no answer. + const isCorrectPassword = await verifyUserPassword(userId, password); if (!isCorrectPassword) { throw new InvalidInputError("Incorrect password"); } @@ -81,14 +78,16 @@ export const enableTwoFactorAuth = async (id: string, code: string) => { throw new ResourceNotFoundError("user", id); } - if (!user.password) { - throw new InvalidInputError("User does not have a password set"); - } - if (user.identityProvider !== "email") { throw new InvalidInputError("Third party login is already enabled"); } + // Requires a credential account (password lives there post-ENG-1054, not on User.password). + const passwordHash = await getCredentialPasswordHash(id); + if (!passwordHash) { + throw new InvalidInputError("User does not have a password set"); + } + if (user.twoFactorEnabled) { throw new InvalidInputError("Two factor authentication is already enabled"); } @@ -142,10 +141,6 @@ export const disableTwoFactorAuth = async (id: string, params: TDisableTwoFactor throw new ResourceNotFoundError("user", id); } - if (!user.password) { - throw new InvalidInputError("User does not have a password set"); - } - if (!user.twoFactorEnabled) { throw new InvalidInputError("Two factor authentication is not enabled"); } @@ -155,8 +150,8 @@ export const disableTwoFactorAuth = async (id: string, params: TDisableTwoFactor } const { code, password, backupCode } = params; - const isCorrectPassword = await verifyPassword(password, user.password); - + // Delegate password verification (credential lookup + fail-closed) to verifyUserPassword. + const isCorrectPassword = await verifyUserPassword(id, password); if (!isCorrectPassword) { throw new InvalidInputError("Incorrect password"); } diff --git a/docs/self-hosting/configuration/environment-variables.mdx b/docs/self-hosting/configuration/environment-variables.mdx index f8aa6060e900..5e27f7e954e0 100644 --- a/docs/self-hosting/configuration/environment-variables.mdx +++ b/docs/self-hosting/configuration/environment-variables.mdx @@ -44,6 +44,7 @@ For `AI_PROVIDER=openai-compatible`, the LLM GA v1 self-hosted path is Qwen serv | IMPRINT_ADDRESS | Address for imprint. | optional | | | EMAIL_AUTH_DISABLED | Disables the ability for users to signup or login via email and password if set to 1. | optional | | | PASSWORD_RESET_DISABLED | Disables password reset functionality if set to 1. | optional | | +| PASSWORD_HIBP_CHECK_DISABLED | Disables the Have-I-Been-Pwned breach check on password set (signup / reset) if set to 1. The check calls api.pwnedpasswords.com and fails open on network errors; set this on air-gapped / closed-network deployments to skip the outbound call. | optional | | | PASSWORD_RESET_TOKEN_LIFETIME_MINUTES | Configures how long password reset links remain valid in minutes. Accepted values are integers from 5 to 120. | optional | 30 | | EMAIL_VERIFICATION_DISABLED | Disables email verification if set to 1. | optional | | | RATE_LIMITING_DISABLED | Disables only the application-level rate limiter if set to 1. It does not disable Envoy or an equivalent edge rate limiter. | optional | | diff --git a/packages/types/errors.ts b/packages/types/errors.ts index c96ff0a95495..b1a3ae1359a8 100644 --- a/packages/types/errors.ts +++ b/packages/types/errors.ts @@ -202,6 +202,14 @@ export const RESPONSE_ALREADY_FINISHED_ERROR_CODE = "response_already_finished"; */ export const SIGNUP_EMAIL_DOMAIN_BLOCKED_ERROR_CODE = "email_domain_not_allowed"; +/** + * Stable, locale-independent marker used when a password is rejected because it appears in the + * Have-I-Been-Pwned breach corpus (ENG-1587). Set as the `code` on the Better Auth APIError thrown + * by the breach-check plugin, then re-surfaced as the sign-up / reset action's `serverError` + * sentinel so the client can map it to a localized message. + */ +export const PASSWORD_COMPROMISED_ERROR_CODE = "password_compromised"; + export interface ApiErrorResponse { code: | "not_found" diff --git a/turbo.json b/turbo.json index a39d5fd8bd82..33249fcf93c1 100644 --- a/turbo.json +++ b/turbo.json @@ -316,6 +316,7 @@ "OIDC_DISPLAY_NAME", "OIDC_ISSUER", "OIDC_SIGNING_ALGORITHM", + "PASSWORD_HIBP_CHECK_DISABLED", "PASSWORD_RESET_DISABLED", "PASSWORD_RESET_TOKEN_LIFETIME_MINUTES", "PLAYWRIGHT_CI",