When a user specifies SSL explicitly, never try without it (#13304)

Previously, when adding a database, we would try to connect to the
database once with SSL, and once without. If connecting without SSL
works, but connecting with did not, we save the database as not needing
SSL. This ignored the SSL setting the user specified.

This makes the user's setting take precedence in only allowing SSL
connections.

Resolves #13295

src/metabase/api/database.clj

@@ -437,21 +437,26 @@
 (s/defn ^:private test-connection-details :- su/Map
   "Try a making a connection to database `engine` with `details`.
 
-  Tries twice: once with SSL, and a second time without if the first fails. If either attempt is successful, returns
+  If the `details` has SSL explicitly enabled, go with that and do not accept plaintext connections. If it is disabled,
+  try twice: once with SSL, and a second time without if the first fails. If either attempt is successful, returns
   the details used to successfully connect. Otherwise returns a map with the connection error message. (This map will
   also contain the key `:valid` = `false`, which you can use to distinguish an error from valid details.)"
   [engine :- DBEngineString, details :- su/Map]
-  (let [details (if (supports-ssl? (keyword engine))
-                  (assoc details :ssl true)
-                  details)]
+  (if (and (supports-ssl? (keyword engine))
+           (true? (:ssl details)))
+    (let [error (test-database-connection engine details)]
+      (or error details))
+    (let [details (if (supports-ssl? (keyword engine))
+                    (assoc details :ssl true)
+                    details)]
     ;; this loop tries connecting over ssl and non-ssl to establish a connection
     ;; if it succeeds it returns the `details` that worked, otherwise it returns an error
-    (loop [details details]
-      (let [error (test-database-connection engine details)]
-        (if (and error
-                 (true? (:ssl details)))
-          (recur (assoc details :ssl false))
-          (or error details))))))
+      (loop [details details]
+        (let [error (test-database-connection engine details)]
+          (if (and error
+                   (true? (:ssl details)))
+            (recur (assoc details :ssl false))
+            (or error details)))))))
 
 (def ^:private CronSchedulesMap
   "Schema with values for a DB's schedules that can be put directly into the DB."

test/metabase/api/database_test.clj

@@ -683,7 +683,34 @@
           (testing "via the API endpoint"
             (is (= {:valid false}
                    ((mt/user->client :crowberto) :post 200 "database/validate"
-                    {:details {:engine :h2, :details {:db "ABC"}}})))))))))
+                    {:details {:engine :h2, :details {:db "ABC"}}})))))))
+
+    (let [call-count (atom 0)
+          ssl-values (atom [])]
+      (with-redefs [database-api/test-database-connection (fn [_ details]
+                                                            (swap! call-count inc)
+                                                            (swap! ssl-values conj (:ssl details))
+                                                            {:valid true})]
+          (testing "with SSL enabled, do not allow non-SSL connections"
+            (#'database-api/test-connection-details "presto" {:ssl true})
+            (is (= 1 @call-count))
+            (is (= [true] @ssl-values)))
+
+          (reset! call-count 0)
+          (reset! ssl-values [])
+
+          (testing "with SSL disabled, try twice (once with, once without SSL)"
+            (#'database-api/test-connection-details "presto" {:ssl false})
+            (is (= 2 @call-count))
+            (is (= [true false] @ssl-values)))
+
+          (reset! call-count 0)
+          (reset! ssl-values [])
+
+          (testing "with SSL unspecified, try twice (once with, once without SSL)"
+            (#'database-api/test-connection-details "presto" {})
+            (is (= 2 @call-count))
+            (is (= [true false] @ssl-values)))))))
 
 
 ;;; +----------------------------------------------------------------------------------------------------------------+