Authentication is against first user in MongoDB even if email does not match #9

Closed
wants to merge 1 commit into from

3 participants

@onyxrev

I'm using CompoundJS with MongoDB with the local strategy. The way the code for compound-password is written, it adopts the default findOrCreate method on User from the CompoundJS framework, although it provides a shim in user.js. findOrCreate accepts a query such as {where: {email: "foo@bar.com"}}. It just so happens that if the query omits the "where" as in {email:"foo@bar.com}, it matches the first entry in the User collection. This is the way the local strategy is written right now.

Thus it always authenticates against the first user in the User collection regardless of email.

I'm trying to figure out the right way to fix this.

@onyxrev onyxrev Fix for #9 - authentication always happens against first user in MongoDB
First of all - I basically don't know why you'd want to use
firstOrCreate for the local strategy.  Making empty users in the
authentication stage is a bad idea.  Second, the fallback for
firstOrCreate was just basically an alias for findOne - it didn't
need to be in there at all.

The big problem, as described in issue #9, is that without the
where key in the firstOrCreate query it will match the first
User in the User collection rather than the user with the email
intended to be authenticated.
44454ac
@onyxrev

This pull request at least solves the issue at hand. I'm not sure if it does so in the flavor originally intended, though. It seems as though the firstOrCreate was merely being used for consistency or something. Like are we concerned that in some DB adapters findOne doesn't exist?

@1602
Owner
  1. find one exists in core, so every adapter works
  2. local strategy should create user if it's not exist
  3. if you have issues with findOrCreate - fix this method, and not just remove it with breaking module logic.
@1602 1602 closed this Mar 30, 2013
@onyxrev

findOrCreate delegates to findOne which delegates to all. I don't think the implementations of those methods are unreasonable but they do differ from some other Mongo ORMs such as Mongoid where you have a "first" method to return the first match. The problem is that this passport module's firstOrCreate query always fails to do what it aims to do. The firstOrCreate method searches for an email but does so without the 'where' key in the local strategy, so it always returns the same thing: the first user in the User collection. That's not right. The module should be finding the user with the email and to do so with the current JugglingDB implementation you need to pass in a 'where' clause.

Do you see?

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