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

specs says type in header should at+jwt #990

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

Conversation

arthurchan35
Copy link
Contributor

@arthurchan35
Copy link
Contributor Author

@amarkevich
@reta

@@ -646,7 +647,12 @@ protected String processJwtAccessToken(JwtClaims jwtCliams) {
// It will JWS-sign (default) and/or JWE-encrypt
OAuthJoseJwtProducer processor =
getJwtAccessTokenProducer() == null ? new OAuthJoseJwtProducer() : getJwtAccessTokenProducer();
return processor.processJwt(new JwtToken(jwtCliams));

JwsHeaders jwsHeaders = new JwsHeaders();
Copy link
Member

@reta reta Aug 22, 2022

Choose a reason for hiding this comment

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

@arthurchan35 it does not seem to be solving the problem at large:

  • as you may see in the comment [1], it could be JWS or JWE
  • for JWE, the JwsHeaders are not used, the JweHeaders are

Looking into the right place to apply the spec recommendation, but on more general note, we need to introduce a member to JoseType for at+JWT and respective constant to JoseConstants.

[1] https://github.com/apache/cxf/pull/990/files#diff-1c24cdb27ac335b1f77f921093e723cfeeda77ce8e14d3196d2d1977a3d1effaR648

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @reta, thanks for the review!

According to RFC 9068 sections 2.1 and section 4, a JWT access token must be signed, optionally encrypted. As I interpret the comment, it means the same thing as specs required?

Regarding JoseType and JoseConstants, I will look into them bit more.

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