From c78e8ebb9441f5f01b878031ed221c037e649aee Mon Sep 17 00:00:00 2001
From: Arthur Ulfeldt <arthur@ulfeldt.com>
Date: Fri, 14 Apr 2017 11:59:18 -0700
Subject: [PATCH] Autocorrect email security settings where possible
---
src/metabase/api/email.clj | 31 +++++++++++++++++++------
src/metabase/email.clj | 47 ++++++++++++++++++++++++++++----------
2 files changed, 59 insertions(+), 19 deletions(-)
diff --git a/src/metabase/api/email.clj b/src/metabase/api/email.clj
index e3452558f1a..9618da34913 100644
--- a/src/metabase/api/email.clj
+++ b/src/metabase/api/email.clj
@@ -1,11 +1,15 @@
(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)})))
diff --git a/src/metabase/email.clj b/src/metabase/email.clj
index ab561327931..40dc7c56b49 100644
--- a/src/metabase/email.clj
+++ b/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))))
--
GitLab