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

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

Open
mulka opened this issue Jun 10, 2016 · 19 comments

Comments

@mulka
Copy link

mulka commented Jun 10, 2016

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.

@peterkappus
Copy link

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: #28

@mulka
Copy link
Author

mulka commented Jun 20, 2016

I was able to work around this issue by applying the same change some other people have done:
WebTheoryLLC/omniauth-twitch@3032f76

Add the following method to your strategy:

def callback_url
full_host + script_name + callback_path
end

@peterkappus
Copy link

peterkappus commented Jun 21, 2016

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

@jeanbaptistevilain
Copy link

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 2615267 introduced in 1.4.0.

@peterkappus
Copy link

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!

@ivanstojkovicapps
Copy link

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?

@codeanpeace
Copy link

codeanpeace commented Oct 31, 2016

@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!

@mrj
Copy link

mrj commented Feb 13, 2017

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.

@joel
Copy link

joel commented Feb 14, 2017

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

@mrj
Copy link

mrj commented Feb 14, 2017

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.

@joel
Copy link

joel commented Feb 14, 2017

@mrj I agree, I will do

@mrj
Copy link

mrj commented Feb 16, 2017

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).

@23tux
Copy link

23tux commented Mar 26, 2019

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?

tavyy pushed a commit to DFE-Digital/get-help-to-retrain that referenced this issue Jan 16, 2020
aguynamedben added a commit to getcommande/omniauth-dropbox-business-api2 that referenced this issue Mar 18, 2020
Unlike most providers, the Dropbox Business (and Dropbox consumer) API
require the callback_url to exactly match what is configured in their
web UI, **including any querystring values**. By default, OmniAuth appends any
incoming querystrings to the callback_url being sent the the
provider.

This means that if your app begins auths with something like:
/auth/dropbox_oauth2?auth_version=v2,

Your callback_url becomes:
/auth/dropbox_oauth2/callback?auth_version=v2

This doesn't exact match Dropbox Business' overly strict requirements
for this URL:
/auth/dropbox_oauth2/callback

The fix is for this provider to override callback_url so that the
querystring is not appended automatically.

There is a long-going disucssion to see whether this should be fixed
in omniauth-oauth2 or within each affected provider strategy:
omniauth/omniauth-oauth2#93

It's not super clear, but the consensus seems to be that this behavior
should be accounted for in the strategy.

Here's the similar issue for Dropbox (consumer):
icoretech/omniauth-dropbox2#2

Unmerged PR in the consumer library:
icoretech/omniauth-dropbox2#2
@aaronpk
Copy link

aaronpk commented Sep 2, 2020

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.

@BobbyMcWho
Copy link
Member

@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.

@masha256
Copy link

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

@BobbyMcWho
Copy link
Member

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

@machadolab Feel free to make a PR.

@masha256
Copy link

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.

@evanbattaglia
Copy link

Dear maintainers, please revert 85fdbe1 .
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).

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

No branches or pull requests