diff --git a/modules/drivers/mongo/src/metabase/driver/mongo/connection.clj b/modules/drivers/mongo/src/metabase/driver/mongo/connection.clj
index 0b0d2a4b43863d323307f036b26d1538124c8837..999d32e2ac2583c5d4f0750a390a87ad5c4063cc 100644
--- a/modules/drivers/mongo/src/metabase/driver/mongo/connection.clj
+++ b/modules/drivers/mongo/src/metabase/driver/mongo/connection.clj
@@ -10,7 +10,7 @@
[metabase.util.log :as log]
[metabase.util.ssh :as ssh])
(:import
- (com.mongodb ConnectionString MongoClientSettings MongoClientSettings$Builder)
+ (com.mongodb ConnectionString MongoClientSettings MongoClientSettings$Builder MongoCredential)
(com.mongodb.connection SslSettings$Builder)))
(set! *warn-on-reflection* true)
@@ -20,11 +20,8 @@
nil)
(defn db-details->connection-string
- "Generate connection string from database details.
-
- - `?authSource` is always prestent because we are using `dbname`.
- - We let the user override options we are passing in by means of `additional-options`."
- [{:keys [use-conn-uri conn-uri host port user authdb pass dbname additional-options use-srv ssl] :as _db-details}]
+ "Generate connection string from database details."
+ [{:keys [use-conn-uri conn-uri host port dbname additional-options use-srv ssl] :as _db-details}]
;; Connection string docs:
;; http://mongodb.github.io/mongo-java-driver/4.11/apidocs/mongodb-driver-core/com/mongodb/ConnectionString.html
(if use-conn-uri
@@ -32,14 +29,12 @@
(str
(if use-srv "mongodb+srv" "mongodb")
"://"
- (when (seq user) (str user (when (seq pass) (str ":" pass)) "@"))
+ ;; credentials are added into MongoClientSettings in [[db-details->mongo-client-settings]]
host
(when (and (not use-srv) (some? port)) (str ":" port))
"/"
dbname
- "?authSource=" (if (empty? authdb) "admin" authdb)
- "&appName=" config/mb-app-id-string
- "&connectTimeoutMS=" (driver.u/db-connection-timeout-ms)
+ "?connectTimeoutMS=" (driver.u/db-connection-timeout-ms)
"&serverSelectionTimeoutMS=" (driver.u/db-connection-timeout-ms)
(when ssl "&ssl=true")
(when (seq additional-options) (str "&" additional-options)))))
@@ -65,17 +60,27 @@
(defn db-details->mongo-client-settings
"Generate `MongoClientSettings` from `db-details`. `ConnectionString` is generated and applied to
- `MongoClientSettings$Builder` first. Then ssl context is udated in the `builder` object.
+ `MongoClientSettings$Builder` first. Then credentials are set and ssl context is updated in the `builder` object.
Afterwards, `MongoClientSettings` are built using `.build`."
^MongoClientSettings
- [{:keys [use-conn-uri ssl] :as db-details}]
+ [{:keys [authdb user pass use-conn-uri ssl] :as db-details}]
(let [connection-string (-> db-details
db-details->connection-string
ConnectionString.)
builder (com.mongodb.MongoClientSettings/builder)]
+ (.applicationName builder config/mb-app-id-string)
(.applyConnectionString builder connection-string)
- (when (and ssl (not use-conn-uri))
- (maybe-add-ssl-context-to-builder! builder db-details))
+ (when-not use-conn-uri
+ ;; NOTE: authSource connection parameter is the second argument of `createCredential`. We currently set it only
+ ;; when some credentials are used (ie. user is not empty), previously we did that in all cases. I've
+ ;; manually verified that's not necessary.
+ (when (seq user)
+ (.credential builder
+ (MongoCredential/createCredential user
+ (or (not-empty authdb) "admin")
+ (char-array pass))))
+ (when ssl
+ (maybe-add-ssl-context-to-builder! builder db-details)))
(.build builder)))
(defn do-with-mongo-client
diff --git a/modules/drivers/mongo/test/metabase/driver/mongo/connection_test.clj b/modules/drivers/mongo/test/metabase/driver/mongo/connection_test.clj
index 2030d27f3a898c202e5cbbf9d019bf024bc9252c..2a7ba274549918ec5ddbd1d00a3efb22d4bb1ac4 100644
--- a/modules/drivers/mongo/test/metabase/driver/mongo/connection_test.clj
+++ b/modules/drivers/mongo/test/metabase/driver/mongo/connection_test.clj
@@ -2,17 +2,16 @@
(:require
[clojure.string :as str]
[clojure.test :refer :all]
+ [metabase.config :as config]
[metabase.driver.mongo.connection :as mongo.connection]
[metabase.driver.mongo.database :as mongo.db]
[metabase.driver.mongo.util :as mongo.util]
[metabase.driver.util :as driver.u]
[metabase.test :as mt])
(:import
- (com.mongodb ServerAddress)
+ (com.mongodb MongoCredential ServerAddress)
(com.mongodb.client MongoDatabase)))
-;;;; TODO: move some tests to db-test?
-
(set! *warn-on-reflection* true)
(def ^:private mock-details
@@ -39,7 +38,12 @@
:use-srv true}]
(testing "mongo+srv connection string used when :use-srv is thruthy"
(is (str/includes? (mongo.connection/db-details->connection-string db-details)
- "mongodb+srv://test-user:test-pass@test-host.place.com/datadb?authSource=authdb")))
+ "mongodb+srv://test-host.place.com/datadb"))
+ (let [settings (mongo.connection/db-details->mongo-client-settings db-details)
+ ^MongoCredential credential (.getCredential settings)]
+ (is (= "test-user" (.getUserName credential)))
+ (is (= "test-pass" (str/join "" (.getPassword credential))))
+ (is (= "authdb" (.getSource credential)))))
(testing "Only fqdn may be used with mongo+srv"
(is (thrown-with-msg? Throwable
#"Using DNS SRV requires a FQDN for host"
@@ -139,3 +143,27 @@
(or (when (instance? java.net.ConnectException e)
(throw e))
(some-> (.getCause e) recur))))))))))
+
+(deftest hard-password-test
+ (mt/test-driver
+ :mongo
+ (testing "Passwords and usernames containing `$ : / ? # [ ] @` are usable (#38697)"
+ (let [s "$ : / ? # [ ] @"
+ settings (mongo.connection/db-details->mongo-client-settings {:user s
+ :pass s
+ :host "localhost"
+ :dbname "justdb"})
+ ^MongoCredential credential (.getCredential settings)]
+ (is (= s (.getUserName credential)))
+ (is (= s (str/join "" (.getPassword credential))))
+ (is (= "admin" (.getSource credential)))))))
+
+(deftest application-name-test
+ (mt/test-driver
+ :mongo
+ (with-redefs [config/mb-app-id-string "$ : / ? # [ ] @"]
+ (let [db-details {:host "test-host.place.com"
+ :dbname "datadb"}
+ settings (mongo.connection/db-details->mongo-client-settings db-details)]
+ (is (= "$ : / ? # [ ] @"
+ (.getApplicationName settings)))))))