graphql-java-kickstart / graphql-spring-boot

GraphQL and GraphiQL Spring Framework Boot Starters - Forked from oembedler/graphql-spring-boot due to inactivity.
https://www.graphql-java-kickstart.com/spring-boot/
MIT License
1.5k stars 326 forks source link

Rest and graphql cors configuration conflict #781

Open jasonmontalto opened 2 years ago

jasonmontalto commented 2 years ago

Bug Description I have a project that contains a both rest and graphql (via this kickstart). I have rest cors configured within a WebMvcConfigurer implementation that overrides the addCorsMapping and adds our custom cors needs. This works perfectly until I enable cors for graphql. Once cors is enabled, the custom mapping is completely ignored, causing all non graphql endpoints to no longer have cors enabled.

To Reproduce

  1. Scaffold a simple spring rest setup
  2. Configure cors mappings by Creating an @Configuration class that implements WebMvcConfigurer and overrides addCorsMappings to add cors mapping
  3. Add this graphql kickstart package
  4. View app and see cors working for rest eps configured above
  5. Enable cors through application.properties graphql.servlet.corsEnabled
  6. View app and see cors no longer working for rest eps configured above, but graphql ep is working with cors

Expected behavior I would assume (without any documentation suggesting otherwise) that both cors configurations should be honored.

Browser for test

Workaround Took a while to unravel what was going on, but was able to figure out a workaround by temporarily allowing overriding of beans and overriding the cors configuration bean that comes packaged with this library. This allowed me to add my custom cors implementation in addition to the graphql cors configuration. Overriding beans is not something I want to add to our codebase, but it showed a path forward.

So my solution was to not use the WebMvcConfigurer:addCorsMapping method and instead create my own custom bean that configures cors directly using CorsConfiguration. Something like below

Example of cors mapping setup:

@Configuration
public class WebMvcConfig implements WebMvcConfigurer {
  @Override
  public void addCorsMappings(CorsRegistry registry) {
    registry.addMapping("/api/**")
      .allowedOriginPatterns("*")
      .allowedHeaders("*")
      .allowedMethods("*")
      .exposedHeaders("Authorization");
  }
}

Example of custom bean that was used as a workaround:

@Configuration
public class AppCorsConfiguration {

  @Bean
  public CorsFilter curionCorsConfiguration() {
    CorsConfiguration corsConfiguration = new CorsConfiguration();
    corsConfiguration.addAllowedOriginPattern("*");
    corsConfiguration.addAllowedHeader("*");
    corsConfiguration.addAllowedMethod("*");
    corsConfiguration.addExposedHeader("ExposedHeaderName");

    Map<String, CorsConfiguration> corsConfigurations = new LinkedHashMap<>(1);
    corsConfigurations.put("/api/**", corsConfiguration);

    UrlBasedCorsConfigurationSource configurationSource = new UrlBasedCorsConfigurationSource();
    configurationSource.setCorsConfigurations(corsConfigurations);
    configurationSource.setAllowInitLookupPath(true);

    return new CorsFilter(configurationSource);
  }
}

I am not sure if this actually a bug or expected behavior, but I found it very confusing. If it is expected behavior, would it be possible to add a note about cors configuration to the documentation?

Thanks, Jason

aloisgeith commented 2 years ago

We had this Problem also! Thx for the workaround

Regards, Alois

nort3x commented 2 years ago

yes i'm also confused, i expected it would be much more transparent and delegate these sort of configuration to spring itself but seems like i'm bound to use properties file as head first solution (which in my case i need to use dynamic resolution for cors from a database) i'm trying to tackle this problem in a more appropriate way as @jasonmontalto stated i neither like to tweak custom filters which i consider tricky and introducing unnecessary complexity

i expect graphql do what it claims it's doing i wouldn't like to tweak cors from within it's configuration