Extend type support on JWT claims #451
Conversation
google-cla
bot
added
the
cla: yes
kokoro-team
removed
the
kokoro:run
| * | ||
| * @param claims Map of claim objects to be validated | ||
| */ | ||
| private static boolean validateClaims(Map<String, ?> claims) { |
There was a problem hiding this comment.
This is interesting code. I wonder if there's a library for this somewhere?
There was a problem hiding this comment.
I asked a fellow programmer friends who are more experience than me on Java before coming up with this and they didn't know of any.
I ultimately got inspired by the logic used on java-jwt though I wanted to provide a more robust support than that implementation.
Open to any suggestions you may have on how to improve this code!
oauth2_http/javatests/com/google/auth/oauth2/JwtClaimsTest.java
Outdated
Show resolved
Hide resolved
oauth2_http/javatests/com/google/auth/oauth2/JwtClaimsTest.java
Outdated
Show resolved
Hide resolved
Additional JWT claims now support more types other than String
| * @param claims Map of claim objects to be validated | ||
| */ | ||
| private static boolean validateClaims(Map<String, ?> claims) { | ||
| if (!validateKeys(claims)) { |
There was a problem hiding this comment.
Is it really required here given that the key type is explicitly defined as String?
| * @param value object to be evaluated. | ||
| */ | ||
| private static final boolean isSupportedValue(Object value) { | ||
| Class clazz = value.getClass(); |
There was a problem hiding this comment.
we can remove this and pass into contains directly? since its trivial and not reused
| @@ -68,10 +73,10 @@ | |||
| * | |||
| * @return additional claims | |||
| */ | |||
| abstract Map<String, String> getAdditionalClaims(); | |||
| abstract Map<String, ?> getAdditionalClaims(); | |||
There was a problem hiding this comment.
this is a breaking change. It should be possible to keep the old one marked as Deprecated and calling this new signature internally
|
Closing as outdated, feel free to reopen if actual again |
Additional JWT claims now support more types other than String
Fixes #439