OidcIdTokenDecoderFactory caches JwtDecoders and ignores ClientRegistration updates

[iigolovko]

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. #16647
If you have a better idea, you are welcome.

[rupert-madden-abbott]

I think this is the same issue as #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.

[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:

1. Remove cache at all and break backward compatibility a bit
2. Add new constructor to allow us ignore existing behaviour

[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.