requests-oauthlib icon indicating copy to clipboard operation
requests-oauthlib copied to clipboard

Bad error message for wrong redirect URI

Open JensErat opened this issue 10 years ago • 18 comments

I just worked through the Facebook authentication tutorial, and was stuck for quite some time until I started tracing the logs.

How to Reproduce

My problem was I registered a URI with trailing / (let it be https://example.org/) at facebook, but used the same URI without slash https://example.org in the python code.

r.text in oauth2_session.py contained a helpful error message:

{"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}}

Expected Output

The error message is printed to the developer (eg., a reasonable exception is thrown).

Actual Output

Instead it was hidden from me, and I got a barely helpful and misleading traceback instead:

Traceback (most recent call last):
  File "./test.py", line 30, in <module>
    facebook.fetch_token(token_url, client_secret=client_secret, authorization_response=redirect_response)
  File "/usr/local/lib/python2.7/dist-packages/requests_oauthlib/oauth2_session.py", line 199, in fetch_token
    self._client.parse_request_body_response(r.text, scope=self.scope)
  File "/usr/lib/python2.7/dist-packages/oauthlib/oauth2/rfc6749/clients/web_application.py", line 271, in parse_request_body_response
    self.token = parse_token_response(body, scope=scope)
  File "/usr/lib/python2.7/dist-packages/oauthlib/oauth2/rfc6749/parameters.py", line 303, in parse_token_response
    validate_token_parameters(params, scope)
  File "/usr/lib/python2.7/dist-packages/oauthlib/oauth2/rfc6749/parameters.py", line 313, in validate_token_parameters
    raise MissingTokenError(description="Missing access token parameter.")
oauthlib.oauth2.rfc6749.errors.MissingTokenError

JensErat avatar Mar 29 '15 10:03 JensErat

If you can still easily reproduce this, what's the status code of that response?

Lukasa avatar Mar 29 '15 11:03 Lukasa

Which status response, where can I fetch it from? There is a code 100 in r.text, I guess that's not what you're after?

JensErat avatar Mar 29 '15 11:03 JensErat

That's not what I'm after, I'm after the HTTP status code. Where you print r.text you can also print r.status_code.

Lukasa avatar Mar 29 '15 11:03 Lukasa

Aye, r for response. ;)

Seems to be a reasonable error code, 400. But indeed, at least in the lines between sending the request and continuing with the response contents I see no code verifying the response code. I guess somewhere around line 199, an exception should be thrown if the response code is in the 400 or 500 range.

JensErat avatar Mar 29 '15 11:03 JensErat

Yeah, that was my line of thinking. We should probably just call r.raise_for_status() just after we invoke the compliance hooks.

Lukasa avatar Mar 29 '15 11:03 Lukasa

Your proposal raises an error if something's broken (and nother otherwise) -- but sadlly when using the facebook compaibility layer, raising the error raises an error.

30.03.2015 18:06:56 DEBUG Error trying to decode a non urlencoded string. Found invalid characters: set([u':', u' ', u'{', u'"', u'}']) in the string: '{"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}}'. Please ensure the request/response body is x-www-form-urlencoded.

I guess the problem is around line 29 of the facebook hook, but I didn't investigate further as I'm unsure what's happening why here.

Putting the r.raise_for_status() above that resolves the issue, but I agree it should occur after (we might want to fix a bad return code).

JensErat avatar Mar 30 '15 18:03 JensErat

Hang on, what's the exact flow here? Why weren't we hitting this before the change?

Lukasa avatar Mar 30 '15 19:03 Lukasa

Oh nevermind, that doesn't appear to be an actual exception, so it wouldn't previously have blocked execution. It's weird that oauthlib doesn't raise exceptions here.

Lukasa avatar Mar 30 '15 19:03 Lukasa

So this seems to be a bug in your code:

Error validating verification code. Please make sure your redirect_uri is identical to the one you used in the OAuth dialog request.

Does the status exception still get thrown?

Lukasa avatar Mar 30 '15 19:03 Lukasa

Error validating verification code. Please make sure your redirect_uri is identical to the one you used in the OAuth dialog request.

Well, this was my original problem (and that's what I am using to trigger a non-successfull (non-200) HTTP status code. By now it is resolved for my actual code, but for testing this I broke it again.

As far as I understand the issue following is happening (a different situation per bullet point):

  • I don't have an issue in my code, everything's fine.
  • Given I have broken code (eg., a wrong redirect URI leading to the message quoted above) and the r.raise_for_status() line is not included: I get a weird and barely helpful error message, see my original post.
  • I have broken code, but with the line included after the Facebook hook was called: I get an exception that contains the error message Facebook is sending, but because the message was in an unexpected format/encoding. This is the "wrong exception", it seems to be thrown by the requests module while it tries to raise the expected exception.
  • I have broken code, but included the r.raise_for_status() line before the Facebook hook is called: everything's fine, an exception is thrown indicating a non-successful HTTP response was returned.

JensErat avatar Mar 30 '15 21:03 JensErat

So in case 3, can you show me what exception was actually thrown? Stack trace and all, please. =)

Lukasa avatar Mar 31 '15 06:03 Lukasa

I will create a minimal example as soon as I get round to it, will take some days.

JensErat avatar Mar 31 '15 12:03 JensErat

That's alright, there's no rush. =)

