Skip to content

Add configuration option for controlling the usage of PKCE when used with OAuth2 confidential clients #40978

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
randomstuff opened this issue Jun 3, 2024 · 3 comments
Labels
for: external-project For an external project and not something we can fix status: declined A suggestion or change that we don't feel we should currently apply

Comments

@randomstuff
Copy link

Currently confidential OAuth2 clients do not use PKCE by default. PKCE can be enabled through code. However, it would be a lot more convenient be have an option to control this using configuration instead. Otherwise everyone has to implement it by theirselves.

PKCE can be used with confidential clients and the the latest drafts suggests to use it even with confidential clients:

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.

Related:

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jun 3, 2024
@wilkinsona
Copy link
Member

Thanks for the suggestion.

We don't have a great deal of auto-configuration for this and it currently uses Spring Security's defaults for OAuth2 login:

@Configuration(proxyBeanMethods = false)
@ConditionalOnDefaultWebSecurity
static class OAuth2SecurityFilterChainConfiguration {
@Bean
SecurityFilterChain oauth2SecurityFilterChain(HttpSecurity http) throws Exception {
http.authorizeHttpRequests((requests) -> requests.anyRequest().authenticated());
http.oauth2Login(withDefaults());
http.oauth2Client(withDefaults());
return http.build();
}
}

This configuration backs off once any custom security configuration is provided.

I'm not sure that we should start offering properties that are intended to take the place of a Customizer<OAuth2LoginConfigurer<HttpSecurity>> passed to oauth2Login. We would be in danger of trying to recreate Spring Security's DSL in properties and of encouraging people to program through properties.

What's your take on this please, @jgrandja? You said in spring-projects/spring-security#12219 (comment) that "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". This was 18 months ago so I wonder if your opinion has changed since then.

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.

I don't think this is something that we'd do in Spring Boot as we prefer to keep our defaults aligned with Spring Security's. If you would like to see PKCE enabled by default, please raise a Spring Security issue.

@wilkinsona wilkinsona added the status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team label Jun 3, 2024
@jgrandja
Copy link

@wilkinsona

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

My comment was meant to be more of a general statement but I see that I was a conflicting statement instead. To be clear, a "use-pkce" configuration option per client registration would not make sense to add as a Spring Boot property.

We do not want to promote:

We would be in danger of trying to recreate Spring Security's DSL in properties and of encouraging people to program through properties.

@wilkinsona Please close this issue and we'll take it over in spring-security#12219.

@randomstuff Let's take this conversation to spring-security#12219 and see what we can do to simplify things further.

@sjohnr has an idea that he will propose.

@wilkinsona
Copy link
Member

Thanks very much, @jgrandja.

@wilkinsona wilkinsona closed this as not planned Won't fix, can't repro, duplicate, stale Jun 10, 2024
@wilkinsona wilkinsona added status: declined A suggestion or change that we don't feel we should currently apply for: external-project For an external project and not something we can fix and removed status: waiting-for-triage An issue we've not yet triaged status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team labels Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: external-project For an external project and not something we can fix status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

4 participants