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

Can't use with two different strategies #62

Closed
diversario opened this issue Sep 2, 2011 · 17 comments
Closed

Can't use with two different strategies #62

diversario opened this issue Sep 2, 2011 · 17 comments

Comments

@diversario
Copy link

I'm trying to use connect-auth for Facebook and Twitter auth:

.use(c_auth({strategies:[ c_auth.Twitter({consumerKey: keys.twitterConsumerKey, consumerSecret: keys.twitterConsumerSecret}),
    c_auth.Facebook({appId : keys.fbId, appSecret: keys.fbSecret, scope: "email", callback: keys.fbCallbackAddress})],
    trace: true,
    logoutHandler: require('../node_modules/connect-auth/lib/events').redirectOnLogout("/"),
    firstLoginHandler: firstLoginHandler
}))

The problem is, if user authenticates with one strategy (say, FB) and then tries to authenticate with another (Twitter) - connect-auth returns true for the second strategy even if user not even logged in into the service.

It doesn't matter which strategy is first, the second one always breaks after the first one.

I'm using the authentication middleware from one of your examples:

exports.auth_middleware= function auth_middleware(){
  return function(req, res, next) {
    var urlp= url.parse(req.url, true)
    if( urlp.query.login_with ) {
      req.authenticate([urlp.query.login_with], function(error, authenticated) {
        if( error ) {
          // Something has gone awry, behave as you wish.
          console.log( error );
          res.end();
        } else {
          if( authenticated === undefined ) {
            // The authentication strategy requires some more browser interaction, suggest you do nothing here!
          }
          else {
            // We've either failed to authenticate, or succeeded (req.isAuthenticated() will confirm, as will the value of the received argument)
            console.log('from auth:', req.isAuthenticated(), authenticated);
            next();
          }
      }});
    } else {
      next();
    }
  }
};

How do I fix this?

@misterinterrupt
Copy link

Just a guess, but what happens if you duplicate your .use() statement for each additional strategy and or create 2 middleware functions? Basically chain connect-auth to itself using a different strategy..

@diversario
Copy link
Author

I split the .use() in two but nothing changed.

I see some mentions of some scope in the issues but can't figure out if that's what I need. Do you have any ideas?

@misterinterrupt
Copy link

I saw mention of scopes but I thought it had to do with passing permissions scope to facebook. Did you assign require('connect-auth') twice, so that you had two different vars i.e. c_auth1 and c_auth2? I feel like this solution is kind of hackish, but if you treat the modules as being separate middle-ware, they should ideally play nice.. Also, in order for others to help, you may need to provide more info about what is 'breaking' and the error messages that you are getting. Sorry I only have guesses.. I am kinda new to node :)

@diversario
Copy link
Author

No, there's some other scope related to connect-auth itself, saw it in the issue #1.

Requiring the module multiple times is indeed hackish... not sure about that. That doesn't scale either (even though I only have two strategies for now).

As for the error and stuff - I'm not getting any, I just get

18:00:35-851 [tFuUmX] >>> Authenticating (domain.com/connect?login_with=twitter)
18:00:35-851 [tFuUmX] <<< Authentication successful (Already Authenticated)

and that's it, it bypasses the actual authentication with the provider.

@misterinterrupt
Copy link

looks like it is on the todo list.. at line 49 of strategyExecutor

@misterinterrupt
Copy link

I'd start by forking the repo and git grep '(Already Authenticated)' but right now I have a little deadline that only needs the one strategy.

@diversario
Copy link
Author

Oh, damn. That's not good. I thought someone already had a similar issue and it was resolved, though.

@misterinterrupt
Copy link

Right, I see this issue was resolved.. try passing a scope param in with your strategy config objects and then in your middleware, utilize the isAuthenticated(scope) method as mentioned by the author.

@ciaranj
Copy link
Owner

ciaranj commented Sep 4, 2011

Sorry I've been out of the country. Yes scopes is how I would tackle this, I'm going to take a quick look at your use-case and check that it is possible to implement ;)

@ciaranj
Copy link
Owner

ciaranj commented Sep 4, 2011

:( It seems scope-passing between requests/re-directs won't currently work ! [this is bad as it isn't just broken for your use-case... but means it is broken for any use-case!]

@diversario
Copy link
Author

Oh... Hmm. Well, what about requiring twice? Would that work? I haven't had a chance to test that yet.

@ciaranj
Copy link
Owner

ciaranj commented Sep 4, 2011

I doubt it... the problem is a little more fundamental.... I'm working up a fix at the mo, but it will require sessions (but the twitter + facebook strategies need that anyway!)

@diversario
Copy link
Author

I'm ready to test whatever you got, my deadline is approaching (like, in 2 days!) :)

@ciaranj
Copy link
Owner

ciaranj commented Sep 4, 2011

Try now. I've published a new version of the API (0.4.1) and a gist can be found here of my testing harness (that I took from your samples) : https://gist.github.com/1193637

Hope this helps :)

P.s. This isn't what I had in mind originally with scopes (that was more for 'log in as Admin' or 'view as end user' style authentication ... but I think this use-case is still valid for that approach!)

@ciaranj ciaranj closed this as completed Sep 4, 2011
@diversario
Copy link
Author

Cool! I'll test as soon as I get off the bus :D

@diversario
Copy link
Author

It looks like it's working! Woo!

Thank you!

@ciaranj
Copy link
Owner

ciaranj commented Sep 5, 2011

No wOrries, glad you're sorted :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants