digitalbazaar / bedrock-mongodb

Bedrock mongodb module
Apache License 2.0
2 stars 3 forks source link

Use database specified in the connect string when calling `client.db()`. #57

Closed mattcollier closed 4 years ago

mattcollier commented 4 years ago

This in service of specifying ONLY the url when connecting. The operative bit here is the removal of config.name from the client.db() call that was on L592, and is now on L596. The unexpected behavior caused by this has been haunting me for a while. The unexpected behavior occurs when one is specifying a self contained url string that is designed to provide the database, username and password, whereby none of these values are set to non-default values in the config.

For instance: mongodb://my-user:my-password@mongos.example.com:27075/my_database?ssl=true. With this connect string solely, a valid authenticated database connection can be made. However, with line client.db(config.name), the client tries to connect to a non-existent database, bedrock_dev (the default value) and an authentication error ensues because the user that has authenticated to the database my_database has no permissions for the bedrock_dev database.

client.db() when called with no options, returns the database that was from the connect string, and therefore it is redundant at best, and creates a problem if config.name is not being set because the url config is in effect.

See: https://mongodb.github.io/node-mongodb-native/3.5/api/MongoClient.html#db

mattcollier commented 4 years ago

I forgot to add that this is simply a patch release as it does not impact typical dev use cases where the typical bedrock.mongodb config options are utilized. These are composed into the connect string, and the default database returned by client.db() is still the right database.

mattcollier commented 4 years ago

@aljones15 this branch has now been tested using authentication based solely on the URL config option (user provided connection string), specifying username/password via regular bedrock config (auto-composed connect string) and no authentication (test suite).

This does not alter any existing error handling.