SpringBoot CORS global configuration support

The recent Spring Framework 4.2 RC1 release has introduced CORS first class support. It provides fine grained CORS support thanks to the @CrossOriginannotation (nothing to do on Spring Boot side to support that except depending on Spring Framework 4.2 ...), but that may be interesting to provide a Spring Boot alternative to the CorsConfigurer Spring Framework JavaConfig API.

A key class in Spring Framework CORS support is CorsConfiguration, it allows to specify all the CORS properties to use. My proposal would be to add Spring Boot support for this kind of declaration for CORS global configuration:

@Bean
CorsConfiguration cors() {
        config.addAllowedOrigin("http://domain1.com");
        config.addAllowedHeader("header1");
}

For supporting mapped CORS configuration (since Spring Framework 4.2 support them), what about using bean names to specify the path pattern to use:

@Bean(name = "/api")
CorsConfiguration corsForApi() {
        config.addAllowedOrigin("http://domain1.com");
        config.addAllowedHeader("header1");
}

@Bean(name = "/resources")
CorsConfiguration corsForResources() {
        config.addAllowedOrigin("http://domain2.com");
        config.addAllowedHeader("header2");
}

These beans could then be retrieved by Spring Boot Cors auto-configuration facilities in a Map and registered accordingly on the various AbstractHandlerMapping instances.

Any thoughts ?

Comment From: snicoll

A couple of remarks: 1. We could reuse the configuration keys infrastructure to map the default CorsConfiguration instead of a bean definition (spring.mvc.cors.default or something) 2. I wouldn't use the name to specify the path pattern to use. What If you have XYZConfiguration tomorrow that applies to path pattern as well?

Configuring the default cors configuration via the Environment seems indeed a Spring Boot specific. I am not sure about the other one. What would you do in plain Spring Framework?

Comment From: sdeleuze

1. Indeed, reusing the configuration keys infrastructure is an interesting alternative. Even with multiple mappings. Maybe something like spring.mvc.cors.api.allowed-origins = http://domain1.com would map the CORS configuration to /api/**, with the possibility to provide a customized path pattern with the spring.mvc.cors.api.path key.
2. Indeed, there will be an issue if they use the same name.

In plain Spring Framework, you will do:

@Configuration
@EnableWebMvc
public class WebConfig extends WebMvcConfigurerAdapter {

    @Override
    public void configureCors(CorsConfigurer configurer) {
        configurer.enableCors("/api/**")
            .allowedOrigins("http://domain1.com")
            .allowedHeaders("header1");
        configurer.enableCors("resources/**")
            .allowedOrigins("http://domain2.com")
            .allowedHeaders("header2");
    }
}

Comment From: sdeleuze

Based on @snicoll feedback and more thoughts, my proposal is now:

Supporting only global CORS configuration based on CorsConfiguration bean declaration:

@Bean
CorsConfiguration cors() {
        config.addAllowedOrigin("http://domain1.com");
        config.addAllowedHeader("header1");
}

Supporting these 3 kinds of mapping thanks to Spring Boot configuration keys infrastructure:

# Default mapping on /**
spring.mvc.cors.allowed-origins = http://domain.com
spring.mvc.cors.allowed-methods = GET, POST
spring.mvc.cors.allowed-headers = header1, header2
spring.mvc.cors.exposed-headers = header1, header2
spring.mvc.cors.max-age = 3600
spring.mvc.cors.alllow-credentials = false

# Mapping on /foo/**
spring.mvc.cors.foo.allowed-origins = http://domain.com
spring.mvc.cors.foo.allowed-methods = GET, POST
spring.mvc.cors.foo.allowed-headers = header1, header2
spring.mvc.cors.foo.exposed-headers = header1, header2
spring.mvc.cors.foo.max-age = 3600
spring.mvc.cors.foo.alllow-credentials = false

# Custom mapping on /**/*.json
spring.mvc.cors.bar.path = /**/*.json
spring.mvc.cors.bar.allowed-origins = http://domain.com
spring.mvc.cors.bar.allowed-methods = GET, POST
spring.mvc.cors.bar.allowed-headers = header1, header2
spring.mvc.cors.bar.exposed-headers = header1, header2
spring.mvc.cors.bar.max-age = 3600
spring.mvc.cors.bar.alllow-credentials = false

Comment From: snicoll

