spring-projects / spring-projects/spring-security
MappedJwtClaimSetConverter calls Converter#convert with null
Nobody has claimed this yet.
- Dominant language
- Java
- Stars
- 9.6k
- Forks
- 6.3k
- Avg merge
- 2d 11h
- Merged PRs (30d)
- 52
Description
Describe the bug
Documentation of org.springframework.core.convert.converter.Converter#convert clearly says "never null":
* @param source the source object to convert, which must be an instance of {@code S} (never {@code null})
But lines 156-158 of org.springframework.security.oauth2.jwt.MappedJwtClaimSetConverter haven't got null-checks in them:
Object claim = claims.get(claimName); // claim is nullable
Object mappedClaim = converter.convert(claim); // convert accepts non-null !!!
mappedClaims.compute(claimName, (key, value) -> mappedClaim);
This results in NullPointerException when accepting JWT without particular claims (which of them that haven't got custom converters for them).
What's interesting is that default converters (e.g. convertInstant) has null-checks INSIDE of them:
private static Instant convertInstant(Object source) {
if (source == null) {
return null;
}
// ... snip ...
}
To Reproduce
Set custom ClaimSetConverter in NimbusJwtDecoder:
decoder.setClaimSetConverter(MappedJwtClaimSetConverter
.withDefaults(Map.of(JwtClaimNames.SUB, source -> {
return "Custom conversion result";
})));
Then make a request with JWT without "sub" claim.
Expected behavior
Conversion is not called for absent claims.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/MappedJwtClaimSetConverter.java at lines 156-158, then follow the NimbusJwtDecoder claim-set converter entry point. Reproduce the case with a custom converter and a JWT missing the configured claim. Done means absent claims no longer invoke the converter or cause a NullPointerException.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- authentication, security
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 48/100