-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Automatic per-socket authentication in Connection class #5
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
…entials are provided to Connection
To clarify the code changes in this pull request, the intent is to provide automatic authentication to MongoDB but only when it is really necessary: when a new, previously unauthenticated socket is used. After a socket connection has been authenticated once, it is not authenticated again. Because pymongo already pools socket resources this approach should make authentication very efficient, and minimise the number of auth operations a multi-threaded application will perform. In short, no more need to pre-emptively authenticate every request in multi-threaded apps that use pymongo on the off chance a socket has gone stale. |
Just took a quick look at this patch. My first thought is that I wonder if there's anyway we can avoid doing branching in _send_message - that method runs all the time, so branching there will probably have significant performance impact. Why can't we just auth when we create a new socket? |
Maybe the auth-checking and performance logic could be moved into the My approach only authenticates to specific databases on-demand, when you first access them. It sounds like it would be better to a) detect when a new socket has been created, and b) authenticate that new socket for all credentials available, or just for 'admin' if those credentials are known. What do you think? On a related note, I wonder if it would be better for users to specify their auth credentials only in the constructor, not at arbitrary times using the add/remove_db_auth methods? That way the set of known database credentials wouldn't change over time and I could get away with pre-authenticating each socket. |
I think the auth credentials should just get picked up by the normal Database.authenticate method - hopefully no need to add anything new to the API. Yeah I think it'd be better to just authenticate to everything on socket creation. |
Do you mean that Database.authenticate would use the Connection's pre-defined credentials (provided in the Connection's constructor), or that the credentials would only be set by Database.authenticate method calls (and not directly on the Connection at all)? When I have a good grasp of your recommended changes I will update my patch. Thanks for the feedback! |
I was thinking that credentials would be set by calls to Database.authenticate, just like they are now. The only difference would be that those calls would persist across sockets. |
Cool. I'll work on an a reworked patch and create another Pull request once it's ready. Unless there's a better way to propose a patch? |
okay great! pull requests are good. might be worth creating a case on jira.mongodb.org and linking it up w/ the pull requests though - things tend to get more eyes there. |
Created a new pull request following feedback, and a Jira ticket. http://github.com/mongodb/mongo-python-driver/pull/6 |
I have modified the pymongo's Connection class to perform per-socket authentication for databases when credentials are provided in advance. The connection caches information about already-authenticated socket+database combinations, so authentication is performed only once for each new socket.
Please consider pulling this feature because it will make it much easier to efficiently use MongoDB's auth within multi-threaded web frameworks.
The changeset includes unit tests that exercise the authentication in single- and multi-threaded scenarios -- these tests are skipped if the local server does not have auth enabled. There is no API documentation yet for the new methods on pymongo.Connection, but I would be happy to add some if it's likely my changes will be accepted.
Cheers,
James