breaking api change in versions greater than v4.2.6

[pakwfoley]

The Issue

There is a breaking api change in versions greater than v4.2.6. This is in the native redirect uri authorization code flow. Up until then the code response route, after resource owner authentication, was /oauth/authorize/<code> . Now the response is oauth/authorize/native?code=<code>. This breaks the native oauth flows for clients that previously parsed the response from /oauth/authorize/<code>. This is what has kept us from upgrading doorkeeper. But now that we have to upgrade to 4.4.2 to get the backport for the implicit token revocation fix #1120, this has become a more pressing issue. It would be nice to have a initializer option that either lets us opt out of the breaking api change that exists in the minor version bump from v4.2.6 - v4.4.2 or add the option to customize this route. I would be happy to put in a PR for either option if they seem reasonable.

Expected behavior

In a minor version bump I would not expect a breaking api change, I would expect the authorization code response for a native auth code flow to be: /oauth/authorize/<code>.

Actual behavior

The response is oauth/authorize/native?code=<code>.

[nbulaj]

Hi @pakwfoley . Yes, you are absolutely right. This change was made to help applications that automatically authorizes (take a look at original PR). I think we need to revert this for 4.x release, but save it for 5.x with notice in upgrade guides and changelog.

[pakwfoley]

@nbulaj Thank you for the quick response! The only concern I have for reverting would be for those applications who have already bumped past 4.2.6 and asked clients to move over to the new api. I know it’s not easy to ask clients to move to a new api, and it’s especially not easy to ask them to revert after changing. Would it be unreasonable to add an option to opt out of the api change in the 4.x release, and remove that option for 5.x?

[nbulaj]

@pakwfoley I think you are right. It would be great if you can send a PR with such an option

[nbulaj]

Released with 4.4.3 @pakwfoley

[nbulaj]

Hi @pakwfoley . Can we close this issue ?

[pakwfoley]

Released with 4.4.3

[starsolutions]

Hi @nbulaj @pakwfoley ,

Our team has run into a similar issue with this change, where we accidentally had production clients using this API. We're working to mitigate this and have them move to non-native redirect URIs but unfortunately it will be a while till we get all users on to a new version of our application that uses the new redirect URIs. I was wondering if you'd be ok with continuing to have the flag to enable to old behavior in the 5.x chain, so that we can continue to update past 4.x.

If this would be ok, I've created a PR that merges the changes from the 4.x branch to the main branch. Thank you!

I raised a PR for this as well: #1597