Affects: 5.2.x

Hello, the intent of this issue is to explore ways to ease the configuration of CORS through WebMvcConfigurer#corsConfigurer and a CorsConfiguration instance that might have been obtained e.g. via a Spring Boot ConfigurationProperties on a CorsConfiguration bean.

Currently, if one wants to leverage Spring Boot properties to override CorsConfiguration default values, one needs to do something like this, which is really verbose, duplicates existing code, and prone to error (NullPointerException protection, etc.):

    @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());
        }
    }

I see at least two ways in which it could be simplified in the calling code:

    @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("/**").combineWithConfiguration(globalCorsConfiguration);
            }
        };
    }

The first solution would be to change the visibility of the CorsRegistration#getCorsConfiguration() method and set it public.

The second solution would be to add a new CorsRegistration#combineWithConfiguration() method that would delegate to the CorsConfiguration#combine() method of the internal configuration field, and return this (the CorsRegistration configuration) to not break the fluent API style of CorsRegistration.

What do you think?

Not sure it's a good practice, but let me cc @sdeleuze which was involved in a Spring Boot issue around the same topic, earlier: https://github.com/spring-projects/spring-boot/issues/3052

Comment From: rstoyanchev

Exposing a combine(CorsConfiguration) method on CorsRegistration is pretty straight-forward and makes sense.

Comment From: lpetit-yseop

Exposing a combine(CorsConfiguration) method on CorsRegistration is pretty straight-forward and makes sense.

Should this issue be vetted, and if you think it's a low-enough hanging fruit to start with, I'd be glad to try to provide a PR including tests and doc updates.

Comment From: rstoyanchev

Having a combine method as a convenient way of applying configuration on top of the defaults is a straight-forward affair, and if it helps you with your case, then that's all the vetting we need I think.

Feel free to send a PR. It should come down to a change in CorsRegistration although keep in mind there are two of those, one in webmvc and another in webflux.