requests / requests-oauthlib

OAuthlib support for Python-Requests!
https://requests-oauthlib.readthedocs.org/
ISC License
1.71k stars 422 forks source link

Adding refresh_token_request and access_token_request compliance hooks #433

Closed violuke closed 2 years ago

violuke commented 3 years ago

This is based upon #214, so thanks to @jimcortez for basically solving this for me :+1: :pray:

We needed this for a custom internal OAuth solution hence no included compliance fix example.

This solved the problem we're having and I believe will solve some or all of the points in issues #182, #379, #430 and #264 so should be useful for the wider community, despite not including a specific compliance fix example.

Here's our internal compliance fix:

import base64
from oauthlib.common import add_params_to_uri

def internal_oauth_compliance_fix(session, client_secret: str):
    def _non_compliant_auth_header(url, headers, body):
        basic_auth_value = "{}:{}".format(session._client.client_id, client_secret)
        headers["Authorization"] = 'Basic {}'.format(base64.b64encode(basic_auth_value.encode("utf-8")).decode("utf-8"))
        token = [('token', session._client.access_token)]
        url = add_params_to_uri(url, token)
        return url, headers, body

    session.register_compliance_hook('refresh_token_request', _non_compliant_auth_header)
    return session

I hope this helps someone else, thanks again to @jimcortez for the original work and if this could be merged soon that would be much appreciated. :+1:

coveralls commented 3 years ago

Coverage Status

Coverage increased (+0.5%) to 90.31% when pulling 4e67803e410f2abb5b3894f807cb61df7a7c0316 on violuke:master into 6ac61335f3049b616138839261e5591de568c399 on requests:master.

coveralls commented 3 years ago

Coverage Status

Coverage decreased (-0.3%) to 89.817% when pulling 460521c455ba343a0845a95c125764bb918c165e on violuke:master into 46f886ccb74652fc9c850ece960edcf2bce765a5 on requests:master.

timdawborn commented 3 years ago

An example of a large public application that needs Basic Auth for the refresh tokens is the Xero API: https://developer.xero.com/documentation/oauth2/auth-flow#refresh

violuke commented 2 years ago

I've just also added support for an access_token_request hook. We were in the same situation as #244 (needing to send the data as json on a password grant type), but in our case the API was unable to accept via application/x-www-form-urlencoded. We also had to remove the redirect_uri key from the JSON or the request was rejected.

I know this is against the Oauth2 specs, but that's the whole point of compliant hooks :wink: Here's an example usage:

def redacted_company_compliance_fix(session):
    def _request_in_json_not_form_urlencoded(url, headers, request_kwargs):
        headers["Content-Type"] = "application/json"

        if "redirect_uri" in request_kwargs["data"]:
            del request_kwargs["data"]["redirect_uri"]

        request_kwargs["json"] = request_kwargs["data"]

        del request_kwargs["data"]

        return url, headers, request_kwargs

    session.register_compliance_hook("access_token_request", _request_in_json_not_form_urlencoded)
    return session
violuke commented 2 years ago

@jtroussard would be please be possible for this to be reviewed? I think it will help in lots of incompliant situations. Thank you 🙏

violuke commented 2 years ago

@JonathanHuot I've added a test now for both of the new hooks. Thanks.

JonathanHuot commented 2 years ago

Can you execute tox -e black and tox -e py27 as they seem not passing in pipeline. Thanks in advance

violuke commented 2 years ago

Both are sorted now, thanks.

JonathanHuot commented 2 years ago

Thanks for this useful feature!

violuke commented 2 years ago

@JonathanHuot would you please be able to make a release on PyPI for this so that we can switch our package manager over to the main project again? Thank you.

violuke commented 1 year ago

Any chance of that PyPI release? 😉

JonathanHuot commented 1 year ago

Hi @violuke, I will prepare a release this month. Thanks for your patience.