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.

Comment From: eleftherias

Thanks for reaching out @mitasov-ra! I believe this issue describes 2 problems.

The first problem is described in the "To Reproduce" section, which is that a NullPointerException is thrown when accepting JWT without particular claims. I believe this could be a duplicate of gh-10142. Could you confirm which version of Spring Security you are using?

The second problem is described in the issue title, which is that MappedJwtClaimSetConverter calls Converter#convert with null. It is true that this implementation does not adhere to the Javadoc, although it's not clear if it alone is causing any bugs. We may need to have a larger team discussion about whether we should consider an alternative implementation.

For now, let's focus on the first problem, since that is the one that is causing errors.

Comment From: spring-projects-issues

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

Comment From: mitasov-ra

Yeah, my bad, I've put very unclear "To Reproduce" example.

What I wanted to show is that it's definitely is a single problem, caused by public API violation. I'll try again.

To Reproduce

  1. Set custom ClaimSetConverter in NimbusJwtDecoder:
decoder.setClaimSetConverter(MappedJwtClaimSetConverter
    .withDefaults(Map.of(JwtClaimNames.SUB, sub -> {
        return "Sub claim is: " + sub.toString(); // .toString() will cause NPE
    })));
  1. Make a request with JWT token without sub claim.

What i've wrote above is a completely correct usage of MappedJwtClaimSetConverter according to docs. Lambda passed to withDefaults method has type Converter, and docs says that Converters are never called with null parameters.

To fix NPE above, I simply need to add null-check:

decoder.setClaimSetConverter(MappedJwtClaimSetConverter
    .withDefaults(Map.of(JwtClaimNames.SUB, sub -> {
        if (sub == null) return null;
        return "Sub claim is: " + sub.toString(); // .toString() will cause NPE
    })));

The problem is that that null-check is not required according to documentation.

I've already proposed PR with fix that is completely compatible with previous versions, as all what it fixes is inner implementation details.

Oh I'm sorry, the answers to your questions:

  • I'm using 2.6.0 version of spring-security as transitive dependency gathered from org.springframework.boot:spring-boot-starter-oauth2-resource-server:2.6.1

  • No this is not a duplicate of #10142. NPE above is not fixed in that PR

Comment From: mitasov-ra

Another possible solution for this NPE is to change functional interface from Converter to another, which, by docs, works with null values.

OR we can simply mention this detail in MappedJwtClaimSetConverter docs, something like "Warning! Although is's stated in Converter documentation that it never accepts null values, this class in fact, passes null into it".

P.S. In fact this is not a solution, but docs will be clearer at least.

Comment From: mitasov-ra

Just checked #10549 comments and realized that MappedJwtClaimSetConverter intended to accept (null -> smth) lambdas. Well then remaining fix is to make it a little clearer in the docs, that MappedJwtClaimSetConverter uses Converter in a different way.