create new field in ClientRegistration (e.g. "alwaysPkce") to enable PKCE for confidential clients

[drahkrub]

Since #6548 it is possible to enable PKCE for confidential clients - great!

Unfortunately, this can only be configured in a programmatic way.

It would be nice to encode this information ("use PKCE for confidential clients, yes or no") inside the ClientRegistration, such that it is handled automatically per client.

The current programmatic configuration seems to force the usage of some custom OAuth2AuthorizationRequestResolver (which delegates to a customized or uncustomized version of DefaultOAuth2AuthorizationRequestResolver) if different confidential clients need different pkce handling.

[sjohnr]

Thanks for the suggestion, @drahkrub!

In general, I think adding additional specific flags/configuration to ClientRegistration would be rare. Given the importance of PKCE in specs like OAuth 2.1 and OpenID Connect 1.0, perhaps it could be made configurable in some way (without code). @jgrandja any thoughts on this?

[jgrandja]

@drahkrub PKCE is only applicable for the authorization_code grant and not all ClientRegistration's would be configured with AuthorizationGrantType.AUTHORIZATION_CODE - some may be configured with AuthorizationGrantType.CLIENT_CREDENTIALS. Given this, it would not make sense to introduce a new field ClientRegistration.applyPkce (or similar).

This type of configuration/setting would make more sense in the Spring Boot auto-configuration classes and properties.

However, I don't feel it's necessary as the configuration is pretty straight forward as per example 1 and example 2.

I'm going to close this based on the explanation provided.

[sjohnr]

@randomstuff as a follow-up to this comment, I'd like to propose that we enhance Spring Security to support configuring the OAuth2AuthorizationRequestResolver by publishing as a @Bean. This would simplify this customization and allow for the following configuration:

@Bean
public OAuth2AuthorizationRequestResolver authorizationRequestResolver(
		ClientRegistrationRepository clientRegistrationRepository) {

	var authorizationRequestResolver =
		new DefaultOAuth2AuthorizationRequestResolver(
			clientRegistrationRepository,
			OAuth2AuthorizationRequestRedirectFilter.DEFAULT_AUTHORIZATION_REQUEST_BASE_URI);
	authorizationRequestResolver.setAuthorizationRequestCustomizer(
		OAuth2AuthorizationRequestCustomizers.withPkce());

	return authorizationRequestResolver;
}

I believe the above proposal is the best middle ground, as this enhancement would be in line with other OAuth2-related "simplification" enhancements as well as a general approach in Spring Security to bean-based configuration. As mentioned on the Spring Boot issue, we wouldn't pursue enabling such a configuration via a Spring Boot configuration property, so it would continue to be an application-level requirement to add this configuration. Any thoughts on that? Should we open a new issue for this?

Separately, I also wonder if configuring PKCE on a per-client basis is common? If it is (research and feedback required), another enhancement we could look into is providing a delegating implementation of OAuth2AuthorizationRequestResolver that delegates per client registration id. But this would be purely for convenience, so I don't see it being required.

Lastly,

I think it would make sense to have this enabled by default in the medium term (apparently, there is fear that this might break some authorization servers) so maybe an option whose default value could be changed in the future would be nice.

Regarding enabling PKCE for confidential clients by default, this has already been discussed and isn't likely to change unless it becomes required by a relevant specification.

[randomstuff]

As mentioned on the Spring Boot issue, we wouldn't pursue enabling such a configuration via a Spring Boot configuration property, so it would continue to be an application-level requirement to add this configuration

The thing is I that whether to use PKCE for a given provider looks like some something that really wants to be (per-provider) configuration : it depends on the provider (some provider support it, some don't support it, some might advertise they support it be in reality their support is broken so you wouldn't want to use it, maybe (?) some support it only for public clients).

Now, maybe nobody actually would use such a setup in practice 🤷 but I would find it weird.

Regarding enabling PKCE for confidential clients by default, this has #6548 (comment) and isn't likely to change unless it becomes required by a relevant specification.

Yes, I was hinting at the fact that AFAIU, OAauth 2.1 is expected to be REQUIRED:

To prevent injection of authorization codes into the client, using code_challenge and code_verifier is REQUIRED for clients, and authorization servers MUST enforce their use, unless both of the following criteria are met:

• The client is a confidential client.

• In the specific deployment and the specific request, there is reasonable assurance by the authorization server that the client implements the OpenID Connect nonce mechanism properly.

This is only a draft but if I guess there is a consensus on the fact that PKCE will be required.