callback_url definition

[blarralde]

Hello

Is there any reason why callback_url is overriden line 37?
It's losing query_string from its original definition, which I find very useful...

Cheers
Ben

[tmilewski]

I couldn't find a reason as to why this would be the case. I would like @mbleigh to weigh in before merging this into master.

Per RFC3986:

The endpoint URI MAY include an "application/x-www-form-urlencoded" formatted (per Appendix B) query component ([RFC3986] Section 3.4), which MUST be retained when adding additional query parameters.

http://tools.ietf.org/html/rfc6749#section-3.1.2

[jeroenj]

Is there any news on this? It took me quite some time debugging why the callback url would lose the querystring.

[h0tw1r3]

Patch breaks google oauth2, but twitter and facebook work fine.

[stereobooster]

Breaks Spotify and Beats (which do not accept callback urls with get params).

You can use request.env['omniauth.params'] in callback controller

[chamnap]

@stereobooster, request.env['omniauth.params'] is nil inside my callback controller.

[NobodysNightmare]

A little silent in here...

I think the override was removed in #70.

However, now the callback_url contains the code and state query parameters for me:

>>  callback_url
=> "http://localhost:3005/auth/XXX/callback?code=63e874156af1c6e9c056c34f5225ac7fa58f4cb0ba1ce19f12674fe51f20739c&state=864417e69b314d78dd33201169c7ca58dad54070ea6f80f1"

At least the code parameter was provided by the AS in response to the Authorization Request. However, a spec compliant AS must compare the redirect_uri it received in the Authorization Request with the one in the Access Token Request.

Since the former request could not possibly contain the code parameter, this parameter (and probably also state) should not be transmitted in the latter request.

tl; dr: By transmitting the code parameter in the Access Token Request, the AS verification of the redirect_uri fails.

I don't quite know why this is no issue for many more people...

[caseyprovost]

+1 sameissue here

[jmanian]

I had the same issue @NobodysNightmare mentioned above — the removal of the callback_url definition in #70 broke my custom strategies, because the redirect_uri during the callback phase does not match the original one.

I had to add this to all my custom strategies to get them working again.

def callback_url
  full_host + callback_path
end

I also use omnituah-google-oauth2, which continued to work after #70, and I was wondering why it didn't have the same problem with redirect_url mismatch, and it's because it has basically the same override.

[sirwolfgang]

I just ran into this issue with trying to implement Airtable; Where they were rejecting the redirect_uri if it had the additional QSP.

While it seems like the spec is well defined, it does seem common enough of an implementation detail that it might be wise to uplevel this to an option that may be configured depending on how the server is behaving.

[WillPowelson-at]

Chiming in from Airtable, our reading of RFC3986:

The endpoint URI MAY include an "application/x-www-form-urlencoded" formatted (per Appendix B) query component ([RFC3986] Section 3.4), which MUST be retained when adding additional query parameters.

http://tools.ietf.org/html/rfc6749#section-3.1.2

was that query parameters included in redirect URI stored in authorization server's database must be preserved. e.g. if you've registered the URI https://www.mysite.com/oauth-redirect?myParam=staticArg then myParam=staticArg must be preserved when we redirect the user, note that we should allow additional or dynamic query parameters during authorization (we may relax this in the future but I can't offer any timelines, sorry!)