requests-oauthlib
requests-oauthlib copied to clipboard
Support non-x-www-form-urlencoded bodies returned from refresh_token_request compliance hook
Addresses https://github.com/requests/requests-oauthlib/issues/544
Adding similar capabilities to the refresh_token_request
compliance hook as is possible in the access_token_request
hook, to allow for non standard, non x-www-form-urlencoded
request bodies in token refresh requests.
Additionally, adding in a Wix compliance fix showing how to use the access_token_request
and refresh_token_request
compliance hooks to successfully request and refresh tokens with Wix. Docs here: https://dev.wix.com/docs/rest/app-management/oauth-2/request-an-access-token
I added tests for the compliance fix to show the refresh_token_request
updates work (as well as testing them manually against the Wix API from my own project). I'm happy to add tests in test_oauth2_session.py
as well, I just wasn't sure exactly what to add, as the code I added should only ever be triggered by compliance hooks changing the request body to something non-standard.
I am also open to a completely different implementation here, this was just the option that I saw would solve my problem and looked to be fully backwards compatible with anyone's existing refresh_token_request
compliance hooks.
Instead of relying on a ValueError would it not make more sense to inspect the Content-Type in the headers? If the value is not x-www-form-urlencoded
then the Content-Type should be something other than application/x-www-form-urlencoded
and conversely if the Content-Type is not application/x-www-form-urlencoded
then it makes little sense to force the value to be x-www-form-urlencoded
.
Relying on a ValueError is a bit iffy since it relies on the underlying oauthlib function to throw the correct Exceptions.
We would also find this very useful, so if this could be merged that would be very much appreciated (Wix is also our use case) :pray:
Instead of relying on a ValueError would it not make more sense to inspect the Content-Type in the headers? If the value is not x-
www-form-urlencoded
then the Content-Type should be something other thanapplication/x-www-form-urlencoded
and conversely if the Content-Type is notapplication/x-www-form-urlencoded
then it makes little sense to force the value to bex-www-form-urlencoded
.Relying on a ValueError is a bit iffy since it relies on the underlying oauthlib function to throw the correct Exceptions.
I agree this would be a better solution.
@JonathanHuot would you consider accepting this? Please and thank you 🙏
@violuke @JoepvandenHoven-Bluemine good points, I'm happy to change the logic to check the content-type, instead of handling the ValueError.
Before doing any more work here I'd like to make sure this PR will be accepted by one of the project maintainers, then I can do any other changes needed and update tests all at once.
I had some extra time on my hands so I went ahead and updated the code based on feedback so far.
Does anyone have advice for getting this reviewed by a maintainer?