spring-projects / spring-projects/spring-security

MappedJwtClaimSetConverter calls Converter#convert with null

Open
#10,548 5 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

in: oauth2 type: enhancement
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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.