I am still having a hard time with this. All I can see here points to the direction of a registry that would map a path to a CorsConfiguration. I don't like the special case being mixed with a "per-name" mapping. /**. Don't forget that if you have a Map<String,CorsConfiguration in configuration properties, you need to give him a dedicated namespace (since it's eating *).

Are users going to be willing to tune this via configuration or are they going to configure that in code? If they have several CorsConfiguration they may want to share some of these settings. With the properties-based solution, you'd have to copy/paste (as in your example).

I guess what I am saying is that I see little value of "simply" exposing the core contract to configuration. But again, I don't know how users are going to use this so more info on that may help figuring out the right level of customization. Thanks!

Comment From: snicoll

@sdeleuze and I had a call on this and I see now what we're after. So to summarize what we discussed (which answers my concerns above).

The general MvcCorsProperties may look like the following (simplified)

@ConfigurationProperties("spring.mvc.cors")
public class MvcCorsProperties {
    // property javadoc 
   private final Map<String,PathCorsConfiguration> mappings ...

    public Map<String,PathCorsConfiguration> getMappings();
}

This allows extra cors-related configuration properties to be defined in the future. PathCorsConfiguration could be an extension of CorsConfiguration that exposes the path property.

That map has a special key called default. If one registers a PathCorsConfiguration for that key, this will configure the default cors configuration. If the user creates extra configuration (for dedicated mappings), any property that is not set will actually use the default value. This should reduce the copy-paste significantly.

We may want to revisit MvcEndpointCorsProperties as it's only doing a mapping for the sake of having proper meta-data (description for each property). I think it's interesting that we keep that so we could extract that so that this feature can benefit from it as well.

Comment From: sdeleuze

@snicoll Maybe for @Bean based CORS configuration we could support CorsRegistry beans.

It is originally designed to be used in @EnableWebMvc Spring MVC configuration, but it has a nice fluent API designed to define mapped CORS configuration and could maybe be reused in Spring Boot if we make CorsRegistry#getCorsConfigurations () public.

Comment From: snicoll

Given the amount of code, I don't think it matters much. I am still not sure in which camp I sit. There are two options: - Detect the presence of a CorsRegistry bean and basically use that to "auto-configure" the CORS support - Expose a @ConfigurationProperties pojo with a Map<String,CorsConfiguration>

Doing both will be confusing IMO.

Comment From: sdeleuze

Maybe: - Simple case : a @ConfigurationProperties pojo with a CorsConfiguration that will be mapped for all the application (/**) - More advanced cases will be configured with a CorsRegistry bean

Comment From: wilkinsona

I agree with @snicoll that doing both will be confusing. We also need to make sure that whatever we do here doesn't lead to confusion with the Actuator's new CORS support.

I'm not keen on the @ConfigurationProperties Map<String,CorsConfiguration> approach. It feels like programming via properties/yaml to me. Auto-configuring a CorsRegistry bean is, imo, the most Spring Boot-like approach that's been described thus far but I'm not really sure that it's that much better than Spring Framework's existing API for configuring CORS and there's a lot to be said for only having a single way to do something.

Comment From: sdeleuze

In fact, what I like in the CorsRegistry bean solution is that would be the same API, the only thing we will add is the possibility to use it in Spring Boot applications without having to disable the web autoconfiguration (= without having to use @EnableWebMvc).

Basically the same API but natively integrated in Spring Boot.

Comment From: snicoll

Then we should detect if a bean of type CorsRegistry is present in the context and if so auto-bind it to our configuration. Would that work for you?

Comment From: sdeleuze

:+1:

Basically that means generating a Map<String,CorsConfiguration> using the CorsRegistry bean, and use it to set the corsConfiguration properties of all AbstractHandlerMapping instances created by Spring Boot (exactly what we do in WebMvcConfigurationSupport).

I will send a PR if you and @wilkinsona are ok.

Comment From: wilkinsona

Sounds good to me

Comment From: sdeleuze

After discussing with @rstoyanchev and @wilkinsona, it appears that we have already a way to declare CORS configuration without disabling Spring Boot web autoconfiguration (so for this part, I guess updating the documentation will be enough) by declaring a WebMvcConfigurer bean:

@Bean
public WebMvcConfigurer mvcConfigurer() {
    return new WebMvcConfigurerAdapter() {
        @Override
        public void addCorsMappings(CorsRegistry registry) {
            registry.addMapping("/**").allowedMethods("GET", "PUT");
        }
    };
}

But I wonder if, in addition to these advanced capabilities, we should provide a more simple global CORS configuration by mapping CorsConfiguration on /**, in order to provide an easier syntax for people with simple needs (enabling CORS on "/**" is a very common use case):

@Bean
public CorsConfiguration corsConfig() {
    CorsConfiguration config = new CorsConfiguration();
    config.addAllowedMethod("GET");
    config.addAllowedMethod("PUT");
    return config;
}

Any thoughts ?

Comment From: wilkinsona

I'm not keen on providing two similar ways to do the same thing. Also, I think the CorsConfiguration @Bean approach isn't as expressive as using a WebMvcConfigurerAdapter – it's not clear looking at the code which paths the CORS configuration will be applied to. In short, I'm a big fan of Spring Framework's API for configuring CORS and I don't think Boot needs to do anything special here.

Comment From: sdeleuze

Ok thanks, I will just update the documentation.

Comment From: sdeleuze

Could you have a look to the pull request https://github.com/spring-projects/spring-boot/pull/3523 I have just submitted?

Comment From: lpetit-yseop

Hello @sdeleuze @snicoll @wilkinsona, with the current state of the code, if one wants to leverage Spring Boot properties to override CorsConfiguration default values, one needs to do something like this:

    @Bean
    @ConfigurationProperties("cors")
    public CorsConfiguration corsConfiguration() {
        final CorsConfiguration corsConfiguration = new CorsConfiguration();
        corsConfiguration.applyPermitDefaultValues();
        return corsConfiguration;
    }

    @Bean
    public WebMvcConfigurer corsConfigurer(CorsConfiguration globalCorsConfiguration) {
        return new WebMvcConfigurer() {

            @Override
            public void addCorsMappings(CorsRegistry registry) {
                final CorsRegistration registration = registry.addMapping("/**");
                combine(registration, globalCorsConfiguration);
            }
        };
    }

    /**
     * Combines the registration's {@link CorsConfiguration} with {@code corsConfiguration},
     * as if {@link CorsConfiguration#combine(CorsConfiguration)} had been called on the registration's
     * {@link CorsConfiguration}.
     * <p>
     * This method is required since {@link CorsRegistration#getCorsConfiguration()} is not public,
     * it cannot be called and thus {@link CorsConfiguration#combine(CorsConfiguration)} cannot be called.
     */
    void combine(CorsRegistration registration, CorsConfiguration corsConfiguration) {
        if (corsConfiguration.getAllowCredentials() != null) {
            registration.allowCredentials(corsConfiguration.getAllowCredentials());
        }
        if (corsConfiguration.getAllowedHeaders() != null) {
            registration
                    .allowedHeaders(corsConfiguration.getAllowedHeaders().toArray(new String[corsConfiguration.getAllowedHeaders().size()]));
        }
        if (corsConfiguration.getAllowedMethods() != null) {
            registration
                    .allowedMethods(corsConfiguration.getAllowedMethods().toArray(new String[corsConfiguration.getAllowedHeaders().size()]));
        }
        if (corsConfiguration.getAllowedOrigins() != null) {
            registration
                    .allowedOrigins(corsConfiguration.getAllowedOrigins().toArray(new String[corsConfiguration.getAllowedHeaders().size()]));
        }
        if (corsConfiguration.getExposedHeaders() != null) {
            registration
                    .exposedHeaders(corsConfiguration.getExposedHeaders().toArray(new String[corsConfiguration.getAllowedHeaders().size()]));
        }
        if (corsConfiguration.getMaxAge() != null) {
            registration.maxAge(corsConfiguration.getMaxAge());
        }
    }

whereas, if the CorsRegistration#getCorsConfiguration() method was made public, one could just write this:

    @Bean
    @ConfigurationProperties("cors")
    public CorsConfiguration corsConfiguration() {
        final CorsConfiguration corsConfiguration = new CorsConfiguration();
        corsConfiguration.applyPermitDefaultValues();
        return corsConfiguration;
    }

    @Bean
    public WebMvcConfigurer corsConfigurer(CorsConfiguration globalCorsConfiguration) {
        return new WebMvcConfigurer() {

            @Override
            public void addCorsMappings(CorsRegistry registry) {
                // Design 1: with CorsRegistration#getCorsConfiguration() made public instead of protected
                final CorsRegistration registration = registry.addMapping("/**");
                registration.getCorsConfiguration().combine(globalCorsConfiguration);

                // Design 2: with an additional CorsRegistration#combineConfiguration() method
                registry.addMapping("/**").combineConfiguration(globalCorsConfiguration);
            }
        };
    }

which would be a big win, IMHO.

What do you think? Would you consider changing the visibility of the CorsRegistration#getCorsConfiguration() method? Or add a new CorsRegistration#combine() method that would delegate to the CorsConfiguration#combine() method and return the CorsRegistration to not break the fluent API paradigm?

Comment From: snicoll

@lpetit-yseop thanks for the feedback but this issue is closed and CorsRegistration is part of Spring Framework. If you want to suggest an enhancement, please open a separate issue in the Spring Framework issue tracker.

Comment From: lpetit-yseop

@lpetit-yseop thanks for the feedback but this issue is closed and CorsRegistration is part of Spring Framework. If you want to suggest an enhancement, please open a separate issue in the Spring Framework issue tracker.

Thank you for the suggestion. Issue created there: https://github.com/spring-projects/spring-framework/issues/25716