atmos / warden-github

:lock: warden strategy for github oauth
MIT License
54 stars 41 forks source link

Improve strategy and fix redirect #18

Closed fphilipe closed 11 years ago

fphilipe commented 11 years ago

Hey @atmos, me again.

I made some improvements to the strategy. Here's what's changed:

One nice thing is that there should be no clashes with params when GitHub redirects back to the app. E.g. when you open http://localhost:9292/profile?code=abc&state=123 which looks like a callback, it will not be recognized as the callback but instead will redirect to GitHub, which redirects back to http://localhost:9292/profile?code=<code>&state=<state>, which in turn redirects back to the original url http://localhost:9292/profile?code=abc&state=123.

Let me know what you think.

atmos commented 11 years ago

Kinda weary of the code and state params being preserved but overall it seems sane. I'll see about testing and merging it later tonight.

Does the callback url change for existing apps here or does it still default to '/auth/github/callback' ?

fphilipe commented 11 years ago

The callback path /auth/github/callback wasn't hardcoded anywhere. If you specify it in your app as config (which was required anyways) then it will still be respected.

Not sure what you mean with the state being preserved. The state was being stored in the session (which is still the case), and cleared after authentication (which wasn't the case iirc).