Don't throw with invalid address on password reset

If the user doesn't enter a value on the password reset form, we look up
a user with an email of "", which matches deleted users. We then try to
send an email to that address, which throws.

This now catches and logs that case, and gives the default message to
the user that we will send an email if a user was found.

Author: tobias

src/clojars/email.clj

@@ -1,5 +1,6 @@
 (ns clojars.email
   (:require
+   [clojars.config :as config]
    [clojars.log :as log]
    [clojure.edn :as edn]
    [clojure.java.io :as io])
@@ -15,29 +16,35 @@
 (def ^:private email-denylist
   (edn/read-string (slurp (io/resource "email-denylist.edn"))))
 
-(defn simple-mailer [{:keys [hostname username password port tls? from]}]
+(defn- build-email
+  [{:keys [hostname username password port tls? from]} ^String to subject message]
+  (if (contains? email-denylist to)
+    (log/info {:status :denylist})
+    (let [mail (doto (SimpleEmail.)
+                 (.setHostName (or hostname "localhost"))
+                 (.setSmtpPort (or port 25))
+                 (.setStartTLSEnabled (boolean tls?))
+                 (.setStartTLSRequired (boolean tls?))
+                 (.setFrom (or from "contact@clojars.org") "Clojars")
+                 (.addTo to)
+                 (.setSubject subject)
+                 (.setMsg message))]
+      (when tls?
+        (.setSslSmtpPort mail (str (or port 25))))
+      (when (and username password)
+        (.setAuthentication mail username password))
+      mail)))
+
+(defn simple-mailer
+  [config]
   (fn [^String to subject message]
     (log/with-context {:tag :email
                        :email-to to
                        :email-subject subject}
       (try
-        (if (contains? email-denylist to)
-          (log/info {:status :denylist})
-          (let [mail (doto (SimpleEmail.)
-                       (.setHostName (or hostname "localhost"))
-                       (.setSmtpPort (or port 25))
-                       (.setStartTLSEnabled (boolean tls?))
-                       (.setStartTLSRequired (boolean tls?))
-                       (.setFrom (or from "contact@clojars.org") "Clojars")
-                       (.addTo to)
-                       (.setSubject subject)
-                       (.setMsg message))]
-            (when tls?
-              (.setSslSmtpPort mail (str (or port 25))))
-            (when (and username password)
-              (.setAuthentication mail username password))
-            (.send mail)
-            (log/info {:status :success})))
+        (let [^SimpleEmail mail (build-email config to subject message)]
+          (.send mail)
+          (log/info {:status :success}))
         (catch Exception e
           (log/error {:status :failed
                       :error e})
@@ -67,5 +74,7 @@
   (defn mock-mailer []
     (expect-mock-emails)
     (fn [to subject message]
-      (swap! mock-emails conj [to subject message])
+      ;; Allows us to test the email generation logic
+      (let [email (build-email (:mail (config/config)) to subject message)]
+        (swap! mock-emails conj [to subject message email]))
       (.countDown ^CountDownLatch @email-latch))))

src/clojars/web/user.clj

@@ -191,24 +191,32 @@
                                   :email-or-username)
                       (submit-button "Email me a password reset link"))]))
 
+(defn- send-forgot-password-email
+  [db mailer user details]
+  (let [reset-code (db/set-password-reset-code! db (:user user))
+        base-url (:base-url (config/config))
+        reset-password-url (str base-url "/password-resets/" reset-code)]
+    (log/info {:tag :password-reset-code-generated})
+    (try
+      (mailer (:email user)
+              "Password reset for Clojars"
+              (->> ["Hello,"
+                    (format "We received a request from someone, hopefully you, to reset the password of the clojars user: %s." (:user user))
+                    "To continue with the reset password process, click on the following link:"
+                    reset-password-url
+                    "This link is valid for 24 hours, after which you will need to generate a new one."
+                    (notif-common/details-table details)
+                    "If you didn't reset your password then you can ignore this email."]
+                   (interpose "\n\n")
+                   (apply str)))
+      (catch Exception e
+        (log/error {:tag :failed-password-reset-email
+                    :error e})))))
+
 (defn forgot-password [db mailer {:keys [email-or-username]} details]
   (log/with-context {:email-or-username email-or-username}
     (if-let [user (find-user-by-user-or-email db email-or-username)]
-      (let [reset-code (db/set-password-reset-code! db (:user user))
-            base-url (:base-url (config/config))
-            reset-password-url (str base-url "/password-resets/" reset-code)]
-        (log/info {:tag :password-reset-code-generated})
-        (mailer (:email user)
-                "Password reset for Clojars"
-                (->> ["Hello,"
-                      (format "We received a request from someone, hopefully you, to reset the password of the clojars user: %s." (:user user))
-                      "To continue with the reset password process, click on the following link:"
-                      reset-password-url
-                      "This link is valid for 24 hours, after which you will need to generate a new one."
-                      (notif-common/details-table details)
-                      "If you didn't reset your password then you can ignore this email."]
-                     (interpose "\n\n")
-                     (apply str))))
+      (send-forgot-password-email db mailer user details)
       (log/info {:tag :password-reset-user-not-found})))
   (html-doc "Forgot password?" {}
             [:div.small-section [:h1 "Forgot password?"]

test/clojars/integration/users_test.clj

@@ -360,6 +360,23 @@
     (is (re-find #"has changed the password on your 'fixture'" body))
     (is (re-find #"Client IP" body))))
 
+(deftest user-can-enter-invalid-email-for-password-reset
+  (email/expect-mock-emails 0)
+  (-> (session (help/app))
+      (register-as "fixture" "fixture@example.org" "password"))
+  ;; force an invalid email address
+  (db/update-user help/*db* "fixture" "" nil)
+  (-> (session (help/app))
+      (visit "/")
+      (follow "login")
+      (follow "Forgot your username or password?")
+      (fill-in "Email or Username" "fixture")
+      (press "Email me a password reset link")
+      (has (status? 200))
+      (within [:p]
+        (has (text? "If your account was found, you should get an email with a link to reset your password soon."))))
+  (is (empty? @email/mock-emails)))
+
 (deftest bad-reset-code-shows-message
   (-> (session (help/app))
       (visit "/password-resets/this-code-does-not-exist")