🐛 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
This commit is contained in:
Andrey Antukh 2026-08-27 10:34:48 +00:00
parent 06b009938c
commit e168e5d136
2 changed files with 59 additions and 22 deletions

View File

@ -181,11 +181,17 @@
(update-password [conn profile-id] (update-password [conn profile-id]
(let [pwd (auth/derive-password password)] (let [pwd (auth/derive-password password)]
(db/update! conn :profile {:password pwd :is-active true} {:id profile-id}) (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) (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)) nil))

View File

@ -10,6 +10,7 @@
[app.common.uuid :as uuid] [app.common.uuid :as uuid]
[app.config :as cf] [app.config :as cf]
[app.db :as db] [app.db :as db]
[app.email :as eml]
[app.email.blacklist :as email.blacklist] [app.email.blacklist :as email.blacklist]
[app.email.whitelist :as email.whitelist] [app.email.whitelist :as email.whitelist]
[app.nitrate :as nitrate] [app.nitrate :as nitrate]
@ -1164,26 +1165,29 @@
(t/deftest update-profile-password (t/deftest update-profile-password
(let [profile (th/create-profile* 1) (with-mocks [_ {:target 'app.email/send! :return nil}]
data {::th/type :update-profile-password (let [profile (th/create-profile* 1)
::rpc/profile-id (:id profile) data {::th/type :update-profile-password
:old-password "Test123!" ::rpc/profile-id (:id profile)
:password "Foobar12!"} :old-password "Test123!"
out (th/command! data)] :password "Foobar12!"}
(t/is (nil? (:error out))) out (th/command! data)]
(t/is (nil? (:result out))))) (t/is (nil? (:error out)))
(t/is (nil? (:result out))))))
(t/deftest update-profile-password-bad-old-password (t/deftest update-profile-password-bad-old-password
(let [profile (th/create-profile* 1) (with-mocks [mock {:target 'app.email/send! :return nil}]
data {::th/type :update-profile-password (let [profile (th/create-profile* 1)
::rpc/profile-id (:id profile) data {::th/type :update-profile-password
:old-password "badpassword" ::rpc/profile-id (:id profile)
:password "Foobar12!"} :old-password "badpassword"
{:keys [result error] :as out} (th/command! data)] :password "Foobar12!"}
(t/is (th/ex-info? error)) {:keys [result error] :as out} (th/command! data)]
(t/is (th/ex-of-type? error :validation)) (t/is (th/ex-info? error))
(t/is (th/ex-of-code? error :old-password-not-match)))) (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 (t/deftest update-profile-password-email-as-password
@ -1365,7 +1369,11 @@
out (th/command! data)] out (th/command! data)]
(t/is (nil? (:error out))) (t/is (nil? (:error out)))
(t/is (nil? (:result 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 (t/deftest update-profile-password-sends-notification-for-first-password
@ -1377,4 +1385,27 @@
out (th/command! data)] out (th/command! data)]
(t/is (nil? (:error out))) (t/is (nil? (:error out)))
(t/is (nil? (:result 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))))))