Add possibility to customize JwkSource of NimbusJwtDecoder

[marbon87]

We want to increase the cache-ttl of the NimbusJwtDecoder respectively the underlying JWKSource. Furthermore we want to use the refresh-ahead caching of JWKSource.

The com.nimbusds.jose.jwk.source.JWKSourceBuilder provides a lot of features to customize how jwks are cached / update etc. but currently there is no way to customize it.
The implemented possiiblity to customiz the used JWKSourceBuilder in NimbusJwtDecoder allows to use all features from JWKSourceBuilder.

[jzheaux]

Thanks for the PR, @marbon87! I'm curious if it would be easier to construct your own JWKSource instance?

If so, this PR instead might add something similar to NimbusReactiveJwtDecoder#withJwkSource.

[marbon87]

Hi @jzheaux, thanks for your feedback!

If i unterstand the current implementation correctly, adding a separate builder-creation method like NimbusReactiveJwtDecoder#withJwkSource would mean that the jwkSetUri must be specified explicitly / constructed manually. I prefer configuring the issuer-uri and let spring query the jwkSetUri by calling the well-known-config endpoint.

Furthermore i cannot use SpringJWKSource because it's private nor can i customize it because it's final.

Overall I have to write a lot more code as a user of spring-security, if i want to increase the timeouts.

[jzheaux]

Possibly, though I wonder if it is as much as you are thinking (perhaps it's also more than I'm thinking). SpringJWKSource allows you to use RestOperations and Spring Cache instances. However, you are wanting to use Nimbus's caching. Because of that, I imagine you'll be satisfied with working with JWKSourceBuilder directly.

As for the URI, I think that Nimbus has an API that's quite adept at this:

AuthorizationServerMetadata metadata = AuthorizationServerMetadata
    .resolve(new Issuer("https://example.org/issuer"));
String jwkSetUri = metadata.getJwkSetUri();
// ...

It seems to me that this would result in the following:

@Bean
JwtDecoder jwtDecoder(String issuer) {
    AuthorizationServerMetadata metadata = AuthorizationServerMetadata
        .resolve(new Issuer("https://example.org/issuer"));
    String jwkSetUri = metadata.getJwkSetUri();
    JWKSource<SecurityContext> source = JWKSourceBuilder.create(new URL(jwkSetUri))
        // your caching settings
        .build();
    return NimbusJwtDecoder.withJwkSource(source)
        // your decoding settings
        .build();
}

Alternatively, Spring Cache is also quite adept at timeouts and other caching functions if you are open to working with that instead.

What complexities am I failing to consider?

[marbon87]

Your suggestion should work but i am not sure what's missing to get it "completed".
For example the SpringJWKSource seems to be thread-safe, because of the usage of a ReentrantLock.
Furthermore i thought that there must be a reason why spring does not use your approach as the default and instead implements a custom JWKSetSource.

Regarding to spring cache, i did not find an simple solutation for a refresh or/and fault tolerant ahead cache.

[jzheaux]

Gotcha, I can see how the code gives that impression. It uses a custom JWKSetSource to allow applications to use their RestOperations and Cache instances.

If you want to look further into Spring Cache, I think Spring's JCache integration would make sense since you can integrate in this way with Hazelcast and other caches that support refresh-ahead. That said, you know your requirements best and whether the ease of Nimbus's in-memory caching will suit your needs.

Your suggestion should work but i am not sure what's missing to get it "completed".

Am I understanding correctly that you are asking for how the PR would change? To add withJwkSource, you would need to add the appropriate static method to NimbusJwtDecoder; it's implementation would likely be similar to withJwkSource in NimbusReactiveJwtDecoder. Would you like me to push a stub implementation to your branch to get you started?

[marbon87]

Am I understanding correctly that you are asking for how the PR would change? To add withJwkSource, you would need to add the appropriate static method to NimbusJwtDecoder; it's implementation would likely be similar to withJwkSource in NimbusReactiveJwtDecoder. Would you like me to push a stub implementation to your branch to get you started?

Thanks for your offer, but i think i can manage that. Should i close this pr and create a new one referencing this?

[jzheaux]

Up to you, @marbon87. Honestly, the title of the PR still seems to work well with what we are talking about now and I'd be fine with reusing it.

[marbon87]

Hi @jzheaux,

i updated the pr. Please have a look.

[jzheaux]

Thanks for your patience, @marbon87! I realized once your PR was submitted that #17181 was needed first. With that completed, I've left some inline feedback for you.

In addition to that feedback, will you please help me with your commit message as well? It looks like there isn't a blank line between your title and your sign-off message which can mess with how certain tools render the commit.

Finally, some of these changes I'd normally do for the contributor, however this is trickier when the PR is from your main branch, which I don't want to force push to. In the future, if you first cut a branch and then submit, it's a little easier for me to apply some of the needed changes.

[jzheaux]

Please add @since 7.0

[marbon87]

Wouldn't it be possible to backport this PR to the 6.5er branch?

[jzheaux]

No, we cannot backport it to 6.x as we only add features in minor branches. Since there is no 6.6 planned, 7.0 is the next available release.

[jzheaux]

Since providing a JWKSource supercedes a RestOperations and a Cache, please create JwkSourceJwtDecoderBuilder instead of repurposing JwkSetUriJwtDecoderBuilder. In that way, people won't provide a JWKSource and then also try and provide a RestOperations or Cache.

[jzheaux]

Note that JwkSourceJwtDecoderBuilder will not need the validateTypes flag since that was added as part of migrating from 6.x to 7.0. Since this class will be added in 7.0, it doesn't need the same transition flag.

[jzheaux]

This looks like it might be a stray line from a copy-paste. Since this test wasn't added as part of #7056, let's please leave it off.

[jzheaux]

Thanks, @marbon87, for this PR! It has been merged to main.

In the meantime on 6.x, please consider using this snippet:

@Bean 
JwtDecoder jwtDecoder() {
  AuthorizationServerMetadata metadata = AuthorizationServerMetadata
          .resolve(new Issuer("https://example.org/issuer"));
  String jwkSetUri = metadata.getJwkSetUri();
  JWKSource<SecurityContext> source = JWKSourceBuilder.create(new URL(jwkSetUri))
      // your caching settings
      .build();
  ConfigurableJWTProcessor<SecurityContext> jwtProcessor = new DefaultJWTProcessor<>();
  JWSKeySelector<SecurityContext> selector = new JWSVerificationKeySelector<>(
    Set.of(JWSAlgorithm.RS256), jwkSource)
  jwtProcessor.setJWSKeySelector(selector);
  return new NimbusJwtDecoder(jwtProcessor);
}