Allow configurable JwtDecoder for IdToken verification

[jgrandja]

We should expose a configuration in the oauth2Login() DSL that will allow users to configure a custom JwtDecoder to be used for verifying the signature of the OidcIdToken during an OpenID Connect flow.

The OidcAuthorizationCodeAuthenticationProvider contains the property private final Map<String, JwtDecoder> jwtDecoders, which maps clientRegistrationId to JwtDecoder. This property is currently hidden so there is no way to configure it. We should leverage the same property and existing logic internally, however, the DSL should allow for the configuration of this property.

For example:

oauth2Login().tokenEndpoint().oidcIdTokenDecoder(String clientRegistrationId, JwtDecoder jwtDecoder)

OR

oauth2Login().tokenEndpoint().oidcIdTokenDecoders(Map<String, JwtDecoder> jwtDecoders)

[rwinch]

@jgrandja I'd like to suggest a little refactoring of the initial API above.

I don't think that providing a mapping of clientRegistrationId to JwtDecoder. At this time the user does not easily have the information necessary (i.e. the keys) to create the JwtDecoder.

Instead, I think we should provide an API that accepts a ClientRegistration and returns a JwtDecoder (i.e. Converter<ClientRegistration,JwtDecoder>). This will ensure that the developer won't need to look up every ClientRegistration at startup time. This is not only impractical for large numbers, but reduces the benefit of Spring Boot creating the ClientRegistrations for the user since they must now look them all up.

[jgrandja]

@rwinch

At this time the user does not easily have the information necessary (i.e. the keys) to create the JwtDecoder

My assumption is if the user provides a custom implementation of JwtDecoder than they have all the info they need to implement the decoding and signature verification in their implementation. They can than configure the mapping using the clientRegistrationId and their JwtDecoder impl.

If there is no mapping configured for a clientRegistrationId than the existing logic will be applied where we default to NimbusJwtDecoderJwkSupport.

I do agree though that a factory method that accepts a ClientRegistration and returns a JwtDecoder provides greater flexibility. However, using the Converter doesn't make sense to me as it's not converting. I prefer Function over Converter, as is currently implemented in OidcAuthorizationCodeReactiveAuthenticationManager.DefaultDecoderFactory. However, is Function too generic? I'm not sure we want to introduce a new API at this point either?

[rwinch]

If there is no mapping configured for a clientRegistrationId than the existing logic will be applied where we default to NimbusJwtDecoderJwkSupport.

My point is that if there is 1M client registrations, then with the current approach the developer must lookup 1M client registrations at startup time to perform the mappings. This is not practical. One might say 1M client registrations is unrealistic, but I think very large numbers are possible (especially for a multi tenancy application).

I do agree though that a factory method that accepts a ClientRegistration and returns a JwtDecoder provides greater flexibility.

Ok seems like we are on the same page here.

I prefer Function over Converter, as is currently implemented in OidcAuthorizationCodeReactiveAuthenticationManager.DefaultDecoderFactory. However, is Function too generic? I'm not sure we want to introduce a new API at this point either?

Is there a reason you prefer Function other than that is how it is in the reactive side? I'm wondering, because since it isn't public we can certainly change the reactive side.

My concern is that Function is too generic. I'm not sure I'm set on Converter, but I am fairly convinced we should have an API that accepts a ClientRegistration and returns the JwtDecoder.

As you have alluded to, I'm also convinced we don't want a new API.

[jgrandja]

Is there a reason you prefer Function

As I mentioned, I prefer Function over Converter. However, I'm not saying I would go with Function either as it's too generic. I know Converter doesn't make sense to me.

We agree that we need an API that accepts a ClientRegistration and returns a JwtDecoder. The question is which existing API can we leverage that makes sense? Or do we wait until after 5.1 and possibly introduce a new API?

[rwinch]

I do not think we should have a new API for this. I'd say for now anyone wanting to help with this can use Function<ClientRegistration, JwtDecoder> and can expect that that part might change, but it should be a pretty simple change.