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

native_redirect_uri is not a valid redirect_uri in doorkeeper 4.4.1 #1130

Closed
philkcw opened this issue Aug 1, 2018 · 16 comments
Closed

native_redirect_uri is not a valid redirect_uri in doorkeeper 4.4.1 #1130

philkcw opened this issue Aug 1, 2018 · 16 comments

Comments

@philkcw
Copy link

philkcw commented Aug 1, 2018

Steps to reproduce

After generating a grant token, attempt to get the corresponding access token like the following. The important keys are the grant_type and redirect_uri.

    response = page.driver.post(token_url,
      code: access_grant_token,
      client_id: application.uid,
      client_secret: application.secret,
      redirect_uri: "urn:ietf:wg:oauth:2.0:oob",
      grant_type: "authorization_code"
    )

We happen to be using capybara in this test, but it's just an HTTP POST.

Expected behavior

With doorkeeper 4.2.6, we get the access code in the response body, as expected.

Actual behavior

With doorkeeper 4.4.1, we get this error:
{"error":"invalid_grant","error_description":"The provided authorization grant is invalid, expired, revoked, does not match the redirection URI used in the authorization request, or was issued to another client."}

I believe this is happening because this line of code which is in master does not exist in version 4.4.1.

https://github.com/doorkeeper-gem/doorkeeper/blob/master/lib/doorkeeper/oauth/helpers/uri_checker.rb#L6

@philkcw philkcw changed the title native_redirect_uri is not considered a valid redirect_uri in doorkeeper 4.4.1 native_redirect_uri is not a valid redirect_uri in doorkeeper 4.4.1 Aug 1, 2018
@walterlongoneto
Copy link

It's happening to me too.

I had to update to 5.0.0.rc1 version to solve.

@philkcw
Copy link
Author

philkcw commented Aug 1, 2018

Thanks @WalterA . Does anyone know when 5.0.0 will be released?

@nbulaj
Copy link
Member

nbulaj commented Aug 2, 2018

Hi @philkcw . Couldn't say when 5.0.0 will be released, but I'll merge a native redirect URI fix in a 4.4.2 release. Feel free to use 5.0.0.rc2 if you want to have the latest gem features.

@philkcw
Copy link
Author

philkcw commented Aug 2, 2018

@nbulaj thanks! Looking forward to 4.4.2!

@philkcw
Copy link
Author

philkcw commented Aug 7, 2018

Hi @nbulaj do you have an ETA on the fix for 4.4.2? There is some urgency to this because of a CVE in doorkeeper: https://nvd.nist.gov/vuln/detail/CVE-2018-1000211

@nbulaj
Copy link
Member

nbulaj commented Aug 7, 2018

Hi @philkcw . This CVE already fixed with 4.4.0 release. What about native redirect uri - ASAP, currently I'm too busy, sorry

@philkcw
Copy link
Author

philkcw commented Aug 7, 2018

@nbulaj 4.4.0 has the regression as well. I'll bug you next week or submit a PR.

@ryansch
Copy link
Contributor

ryansch commented Aug 13, 2018

I'm having success with 4.4.1 with this patch: 4.4.0...outstand:backport-1060

@philkcw
Copy link
Author

philkcw commented Aug 14, 2018

@ryansch I think your patch is correct. Thanks! It could use a regression test like this one: https://github.com/doorkeeper-gem/doorkeeper/blob/master/spec/lib/oauth/helpers/uri_checker_spec.rb#L46

Are you going to submit a PR?

@philkcw
Copy link
Author

philkcw commented Aug 16, 2018

@nbulaj @ryansch please review: #1136

@nbulaj
Copy link
Member

nbulaj commented Aug 19, 2018

I'll release 4.4.2 tomorrow with the fix for this issue

@nbulaj
Copy link
Member

nbulaj commented Aug 20, 2018

@philkcw released to Rubygems

@philkcw
Copy link
Author

philkcw commented Aug 21, 2018

Thanks everyone!

@philkcw
Copy link
Author

philkcw commented Aug 22, 2018

Unfortunately the fix which has been merged has introduced a different regression. 😢 Stay tuned for more details.

@jhubert
Copy link

jhubert commented Aug 22, 2018

@philkcw Oh noes! What's the regression?

@philkcw
Copy link
Author

philkcw commented Aug 23, 2018

Sorry! 😄It was NOT a regression, but a flawed test of ours. Thanks everyone for your help with this issue!

@philkcw philkcw closed this as completed Aug 23, 2018
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

5 participants