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.

[randomstuff]

From a defense-in-depth perspective, it would make sense to automatically enable PKCE if is is supported by the AS. Now, what is not clear to me is whether this might lead to some regression for some ASs.

For reference, I found that (some versions of) Keycloak could advertise a PAR endpoint (pushed_authorization_request_endpoint) even if PAR was disabled: blindly enabling PAR because pushed_authorization_request_endpoint is published could lead to some regression in this case.

[sjohnr]

Thanks for replying @randomstuff!

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

If you haven't already, please take a look at DefaultOAuth2AuthorizationRequestResolver and OAuth2AuthorizationRequestCustomizers.withPkce() (which was added with gh-6548). It is out of scope for Spring Boot to provide their own OAuth2AuthorizationRequestResolver, and therefore it wouldn't be enabled via configuration properties.

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

As mentioned above, I think using delegation on a per-client basis could be worth exploring but this can already be done today as a customization with the existing components. Can you please clarify what enhancement you are recommending?

From a defense-in-depth perspective, it would make sense to automatically enable PKCE if is is supported by the AS.

Can you clarify this point further? How are you proposing that it would be automatically enabled?

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.

The section of OAuth 2.1 you provided makes it clear that it is REQUIRED for public clients, not confidential clients. Spring Security already meets this requirement. Are you seeing it differently than I am?

Now, what is not clear to me is whether this might lead to some regression for some ASs.

Making PKCE enabled by default for confidential clients would absolutely be a breaking change, and as such can't be done in a minor release. Pursuing it for a major release would be possible if it is REQUIRED per the spec.

[randomstuff]

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.

The section of OAuth 2.1 you provided makes it clear that it is REQUIRED for public clients, not confidential clients. Spring Security already meets this requirement. Are you seeing it differently than I am?

Yes, I don't read it this way. I would say that the wording means that there is one specific case where you are allowed not to use PKCE (but still should), when all the following are true at the same time:

1. the client must be a confidential client;
2. and the authorization server must properly support OIDC nonce (which AFAIU excludes pure OAuth flows because nonce cannot be used in this case as it is supposed to be echoed back by the AS as part of the ID token claims*);
3. and you must have reasons to believe that nonce is properly supported.

If we only consider (2), this means that PKCE MUST be used for non-OIDC flows. We might skip it for OIDC flows. For pure OAuth flows (when openid is not present in the scope), you MUST use PKCE.

For (3), we could assume that OIDC authorization servers (OpenID providers) properly support nonce. However, I would claim that a strict interpretation of the wording would lead to an opt-out mechanism per authorization server: you can opt-out if you know that the authorization server actually handles the nonce properly.

Other (some non-normative) paragraphs which suggest that "really you really really should be using PKCE" are:

Specifically, features not specified in OAuth 2.0 core, such as PKCE, are required in OAuth 2.1.

[…]

The authorization code grant is extended with the functionality from PKCE ([RFC7636]) such that the default method of using the authorization code grant according to this specification requires the addition of the PKCE parameters

(i.e. by default you should be using PKCE, i.e. PKCE should be opt-out)

and

Clients MUST use code_challenge and code_verifier and authorization servers MUST enforce their use except under the conditions described in Section 7.5.1. In this case, using and enforcing code_challenge and code_verifier as described in the following is still RECOMMENDED.

From a defense-in-depth perspective, it would make sense to automatically enable PKCE if is is supported by the AS.

I mean, if PKCE support must be enabled through code, only the experts in OAuth will wonder if this is enabled and might enable it. Or maybe people will ultimately find about this after checking an OWASP ASVS check list and might make the extra effort to actually enable it. In many cases, it won't be enabled even if it could because people don't know what this is and what the impact is.

Can you clarify this point further? How are you proposing that it would be automatically enabled?

I was thinking about enabling it by default if support is detected through server metadata (code_challenge_methods_supported):

