omniauth/omniauth-oauth2

1.4.0 makes my rails app unable to sign in with facebook

PikachuEXE opened this issue · 41 comments

rails 4.1.13
devise 3.5.2
omniauth (1.2.2)
omniauth-facebook (2.0.1)

1.3.1 was fine

Error:

(facebook) Authentication failure! invalid_credentials: OAuth2::Error, :
{"error":{"message":"Error validating verification code. Please make sure your redirect_uri is identical to the one you used in the OAuth dialog request","type":"OAuthException","code":100,"fbtrace_id":"GjHr4Inn5Rq"}}
hmnhf commented

I'm getting the same error with omniauth-google-oauth2, after upgrading to 1.4.0.

(google_oauth2) Authentication failure! invalid_credentials: OAuth2::Error, redirect_uri_mismatch: 
{
  "error" : "redirect_uri_mismatch"
}

+1 here with google. getting redirect_uri_mismatch as well

I guess it has to do with this commit:

2615267

I'm getting the same problem.
"message":"Error validating verification code. Please make sure your redirect_uri is identical to the one you used in the OAuth dialog request","type":"OAuthException","code":100,"

Yes it's because of 2615267 by @sferik . The redefinition of callback_url was deleted from OmniAuth::Strategies::OAuth2 while it was used in the strategy to get redirect_uri, which should be the same redirect_uri which we sent to facebook without params (code, etc).

You can use 1.3.1 version of the gem before the bug would be fixed.

dja commented

+1 to 1.3.1 temporarily fixes the issue

i struggled with this for about a week before downgrading to 1.2!
This should be fixed asap! It was an absolute nightmare!

Yeah same here!
I got it working again by changing my Gemfile to:
gem 'omniauth-oauth2', '~> 1.3.1'

What does provider_ignores_state do?
Any doc?

unfortunately, google oauth2 doesn't work even with provider_ignores_state: true

maybe a simple fix like this would work: #82

I lost 2 days of work trying to track down why my custom provider was returning an invalid_grant error. This really should have been checked more before being merged.

According to Section 3.1.2 of the OAuth 2 spec:

query component…MUST be retained when adding additional query parameters

I’m sorry implementing this part of the spec has caused some OAuth providers to break. Gems for such providers should specify their omniauth-oauth2 dependency like this (until we can find a better solution):

spec.add_dependency 'omniauth-oauth2', '~> 1.3.1'

I fix using: gem 'omniauth-oauth2', '~> 1.3.1'

I test the gem 'omniauth-oauth2', '~> 1.3.1' fix, working localy not in production.
Somebody for more infos on that ?

Ok just figure out some configuration not deleted when trying to fix the problem myself.
So using 1.3.1 version is the good fix.

I'm noticing this with my custom strategy for a doorkeeper backend. I understand that it is not following the spec, but it should probably be documented how to override it since even doorkeeper breaks it.

Same issue with the Office 365 adapter too. Again forcing a downgrade to 1.3.1 in my GemFile.lock solved it.

So, what is the suggested way to fix this? Should I fix that in my omniauth-dropbox-oauth2 gem by changing the definition for callback_url or should I wait for this to be fixed?

Wouldn't it be better to have this change activated by some kind of flag? It has proved to be non backwards compatible and since there's no changelog and the project does not follow semver it is pretty hard to detect something like that might happen.

Would a contribution in that path be welcome?

Should callback_url be overridden by specific omniauth strategy?
Maybe some providers require the original definition so that's why this should be removed / moved to specific strategy gems?
Example: omniauth fix this in 3.0.0

Should callback_url be overridden by specific omniauth strategy?
Maybe some providers require the original definition so that's why this should be removed / moved to specific strategy gems?

I agree with you, but making a change which breaks backwards compatibility without any deprecation notice or changelog is not the best either and breaks a lot of stuff.

We encountered the same problem with an app that authenticates with Cloud Foundry's UAA. The UAA rejects the input because the callback path is malformed because, in effect, the query-param starting ? appears twice:

params_there_from_earlier=values_here&redirect_uri=http://localhost/3000/auth/oauth2/callback?other_params_added_later=values_here

(The actual payload is urlencoded and quite different, I decoded and modified it for clarity)

The change in 2615267 means that the base strategy is used, which appends the query params. But it simply appends them, rather than trying to merge them.

