jaredhanson / passport

Simple, unobtrusive authentication for Node.js.
https://www.passportjs.org?utm_source=github&utm_medium=referral&utm_campaign=passport&utm_content=about
MIT License
22.7k stars 1.24k forks source link

passport.authorize does not recognize successRedirect #667

Open copecopecope opened 6 years ago

copecopecope commented 6 years ago

I'm using passport.authorize to integrate third-party authorization in my app with an already authenticated user. It looks like all this method does differently than authenticate is set the assignProperty option to account before calling the internal authenticate method.

https://github.com/jaredhanson/passport/blob/df6e2b7938ebd92c26491e47b73b20ee3516f823/lib/middleware/authenticate.js#L244

In this method, if options.assignProperty is not undefined, next() is called immediately and the function returns before calling req.logIn, which makes sense. But shouldn't the method check for successRedirect first? Or is there some reason why that isn't the case?

timiscoding commented 6 years ago

I've come across this as well and was confused as to why my successRedirect route was failing.

It seems to be intentionally designed that authorize() will automatically handle the failureRedirect and not the successRedirect because if you think about it from passports point of view, it's concern should only be on authenticating users and not linking accounts. This can be seen in the code samples in the Authorize section in the docs.

But the docs seem to contradict that idea in the Association in Verify Callback section where they link accounts directly in the callback. The key part in doing it this way seems to be embedded in the comment

Preserve the login state by supplying the existing user after association. 
return done(null, req.user);

By returning the already logged in user, this lets you use authenticate() so that you can use successRedirect again at the expense of recreating a new login session. https://github.com/jaredhanson/passport/issues/81#issuecomment-10740675 demonstrates its usage. In Routes.js, he uses authenticate() exclusively whether its for logging in or linking.

So to sum up:

  1. Use authorize() if you wish to link accounts outside the verify callback and don't have a need for successRedirect
  2. Use authenticate() if you link accounts inside the verify callback and want to use successRedirect. But if you do this, you have to make sure to return the logged in user in the verify callback so that the user will remain logged into their account.