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

feat: enable user to disable check for token exchange response body #281

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

PixelWeaver
Copy link

Hi there, great lib, thanks for your work on this! I've been using a fork of it for a bit it's been working really well!

Some APIs (e.g. Officient) do not respond to the token exchange request with one of the recognized Content-Type headers, causing check_response_body to return false.

Adding the disable_check_response_body function to the builder of CodeTokenRequest would offer a trifle more flexibility to users of this library without altering the default behavior.

@PixelWeaver PixelWeaver changed the title feat: enable user to disable token exchange response body feat: enable user to disable check for token exchange response body Aug 6, 2024
@ramosbugs
Copy link
Owner

What Content-Type header value is returned? The spec is pretty clear that the response body needs to be JSON:

The parameters are included in the entity-body of the HTTP response
using the "application/json" media type as defined by [RFC4627]

This library treats missing Content-Type headers as if they're JSON, so if check_response_body() is returning an error it means that either the Content-Type response header is explicitly set to something other than application/json, or the response body is missing entirely. Neither of those cases are compliant with the spec, so I don't think it makes sense to add explicit support to this library.

My suggestion, as for most other deviations from the spec, is to define a custom HTTP client as a shim (passed to request()/request_json()) that converts the non-compliant response into a compliant one before returning it to this library. For example, that could mean setting the Content-Type header to application/json in the HTTP response.

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.

2 participants