code_challenge_methods_supported
OPTIONAL. JSON array containing a list of Proof Key for Code
Exchange (PKCE) [RFC7636] code challenge methods supported by this
authorization server. Code challenge method values are used in
the "code_challenge_method" parameter defined in Section 4.3 of
[RFC7636]. The valid code challenge method values are those
registered in the IANA "PKCE Code Challenge Methods" registry
[IANA.OAuth.Parameters]. If omitted, the authorization server
does not support PKCE.

Actually (in the long term), a per-provider property such as spring.security.oauth2.client.provider.[providerId].code-challenge-methods-supported (Edit : or maybe spring.security.oauth2.client.registration.[registrationId].code-challenge-methods-supported) could be used to override the default default value which would:

• be detected using server metadata if available;
• be empty otherwise (so that PKCE would still be disabled by default when no server metadata is used).

PKCE could be manually enabled by setting spring.security.oauth2.client.provider.[providerId].authorization-uri.code-challenge-methods-supported to S256 … or manually disabled by setting it to en empty value.

Making PKCE enabled by default for confidential clients would absolutely be a breaking change, and as such can't be done in a minor release. Pursuing it for a major release would be possible if it is REQUIRED per the spec.

Yes when I was saying that in the long run, I would expect it would be enabled by default, this was possibly in the next major release.

(*): For example, nonce verification is implemented in OidcAuthorizationCodeAuthenticationProvider.validateNonce() but not in OAuth2AuthorizationCodeAuthenticationProvider.

[randomstuff]

As #12219 (comment), I think using delegation on a per-client basis could be worth exploring but this can already be done today as a customization with the existing components. Can you please clarify what enhancement you are recommending?

Oh sorry, I missed (or did not properly parse 😄) the section about per-registration customization. This looks interesting.

I'm wondering if there is some way such a setup could use the server metadata (actually more likely to be in ClientRegistration) so that it would be possible to implement a withAutomaticPkce() customizer (which would enable PKCE if support is detected in the the server metadata).

[sjohnr]

@randomstuff

The section of OAuth 2.1 you provided makes it clear that it is REQUIRED for public clients, not confidential clients. Spring Security already meets this requirement. Are you seeing it differently than I am?

Yes, I don't read it this way.

If I'm understanding you correctly, I see your point that non-OIDC flows appear to always require PKCE -- if OAuth 2.1 were promoted to RFC status.

Other (some non-normative) paragraphs which suggest that "really you really really should be using PKCE" are:

I think we have to be careful reading too much into the summary and overview information in the spec. The specific details we've discussed in Section 7.5.2 are key. However, I think we're reading them the same way.

Regardless, without another way to determine that the AS supports PKCE, it can't simply be enabled due to the potential for breaking deployments, and I think that would be the case even in a major where we can make some breaking changes (though I could be wrong on that point). Which is why...

I'm wondering if there is some way such a setup could use the server metadata (actually more likely to be in ClientRegistration) so that it would be possible to implement a withAutomaticPkce() customizer (which would enable PKCE if support is detected in the the server metadata).

I like this idea! However, the ClientRegistration is not currently available in the customizer so there are challenges to this we'd have to look at. Let me take this away and discuss with the team, and see if it makes sense to open an issue for this idea as well as determine details of the enhancement we would propose. I'll tag you on any new issues opened as a result of this conversation.

Lastly, regarding my other comment:

I'd like to propose that we enhance Spring Security to support configuring the OAuth2AuthorizationRequestResolver by publishing as a @Bean.

I feel it helps with any of the other enhancements we propose, so that customization options for enabling PKCE can be applied using bean configuration instead of customizing only via the DSL. Do you agree that this would also be a worthwhile enhancement?

[randomstuff]

I'd like to propose that we enhance Spring Security to support configuring the OAuth2AuthorizationRequestResolver by publishing as a @bean.

Do you agree that this would also be a worthwhile enhancement?

Yes, I agree this would certainly help.

[sjohnr]

Thanks so much @randomstuff! I appreciate the conversation immensely.

[rwinch]

For those following this issue as an FYI I have created gh-16382