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"