As @sferik points out, that s3.1.2 of the RFC requires query parameters to be retained. But it seems to me that 3.1.2 could be satisfied by merging the parameters, rather than physically appending them in a way which seems to break every interaction with upstream authentication servers.

Same problem with rails + omniauth + doorkeeper, fixed by downgrading to 1.3.1

Same problem here but with a different provider (a custom NodeJS OAuth2 server).
Ruby/Rails spit out this error whenever it tried to authorize the callback code:
"invalid_request: Missing required parameter: code {"error":"invalid_request","error_description":"Missing required parameter: code"}"

Downgrading it to 1.3.1 fixed it.

Could somebody make a big note on the "README.md" for those unaware of this issue until it is resolved? =)
Thanks

Yet another me too - just spent a few hours tracking this down with Windows Live Oauth failing - same issue, the redirect_uri fails to match if the query string params are included

@samuraraujo Thanks. That fixed my problem as well.

gem 'omniauth-oauth2', '~> 1.3.1'

1.3.1 works for me, thanks.

The 1.3.1 fix won't work for rails 5.0.0.beta1 due to differences in rack versions,

~/root/sides/herds(branch:master*) » bundle                                                                                                gokul@noob
Fetching gem metadata from https://rubygems.org/..........
Fetching version metadata from https://rubygems.org/...
Fetching dependency metadata from https://rubygems.org/..
Resolving dependencies....
Bundler could not find compatible versions for gem "rack":
  In Gemfile:
    rails (< 5.1, >= 5.0.0.beta1) ruby depends on
      railties (= 5.0.0.beta1) ruby depends on
        actionpack (= 5.0.0.beta1) ruby depends on
          rack (~> 2.x) ruby

    rails (< 5.1, >= 5.0.0.beta1) ruby depends on
      railties (= 5.0.0.beta1) ruby depends on
        actionpack (= 5.0.0.beta1) ruby depends on
          rack-test (~> 0.6.3) ruby depends on
            rack (>= 1.0) ruby

    omniauth-oauth2 (~> 1.3.1) ruby depends on
      oauth2 (~> 1.0) ruby depends on
        rack (~> 1.2) ruby

Is there a plan to actually fix this given the Rails 5.0.0 incompatibility?

Seems like the decision is to have downstream gems make the required change as per #82.

Could have done better with deprecation, but I think we can settle this now and start opening PRs downstreams.

I commented on the closed PR #82, but just wanted to add that this was another several hours wasted here.

Just spent a good couple days trying to find the root of my error. Finally boiled it down to this.

+1 redirect_uri_mismatch

It was fixed when I downgraded to 1.1.2

Do we have a rails 5 fix?

Most omniauth strategy plugins (some of which are a few years old, but still working) do not stipulate a dependency on a specific version of omniauth-oauth2, just on omniauth itself. That's the main reason this has broken everybody's app - now each app maintainer has to stipulate that dependency themselves.

I'd consider rolling this bugfix back and making a minor release, which will "miraculously" fix all broken users of this gem (applications) upon their next deploy/bundle install invocation.

I get that it's a bug/spec fix, but those who would benefit from it are a far smaller audience (to my untrained eye) than those who would be harmed. Those who need this bugfix could benefit from a branch of this gem until a way can be found to integrate the fix into master without breaking the various omniauth strategy gems floating around (or until most release a newer version which does whatever this bugfix requires it to do).

@sferik is there not a better solution for this yet? 6 months later and I'm still running into this problem in oAuth Strategies.

How is this acceptable to just ignore?

cmar commented

I was able to fix the issue by restoring the callback_url method to my subclass of OAuth2

module OmniAuth
  module Strategies
    class MyStrategy < OmniAuth::Strategies::OAuth2
        ...

        def callback_url
           full_host + script_name + callback_path
        end

        ...

see breaking change

urkle commented

It seems rediculous that this change occurred for ONE strategy and broke every other single strategy out there.. IMHO it would have made more sense for the one strategy that needed the query parameters to override callback_url in it's own strategy.

Is this a wontfix?

We're not planning on reverting, no. This was implemented a year and a half ago to adhere to the OAuth 2 spec.

I recommend reaching out to the specific gem providers you require for them to make the necessary updates.