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

Support non-x-www-form-urlencoded bodies returned from refresh_token_request compliance hook #545

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

skray
Copy link

@skray skray commented Apr 19, 2024

Addresses #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.

…request

compliance hook

* Handle the case where a non-x-www-form-urlencoded string is returned
from the refresh_token_request compliance hook, and allow it to be
used as a raw string in the subsequent request
* Add wix compliance fix to send token request and refresh requests
with JSON bodies
* Add unit tests for wix compliance fixes
@JoepvandenHoven-Bluemine

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.

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

Successfully merging this pull request may close these issues.

None yet

2 participants