Autocorrect email security settings where possible

src/metabase/api/email.clj

 (ns metabase.api.email
   "/api/email endpoints"
-  (:require [clojure.tools.logging :as log]
-            [clojure.set :as set]
-            [compojure.core :refer [GET PUT DELETE POST]]
+  (:require [clojure
+             [data :as data]
+             [set :as set]
+             [string :as string]]
+            [clojure.tools.logging :as log]
+            [compojure.core :refer [POST PUT]]
+            [metabase
+             [config :as config]
+             [email :as email]]
             [metabase.api.common :refer :all]
-            [metabase.config :as config]
-            [metabase.email :as email]
             [metabase.models.setting :as setting]
             [metabase.util.schema :as su]))
 
@@ -51,6 +55,14 @@
         #".*"
         {:message "Sorry, something went wrong.  Please try again."}))))
 
+(defn humanize-email-corrections [corrections]
+  (into {}
+        (mapv (fn [[k v]]
+                [k (format "%s was autocorrected to %s"
+                           (name (mb-to-smtp-settings k))
+                           (string/upper-case v))])
+              corrections)))
+
 (defendpoint PUT "/"
   "Update multiple `Settings` values.  You must be a superuser to do this."
   [:as {settings :body}]
@@ -63,10 +75,15 @@
                          ;; in normal conditions, validate connection
                          (email/test-smtp-connection smtp-settings)
                          ;; for unit testing just respond with a success message
-                         {:error :SUCCESS})]
+                         {:error :SUCCESS})
+        tested-settings  (merge settings (select-keys response [:port :security]))
+        [_ corrections _] (data/diff settings tested-settings)
+        properly-named-corrections (set/rename-keys corrections (set/map-invert mb-to-smtp-settings))
+        corrected-settings (merge email-settings properly-named-corrections)]
     (if (= :SUCCESS (:error response))
       ;; test was good, save our settings
-      (setting/set-many! email-settings)
+      (assoc (setting/set-many! corrected-settings)
+        :with-corrections (humanize-email-corrections properly-named-corrections))
       ;; test failed, return response message
       {:status 500
        :body   (humanize-error-messages response)})))

src/metabase/email.clj

@@ -87,23 +87,12 @@
       {:error   :ERROR
        :message (.getMessage e)})))
 
-
-(defn test-smtp-connection
-  "Test the connection to an SMTP server to determine if we can send emails.
-
-   Takes in a dictionary of properties such as:
-       {:host     \"localhost\"
-        :port     587
-        :user     \"bigbird\"
-        :pass     \"luckyme\"
-        :sender   \"foo@mycompany.com\"
-        :security \"tls\"}"
+(defn- run-smtp-test
   [{:keys [host port user pass sender security] :as details}]
   {:pre [(string? host)
          (integer? port)]}
   (try
     (let [ssl?      (= security "ssl")
-          starttls? (= security "starttls")
           proto     (if ssl? "smtps" "smtp")
           details (-> details
                       (assoc :proto proto
@@ -120,3 +109,37 @@
       (log/error "Error testing SMTP connection:" (.getMessage e))
       {:error   :ERROR
        :message (.getMessage e)})))
+
+(def ^:private email-security-order ["tls" "starttls" "ssl"])
+
+(defn- guess-smtp-security [details]
+  (loop [[security-type & more-to-try] email-security-order]
+    (when security-type
+      (let [test-result (run-smtp-test (assoc details :security security-type))]
+        (if (not= :ERROR (:error test-result))
+          (assoc test-result :security security-type)
+          (do
+            (Thread/sleep 500) ;; try not to get banned
+            (recur more-to-try)))))))
+
+(defn test-smtp-connection
+  "Test the connection to an SMTP server to determine if we can send emails.
+
+   Takes in a dictionary of properties such as:
+       {:host     \"localhost\"
+        :port     587
+        :user     \"bigbird\"
+        :pass     \"luckyme\"
+        :sender   \"foo@mycompany.com\"
+        :security \"tls\"}"
+  [details]
+  (let [inital-attempt (run-smtp-test details)
+        it-worked?     (= :SUCCESS (:error inital-attempt))
+        attempted-fix  (if (not it-worked?)
+                         (guess-smtp-security details))
+        we-fixed-it?     (= :SUCCESS (:error attempted-fix))]
+    (if it-worked?
+      inital-attempt
+      (if we-fixed-it?
+        attempted-fix
+        inital-attempt))))