Doorkeeper: native_redirect_uri is not a valid redirect_uri in doorkeeper 4.4.1

[philkcw]

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

[waltera]

It's happening to me too.

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

[philkcw]

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

[nbulaj]

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]

@nbulaj thanks! Looking forward to 4.4.2!

[philkcw]

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]

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

[philkcw]

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

[ryansch]

I'm having success with 4.4.1 with this patch: https://github.com/doorkeeper-gem/doorkeeper/compare/4.4.0...outstand:backport-1060

[philkcw]

@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]

@nbulaj @ryansch please review: https://github.com/doorkeeper-gem/doorkeeper/pull/1136

[nbulaj]

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

[nbulaj]

@philkcw released to Rubygems

[philkcw]

Thanks everyone!

[philkcw]

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

[jhubert]

@philkcw Oh noes! What's the regression?

[philkcw]

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