From 81e44afbe3c76096e00c0be809aa74a508417f6c Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 6 Aug 2026 10:17:14 +0200 Subject: [PATCH] :bug: Add backend password validation with complexity rules and dictionary check (#11059) * :bug: Add backend password validation with complexity rules and dictionary check Enforce minimum 8-character password length, require at least 1 lowercase letter, 1 uppercase letter, 1 digit, and 1 special character, and reject common passwords using Passay library with a 10k-entry wordlist from SecLists during registration and password change flows. AI-assisted-by: mimo-v2.5-pro * :sparkles: Improve user feedback When the password is invalid, the user now gets extra indications to make it stronger, so it can be valid. * :bug: Fix remove unneeded common password check The dictionary check is only relevant for passwords that meet all other requirements, but all 10,000 common passwords would fail the character requirements, so this check is not needed --------- Co-authored-by: Luis de Dios --- backend/deps.edn | 1 + backend/src/app/auth/passwords.clj | 53 +++++++++++ backend/src/app/rpc/commands/auth.clj | 5 + backend/src/app/rpc/commands/profile.clj | 4 + backend/test/backend_tests/helpers.clj | 2 +- .../test/backend_tests/rpc_profile_test.clj | 94 ++++++++++++++----- frontend/src/app/main/data/profile.cljs | 7 +- frontend/src/app/main/ui/auth/recovery.cljs | 16 +++- frontend/src/app/main/ui/auth/register.cljs | 17 +++- .../src/app/main/ui/components/forms.cljs | 10 +- .../src/app/main/ui/components/forms.scss | 10 ++ .../src/app/main/ui/settings/password.cljs | 8 ++ frontend/translations/en.po | 28 ++++++ frontend/translations/es.po | 28 ++++++ 14 files changed, 249 insertions(+), 34 deletions(-) create mode 100644 backend/src/app/auth/passwords.clj diff --git a/backend/deps.edn b/backend/deps.edn index 2599066e0b..1450f4de58 100644 --- a/backend/deps.edn +++ b/backend/deps.edn @@ -48,6 +48,7 @@ buddy/buddy-hashers {:mvn/version "2.0.167"} buddy/buddy-sign {:mvn/version "3.6.1-359"} + org.passay/passay {:mvn/version "1.6.6"} com.github.ben-manes.caffeine/caffeine {:mvn/version "3.2.4"} diff --git a/backend/src/app/auth/passwords.clj b/backend/src/app/auth/passwords.clj new file mode 100644 index 0000000000..cc75b202b7 --- /dev/null +++ b/backend/src/app/auth/passwords.clj @@ -0,0 +1,53 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS INC Sucursal en España SL + +(ns app.auth.passwords + "Password strength validation using Passay library." + (:require + [app.common.exceptions :as ex]) + (:import + [org.passay CharacterCharacteristicsRule CharacterRule EnglishCharacterData PasswordData])) + +(defonce ^:private passay-code->translation-key + {"INSUFFICIENT_LOWERCASE" "errors.weak-password.insufficient-lowercase" + "INSUFFICIENT_UPPERCASE" "errors.weak-password.insufficient-uppercase" + "INSUFFICIENT_DIGIT" "errors.weak-password.insufficient-digits" + "INSUFFICIENT_SPECIAL" "errors.weak-password.insufficient-special"}) + +(defonce ^:private character-characteristics-rule + (doto (CharacterCharacteristicsRule.) + (.setRules [(CharacterRule. EnglishCharacterData/LowerCase 1) + (CharacterRule. EnglishCharacterData/UpperCase 1) + (CharacterRule. EnglishCharacterData/Digit 1) + (CharacterRule. EnglishCharacterData/Special 1)]) + (.setNumberOfCharacteristics 4))) + +(defn validate-password + "Validates password strength. + Returns nil if valid, or raises exception if invalid. + Checks: + - Minimum length of 8 characters + - At least 1 lowercase letter + - At least 1 uppercase letter + - At least 1 digit + - At least 1 special character" + [password] + (when (< (count password) 8) + (ex/raise :type :validation + :code :weak-password + :hint "password must be at least 8 characters" + :details ["errors.weak-password.too-short"])) + + (let [password-data (PasswordData. password) + char-result (.validate character-characteristics-rule password-data)] + (when-not (.isValid char-result) + (ex/raise :type :validation + :code :weak-password + :hint "password must contain at least 1 lowercase letter, 1 uppercase letter, 1 digit, and 1 special character" + :details (->> (.getDetails char-result) + (mapv #(.getErrorCode %)) + (mapv passay-code->translation-key) + (filterv some?)))))) diff --git a/backend/src/app/rpc/commands/auth.clj b/backend/src/app/rpc/commands/auth.clj index 933411a489..07460633c4 100644 --- a/backend/src/app/rpc/commands/auth.clj +++ b/backend/src/app/rpc/commands/auth.clj @@ -8,6 +8,7 @@ (:require [app.auth :as auth] [app.auth.oidc :as oidc] + [app.auth.passwords :as passwords] [app.common.data :as d] [app.common.exceptions :as ex] [app.common.features :as cfeat] @@ -182,6 +183,7 @@ (db/update! conn :profile {:password pwd :is-active true} {:id profile-id}) nil))] + (passwords/validate-password password) (->> (validate-token token) (update-password conn)) @@ -240,6 +242,9 @@ :code :email-as-password :hint "you can't use your email as password")) + ;; Validate password strength against common password dictionary + (passwords/validate-password (:password params)) + (when (eml/has-bounce-reports? cfg (:email params)) (ex/raise :type :restriction :code :email-has-permanent-bounces diff --git a/backend/src/app/rpc/commands/profile.clj b/backend/src/app/rpc/commands/profile.clj index 36d02ba2d9..26716cc411 100644 --- a/backend/src/app/rpc/commands/profile.clj +++ b/backend/src/app/rpc/commands/profile.clj @@ -7,6 +7,7 @@ (ns app.rpc.commands.profile (:require [app.auth :as auth] + [app.auth.passwords :as passwords] [app.common.data :as d] [app.common.exceptions :as ex] [app.common.schema :as sm] @@ -212,6 +213,9 @@ :code :email-as-password :hint "you can't use your email as password")) + ;; Validate password strength against common password dictionary + (passwords/validate-password (:password params)) + (update-profile-password! cfg (assoc profile :password password)) (->> (rph/get-request params) diff --git a/backend/test/backend_tests/helpers.clj b/backend/test/backend_tests/helpers.clj index f839f222b9..0f582497d9 100644 --- a/backend/test/backend_tests/helpers.clj +++ b/backend/test/backend_tests/helpers.clj @@ -189,7 +189,7 @@ (let [params (merge {:id (mk-uuid "profile" i) :fullname (str "Profile " i) :email (str "profile" i ".test@nodomain.com") - :password "123123" + :password "Test123!" :is-demo false} params)] (db/run! system diff --git a/backend/test/backend_tests/rpc_profile_test.clj b/backend/test/backend_tests/rpc_profile_test.clj index ffbb55ca7c..f846cfb343 100644 --- a/backend/test/backend_tests/rpc_profile_test.clj +++ b/backend/test/backend_tests/rpc_profile_test.clj @@ -42,7 +42,7 @@ (let [profile (th/create-profile* 1) data {::th/type :login-with-password :email "profile1.test@nodomain.com" - :password "foobar"} + :password "Foobar12!"} out (th/command! data)] #_(th/print-result! out) @@ -56,7 +56,7 @@ (let [profile (th/create-profile* 1) data {::th/type :login-with-password :email "profile1.test@nodomain.com" - :password "123123"} + :password "Test123!"} out (th/command! data)] ;; (th/print-result! out) (let [error (:error out)] @@ -69,7 +69,7 @@ (let [profile (th/create-profile* 1 {:is-active true}) data {::th/type :login-with-password :email "profile1.test@nodomain.com" - :password "123123"} + :password "Test123!"} out (th/command! data)] ;; (th/print-result! out) (t/is (nil? (:error out))) @@ -403,7 +403,7 @@ (let [data {::th/type :prepare-register-profile :email "user@example.com" :fullname "foobar" - :password "foobar" + :password "Foobar12!" :utm_campaign "utma" :mtm_campaign "mtma"} out (th/command! data) @@ -444,7 +444,7 @@ (let [data {::th/type :prepare-register-profile :email "hello@example.com" :fullname "foobar" - :password "foobar"} + :password "Foobar12!"} out (th/command! data) token (get-in out [:result :token])] (t/is (th/success? out)) @@ -463,7 +463,7 @@ (let [data {::th/type :prepare-register-profile :email "hello@example.com" :fullname "foobar" - :password "foobar"} + :password "Foobar12!"} out (th/command! data) token (get-in out [:result :token])] (t/is (th/success? out)) @@ -498,7 +498,7 @@ (let [data {::th/type :prepare-register-profile :email "hello@example.com" :fullname "foobar" - :password "foobar"} + :password "Foobar12!"} out (th/command! data) token (get-in out [:result :token])] (t/is (th/success? out)) @@ -521,7 +521,7 @@ (let [data {::th/type :prepare-register-profile :email "hello@example.com" :fullname "foobar" - :password "foobar"} + :password "Foobar12!"} out (th/command! data) token (get-in out [:result :token])] (t/is (th/success? out)) @@ -547,7 +547,7 @@ (let [data {::th/type :prepare-register-profile :email "hello@example.com" :fullname "foobar" - :password "foobar"} + :password "Foobar12!"} out (th/command! data) token (get-in out [:result :token])] (t/is (th/success? out)) @@ -576,7 +576,7 @@ (let [data {::th/type :prepare-register-profile :email "hello@example.com" :fullname "foobar" - :password "foobar"} + :password "Foobar12!"} out (th/command! data) token (get-in out [:result :token])] (t/is (th/success? out)) @@ -614,7 +614,7 @@ :invitation-token itoken :fullname "foobar" :email "user@example.com" - :password "foobar"} + :password "Foobar12!"} {prep-result :result prep-error :error} (th/command! prep-data)] (t/is (nil? prep-error)) @@ -659,7 +659,7 @@ :invitation-token itoken :fullname "foobar" :email "user@example.com" - :password "foobar"} + :password "Foobar12!"} {prep-result :result prep-error :error} (th/command! prep-data)] (t/is (nil? prep-error)) @@ -692,7 +692,7 @@ :invitation-token itoken :email "user@example.com" :fullname "foobar" - :password "foobar"} + :password "Foobar12!"} out (th/command! data)] (t/is (not (th/success? out))) @@ -712,7 +712,7 @@ :invitation-token itoken :fullname "foobar" :email "user@example.com" - :password "foobar"} + :password "Foobar12!"} out (th/command! data)] (t/is (not (th/success? out))) @@ -733,7 +733,7 @@ :invitation-token itoken :email "user@example.com" :fullname "foobar" - :password "foobar"} + :password "Foobar12!"} out (th/command! data)] (t/is (not (th/success? out))) @@ -754,7 +754,7 @@ :invitation-token itoken :fullname "foobar" :email "user@example.com" - :password "foobar"} + :password "Foobar12!"} out (th/command! data)] (t/is (not (th/success? out))) @@ -767,7 +767,7 @@ (let [data {::th/type :prepare-register-profile :fullname "foobar" :email "user@example.com" - :password "foobar"} + :password "Foobar12!"} out (th/command! data)] (t/is (not (th/success? out))) @@ -780,7 +780,7 @@ data {::th/type :prepare-register-profile :fullname "foobar" :email (:email profile) - :password "foobar"} + :password "Foobar12!"} out (th/command! data)] ;; (th/print-result! out) (t/is (th/success? out)) @@ -793,7 +793,7 @@ data {::th/type :prepare-register-profile :fullname "foobar" :email "user@example.com" - :password "foobar"}] + :password "Foobar12!"}] (th/create-global-complaint-for pool {:type :bounce :email "user@example.com"}) @@ -808,7 +808,7 @@ data {::th/type :prepare-register-profile :fullname "foobar" :email "user@example.com" - :password "foobar"}] + :password "Foobar12!"}] (th/create-global-complaint-for pool {:type :complaint :email "user@example.com"}) @@ -1131,8 +1131,8 @@ (let [profile (th/create-profile* 1) data {::th/type :update-profile-password ::rpc/profile-id (:id profile) - :old-password "123123" - :password "foobarfoobar"} + :old-password "Test123!" + :password "Foobar12!"} out (th/command! data)] (t/is (nil? (:error out))) (t/is (nil? (:result out))))) @@ -1143,7 +1143,7 @@ data {::th/type :update-profile-password ::rpc/profile-id (:id profile) :old-password "badpassword" - :password "foobarfoobar"} + :password "Foobar12!"} {:keys [result error] :as out} (th/command! data)] (t/is (th/ex-info? error)) (t/is (th/ex-of-type? error :validation)) @@ -1154,7 +1154,7 @@ (let [profile (th/create-profile* 1) data {::th/type :update-profile-password ::rpc/profile-id (:id profile) - :old-password "123123" + :old-password "Test123!" :password "profile1.test@nodomain.com"} {:keys [result error] :as out} (th/command! data)] (t/is (th/ex-info? error)) @@ -1271,3 +1271,49 @@ (t/is (th/ex-info? (:error out))) (t/is (th/ex-of-type? (:error out) :validation)) (t/is (th/ex-of-code? (:error out) :params-validation)))) + + +(t/deftest prepare-register-profile-password-too-short + (let [data {::th/type :prepare-register-profile + :email "user@example.com" + :fullname "foobar" + :password "123"} + out (th/command! data)] + (t/is (th/ex-info? (:error out))) + (t/is (th/ex-of-type? (:error out) :validation)) + (t/is (th/ex-of-code? (:error out) :weak-password)))) + + +(t/deftest prepare-register-profile-weak-password + (let [data {::th/type :prepare-register-profile + :email "user@example.com" + :fullname "foobar" + :password "password123"} + out (th/command! data)] + (t/is (th/ex-info? (:error out))) + (t/is (th/ex-of-type? (:error out) :validation)) + (t/is (th/ex-of-code? (:error out) :weak-password)))) + + +(t/deftest update-profile-password-too-short + (let [profile (th/create-profile* 1) + data {::th/type :update-profile-password + ::rpc/profile-id (:id profile) + :old-password "Test123!" + :password "123"} + out (th/command! data)] + (t/is (th/ex-info? (:error out))) + (t/is (th/ex-of-type? (:error out) :validation)) + (t/is (th/ex-of-code? (:error out) :weak-password)))) + + +(t/deftest update-profile-password-weak-password + (let [profile (th/create-profile* 1) + data {::th/type :update-profile-password + ::rpc/profile-id (:id profile) + :old-password "Test123!" + :password "qwerty"} + out (th/command! data)] + (t/is (th/ex-info? (:error out))) + (t/is (th/ex-of-type? (:error out) :validation)) + (t/is (th/ex-of-code? (:error out) :weak-password)))) diff --git a/frontend/src/app/main/data/profile.cljs b/frontend/src/app/main/data/profile.cljs index e37dc08e45..93a948c119 100644 --- a/frontend/src/app/main/data/profile.cljs +++ b/frontend/src/app/main/data/profile.cljs @@ -462,11 +462,14 @@ ptk/WatchEvent (watch [_ _ _] (let [{:keys [on-error on-success] - :or {on-error rx/throw + :or {on-error identity on-success identity}} (meta data)] (->> (rp/cmd! :recover-profile data) (rx/tap on-success) - (rx/catch on-error))))))) + (rx/catch (fn [err] + (on-error err) + (rx/empty))) + (rx/ignore))))))) ;; --- EVENT: fetch-team-webhooks diff --git a/frontend/src/app/main/ui/auth/recovery.cljs b/frontend/src/app/main/ui/auth/recovery.cljs index 5ee49525db..d91f200db9 100644 --- a/frontend/src/app/main/ui/auth/recovery.cljs +++ b/frontend/src/app/main/ui/auth/recovery.cljs @@ -28,8 +28,18 @@ (= password-1 password-2))]]) (defn- on-error - [_form _error] - (st/emit! (ntf/error (tr "errors.invalid-recovery-token")))) + [form error] + (let [{:keys [type code] :as edata} (ex-data error)] + (if (= [:validation :weak-password] [type code]) + (let [details (:details edata) + options (when (seq details) + (mapv tr details))] + (swap! form assoc-in [:extra-errors :password-1] + {:message (tr "errors.weak-password") + :options options})) + + (let [msg (tr "errors.invalid-recovery-token")] + (st/emit! (ntf/error msg)))))) (defn- on-success [_] @@ -38,7 +48,7 @@ (defn- on-submit [form _event] - (let [mdata {:on-error on-error + (let [mdata {:on-error (partial on-error form) :on-success on-success} params {:token (get-in @form [:clean-data :token]) :password (get-in @form [:clean-data :password-2])}] diff --git a/frontend/src/app/main/ui/auth/register.cljs b/frontend/src/app/main/ui/auth/register.cljs index bb8966d12b..be5f3f280b 100644 --- a/frontend/src/app/main/ui/auth/register.cljs +++ b/frontend/src/app/main/ui/auth/register.cljs @@ -21,6 +21,7 @@ [app.util.i18n :as i18n :refer [tr]] [app.util.storage :as storage] [beicon.v2.core :as rx] + [cuerdas.core :as str] [rumext.v2 :as mf])) ;; --- PAGE: Register @@ -103,8 +104,20 @@ (st/emit! (ntf/error (tr "errors.email-already-exists"))) [:validation :email-as-password] - (swap! form assoc-in [:errors :password] - {:message (tr "errors.email-as-password")}) + (st/emit! (ntf/error (tr "errors.email-as-password"))) + + [:validation :weak-password] + (let [details (:details edata) + items (when (seq details) + (->> details + (map #(str "
  • " (tr %) "
    • ")) + (str/join ""))) + detail (when items + (str ""))] + (st/emit! (ntf/show {:content (tr "errors.weak-password") + :detail detail + :type :toast + :level :error}))) (do (when-let [explain (get edata :explain)] diff --git a/frontend/src/app/main/ui/components/forms.cljs b/frontend/src/app/main/ui/components/forms.cljs index 88585a753e..73632ea4eb 100644 --- a/frontend/src/app/main/ui/components/forms.cljs +++ b/frontend/src/app/main/ui/components/forms.cljs @@ -180,11 +180,17 @@ (cond (and touched? (:message error) show-error) - (let [message (:message error)] + (let [message (:message error) + options (:options error)] [:div {:id (dm/str "error-" input-name) :class (stl/css :error) :data-testid (dm/str data-testid "-error")} - message]) + message + (when (seq options) + [:ul {:class (stl/css :error-options)} + (for [opt options] + [:li {:key opt + :class (stl/css :error-option)} opt])])]) ;; FIXME: DEPRECATED (and touched? (:code error) show-error) diff --git a/frontend/src/app/main/ui/components/forms.scss b/frontend/src/app/main/ui/components/forms.scss index 30901f43ec..a9aed792e3 100644 --- a/frontend/src/app/main/ui/components/forms.scss +++ b/frontend/src/app/main/ui/components/forms.scss @@ -168,6 +168,16 @@ font-size: deprecated.$fs-14; } +.error-options { + margin-block: var(--sp-xxs); + padding-inline-start: var(--sp-l); + list-style-type: disc; +} + +.error-option { + margin-block: var(--sp-xxs); +} + .hint { @include t.use-typography("body-small"); diff --git a/frontend/src/app/main/ui/settings/password.cljs b/frontend/src/app/main/ui/settings/password.cljs index cf3d60ed8d..96d0003001 100644 --- a/frontend/src/app/main/ui/settings/password.cljs +++ b/frontend/src/app/main/ui/settings/password.cljs @@ -28,6 +28,14 @@ (swap! form assoc-in [:extra-errors :password-1] {:message (tr "errors.email-as-password")}) + :weak-password + (let [details (:details data) + options (when (seq details) + (mapv tr details))] + (swap! form assoc-in [:extra-errors :password-1] + {:message (tr "errors.weak-password") + :options options})) + (let [msg (tr "generic.error")] (st/emit! (ntf/error msg)))))) diff --git a/frontend/translations/en.po b/frontend/translations/en.po index 641b3e014d..b9621af9e4 100644 --- a/frontend/translations/en.po +++ b/frontend/translations/en.po @@ -1748,6 +1748,34 @@ msgstr "Confirmation password must match" msgid "errors.password-too-short" msgstr "Password should at least be 8 characters" +#: src/app/main/ui/settings/password.cljs, src/app/main/ui/auth/register.cljs +msgid "errors.weak-password" +msgstr "Password does not meet the requirements" + +#: src/app/main/ui/settings/password.cljs, src/app/main/ui/auth/register.cljs +msgid "errors.weak-password.too-short" +msgstr "At least 8 characters" + +#: src/app/main/ui/settings/password.cljs, src/app/main/ui/auth/register.cljs +msgid "errors.weak-password.insufficient-lowercase" +msgstr "At least 1 lowercase letter" + +#: src/app/main/ui/settings/password.cljs, src/app/main/ui/auth/register.cljs +msgid "errors.weak-password.insufficient-uppercase" +msgstr "At least 1 uppercase letter" + +#: src/app/main/ui/settings/password.cljs, src/app/main/ui/auth/register.cljs +msgid "errors.weak-password.insufficient-digits" +msgstr "At least 1 digit" + +#: src/app/main/ui/settings/password.cljs, src/app/main/ui/auth/register.cljs +msgid "errors.weak-password.insufficient-special" +msgstr "At least 1 special character" + +#: src/app/main/ui/settings/password.cljs, src/app/main/ui/auth/register.cljs +msgid "errors.weak-password.in-dictionary" +msgstr "Password is too common" + #: src/app/main/errors.cljs:267 msgid "errors.paste-data-validation" msgstr "Invalid data in clipboard" diff --git a/frontend/translations/es.po b/frontend/translations/es.po index 1d051dbaf4..51165035e2 100644 --- a/frontend/translations/es.po +++ b/frontend/translations/es.po @@ -1717,6 +1717,34 @@ msgstr "La contraseña de confirmación debe coincidir" msgid "errors.password-too-short" msgstr "La contraseña debe tener 8 caracteres como mínimo" +#: src/app/main/ui/settings/password.cljs, src/app/main/ui/auth/register.cljs +msgid "errors.weak-password" +msgstr "La contraseña no cumple los requisitos" + +#: src/app/main/ui/settings/password.cljs, src/app/main/ui/auth/register.cljs +msgid "errors.weak-password.too-short" +msgstr "Al menos 8 caracteres" + +#: src/app/main/ui/settings/password.cljs, src/app/main/ui/auth/register.cljs +msgid "errors.weak-password.insufficient-lowercase" +msgstr "Al menos 1 letra minúscula" + +#: src/app/main/ui/settings/password.cljs, src/app/main/ui/auth/register.cljs +msgid "errors.weak-password.insufficient-uppercase" +msgstr "Al menos 1 letra mayúscula" + +#: src/app/main/ui/settings/password.cljs, src/app/main/ui/auth/register.cljs +msgid "errors.weak-password.insufficient-digits" +msgstr "Al menos 1 dígito" + +#: src/app/main/ui/settings/password.cljs, src/app/main/ui/auth/register.cljs +msgid "errors.weak-password.insufficient-special" +msgstr "Al menos 1 carácter especial" + +#: src/app/main/ui/settings/password.cljs, src/app/main/ui/auth/register.cljs +msgid "errors.weak-password.in-dictionary" +msgstr "La contraseña es demasiado común" + #: src/app/main/errors.cljs:267 msgid "errors.paste-data-validation" msgstr "Datos inválidos en el portapapeles"