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

fix for Doorkeeper incompatibility #269

Closed
wants to merge 3 commits into from

Conversation

SophieDeBenedetto
Copy link

Fixes issue #265

When secondary authorization request is made to the token url, OAuth2 appends the status code the to redirect URI. This is incompatible with Doorkeeper which then refuses to recognize the registered callback url. I made a minor change to client.rb at LOC 92-98--by checking to see if the request being made was the secondary request to the token URL, and changing the redirect URI to remove the appended authorization code and status if so.

@tswayne
Copy link

tswayne commented Aug 25, 2016

Looks like this pull request will depend on #263 to be merged in order for the build to run successfully.

@neerfri
Copy link

neerfri commented Oct 27, 2016

I think the problem you are referring to is in the omniauth-oauth2 gem.
See: omniauth/omniauth-oauth2#93
A workaround is given at: omniauth/omniauth-oauth2#93 (comment)
The PR that causes the issue: See https://github.com/intridea/omniauth-oauth2/pull/70/files

@dankozlowski
Copy link

Any way we could push this fix over the finish line?

@josephpage josephpage added the bug label Jan 8, 2018
@johaywood
Copy link

johaywood commented Feb 7, 2018

@neerfri is correct. this isn't an issue with this gem, but rather with omniauth-oauth2. I've used the workaround mentioned above in many omniauth strategies.

@pboling pboling closed this Feb 7, 2018
@pboling
Copy link
Member

pboling commented Feb 7, 2018

Thanks for the fix to the omniauth-oauth2 gem @neerfri & for the analysis and bump @johaywood .

Any interest in taking on a broader maintenance role in this project?

We, the new team of maintainers, are pushing for a release ASAP, but there is still quite a backlog of issues and PRs to wade through from the long dark winter of the sale of Intridea and loss of their corporate backing.

@johaywood
Copy link

@pboling I'd be interested in helping out with maintenance. I've certainly relied on this project heavily, and continue to, so I definitely don't want to see it fall by the wayside. Let me know where the priorities are and I'll see where I can jump in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
8 participants