Skip to content

feat: add the ability to set jwt header type#737

Merged
aeneasr merged 1 commit into
ory:masterfrom
mgyongyosi:master
Mar 17, 2023
Merged

feat: add the ability to set jwt header type#737
aeneasr merged 1 commit into
ory:masterfrom
mgyongyosi:master

Conversation

@mgyongyosi

Copy link
Copy Markdown
Contributor

First, thank you for all your work on this library! 🙇

We are implementing RFC9068 and during that we found the following issue: the JWT header's typ parameter is implicitly set to "JWT" inside token.go by the the unsignedToken function. Also the ToMap function filters out the typ as well and unfortunately it cannot be changed from the Session for example. And based on https://datatracker.ietf.org/doc/html/rfc9068#section-2.1-3 the typ must be set to at+jwt.

This PR adds the ability to set the typ parameter from the jwt.Headers and when the typ is not set explicitly then the typ will be set to "JWT".

Checklist

  • I have read the contributing guidelines and signed the CLA.
  • I have referenced an issue containing the design document if my change introduces a new feature.
  • I have read the security policy.
  • I confirm that this pull request does not address a security vulnerability.
    If this pull request addresses a security vulnerability,
    I confirm that I got approval (please contact security@ory.sh) from the maintainers to push the changes.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added the necessary documentation within the code base (if appropriate).

Further comments

@mgyongyosi mgyongyosi requested a review from aeneasr as a code owner February 22, 2023 11:06
@CLAassistant

CLAassistant commented Feb 22, 2023

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@aeneasr

aeneasr commented Feb 26, 2023

Copy link
Copy Markdown
Member

Thank you for the PR! This looks quite good to me :) Unfortunately, OIDC Conformity tests are failing. I think they are also failing on master, but given this change we need them to be green. I'll spend a bit of time to see if I can fix this on master but if not, merging this PR is blocked by that requirement :(

@mgyongyosi

Copy link
Copy Markdown
Contributor Author

Thank you for the PR! This looks quite good to me :) Unfortunately, OIDC Conformity tests are failing. I think they are also failing on master, but given this change we need them to be green. I'll spend a bit of time to see if I can fix this on master but if not, merging this PR is blocked by that requirement :(

@aeneasr, is there anything I could do/assist with to fix the failing oidc-conformity tests?

@aeneasr aeneasr merged commit 45a6785 into ory:master Mar 17, 2023
shipperizer pushed a commit to shipperizer/fosite that referenced this pull request Jan 3, 2024
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.

3 participants