Cannot override cache for Nimbus(Reactive)JwtDecoder in (Reactive)OidcIdTokenDecoderFactory

[afiluba]

Expected Behavior

It should be possible to customize cache in NimbusJwtDecoder created by OidcIdTokenDecoderFactory.
NimbusJwtDecoder currently supports spring cache as possible implementation.
If only OidcIdTokenDecoderFactory exposed possibility to set cache on NimbusJwtDecoder builder...

Current Behavior

OidcIdTokenDecoderFactory does not expose possibility to pass cache implementation to NimbusJwtDecoder builder.
That leads to use DefaultJWKSetCache with hardcoded lifespan and refresh time.

Context

I would like to have more control over the frequency of jwkset endpoint pooling.
I'm aware that I can create my own JwtDecoderFactory implementation but for me it looks like it would suit others too and fits into design.

I can try to prepare a PR if the idea is accepted.

[jzheaux]

Thanks, @afiluba, for the idea and the offer. I think the concern is that the cache factory would only apply for certain algorithm settings, which could be confusing. For example, a cache isn't needed when this.jwsAlgorithmResolver produces a MacAlgorithm. It isn't possible for Spring Security at that point to validate the configuration and indicate whether or not it makes sense to use setCacheFactory and setJwsAlgorithmResolver together. While this might not be a problem for advanced users, it may prove a difficult API for beginner users.

That said, I think another implementation of JwtDecoderFactory that is focused only on deriving a JwtDecoder from a JWK Set URI could work. OidcIdTokenDecoderFactory could use this implementation when the algorithm is of type SignatureAlgorithm. Would you be interested in exploring that idea with me in a PR?

[afiluba]

Hi,
I agree with Your API clearness perspective and can try to explore that further.

I looked a little bit into that and it looks for me that I need to introduce an AbstractTokenDecoderFactory, otherwise there will be a lot of duplicated code as the only method I want to customize is
private NimbusJwtDecoder buildDecoder(ClientRegistration clientRegistration) {
Going further I'm thinking if we should introduce 2 implementation, one for signature algorithms and one for mac algorithms?
And finally current OidcIdTokenDecoderFactory should delegate to one of new introduced factories?

I do not know if I'm not going to far...

I also noticed that currently OidcIdTokenDecoderFactory already has a factory for rest template which is relevant only for signature algorithms, with dedicated setter public void setRestOperationsFactory(Function<ClientRegistration, RestOperations> restOperationsFactory). Probably that method should also be moved.

I'll try to introduce a PR to further discuss this if You agree with my general idea.

[afiluba]

My second thought about this...Isn't OidcIdTokenDecoderFactory supposed to supply multiple tenants?

Looking at already supported customizations they are provided as Function<ClientRegistration, XXX>.
I'm not an oidc expert but if I understand it correctly it is possible that I have multiple client registrations in my configuration and some of them can use signature while other mac algorithms?

Does it change the perspective and explains why public void setRestOperationsFactory(Function<ClientRegistration, RestOperations> restOperationsFactory was still implemented even if it makes sense only for signature algorithms?

[franticticktick]

I can suggest a solution:

        ReactiveOidcIdTokenDecoderFactory jwtDecoderFactory = new ReactiveOidcIdTokenDecoderFactory();
        jwtDecoderFactory.setReactiveJwtDecoderFactory(
                (reactiveJwtDecoderFactory) ->
                        NimbusReactiveJwtDecoder.withJwkSetUri(jwkSetUri)
                                .jwsAlgorithm(SignatureAlgorithm.RS256)
                                .webClient(WebClient.create())
                                .build()
        );

@jzheaux @afiluba could you please review this?

[jzheaux]

Does it change the perspective and explains why public void setRestOperationsFactory(Function<ClientRegistration, RestOperations> restOperationsFactory was still implemented even if it makes sense only for signature algorithms?

Yes, @afiluba, I was thinking about that as well. Since setRestOperationsFactory has not yet gone GA, this might allow for that to be removed.

[jzheaux]

@CrazyParanoid, @afiluba, I think at that point, you should create your own JwtDecoderFactory, since the contract is the same.

For example:

@Bean 
JwtDecoderFactory<ClientRegistration> jwtDecoderFactory() {
    return (client) -> {
        String jwkSetUri = client.getProviderDetails().getJwkSetUri();
        NimbusJwtDecoder decoder = NimbusJwtDecoder.withJwkSetUri(jwkSetUri).cache(new MyCache()).build();
        decoder.setJwtValidator(new DelegatingOAuth2TokenValidator<>(
                JwtValidators.createDefault(), new OidcIdTokenValidator(client)));
        decoder.setClaimTypeConverter(
                new ClaimTypeConverter(OidcIdTokenDecoderFactory.createDefaultClaimTypeConverters()));
        return decoder;
    }
}

In this way, it's much clearer that you are in charge of setting up the entire NimbusJwtDecoder instance instead of knowing that your code is using the builder and the surrounding component is calling the setters.