8000 Set correct "typ" value "at+jwt" in OAuth 2 access token headers by laurids · Pull Request #34183 · keycloak/keycloak · GitHub
[go: up one dir, main page]
Skip to content

Set correct "typ" value "at+jwt" in OAuth 2 access token headers - #34183

Closed
laurids wants to merge 1 commit into
keycloak:mainfrom
laurids:issue-34146
Closed

Set correct "typ" value "at+jwt" in OAuth 2 access token headers#34183
laurids wants to merge 1 commit into
keycloak:mainfrom
laurids:issue-34146

Conversation

@laurids
@laurids laurids commented Oct 22, 2024
Copy link
Copy Markdown
Contributor

Set correct "typ" value "at+jwt" in OAuth 2 access token headers to comply with RFC 9068.

See https://datatracker.ietf.org/doc/html/rfc9068#section-2.1

I hope I have updated all relevant tests. Not all tests ran successfully locally, so I am not fully certain.

Closes #34146

@laurids
Set correct "typ" value "at+jwt" in OAuth 2 access token headers to c…
491c928 
…omply with RFC 9068.

See https://datatracker.ietf.org/doc/html/rfc9068#section-2.1

Closes keycloak#34146

Signed-off-by: Laurids Møller Jepsen <laurids.jepsen@cryptomathic.com>
@laurids
laurids requested review from a team as code owners October 22, 2024 08:28
@mposolda mposolda self-assigned this Oct 22, 2024
@laurids
laurids commented Jan 22, 2025 •
edited
Loading
Copy link
Copy Markdown
Contributor Author

Hi @mposolda
Do you have any update on the willingness to accept this PR?

If it is too "breaking" for now, would you consider a solution that takes the token type from a configuration instead? Like you have done for Verifiable Credentials: oid4vc/issuance/signing/SigningProperties.java#L37

This issue is quite important to us. We would be grateful if we can find a solution. Thank you in advance.

I went ahead and created a new issue, 36696 since the other one was closed due to inactivity.

@laurids
laurids commented Feb 18, 2025
Copy link
Copy Markdown
Contributor Author

I have created #37439 as an alternative non-breaking solution.

@thomasdarimont
thomasdarimont commented Feb 18, 2025 •
edited
Loading
Copy link
Copy Markdown
Contributor

I think it would be better to have a compatibility setting for this in the advanced client options:
image

Existing client configurations would keep using the old typ value JWT for access tokens.
Newly created clients could use the "at+jwt" type. A new "OpenID Connect Compatibility Modes" like Use JWT as Access Token type: on/off could provide users a way to keep the old behaviour without introducing a feature flag.

There should also be a note in the upgrading guide for this.

@laurids
laurids commented Feb 18, 2025
Copy link
Copy Markdown
Contributor Author

Hi @thomasdarimont
Thank you for your input!
I agree that it would definitely be better to avoid an extra feature flag.
If you think there is a good chance for your solution proposal to be approved, I would like to offer help in implementing this.
I do not have experience in the client side code of Keycloak, but I think I will be able to figure it out.
Would it be valuable to go ahead and implement your solution, or do we need to discuss the solution with someone else first?

@thomasdarimont
thomasdarimont commented Feb 18, 2025 •
edited
Loading
Copy link
Copy Markdown
Contributor

@laurids I think the relevant admin UI parts are here: https://github.com/keycloak/keycloak/blob/main/js/apps/admin-ui/src/clients/advanced/OpenIdConnectCompatibilityModes.tsx

I think the translations can be found here: https://github.com/keycloak/keycloak/blob/main/js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties#L2457

Perhaps the new client attribute could be called: attributes.token.type.jwt, unfortunately the attributes were never properly namespaced, e.g. attributes.compat.token.type.jwt would have been much nicer :)

@laurids laurids mentioned this pull request Feb 21, 2025
Merged
@laurids
laurids commented Feb 21, 2025 •
edited
Loading
Copy link
Copy Markdown
Contributor Author

@thomasdarimont Thanks for the hints. I have created a PR here: #37573

I suggested a slightly different attribute name attributes.access.token.header.type.jwt since there were other attributes starting with id.token. The whole point of the at+jwt type is to be able to distinguish access tokens from other tokens. I added header to avoid confusion with the JWT payload claim typ.

@mposolda or @thomasdarimont, can I ask one of you to review the new PR? #37573

@thomasdarimont
thomasdarimont commented Feb 21, 2025
Copy link
Copy Markdown
Contributor

Thanks for the update @laurids! The new PR looks good #37573.

I assume this PR #34183 should be merged before #37573, or would it be possible to merge the changes into a single PR?

@laurids
laurids commented Feb 21, 2025
Copy link
Copy Markdown
Contributor Author

Thank you so much @thomasdarimont
I think this PR should be closed, because it is a breaking way to solve the same issue.
And #37439 (the feature flag solution) should also just be closed.
I can go ahead and clean up (close them)?

@laurids
laurids commented Mar 3, 2025
Copy link
Copy Markdown
Contributor Author

Superseded by #37573

@laurids laurids closed this Mar 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@mposolda mposolda

Labels

team/cloud-native team/core-clients team/core-iam

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

OAuth 2 access token type at+jwt

3 participants

@laurids @thomasdarimont @mposolda
0