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

Already on GitHub? Sign in to your account

RUBY-529 raise exception for multiple attempts to auth on the same db #171

Merged
merged 1 commit into from Mar 11, 2013

Conversation

Projects
None yet
3 participants
Contributor

brandonblack commented Mar 8, 2013

Server side, MongoDB will log out the first user when a second authentication
is issued for the same database. In the past we've handled this by just
quietly updating the authentication credentials we have stored for that db.

The driver spec for auth now requires that we raise an explicit exception so
that users don't unknowingly log out previous authentications applied to the
database.

@estolfo estolfo commented on the diff Mar 8, 2013

lib/mongo/db.rb
@@ -99,10 +99,8 @@ def initialize(name, client, opts={})
#
# @core authenticate authenticate-instance_method
def authenticate(username, password, save_auth=true)
- if @connection.pool_size > 1
- if !save_auth
- raise MongoArgumentError, "If using connection pooling, :save_auth must be set to true."
- end
+ if @connection.pool_size > 1 && !save_auth
@estolfo

estolfo Mar 8, 2013

Member

maybe we can turn this check @connection.pool_size > 1 into a method #connection_pooling?

@brandonblack

brandonblack Mar 8, 2013

Contributor

If we were doing this all over the place I think that might make sense, but currently we're only calling that in two reasonably isolated places.

Also, we're technically always pooling connections even when pool_size == 1 so a MongoClient#pooling? style method doesn't feel contextually correct to me. Using MongoClient#pool_size is arguably more explicit and clear in my opinion. Encapsulating that logic in a method would only add value here if we were trying to cut down on redundancies which we don't really have at the moment.

This change here was actually not part of my work. I was just cleaning up the if statements.

I think when you and I rework the connection pool it and re-visit auth it would make sense to look at those types of optimizations. Right now though, the call chain between MongoClient, DB, and Pool for authentication is pretty crazy (in some cases almost circular) and there's very little about it that I think is done right.

For now... I'm just trying to achieve spec compliance.

Contributor

TylerBrock commented Mar 11, 2013

LGTM, we spoke about converting strings to symbols for the hash but it's not imperative.

@brandonblack brandonblack RUBY-529 raise exception for multiple attempts to auth on the same db
Server side, MongoDB will log out the first user when a second authentication
is issued for the same database. In the past we've handled this by just quietly
updating the authentication credentials we have stored for that db.

The driver spec for auth now requires that we raise an explicit exception so
that users don't unknowingly log out previous authentications applied to the
database.
6428bf6

@brandonblack brandonblack merged commit 6428bf6 into mongodb:master Mar 11, 2013

@brandonblack brandonblack deleted the unknown repository branch Mar 11, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment