omniauth-oauth2 icon indicating copy to clipboard operation
omniauth-oauth2 copied to clipboard

redirect_uri parameter given to authorization endpoint is not identical to the one given to the token endpoint

Open mulka opened this issue 8 years ago • 19 comments

It seems like the redirect_uri given in the authorization phase is different than the one given when requesting the token. The state and code parameters are added. But, I think according to the oauth spec, this shouldn't be the case.

https://tools.ietf.org/html/rfc6749#section-4.1.3

For context, I'm trying to use this with django-oauth-toolkit which is giving me an access_denied error because the redirect_uri's don't match exactly.

mulka avatar Jun 10 '16 18:06 mulka

I'm having a similar issue... I get the initial code & state from the request but can't get a token because the provider throws an "invalid redirect_uri" error. Feels like the redirect_uri is getting mangled but unsure how to inspect it. Also, I wonder if it's related to this older issue: https://github.com/intridea/omniauth-oauth2/issues/28

peterkappus avatar Jun 20 '16 14:06 peterkappus

I was able to work around this issue by applying the same change some other people have done: https://github.com/WebTheoryLLC/omniauth-twitch/pull/4/commits/3032f76de1c235e8a104fa25b650359ab62b4456

Add the following method to your strategy:

def callback_url full_host + script_name + callback_path end

mulka avatar Jun 20 '16 19:06 mulka

Thanks @mulka! That works beautifully! FYI I discovered I could also manually assign the redirect_uri param like this:

option :token_params, {
          :redirect_uri =>"<....my callback url....>"
}

but your solution is cleaner. I'm now getting the token response but having an issue where the response isn't getting parsed and throws the error below:

OAuth2::Error at /auth/wd/callback

{"access_token":"eyJ2ZXJz...VoR0RpIn0=","token_type":"bearer","expires_in":3600,"user":{"id":"394908","name":"Peter","email":"[email protected]"}}

file: client.rb location: get_token line: 140

It appears that response.parsed is nil. I believe it's related to this issue because the provider is sending the response as text/html instead of applicaiton/json so it's not getting parsed properly.

I'll follow up with the vendor as I don't think this is an issue w/ Omniauth (or the Oauth2 gem) but thought I'd post here in case it helps or sets off any alarm bells for anyone...

Thanks for your help! PK

peterkappus avatar Jun 21 '16 10:06 peterkappus

Hi, I faced the same issue here. I'm guessing you are using the 1.4.0 version of the gem ? Just wanted to highlight the fact that the issue isn't reproductible when using omniauth-oauth2 v1.3.1, so this could be related to the commit https://github.com/intridea/omniauth-oauth2/commit/26152673224aca5c3e918bcc83075dbb0659717f introduced in 1.4.0.

jeanbaptistevilain avatar Jun 29 '16 14:06 jeanbaptistevilain

To update: @mulka's work-around worked perfectly and I've spoken with the vendor and they've corrected the content-type issue on their end so I believe my issues are now solved. Hope this helps!

peterkappus avatar Jun 30 '16 14:06 peterkappus

i am using 1.4.0 and i can't get to override the callback url. Here is my strategy:

require 'omniauth-oauth2'

module OmniAuth
  module Strategies
    class Example < OmniAuth::Strategies::OAuth2

      option :name, 'example'
      option :client_options, {
        site:          'https://my-url.com',
        authorize_url: '/api/oauth2/authorize'
      }

      uid {
        raw_info['id']
      }

      info do
        {
          email: raw_info['email'],
        }
      end

      extra do
        { raw_info: raw_info }
      end

      def raw_info
        @raw_info = User.first.as_json # TODO get the user
      end
    end
  end
end

i have in routes the redirect

scope '/example/authorize' do
        get '/' => redirect('/auth/example')
     end

How can i specify the callback url?

ivanstojkovicapps avatar Sep 27 '16 08:09 ivanstojkovicapps

@ivanstojkovicapps Either add the following option to your strategy

option :token_params, { redirect_uri: "[insert callback url here]" }

or alternatively add the following method to your strategy

def callback_url
  full_host + script_name + callback_path
end

Hope this helps!

codeanpeace avatar Oct 31 '16 15:10 codeanpeace

Some providers require the redirect URL sent in the request and callback phases to exactly match, while some apps require application state parameters to be added as query parameters to the redirect URL.

To simultaneously handle both of these, the redirect_uri built in the callback phase has to be the received callback_url with just the code and state parameters removed.

There is a PR that attempts to do this. However it fails for some providers by removing the entire query string in the callback phase.

I've got two working solutions for removing just the code and state parameters:

  1. Using regular expressions:
      def build_access_token
        verifier = request.params["code"]
        callback_dangling_question_mark = callback_url[-1] == '?'
        redirect_uri = callback_url.gsub(/&?(code|state)=[^&]*/, '')
        redirect_uri.chomp!('?') unless callback_dangling_question_mark
        client.auth_code.get_token(verifier, {:redirect_uri => redirect_uri}.merge(token_params.to_hash(:symbolize_keys => true)), deep_symbolize(options.auth_token_params))
      end
  1. Parsing using URI.parse and Rack::Utils.parse_query:
      def build_access_token
        verifier = request.params['code']
        redirect_uri = URI.parse(callback_url).tap { |uri| uri.query = Rack::Utils.parse_query(uri.query).reject { |k,v| %w(code state).include?(k) }.to_query }.to_s
        client.auth_code.get_token(verifier, {redirect_uri: redirect_uri}.merge(token_params.to_hash(symbolize_keys: true)), deep_symbolize(options.auth_token_params))
      end

Which is better? The second is has a nasty dependency for non-Rack apps, while I'm not entirely sure that the first covers all possibilities.

mrj avatar Feb 13 '17 22:02 mrj

The second one is clearly more readable, I will change my gem.

joel avatar Feb 14 '17 05:02 joel

Joel, since this problem/solution is applicable to all oauth2 providers, it would be more efficient for the url change to be part of this omniauth-oauth2 gem rather than in your omniauth-windowslive gem. But yes, remove the recent code that removes the query string.

mrj avatar Feb 14 '17 05:02 mrj

@mrj I agree, I will do

joel avatar Feb 14 '17 06:02 joel

I've now come across a provider that requires redirect URLs to exactly match one in the registered list. This prevents app parameters being sent through the redirect query string.

I see two solutions:

  1. Get this gem to encode Omniauth auth parameters into and out of the state string. Hopefully there are no providers that both ignore state parameters and require identical redirect parameters.

  2. Abandon app parameters being passed through the auth, and instead rely on cookies. Cookies can however fail if a user blocks or deletes them, or if a user has multiple app sessions going (unless these are distinguished by a token unique to each page render, such as a CSRF token).

mrj avatar Feb 16 '17 23:02 mrj

Is there any news on this? I have the same problem that the redirect_uri must match exactly the one registered by the provider. How is one supposed to identify different users without state parameters?

23tux avatar Mar 26 '19 07:03 23tux

This is pretty important, you should never be sending the authorization code back out by sending it as part of the redirect URL, and providers should be requiring an exact match of the redirect URL to avoid creating open redirectors. The fix proposed here works great and I would love to see that merged into the core OAuth2 strategy.

aaronpk avatar Sep 02 '20 17:09 aaronpk

@aaronpk looking at discussion on #70, you're likely right. This is another issue on the list of things that will need fixed in a breaking version upgrade.

BobbyMcWho avatar Sep 02 '20 18:09 BobbyMcWho

How is this still a thing over 4 years later! Come on guys!

masha256 avatar Sep 22 '20 21:09 masha256

How is this still a thing over 4 years later! Come on guys!

@machadolab Feel free to make a PR.

BobbyMcWho avatar Sep 22 '20 22:09 BobbyMcWho

How is this still a thing over 4 years later! Come on guys!

@machadolab Feel free to make a PR.

There are a plenty of PRs, suggested fixes, references to other strategies that have implemented fixes that work fine. I don't think thats the issue.

masha256 avatar Sep 22 '20 22:09 masha256

Dear maintainers, please revert https://github.com/omniauth/omniauth-oauth2/commit/85fdbe117c2a4400d001a6368cc359d88f40abc7 . Every major oauth strategy I've found has had to override this method, including Facebook and Linkedin, which #70 claimed the commit fixed:

LinkedIn: https://github.com/decioferreira/omniauth-linkedin-oauth2/blob/master/lib/omniauth/strategies/linkedin.rb#L37 Facebook: https://github.com/simi/omniauth-facebook/blob/master/lib/omniauth/strategies/facebook.rb#L87 Github: https://github.com/omniauth/omniauth-github/blob/master/lib/omniauth/strategies/github.rb#L78 Google: https://github.com/zquestz/omniauth-google-oauth2/blob/master/lib/omniauth/strategies/google_oauth2.rb#L117 Windows Live: https://github.com/joel/omniauth-windowslive/blob/master/lib/omniauth/strategies/windowslive.rb#L57

We happened to use an older, little-maintained strategy which broke on us sometime (perhaps the provider stopped accepting the bad redirect_uri that omniauth is giving).

evanbattaglia avatar May 31 '23 01:05 evanbattaglia