RemoteJwkSet is not refreshed when encountering an unknown KID

[tinolazreg]

Describe the bug
If you are setting up a NimbusJwtDecoder using Spring Security, we have the possibility to provide a Cache that will be used for storing the JWK Set. This is a good feature, since we can then override the default TTL, instead of using DefaultJWKSetCache, which has a default refresh time of 5 minutes.

Reference to documentation: https://docs.spring.io/spring-security/reference/servlet/oauth2/resource-server/jwt.html#oauth2resourceserver-jwt-timeouts

The issue occurs when you use your own cache implementation, and you try to decode a JWT with an unknown KID. I would expect this to trigger a refresh of the JWK set, but this is not what is happening.

Example code of setup:

        var jwkSetCache = new ConcurrentMapCache("jwkSetCache", CacheBuilder.newBuilder()
                .expireAfterWrite(Duration.ofMinutes(30))
                .build().asMap(), false);
        var decoder = NimbusJwtDecoder.withJwkSetUri(jwkSetUri)
                .cache(jwkSetCache)
                .build();

We end up here with an unknown KID: https://bitbucket.org/connect2id/nimbus-jose-jwt/src/43b5435c548d70ce250afe6f932a6a55b3833bb8/src/main/java/com/nimbusds/jose/jwk/source/RemoteJWKSet.java#lines-486, and we expect the JWK set to be refetched.

The problem is that the NimbusJwtDecoder.java always uses the CachingResourceRetriever, even when it encounters an unknown KID: https://github.com/spring-projects/spring-security/blob/main/oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusJwtDecoder.java#L406-L407. This will lead to the JWK set only being fetched from the cache, and not refetched from the remote source.

To Reproduce

1. Setup NimbusJwtDecoder with a Cache and remote JWK set.
2. Try to decode a JWT signed with a KID not present in the JWK set.
3. Expect NimbusJwtDecoder to refetch JWK set from jwksetUri.

If we don't specify a cache to the NimbusJwtDecoder builder, it works as expected. The jwk set is refetched when encountering a signed JWS with an unknown KID.

Expected behavior
I expect JWK set to be refetched when encountering a KID not present in the current cached JWK set.

[sjohnr]

Thanks for reaching out @tinolazreg, and thanks for the detailed explanation!

Reviewing the NimbusJwtDecoder.JwkSetUriJwtDecoderBuilder and the Nimbus library class RemoteJWKSet, it looks like this could happen as you outlined, though it would be helpful to verify with a minimal sample.

If you need to use the Spring Cache abstraction, a workaround in the meantime could be tricky. I have not tested this, and don't work directly with the Cache abstraction often, but you could try something like the following:

@Bean
public JwtDecoder jwtDecoder() throws Exception {
    String jwkSetUri = "...";
    URL jwkSetUrl = new URL(jwkSetUri);

    return NimbusJwtDecoder.withJwkSetUri(jwkSetUri)
        .jwtProcessorCustomizer(jwtProcessor -> {
            JWSVerificationKeySelector<SecurityContext> jwsKeySelector =
                (JWSVerificationKeySelector<SecurityContext>) jwtProcessor.getJWSKeySelector();

            RemoteJWKSet<SecurityContext> remoteJwkSet =
                (RemoteJWKSet<SecurityContext>) jwsKeySelector.getJWKSource();

            ResourceRetriever restOperationsResourceRetriever =
                remoteJwkSet.getResourceRetriever();

            JWKSetCache jwkSetCache = new JWKSetCache() {
                private final Cache cache = new ConcurrentMapCache(
                    "jwkSetCache"/*, CacheBuilder.newBuilder()...build()*/, false);

                @Override
                public void put(JWKSet jwkSet) {
                    this.cache.put(jwkSetUri, jwkSet);
                }

                @Override
                public JWKSet get() {
                    return this.cache.get(jwkSetUri, JWKSet.class);
                }

                @Override
                public boolean requiresRefresh() {
                    return this.cache.get(jwkSetUri) == null;
                }
            };

            RemoteJWKSet<SecurityContext> jwkSource = new RemoteJWKSet<>(
                jwkSetUrl, restOperationsResourceRetriever, jwkSetCache);

            JWSVerificationKeySelector<SecurityContext> newJwsKeySelector =
                new JWSVerificationKeySelector<>(JWSAlgorithm.RS256, jwkSource);
            jwtProcessor.setJWSKeySelector(newJwsKeySelector);
        })
        .build();
}

If this works (or gets close), it allows you to re-use the existing encapsulated implementation of ResourceRetriever.

[tinolazreg]

Thank you for the reply. I created a test that reproduce the issue here: https://github.com/spring-projects/spring-security/compare/main...tinolazreg:spring-security:issue_11621_tests?expand=1.
decodeWhenCacheAndUnknownKidShouldTriggerFetchOfJwkSet ends up throwing an expection: org.springframework.security.oauth2.jwt.BadJwtException: An error occurred while attempting to decode the Jwt: Signed JWT rejected: Another algorithm expected, or no matching key(s) found, since the JWK set is only fetched from the cache.

The second test decodeWithoutCacheSpecifiedAndUnknownKidShouldTriggerFetchOfJwkSet works as expected, and handles a new KID being introduced properly.

[sjohnr]

Thanks for the tests @tinolazreg! I've opened a PR (#11638) for a possible fix based on my suggested workaround. Would you mind reviewing and verify if it would fix the issue?

[tinolazreg]

Thank you for picking this up quickly!

I tested your branch against my local setup, and we still have issues when using a provided Cache. The problem now seems to be here: https://bitbucket.org/connect2id/nimbus-jose-jwt/src/43b5435c548d70ce250afe6f932a6a55b3833bb8/src/main/java/com/nimbusds/jose/jwk/source/RemoteJWKSet.java#lines-484. The if statement here evaluates to false, since the code tests if the instance of jwkSetCache.get() is the same as an earlier jwkSetCache.get() call in the same method.

I would assume that this is because SpringJwkSetCache.get() returns a new instance, while DefaultJwkSetCache.get() ends up returning the same instance.

[sjohnr]

Oh, right. I did that to ensure we're caching the same thing (JSON) and not some kind of object. I'll take a look at that after I come back from my trip in a week. Thanks for testing it for me!