Hello. OidcIdTokenDecoderFactory caches JwtDecoders and ignores ClientRegistration updates. It's required to provide uncached way to build JwtDecoder based on actual ClientRegistration.
For example we have a custom ClientRegistrationRepository that uses db to store ClientRegistrations. It is expected that the settings can be changed dynamically. But OidcIdTokenDecoderFactory caches first created JwtDecoder by registrationId. And if we need to change value of jwsAlgorithm either jwksUri, we can't do it without the restarting of our application.
For now it can be asf crutch
OidcIdTokenDecoderFactory must provide a way to prevent caching of JwtDecoders.
For now, this can only be achieved with a dirty crutch below (via creation of new OidcIdTokenDecoderFactory everytime we need to access JwtDecoder).
@RequiredArgsConstructor
public class SsoOidcIdTokenDecoderFactory implements JwtDecoderFactory<ClientRegistration> {
private final Function<ClientRegistration, JwsAlgorithm> jwsAlgorithmResolver;
@Override
public JwtDecoder createDecoder(ClientRegistration clientRegistration) {
OidcIdTokenDecoderFactory tempFactory = new OidcIdTokenDecoderFactory();
tempFactory.setJwsAlgorithmResolver(this.jwsAlgorithmResolver);
return tempFactory.createDecoder(clientRegistration);
}
}
I prepared PR that fixes it. https://github.com/spring-projects/spring-security/pull/16647 If you have a better idea, you are welcome.
Comment From: rupert-madden-abbott
I think this is the same issue as https://github.com/spring-projects/spring-security/issues/12816.
We also have the same issue where we want client registrations to be added dynamically at runtime and so store them in the database. The fact that they are cached means the configuration is frozen in memory the first time somebody logs in using a given client registration. If the client registration then changes (e.g. if they have got the configuration incorrect and want to fix it), the change is ignored due to this cache until we restart the server.
Disabling the cache would work for us if the performance consequences of doing that were not too costly.
Alternatively, being able to evict the cache would also work for us as we could then do that whenever the configuration changed.
Comment From: iigolovko
It's easier to remove cache at all.
To manage cache eviction we should implement equals
for org.springframework.security.oauth2.client.registration.ClientRegistration
and store ClientRegistration somewhere in OidcIdTokenDecoderFactory. And then we have an additional challange in concurrent environment on decoder update.
Nothing terrible to cache results of the same input data. But we are in another situation.
So I suggest one of the ways:
- Remove cache at all and break backward compatibility a bit
- Add new constructor to allow us ignore existing behaviour
Comment From: ioanngolovko
Another problem is that these factories cause memory leaks.
There is no mechanism to clean up jwtDecoders Map. So if we often, for example, rename registrationId, the factories accumulate outdated Decoders without cleaning.
Comment From: jgrandja
Closing as duplicate of gh-12816