Merge pull request #5035 from metabase/fix-ssh-tunnel-credentials-not-being-validated-5016

Fix SSH tunnel credentials not being validated

src/metabase/driver/druid.clj

@@ -47,7 +47,8 @@
 ;;; ### Misc. Driver Fns
 
 (defn- can-connect? [details]
-  (= 200 (:status (http/get (details->url details "/status")))))
+  (ssh/with-ssh-tunnel [details-with-tunnel details]
+    (= 200 (:status (http/get (details->url details-with-tunnel "/status"))))))
 
 
 ;;; ### Query Processing

src/metabase/driver/generic_sql.clj

@@ -202,9 +202,9 @@
   ^clojure.lang.Keyword [k]
   (keyword (hx/escape-dots (name k))))
 
-
 (defn- can-connect? [driver details]
-  (let [connection (connection-details->spec driver details)]
+  (let [details-with-tunnel (ssh/include-ssh-tunnel details)
+        connection (connection-details->spec driver details-with-tunnel)]
     (= 1 (first (vals (first (jdbc/query connection ["SELECT 1"])))))))
 
 (defn pattern-based-column->base-type

src/metabase/driver/oracle.clj

@@ -59,7 +59,7 @@
          (dissoc details :host :port :sid :service-name)))
 
 (defn- can-connect? [details]
-  (let [connection (connection-details->spec details)]
+  (let [connection (connection-details->spec (ssh/include-ssh-tunnel details))]
     (= 1M (first (vals (first (jdbc/query connection ["SELECT 1 FROM dual"])))))))
 
 

test/metabase/driver/druid_test.clj

 (ns metabase.driver.druid-test
   (:require [cheshire.core :as json]
+            [expectations :refer [expect]]
             [metabase
+             [driver :as driver]
              [query-processor :as qp]
              [query-processor-test :refer [rows rows+column-names]]
              [timeseries-query-processor-test :as timeseries-qp-test]
@@ -246,3 +248,21 @@
                :query    {:source-table (data/id :checkins)
                           :aggregation  [:+ ["METRIC" (u/get-id metric)] 1]
                           :breakout     [(ql/breakout (ql/field-id (data/id :checkins :venue_price)))]}})))))
+
+(expect
+  #"com.jcraft.jsch.JSchException:"
+  (try
+    (let [engine :druid
+      details {:ssl false,
+               :password "changeme",
+               :tunnel-host "localhost",
+               :tunnel-pass "BOGUS-BOGUS",
+               :port 5432,
+               :dbname "test",
+               :host "http://localhost",
+               :tunnel-enabled true,
+               :tunnel-port 22,
+               :tunnel-user "bogus"}]
+      (driver/can-connect-with-details? engine details :rethrow-exceptions))
+       (catch Exception e
+         (.getMessage e))))

test/metabase/driver/generic_sql_test.clj

@@ -123,3 +123,23 @@
     0.5)
   (dataset half-valid-urls
     (field-percent-urls datasets/*driver* (db/select-one 'Field :id (id :urls :url)))))
+
+;;; Make sure invalid ssh credentials are detected if a direct connection is possible
+(expect
+  #"com.jcraft.jsch.JSchException:"
+  (try (let [engine :postgres
+             details {:ssl false,
+                      :password "changeme",
+                      :tunnel-host "localhost", ;; this test works if sshd is running or not
+                      :tunnel-pass "BOGUS-BOGUS-BOGUS",
+                      :port 5432,
+                      :dbname "test",
+                      :host "localhost",
+                      :tunnel-enabled true,
+                      :tunnel-port 22,
+                      :engine :postgres,
+                      :user "postgres",
+                      :tunnel-user "example"}]
+         (driver/can-connect-with-details? engine details :rethrow-exceptions))
+       (catch Exception e
+         (.getMessage e))))

test/metabase/driver/mongo/util_test.clj

 (ns metabase.driver.mongo.util-test
   (:require [expectations :refer :all]
-            metabase.driver.mongo.util
+            [metabase.driver :as driver]
             [metabase.test.util :as tu])
   (:import com.mongodb.ReadPreference))
 
@@ -28,3 +28,21 @@
 (expect
   IllegalArgumentException
   (build-connection-options :additional-options "readPreference=ternary"))
+
+(expect
+  #"We couldn't connect to the ssh tunnel host"
+  (try
+    (let [engine :mongo
+      details {:ssl false,
+               :password "changeme",
+               :tunnel-host "localhost",
+               :tunnel-pass "BOGUS-BOGUS",
+               :port 5432,
+               :dbname "test",
+               :host "localhost",
+               :tunnel-enabled true,
+               :tunnel-port 22,
+               :tunnel-user "bogus"}]
+      (driver/can-connect-with-details? engine details :rethrow-exceptions))
+       (catch Exception e
+         (.getMessage e))))

test/metabase/driver/oracle_test.clj

@@ -4,7 +4,7 @@
             [metabase.driver :as driver]
             [metabase.driver
              [generic-sql :as sql]
-             oracle])
+             [oracle :as oracle]])
   (:import metabase.driver.oracle.OracleDriver))
 
 ;; make sure we can connect with an SID
@@ -44,3 +44,21 @@
                                                  :port         1521
                                                  :service-name "MyCoolService"
                                                  :sid          "ORCL"}))
+
+
+(expect
+  com.jcraft.jsch.JSchException
+  (let [engine :oracle
+        details {:ssl false,
+                 :password "changeme",
+                 :tunnel-host "localhost",
+                 :tunnel-pass "BOGUS-BOGUS-BOGUS",
+                 :port 12345,
+                 :service-name "test",
+                 :sid "asdf",
+                 :host "localhost",
+                 :tunnel-enabled true,
+                 :tunnel-port 22,
+                 :user "postgres",
+                 :tunnel-user "example"}]
+    (#'oracle/can-connect? details)))

test/metabase/driver/presto_test.clj

@@ -142,3 +142,20 @@
                :order-by [[:default.categories.id :asc]]}
               {:page {:page  2
                       :items 5}}))
+
+(expect
+  #"com.jcraft.jsch.JSchException:"
+  (try
+    (let [engine :presto
+      details {:ssl false,
+               :password "changeme",
+               :tunnel-host "localhost",
+               :tunnel-pass "BOGUS-BOGUS",
+               :catalog "BOGUS"
+               :host "localhost",
+               :tunnel-enabled true,
+               :tunnel-port 22,
+               :tunnel-user "bogus"}]
+      (driver/can-connect-with-details? engine details :rethrow-exceptions))
+       (catch Exception e
+         (.getMessage e))))