Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

let username and password be specified outside a uri #731

Closed
wants to merge 12 commits into from
22 changes: 14 additions & 8 deletions mongoengine/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,8 @@ def register_connection(alias, name=None, host=None, port=None,
uri_dict = uri_parser.parse_uri(conn_settings['host'])
conn_settings.update({
'name': uri_dict.get('database') or name,
'username': uri_dict.get('username'),
'password': uri_dict.get('password'),
'read_preference': read_preference,
'username': uri_dict.get('username') or username,
'password': uri_dict.get('password') or password,
})
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was redundant, as it gets overriden later anyway

if "replicaSet" in conn_settings['host']:
conn_settings['replicaSet'] = True
Expand Down Expand Up @@ -94,6 +93,7 @@ def get_connection(alias=DEFAULT_CONNECTION_NAME, reconnect=False):
raise ConnectionError(msg)
conn_settings = _connection_settings[alias].copy()

# These settings aren't used until we connect to a specific db
conn_settings.pop('name', None)
conn_settings.pop('username', None)
conn_settings.pop('password', None)
Expand All @@ -111,14 +111,20 @@ def get_connection(alias=DEFAULT_CONNECTION_NAME, reconnect=False):

try:
connection = None
# check for shared connections
connection_settings_iterator = ((db_alias, settings.copy()) for db_alias, settings in _connection_settings.iteritems())
for db_alias, connection_settings in connection_settings_iterator:
# This loop allows us to detect different alias' that connect to the same mongodb instance, and have them
# share a single pymongo object.
# Ie: alias 'production' and 'readOnly' connect to the same database, just with a different
# username/password. This would have both alias' use the same pymongo connection object, but still
# authenticate separately.
connection_settings_iterator = ((alias, settings.copy()) for alias, settings in _connection_settings.iteritems())
for alias, connection_settings in connection_settings_iterator:
# Need to pop these off as we did above so the dict comparison works
connection_settings.pop('name', None)
connection_settings.pop('username', None)
connection_settings.pop('password', None)
if conn_settings == connection_settings and _connections.get(db_alias, None):
connection = _connections[db_alias]
connection_settings.pop('authentication_source', None)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was popped off the setting we were comparing, but not within the loop. This would cause any settings with an authentication_source set to equate as not equal every time.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment got removed due to merge, but I feel it's worth being visible:
this was popped off the setting we were comparing, but not within the loop. This would cause any settings with an authentication_source set to equate as not equal every time.

if conn_settings == connection_settings and _connections.get(alias, None):
connection = _connections[alias]
break

_connections[alias] = connection if connection else connection_class(**conn_settings)
Expand Down
34 changes: 34 additions & 0 deletions tests/test_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from mongoengine import *
import mongoengine.connection
from mongoengine.connection import get_db, get_connection, ConnectionError
import mongoengine.connection as me_connection


class ConnectionTest(unittest.TestCase):
Expand Down Expand Up @@ -101,6 +102,39 @@ def test_connect_uri_without_db(self):
c.admin.system.users.remove({})
c.mongoenginetest.system.users.remove({})

def test_connect_uri_without_username_password(self):
"""Ensure that the connect() method works properly with a uri,
when the username/password is specified outside the uri
"""
c = connect(db='mongoenginetest', alias='admin')
c.admin.system.users.remove({})
c.mongoenginetest.system.users.remove({})

c.admin.add_user("admin", "password")
c.admin.authenticate("admin", "password")
c.mongoenginetest.add_user("username", "password")

conn = connect(alias='test_uri_no_username', host='mongodb://localhost/mongoenginetest', username="username", password="password")
self.assertTrue(isinstance(conn, pymongo.mongo_client.MongoClient))

# Since the mongodb instance used for testing doesn't require
# authentication (and turning that on breaks some 85 tests), and there
# doesn't appear to be any way to check to see if a connection has
# authenticated, I instead expose some internals of mongoengine to
# make sure the correct settings have been saved.
# Without this, instead of the test failing everything would appear to
# work fine, but there would be no username/password on the
# connection.
self.assertEqual(me_connection._connection_settings['test_uri_no_username']['username'], 'username')
self.assertEqual(me_connection._connection_settings['test_uri_no_username']['password'], 'password')
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really don't like doing it this way. If anyone knows how to check the authenticated username on a connection (or anything else that solves this problem) please let me know?


db = get_db(alias='test_uri_no_username')
self.assertTrue(isinstance(db, pymongo.database.Database))
self.assertEqual(db.name, 'mongoenginetest')

c.admin.system.users.remove({})
c.mongoenginetest.system.users.remove({})

def test_register_connection(self):
"""Ensure that connections with different aliases may be registered.
"""
Expand Down