Don't override callback_url Attempt to correct #28 #70
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Overridden method discarded the query_string
|
👍 |
|
This would very very useful for us as well. Works fine with LinkedIn and Facebook. We have a workaround in place for now. Would love to see this merged (at least as a configurable option) |
sferik
added a commit
that referenced
this pull request
Oct 21, 2015
Don't override callback_url Attempt to correct #28
|
Cool, thanks for merging! |
|
@timherby Thanks for bumping it. Released in version 1.4.0. |
iainbeeston
added a commit
to iainbeeston/omniauth-linkedin-oauth2
that referenced
this pull request
Oct 30, 2015
Before omniauth-oauth2 version 1.4, the oauth2 strategy overrode the callback_url method to exclude querystring parameters. However, in version 1.4 (see omniauth/omniauth-oauth2#70) override was removed, which means that querystring parameters are now included (by omniauth-linkedin-oauth2 and every other omniauth-oauth2 gem). Unfortunately, LinkedIn expects a callback without any querystring parameters, and if you use this gem with omniauth-oauth2 version 1.4+ you will get this error: ``` I, [2015-10-29T17:36:27.934054 #49096] INFO -- omniauth: (linkedin) Callback phase initiated. "callback_url: http://localhost:3000/users/auth/linkedin/callback?code=UlArqHqkcV0iHYoJENjq088IlbEcYnYbeXVHu7LzpGi2u5gYDmYHXk8xajWeM1ryKESL41ng3VyIAerJV3Ac3CF4hj4616mmkLWluXNQKXR7Qr0iiQ8&state=940cff5c6d64870a5bc7db6158b534e994860c8f55a55a0e" E, [2015-10-29T17:36:28.754548 #49096] ERROR -- omniauth: (linkedin) Authentication failure! invalid_credentials: OAuth2::Error, invalid_request: missing required parameters, includes an invalid parameter value, parameter more than once. : Unable to retrieve access token : appId or redirect uri does not match authorization code or authorization code expired {"error_description":"missing required parameters, includes an invalid parameter value, parameter more than once. : Unable to retrieve access token : appId or redirect uri does not match authorization code or authorization code expired","error":"invalid_request”} ``` To fix that, I've pulled the override that used to be in omniauth-oauth2 down into omniauth-linkedin-oauth2, so it maintains the same behaviour regardless of which version of omniauth-oauth2 is being used.
tpett
added a commit
to watermarkchurch/wcc-auth
that referenced
this pull request
Nov 11, 2015
This basically reverts the commit recently introduced on the Oauth2 omniauth gem here: omniauth/omniauth-oauth2#70 omniauth/omniauth-oauth2@2615267 The callback_url in the base omniauth gem includes the query string so by removing the overridden version the query string was being included which was making Doorkeeper complain about a callback URL not being equal.
vgarro
added a commit
to G5/omniauth-g5
that referenced
this pull request
Nov 19, 2015
…ge that removes `callback_url` mehtod required for our Auth Workflow omniauth/omniauth-oauth2#70 - Pinning in the mean time doorkeeper fixes it (https://github.com/doorkeeper-gem/doorkeeper/issues/7370 ) [Fixes #108467542]
vgarro
added a commit
to G5/omniauth-g5
that referenced
this pull request
Nov 19, 2015
…ge that removes `callback_url` method required for our Auth Workflow omniauth/omniauth-oauth2#70 - Pinning in the mean time doorkeeper fixes it (https://github.com/doorkeeper-gem/doorkeeper/issues/7370 ) [Fixes #108467542]
jtokoph
added a commit
to jtokoph/omniauth-twitch
that referenced
this pull request
Dec 5, 2015
Bug introduced by omniauth-oauth2 change: omniauth/omniauth-oauth2#70 Fixes WebTheoryLLC#3
jtokoph
added a commit
to jtokoph/omniauth-twitch
that referenced
this pull request
Dec 5, 2015
Bug introduced by omniauth-oauth2 change: omniauth/omniauth-oauth2#70 Fixes WebTheoryLLC#3
theycallmeswift
added a commit
to MLH/omniauth-mlh
that referenced
this pull request
Dec 14, 2015
omniauth/omniauth-oauth2#70 introduced a breaking change for a number of oauth providers. Right now the recommended fix is to lock to 1.3.1 Here's the relevant doorkeeper discussion. doorkeeper-gem/doorkeeper#737
ebababi
added a commit
to GaggleAMP/omniauth-gaggleamp
that referenced
this pull request
Apr 1, 2016
|
This change caused issues for me in writing a custom strategy for ADP. Similar to LinkedIn, the redirect_url must match exactly what they have on file, so having extra parameters in the query string causes the token request to fail. I had to use a workaround as suggested by @jtokoph in this issue WebTheoryLLC/omniauth-twitch#3 So it would appear this patch made life easier for some integrations, but harder for others.. |
pjg
added a commit
to Hello-bar/omniauth-verticalresponse
that referenced
this pull request
Jul 27, 2017
webgago
added a commit
to Hello-bar/omniauth-infusionsoft
that referenced
this pull request
Sep 5, 2017
pjg
added a commit
to Hello-bar/omniauth-constantcontact2
that referenced
this pull request
Oct 6, 2017
jonlunsford
added a commit
to ConvertKit/omniauth-infusionsoft
that referenced
this pull request
Feb 28, 2018
Re-implements callback_url removed by omniauth/omniauth-oauth2#70
This was referenced Feb 28, 2018
walf443
added a commit
to pixiv/omniauth-pixiv-public
that referenced
this pull request
Mar 5, 2018
l1h3r
pushed a commit
to l1h3r/omniauth-infusionsoft
that referenced
this pull request
Mar 16, 2018
* Fix compatibility with omniauth-oauth 1.4+ References: * omniauth/omniauth-oauth2#70 * omniauth/omniauth-oauth2@2615267 * omniauth/omniauth-oauth2#82 * jdennes/omniauth-createsend#3 * WebTheoryLLC/omniauth-twitch#4 * jdennes/omniauth-createsend#5 * DripEmail/omniauth-drip#6 * Don't force https if it's localhost * Allow 127.0.0.1 and localhost to be on http * Lookup IP for the given host and force ssl Avoid to force ssl for local IPs (127/8 network) * Improve resolving * Improve even more * Fix regexp * Allow fe80::* entirely
|
cross-linking/pasting #82 (comment) since this is a different set of folks. In short, by my read of the spec. Looking at the RFC: https://tools.ietf.org/html/rfc6749#section-4.1.3
The RFC says that the |
pboling
added a commit
to rivierapartners/omniauth-greenhouse
that referenced
this pull request
Apr 13, 2020
narikazu
added a commit
to honeypotio/omniauth-greenhouse
that referenced
this pull request
Oct 16, 2020
narikazu
added a commit
to honeypotio/omniauth-greenhouse
that referenced
this pull request
Oct 16, 2020
nickgal
added a commit
to nickgal/omniauth-dealerpulse
that referenced
this pull request
Sep 28, 2021
smcabrera
added a commit
to user-interviews/omniauth-qualtrics
that referenced
this pull request
Jul 5, 2022
This change fixes the configuration addressing a common bug across omniauth strategies that was introduced back in omniauth/omniauth-oauth2#70
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I hope nobody minds me issuing this PR. It bit us pretty hard and would be great to get it merged into master!
Overridden method discarded the query_string