Lukasa avatar Mar 31 '15 12:03 Lukasa

Too bad, the error is resolved in such that Facebook now prints an error message instead of redirecting anyway if the URI is different. I'm not able to trigger such an error message any more. :/

I had to apply a small hack, and generated a new facebook instance. This is an example, derived from the original facebook example:

#!/usr/bin/env python
# Credentials you get from registering a new application
client_id = ''
client_secret = ''

# OAuth endpoints given in the Facebook API documentation
authorization_base_url = 'https://www.facebook.com/dialog/oauth'
token_url = 'https://graph.facebook.com/oauth/access_token'
redirect_uri = 'https://example.org/'     # Should match Site URL

from requests_oauthlib import OAuth2Session
from requests_oauthlib.compliance_fixes import facebook_compliance_fix
facebook = OAuth2Session(client_id, redirect_uri=redirect_uri)
facebook = facebook_compliance_fix(facebook)

# Redirect user to Facebook for authorization
authorization_url, state = facebook.authorization_url(authorization_base_url)
print 'Please go here and authorize,', authorization_url

redirect_uri = 'https://example.org'     # Like above, but without trailing slash
facebook = OAuth2Session(client_id, redirect_uri=redirect_uri)
facebook = facebook_compliance_fix(facebook)

# Get the authorization verifier code from the callback url
redirect_response = raw_input('Paste the full redirect URL here:')

# Fetch the access token
facebook.fetch_token(token_url, client_secret=client_secret,
                     authorization_response=redirect_response)

# Fetch a protected resource, i.e. user profile
r = facebook.get('https://graph.facebook.com/me?')
print r.content

Without my patch, this results in

  File "./minimal.py", line 29, in <module>
    authorization_response=redirect_response)
  File "/usr/local/lib/python2.7/dist-packages/requests_oauthlib/oauth2_session.py", line 199, in fetch_token
    self._client.parse_request_body_response(r.text, scope=self.scope)
  File "/usr/lib/python2.7/dist-packages/oauthlib/oauth2/rfc6749/clients/web_application.py", line 271, in parse_request_body_response
    self.token = parse_token_response(body, scope=scope)
  File "/usr/lib/python2.7/dist-packages/oauthlib/oauth2/rfc6749/parameters.py", line 303, in parse_token_response
    validate_token_parameters(params, scope)
  File "/usr/lib/python2.7/dist-packages/oauthlib/oauth2/rfc6749/parameters.py", line 313, in validate_token_parameters
    raise MissingTokenError(description="Missing access token parameter.")
oauthlib.oauth2.rfc6749.errors.MissingTokenError

With the patch, the raise function fails with following exception:

  File "./minimal.py", line 29, in <module>
    authorization_response=redirect_response)
  File "/usr/local/lib/python2.7/dist-packages/requests_oauthlib/oauth2_session.py", line 199, in fetch_token
    r.raise_for_status()
  File "/usr/lib/python2.7/dist-packages/requests/models.py", line 825, in raise_for_status
    raise HTTPError(http_error_msg, response=self)

JensErat avatar Apr 15 '15 17:04 JensErat

I'd be happy to merge that patch if you'd like to open a pull request.

Lukasa avatar Apr 15 '15 17:04 Lukasa

I added a pull request (and the patch at least fixes the issue that the return code is not verified). But I'm still not sure if there's not yet another problem as described above with the facebook code (as I think another, more specific exception should have been thrown).

I think the Facebook compatibility layer somehow tries to parse/change the returned message body, but I'm not sure what the code is doing why.

JensErat avatar Apr 15 '15 17:04 JensErat

The code change from the pull request has actually broken our code; historically oauthlib would process certain errors from within parse_request_body_response and raise if it wasn't handled (though this would happen via validate_token_parameters and not via checking the status code).

This can be seen here: https://github.com/idan/oauthlib/blob/master/oauthlib/oauth2/rfc6749/parameters.py#L382

With the change from this issue the OAuthlib specific errors from https://github.com/idan/oauthlib/blob/master/oauthlib/oauth2/rfc6749/errors.py aren't raised any longer.

RR2DO2 avatar Sep 22 '15 12:09 RR2DO2

Looking at the original issue, this is due to oauthlib raising MissingTokenError if 'access_token' is not part of the response body. It doesn't handle the 'error' provided by Facebook as it is not a known oauth2 error (as per spec); causing errors.raise_from_error in oauthlib not to raise an exception. The next fall-through there is the missing token exception.

The error handling in oauthlib could probably be improved, both by adding support for registering custom errors (right now this has to be done by monkey-patching raise_from_error) and by having smarter handling about an 'error' key with an unsupported error. Technically that's against the oauth spec so it should become an 'invalid response' error, but with pluggable error handling requests-oauthlib could add a compliance fix for Facebook.

RR2DO2 avatar Sep 29 '15 11:09 RR2DO2