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

ServiceApplicationClient: Add extra_jwt_headers #865

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

Conversation

lukaslihotzki-f
Copy link

This is needed to use the Zitadel token endpoint that needs an extra "kid" header with ServiceApplicationClient.

I named the parameter extra_jwt_headers (not just extra_headers) to disambiguate it from HTTP headers.

@auvipy auvipy self-requested a review September 14, 2023 13:19
Copy link
Contributor

@auvipy auvipy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add some unit tests for this? it is needed

Copy link
Contributor

@auvipy auvipy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets wait for the CI

@lukaslihotzki-f
Copy link
Author

All checks have passed. @auvipy, can you merge it?

@@ -68,6 +68,7 @@ def prepare_request_body(self,
audience=None,
expires_at=None,
issued_at=None,
extra_jwt_headers=None,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it needed for any specific specification or for a particular API/use case only?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Standard header parameters are defined in RFC 7515. In this document, "kid" is defined, but also other parameters that may be meaningfully used here.

@auvipy
Copy link
Contributor

auvipy commented Sep 21, 2023

lets wait for review from another maintainer

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