-
Notifications
You must be signed in to change notification settings - Fork 27
fix username/password authentication #158
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this patch, Ivan!
django_mongodb/base.py
Outdated
@@ -157,20 +159,35 @@ def __getattr__(self, attr): | |||
raise AttributeError(attr) | |||
|
|||
def _connect(self): | |||
settings_dict = self.settings_dict | |||
settings_dict = deepcopy(self.settings_dict) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can tell, there's no need for a copy.
django_mongodb/base.py
Outdated
user = settings_dict.get("USER") | ||
password = settings_dict.get("PASSWORD") | ||
options = settings_dict.get("OPTIONS", {}) | ||
options_user = options.pop("username", None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should ignore the possibility the developer put username and password in OPTIONS as that's just creating extra complexity, and it breaks the assumption in other places that the USER/PASSWORD will be set (e.g. https://github.com/mongodb-labs/django-mongodb/blob/main/django_mongodb/client.py).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, fine!
I reworked it and made it as simple as possible
django_mongodb/base.py
Outdated
password = settings_dict["PASSWORD"] | ||
if user and password and not self.database.authenticate(user, password): | ||
raise ImproperlyConfigured("Invalid username or password.") | ||
# We can check if the credentials are correct by calling server_info(). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the explanation, but I think we can omit the comment since this is no different from the behavior of other backends.
database.authenticate() was removed in PyMongo 4.
This PR updates the MongoDB connection logic in the Django MongoDB integration to be compatible with PyMongo 4.x. The old method of authenticating db via
self.database.authenticate(user, password)
has been removed, since it was deprecated and fully removed in PyMongo 4.x.Changes Made:
_connect()
method to utilize the predefined options to handle user credentials.authenticate(user, password)
method.USER/PASSWORD
(from settings) andOPTIONS['username']
/OPTIONS['password']
.