From e168e5d136a77dba67b6d479af133e75061ccb2a Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 27 Aug 2026 10:34:48 +0000 Subject: [PATCH] :bug: Address code review feedback on password notification - Send password-changed notification from recover-profile too (forgot-password reset path was missing the email). - Strengthen test assertions to verify email factory, recipient, and name via :call-args-list instead of just call-count. - Add negative test: no email sent when old-password is wrong. - Wrap pre-existing update-profile-password test with send! mock to keep its scope focused. Ref: PR #11393 AI-assisted-by: longcat-2.0 --- backend/src/app/rpc/commands/auth.clj | 12 +++- .../test/backend_tests/rpc_profile_test.clj | 69 ++++++++++++++----- 2 files changed, 59 insertions(+), 22 deletions(-) diff --git a/backend/src/app/rpc/commands/auth.clj b/backend/src/app/rpc/commands/auth.clj index b134d2a0c9..ed2892ee28 100644 --- a/backend/src/app/rpc/commands/auth.clj +++ b/backend/src/app/rpc/commands/auth.clj @@ -181,11 +181,17 @@ (update-password [conn profile-id] (let [pwd (auth/derive-password password)] (db/update! conn :profile {:password pwd :is-active true} {:id profile-id}) - nil))] + (db/get-by-id conn :profile profile-id)))] (passwords/validate-password password) - (->> (validate-token token) - (update-password conn)) + + (let [profile (->> (validate-token token) + (update-password conn))] + (eml/send! {::eml/conn conn + ::eml/factory eml/password-changed + :public-uri (cf/get :public-uri) + :to (:email profile) + :name (:fullname profile)})) nil)) diff --git a/backend/test/backend_tests/rpc_profile_test.clj b/backend/test/backend_tests/rpc_profile_test.clj index 6f8cabbe2b..a9f589d649 100644 --- a/backend/test/backend_tests/rpc_profile_test.clj +++ b/backend/test/backend_tests/rpc_profile_test.clj @@ -10,6 +10,7 @@ [app.common.uuid :as uuid] [app.config :as cf] [app.db :as db] + [app.email :as eml] [app.email.blacklist :as email.blacklist] [app.email.whitelist :as email.whitelist] [app.nitrate :as nitrate] @@ -1164,26 +1165,29 @@ (t/deftest update-profile-password - (let [profile (th/create-profile* 1) - data {::th/type :update-profile-password - ::rpc/profile-id (:id profile) - :old-password "Test123!" - :password "Foobar12!"} - out (th/command! data)] - (t/is (nil? (:error out))) - (t/is (nil? (:result out))))) + (with-mocks [_ {:target 'app.email/send! :return nil}] + (let [profile (th/create-profile* 1) + data {::th/type :update-profile-password + ::rpc/profile-id (:id profile) + :old-password "Test123!" + :password "Foobar12!"} + out (th/command! data)] + (t/is (nil? (:error out))) + (t/is (nil? (:result out)))))) (t/deftest update-profile-password-bad-old-password - (let [profile (th/create-profile* 1) - data {::th/type :update-profile-password - ::rpc/profile-id (:id profile) - :old-password "badpassword" - :password "Foobar12!"} - {:keys [result error] :as out} (th/command! data)] - (t/is (th/ex-info? error)) - (t/is (th/ex-of-type? error :validation)) - (t/is (th/ex-of-code? error :old-password-not-match)))) + (with-mocks [mock {:target 'app.email/send! :return nil}] + (let [profile (th/create-profile* 1) + data {::th/type :update-profile-password + ::rpc/profile-id (:id profile) + :old-password "badpassword" + :password "Foobar12!"} + {:keys [result error] :as out} (th/command! data)] + (t/is (th/ex-info? error)) + (t/is (th/ex-of-type? error :validation)) + (t/is (th/ex-of-code? error :old-password-not-match)) + (t/is (= 0 (:call-count @mock)))))) (t/deftest update-profile-password-email-as-password @@ -1365,7 +1369,11 @@ out (th/command! data)] (t/is (nil? (:error out))) (t/is (nil? (:result out))) - (t/is (= 1 (:call-count @mock)))))) + (t/is (= 1 (:call-count @mock))) + (let [{:keys [::eml/factory :to :name]} (first (:call-args-list @mock))] + (t/is (= eml/password-changed factory)) + (t/is (= (:email profile) to)) + (t/is (= (:fullname profile) name)))))) (t/deftest update-profile-password-sends-notification-for-first-password @@ -1377,4 +1385,27 @@ out (th/command! data)] (t/is (nil? (:error out))) (t/is (nil? (:result out))) - (t/is (= 1 (:call-count @mock)))))) + (t/is (= 1 (:call-count @mock))) + (let [{:keys [::eml/factory :to :name]} (first (:call-args-list @mock))] + (t/is (= eml/password-changed factory)) + (t/is (= (:email profile) to)) + (t/is (= (:fullname profile) name)))))) + + +(t/deftest recover-profile-sends-notification + (with-mocks [mock {:target 'app.email/send! :return nil}] + (let [profile (th/create-profile* 1) + token (tokens/generate th/*system* + {:iss :password-recovery + :exp (ct/in-future "15m") + :profile-id (:id profile)}) + data {::th/type :recover-profile + :token token + :password "Foobar12!"} + out (th/command! data)] + (t/is (nil? (:error out))) + (t/is (= 1 (:call-count @mock))) + (let [{:keys [::eml/factory :to :name]} (first (:call-args-list @mock))] + (t/is (= eml/password-changed factory)) + (t/is (= (:email profile) to)) + (t/is (= (:fullname profile) name))))))