Extend the definition of sensitive-fields in the Database model to include any...

Extend the definition of sensitive-fields in the Database model to include any custom password type properties (#14712)

Adding new utility function to determine sensitive fields for a driver, which lives in the driver.utility namespace

Delegating all calls to what used to be the sensitive-fields var to use the new fn instead

Adding new test to confirm that custom :password connection-properties are masked

src/metabase/api/database.clj

@@ -516,7 +516,7 @@
                 (m/update-existing details k (constantly (get-in database [:details k])))
                 details))
             details
-            database/sensitive-fields))))
+            (database/sensitive-fields-for-db database)))))
 
 (api/defendpoint PUT "/:id"
   "Update a `Database`."

src/metabase/driver/util.clj

@@ -142,3 +142,19 @@
     (.init ssl-context nil (.getTrustManagers trust-manager-factory) nil)
 
     (.getSocketFactory ssl-context)))
+
+(def default-sensitive-fields
+  "Set of fields that should always be obfuscated in API responses, as they contain sensitive data."
+  #{:password :pass :tunnel-pass :tunnel-private-key :tunnel-private-key-passphrase :access-token :refresh-token
+    :service-account-json})
+
+(defn sensitive-fields
+  "Returns all sensitive fields that should be redacted in API responses for a given database. Calls get-sensitive-fields
+  using the given database's driver, if that driver is valid and registered. Refer to get-sensitive-fields docstring
+  for full details."
+  [driver]
+  (if-some [conn-prop-fn (get-method driver/connection-properties driver)]
+    (let [all-fields      (conn-prop-fn driver)
+          password-fields (filter #(= (get % :type) :password) all-fields)]
+      (into default-sensitive-fields (map (comp keyword :name) password-fields)))
+    default-sensitive-fields))

src/metabase/models/database.clj

@@ -154,10 +154,17 @@
   "The string to replace passwords with when serializing Databases."
   "**MetabasePass**")
 
-(def ^:const sensitive-fields
-  "List of fields that should be obfuscated in API responses, as they contain sensitive data."
-  [:password :pass :tunnel-pass :tunnel-private-key :tunnel-private-key-passphrase
-   :access-token :refresh-token :service-account-json])
+(defn sensitive-fields-for-db
+  "Gets all sensitive fields that should be redacted in API responses for a given database. Delegates to
+  driver.u/sensitive-fields using the given database's driver (if valid), so refer to that for full details. If a valid
+  driver can't be clearly determined, this simply returns the default set (driver.u/default-sensitive-fields)."
+  [database]
+  (if (and (some? database) (not-empty database))
+      (let [driver (driver.u/database->driver database)]
+        (if (some? driver)
+            (driver.u/sensitive-fields (driver.u/database->driver database))
+            driver.u/default-sensitive-fields))
+      driver.u/default-sensitive-fields))
 
 ;; when encoding a Database as JSON remove the `details` for any non-admin User. For admin users they can still see
 ;; the `details` but remove anything resembling a password. No one gets to see this in an API response!
@@ -171,5 +178,5 @@
                             (reduce
                              #(m/update-existing %1 %2 (constantly protected-password))
                              details
-                             sensitive-fields))))
+                             (sensitive-fields-for-db db)))))
     json-generator)))

test/metabase/models/database_test.clj

@@ -2,6 +2,8 @@
   (:require [cheshire.core :refer [decode encode]]
             [clojure.string :as str]
             [clojure.test :refer :all]
+            [metabase.driver :as driver]
+            [metabase.driver.util :as driver.u]
             [metabase.models :refer [Database]]
             [metabase.models.database :as mdb]
             [metabase.models.permissions :as perms]
@@ -124,3 +126,30 @@
                   "id"          2
                   "engine"      "bigquery"}
                  (encode-decode bq-db))))))))
+
+;; register a dummy "driver" for the sole purpose of running sensitive-fields-test
+(driver/register! :test-sensitive-driver, :parent #{:h2})
+
+;; define a couple custom connection properties for this driver, one of which has :type :password
+(defmethod driver/connection-properties :test-sensitive-driver
+  [_]
+  [{:name         "custom-field-1"
+    :display-name "Custom Field 1"
+    :placeholder  "Not particularly secret field"
+    :type         :string
+    :required     true}
+   {:name         "custom-field-2-secret"
+    :display-name "Custom Field 2"
+    :placeholder  "Has some secret stuff in it"
+    :type         :password
+    :required     true}])
+
+(deftest sensitive-fields-test
+  (testing "get-sensitive-fields returns the custom :password type field in addition to all default ones"
+    (is (= (conj driver.u/default-sensitive-fields :custom-field-2-secret)
+           (driver.u/sensitive-fields :test-sensitive-driver))))
+  (testing "get-sensitive-fields-for-db returns default fields for null or empty database map"
+    (is (= driver.u/default-sensitive-fields
+           (mdb/sensitive-fields-for-db nil)))
+    (is (= driver.u/default-sensitive-fields
+           (mdb/sensitive-fields-for-db {})))))