Skip to content

Provide a way to pass a cache in JwtDecoders #8885

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
nicolas-teck-mox opened this issue Jul 30, 2020 · 7 comments
Closed

Provide a way to pass a cache in JwtDecoders #8885

nicolas-teck-mox opened this issue Jul 30, 2020 · 7 comments
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement

Comments

@nicolas-teck-mox
Copy link

Summary

Module: spring-security-oauth2-jose
Version: 5.4.0-M2

Thanks to this PR, we are now able to pass our own custom cache to store JWK set. However there is no way to pass this cache from JwtDecoders methods

Current Behavior

Currently only the oidc issuer location can be use to create a JwtDecoder. There is no other configuration that can be provided.

JwtDecoders.fromOidcIssuerLocation(uri)

Expected Behavior

Would it be possible to pass the custom cache to this method so it can get pass downstream?

@nicolas-teck-mox nicolas-teck-mox added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels Jul 30, 2020
@jzheaux
Copy link
Contributor

jzheaux commented Jul 30, 2020

@nicolas-teck-mox, thanks for the suggestion.

Since JwtDecoders is designed as a utility for getting a default instance, it's not really the place for adding configurability. I think there are other ways to address your issue, though.

One way would be to introduce NimbusJwtDecoder#withIssuerLocation. This would return a builder like NimbusJwtDecoder#withJwkSetUri does, including a cache(Cache) builder method. Then, for applications that need to customize the cache, they'd call NimbusJwtDecoder#withIssuerLocation instead of JwtDecoders#withIssuerLocation.

Because there's a related conversation going on right now about wiring a custom RestTemplate, I think we should let that conversation progress a bit further before making a change.

In the meantime, you can configure the JWK Set URI directly instead of using discovery:

@Bean 
public JwtDecoder decoder() {
    NimbusJwtDecoder decoder = NimbusJwtDecoder
            .withJwkSetUri(jwkSetUri)
            .cache(cache)
            .build();
    decoder.setJwtValidator(JwtValidators.createDefaultWithIssuer(issuer));
    return decoder;
}

@jzheaux jzheaux added in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 30, 2020
@jzheaux jzheaux self-assigned this Jul 30, 2020
@DarrenForsythe
Copy link
Contributor

DarrenForsythe commented Dec 16, 2020

@jzheaux it would be good for this to encompass all methods to build a NimbusJwtDecoder, my specific use case would be around the public key configuration which doesn't expose the ability to cache a public key for a given time.

If that's possible with wrapping nimbus could be beyond the capabilites?

@jzheaux
Copy link
Contributor

jzheaux commented Dec 16, 2020

Hey, @DarrenForsythe. That's an interesting use case.

When it comes to NimbusJwtDecoder#withPublicKey, for example, you simply provide the key, so configuring a cache wouldn't make sense since there's no way to reload the cache once it expires.

However, I wonder what are your thoughts on #5403. Does that seem like what you are looking for?

@DarrenForsythe
Copy link
Contributor

It is interesting :)

And you are correct our implementation just accepts that as a risk that the IdP might roll the keys and applications will not get the updated key for X time (configurable by applications).

However, depending on ops, could this not also be a risk with caching (or lack there of) of a JWK Set? Our main use case is to simply avoid the I/O

@jzheaux
Copy link
Contributor

jzheaux commented Dec 17, 2020

That's one thing that makes the kid useful. When an IdP rotates keys, the new key has a new globally-unique kid. Once the resource server sees an unrecognized kid, it's a hint to invalidate the cache.

But I'm thinking I might have misunderstood your question since to me the two scenarios don't sound equivalent. If you are doing this (hypothetically):

JwtDecoder jwtDecoder = NimbusJwtDecoder.withPublicKey(key).cache(cache).build();

where will the cache get the new key from once the cache is invalidated? It seems more like you'd need something like:

JwtDecoder jwtDecoder = NimbusJwtDecoder.withPublicKeySource(keySource).cache(cache).build();

This way, when the cache is invalidated, the decoder can invoke the key source to get the latest key or set of keys.

@DarrenForsythe
Copy link
Contributor

Yes that would be equivalent to what I've implemented at the moment

@jzheaux
Copy link
Contributor

jzheaux commented Jun 21, 2022

Closing this ticket as the preferred way to pass a cache is to use the NimbusJwtDecoder builder.

@jzheaux jzheaux closed this as completed Jun 21, 2022
@jzheaux jzheaux added the status: declined A suggestion or change that we don't feel we should currently apply label Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants