Skip to content
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

Automatically enable .cors() if CorsConfigurationSource bean is present #5011

Closed
mbhave opened this issue Feb 9, 2018 · 4 comments
Closed
Assignees
Labels
in: config An issue in spring-security-config type: enhancement A general enhancement
Milestone

Comments

@mbhave
Copy link
Contributor

mbhave commented Feb 9, 2018

This would avoid Boot needing to add that to provide support for CORS out of the box.

@andersonkyle
Copy link
Contributor

IMHO enabling CORS isn't the right default for Spring Security, considering the other defaults currently set (CSRF, PasswordEncoders, etc.) Although CORS isn't entirely analogous to those other features, it still feels like it would be going against the grain, even if the allowed origin & methods are restricted.

@rwinch rwinch changed the title Enable cors by default in WebSecurityConfigurerAdapter Use CorsConfigurationSource Beans by default in WebSecurityConfigurerAdapter Mar 8, 2018
@rwinch
Copy link
Member

rwinch commented Mar 8, 2018

This is really about if a user provides a CorsConfigurationSource then leveraging that by default vs explicitly having to configure CORS

@rwinch rwinch modified the milestones: 5.0.4, 5.1.0.M1 Mar 8, 2018
@rwinch rwinch modified the milestones: 5.1.0.M1, 5.1.0.M2 May 11, 2018
@rwinch rwinch modified the milestones: 5.1.0.M2, 5.1.0.RC1 Jul 26, 2018
@marcusdacoregio
Copy link
Contributor

marcusdacoregio commented Apr 11, 2023

This is done by Spring Security since the beginning, or I am missing something, see

boolean containsCorsSource = context.containsBean(CORS_CONFIGURATION_SOURCE_BEAN_NAME);
if (containsCorsSource) {
CorsConfigurationSource configurationSource = context.getBean(CORS_CONFIGURATION_SOURCE_BEAN_NAME,
CorsConfigurationSource.class);
return new CorsFilter(configurationSource);
.

In other words, if you do http.cors(Customizer.withDefaults()) then Spring Security will also check if there is a bean of type CorsConfigurationSource to use it in the CorsFilter.

In addition to that, we do not want to configure CORS by default if the CorsConfigurationSource is present because it can become a more permissive secure default, and, Spring Security wants users to be explicit when they want to be more flexible on their security configuration.

I'll close this as invalid since this is already working as requested, but if I missed something we can reopen and continue the discussion.

@marcusdacoregio marcusdacoregio self-assigned this Apr 11, 2023
@marcusdacoregio marcusdacoregio added status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged labels Apr 11, 2023
@marcusdacoregio
Copy link
Contributor

marcusdacoregio commented Jun 2, 2023

Thanks to @jzheaux I realized that my previous comment was not giving a good argument not to do this. Instead, we should avoid boilerplate code by not requiring http.cors(...) if a CorsConfigurationSource bean is configured.

Josh's words were pretty convincing: "If an application does not have Spring Security and is using CorsConfigurationSource, then CORS is working for them. But then if they add Spring Security at that point, CORS breaks. They must add .cors() to then get it to work again. This seems broken.".

@marcusdacoregio marcusdacoregio added in: config An issue in spring-security-config type: enhancement A general enhancement and removed status: invalid An issue that we don't feel is valid labels Jun 2, 2023
@marcusdacoregio marcusdacoregio changed the title Use CorsConfigurationSource Beans by default in WebSecurityConfigurerAdapter Automatically enable .cors() if CorsConfigurationSource bean is present Jun 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: config An issue in spring-security-config type: enhancement A general enhancement
Projects
Status: Done
Development

No branches or pull requests

4 participants