auth0 / passport-linkedin-oauth2

Passport Strategy for LinkedIn OAuth 2.0
MIT License
119 stars 106 forks source link

Verify callback is not called when using auto-state #32

Closed oliversalzburg closed 9 years ago

oliversalzburg commented 9 years ago

When I set auto : true in my options, my verify callback is never called when I return from LinkedIn.

When I supply a state manually, the callback is called just fine.

fiznool commented 9 years ago

@oliversalzburg did you try setting state: true? I'm not sure if auto: true is a valid option.

oliversalzburg commented 9 years ago

@fiznool Heh, I really hope that was a typo when I reported it ;D

siacomuzzi commented 9 years ago

Can you confirm it? auto is not a valid option, but state is mandatory (see https://github.com/auth0/passport-linkedin-oauth2#auto-handle-state-param)

Thanks!

oliversalzburg commented 9 years ago

Sorry for taking so long to look into this. I think I was actually using the wrong property, auto, when I initially reported this.

However, when I set state: true, I will actually get &state=true in the request URL. It is my understanding that it should be a "unique string value of your choice that is hard to guess". So either I'm still not doing it right or something is wrong.

fiznool commented 9 years ago

Could you show us a code snippet? It's possibly a configuration issue as I'm using state: true with no issues. On 12 Oct 2015 21:51, "Oliver Salzburg" notifications@github.com wrote:

Sorry for taking so long to look into this. I think I was actually using the wrong property, auto, when I initially reported this.

However, when I set state: true, I will actually get &state=true in the request URL. It is my understanding that it should be a "unique string value of your choice that is hard to guess". So either I'm still not doing it right or something is wrong.

— Reply to this email directly or view it on GitHub https://github.com/auth0/passport-linkedin-oauth2/issues/32#issuecomment-147515964 .

oliversalzburg commented 9 years ago

Due to the way the application is structured, a snippet probably won't do. And I also don't really want to omit anything, because that might just hide my error :P

So, I hope this isn't too convoluted, but this is how we set up our authentication strategies: https://gist.github.com/oliversalzburg/9b12f666a3f080027435

fiznool commented 9 years ago

I believe you are overwriting the state parameter here: https://gist.github.com/oliversalzburg/9b12f666a3f080027435#file-authentication-loader-js-L60

You don't need this if branch. Setting state should only be done when you initially passport.use the strategy, not on each passport.authenticate.

oliversalzburg commented 9 years ago

@fiznool You're absolutely right. I removed the if branch and set state:true in the passport.use call and everything is working perfectly fine. Thanks a lot for your help.