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

Add claimTypeMismatch log message #839

Merged
merged 1 commit into from
Nov 15, 2024

Conversation

sberyozkin
Copy link
Contributor

@sberyozkin sberyozkin commented Nov 15, 2024

When a numerical claim such as updated_at is presented as String by a provider like Auth0 (see quarkusio/quarkus#43924), smallrye-jwt issues a warning, which is fair enough, since this and other standard claims are expected to be numbers, see the OIDC spec, and MP JWT spec itself expects such claims be numbers (long), for example, Claims.updated_at.

What is a bit of a problem is that in quarkusio/quarkus#43924 it happens during the implicit overridden toString() call and getting this warning is confusing given that probably noone is ever trying to check the updated_at claim...

So in this PR I simply made it log a more informative message at debug level...

The problem in general now is that if someone does want to get such claim as String, while it is specified to be of type long, then JsonWebToken API can't help directly... Now, there is a method Object getClaim(String) and I was thinking, I just get that String returned instead of long, but I'm nearly sure it will get me into a lot of trouble :-)

So if some OIDC providers ignore the specification advice and issue claims which are supposed to be numbers as strings, then smallrye-jwt users can still get the raw token with getClaim(Claims.raw_token) and extracting such claim using JsonObject.

Copy link
Member

@MikeEdgar MikeEdgar left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Now, there is a method Object getClaim(String) and I was thinking, I just get that String returned instead of long, but I'm nearly sure it will get me into a lot of trouble :-)

You may be right. Using getClaim(String) might be the only shortcut if there were more demand for it via JsonWebToken.

@sberyozkin
Copy link
Contributor Author

Thanks @MikeEdgar, having something like .getClaim(String name, String.class) can be of help too, but the possibility of it happening at the JsonWebToken API level in the short term future is very low. May be we should consider introducing

interface SmallRyeJsonWebToken extends JsonWebToken {
}

at some point :-)

@sberyozkin sberyozkin merged commit 71d80a8 into smallrye:main Nov 15, 2024
7 checks passed
@sberyozkin sberyozkin deleted the updated_at_string branch November 15, 2024 19:00
@sberyozkin sberyozkin added this to the 4.6.1 milestone Nov 15, 2024
@github-actions github-actions bot added this to the 4.6.1 milestone Nov 15, 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.

2 participants