fix: CORS Defaults v3

.github/workflows/integration-tests.yml

@@ -137,6 +137,8 @@ jobs:
                     APIML_GATEWAY_SERVICESTOLIMITREQUESTRATE: discoverableclient
                     APIML_GATEWAY_SERVICESTODISABLERETRY: discoverableclient
                     APIML_GATEWAY_COOKIENAMEFORRATELIMIT: apimlAuthenticationToken
+                    APIML_SERVICE_CORSENABLED: true
+                    APIML_SERVICE_CORSDEFAULTALLOWEDORIGINS: https://localhost:10010,https://localhost2:10010
                 #The memory is constrained to test large file upload memory issues (#4265)
                 #If the container runs oom, it is hard killed without printing any reasonable message in the log
                 options: --memory 640m --memory-swap 640m
@@ -476,6 +478,8 @@ jobs:
                     EUREKA_CLIENT_INSTANCEINFOREPLICATIONINTERVALSECONDS: 1
                     EUREKA_CLIENT_REGISTRYFETCHINTERVALSECONDS: 1
                     logbackService: ZWEAGW1
+                    APIML_SERVICE_CORSENABLED: true
+                    APIML_SERVICE_CORSDEFAULTALLOWEDORIGINS: https://localhost:10010,https://localhost2:10010
                 options: --memory 640m --memory-swap 640m
             zaas-service:
                 image: ghcr.io/balhar-jakub/zaas-service:${{ github.run_id }}-${{ github.run_number }}
@@ -499,6 +503,8 @@ jobs:
                     APIML_SERVICE_HOSTNAME: gateway-service-2
                     APIML_SERVICE_DISCOVERYSERVICEURLS: https://discovery-service-2:10011/eureka/
                     logbackService: ZWEAGW2
+                    APIML_SERVICE_CORSENABLED: true
+                    APIML_SERVICE_CORSDEFAULTALLOWEDORIGINS: https://localhost:10010,https://localhost2:10010
                 options: --memory 640m --memory-swap 640m
             zaas-service-2:
                 image: ghcr.io/balhar-jakub/zaas-service:${{ github.run_id }}-${{ github.run_number }}

apiml-package/src/main/resources/bin/start.sh

@@ -59,6 +59,9 @@
 # - ZWE_configs_apiml_security_x509_registry_allowedUsers
 # - ZWE_configs_apiml_service_allowEncodedSlashes
 # - ZWE_configs_apiml_service_corsEnabled
+# - ZWE_configs_apiml_service_corsAllowedMethods
+# - ZWE_configs_apiml_service_corsDefaultAllowedOrigins
+# - ZWE_configs_apiml_service_corsDefaultAllowedHeaders
 # - ZWE_configs_apiml_service_forwardClientCertEnabled
 # - ZWE_configs_apimlId
 # - ZWE_configs_certificate_ciphers / ZWE_configs_ciphers
@@ -316,6 +319,8 @@ _BPX_JOBNAME=${ZWE_zowe_job_prefix}${APIML_CODE} ${JAVA_BIN_DIR}java \
     -Dapiml.service.apimlId=${ZWE_components_gateway_apimlId:-${ZWE_configs_apimlId:-}} \
     -Dapiml.service.corsAllowedMethods=${ZWE_components_gateway_apiml_service_corsAllowedMethods:-${ZWE_configs_apiml_service_corsAllowedMethods:-GET,HEAD,POST,PATCH,DELETE,PUT,OPTIONS}} \
     -Dapiml.service.corsEnabled=${ZWE_components_gateway_apiml_service_corsEnabled:-${ZWE_configs_apiml_service_corsEnabled:-false}} \
+    -Dapiml.service.corsDefaultAllowedOrigins=${ZWE_components_gateway_apiml_service_corsDefaultAllowedOrigins:-${ZWE_configs_apiml_service_corsDefaultAllowedOrigins:-}} \
+    -Dapiml.service.corsDefaultAllowedHeaders=${ZWE_components_gateway_apiml_service_corsDefaultAllowedHeaders:-${ZWE_configs_apiml_service_corsDefaultAllowedHeaders:-}} \
     -Dapiml.service.forwardClientCertEnabled=${ZWE_components_gateway_apiml_security_x509_enabled:-${ZWE_configs_apiml_security_x509_enabled:-false}} \
     -Dapiml.service.hostname=${ZWE_haInstance_hostname:-localhost} \
     -Dapiml.service.port=${ZWE_components_gateway_port:-${ZWE_configs_port:-7554}} \

apiml/src/main/resources/application.yml

@@ -197,7 +197,7 @@ apiml:
         forwardClientCertEnabled: false
         hostname: localhost
         id: ${spring.application.name}
-        ignoredHeadersWhenCorsEnabled: Access-Control-Request-Method,Access-Control-Request-Headers,Access-Control-Allow-Origin,Access-Control-Allow-Methods,Access-Control-Allow-Headers,Access-Control-Allow-Credentials,Origin
+        ignoredHeadersWhenCorsEnabled: Access-Control-Request-Method,Access-Control-Request-Headers,Origin
         port: 10010
         scheme: https  # "https" or "http"
         preferIpAddress: false

apiml/src/test/java/org/zowe/apiml/acceptance/AttlsConfigTest.java

@@ -215,15 +215,9 @@ void whenNoKeystore_thenStartupSuccess() {
     @Nested
     @ActiveProfiles({"attlsClient", "attlsServer", "WhenCorsEnabledService"})
     @DirtiesContext
-    @SpringBootTest(classes = {
-            ApimlApplication.class,
-            FreeMarkerConfigurer.class,
-            TestConfig.class
-        },
-        webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT
-    )
     @AcceptanceTest
     @TestInstance(Lifecycle.PER_CLASS)
+    // this test requires a defined port to either match the default allowed origin or set apiml.corsDefaultAllowedOrigins property with the known port
     class WhenCorsEnabledService extends AcceptanceTestWithMockServices {
 
         private static final String VALID_CERT =

apiml/src/test/resources/application.yml

@@ -1,8 +1,8 @@
 logging:
     level:
-        ROOT: INFO
-        #org.springframework: DEBUG
-        #org.springframework.boot.autoconfigure.web: DEBUG
+        ROOT: DEBUG
+        org.springframework: DEBUG
+        org.springframework.boot.autoconfigure.web: DEBUG
         org.springframework.cloud.gateway: DEBUG
         org.springframework.http.server.reactive: DEBUG
         org.springframework.web.reactive: DEBUG
@@ -43,7 +43,7 @@ apiml:
         serviceId: apicatalog
     discovery:
         staticApiDefinitionsDirectories: config/local/api-defs
-        allPeersUrls: http://${apiml.service.hostname}:${apiml.service.port}/eureka/
+        allPeersUrls: https://${apiml.service.hostname}:${apiml.service.port}/eureka/
     internal-discovery:
         address: ${server.address}
         port: 10011

build.gradle

@@ -101,6 +101,17 @@ allprojects {
         }
     }
 
+    sourceSets {
+        main {
+            java { srcDirs = ['src/main/java'] }
+            resources { srcDirs = ['src/main/resources'] }
+        }
+        test {
+            java { srcDirs = ['src/test/java'] }
+            resources { srcDirs = ['src/test/resources'] }
+        }
+    }
+
     configurations.all {
         resolutionStrategy.dependencySubstitution {
             substitute(module('javax.servlet:servlet-api')).using(module('javax.servlet:javax.servlet-api:4.0.1'))
@@ -165,6 +176,7 @@ configure(subprojects.findAll {it.name != 'platform'}) {
         testImplementation libs.junit.platform.commons
         testImplementation libs.junit.platform.engine
     }
+
 }
 
 subprojects {

common-service-core/src/main/java/org/zowe/apiml/util/CorsUtils.java

@@ -10,9 +10,9 @@
 
 package org.zowe.apiml.util;
 
+import lombok.Builder;
 import lombok.NonNull;
 import lombok.extern.slf4j.Slf4j;
-import org.apache.logging.log4j.util.TriConsumer;
 import org.springframework.web.cors.CorsConfiguration;
 
 import java.util.Arrays;
@@ -22,68 +22,108 @@
 import java.util.function.BiConsumer;
 import java.util.regex.Pattern;
 
+import static org.apache.commons.lang3.StringUtils.isNotBlank;
+
 @Slf4j
+@Builder
 public class CorsUtils {
 
-    private final List<String> allowedCorsHttpMethods;
-    private final boolean corsEnabled;
     private static final Pattern gatewayRoutesPattern = Pattern.compile("apiml\\.routes\\.[^.]*\\.gateway\\S*");
+
+    private final List<String> defaultAllowedCorsHttpMethods;
+    private final boolean gatewayCorsEnabled;
     private final List<String> corsAllowedEndpoints;
+    private final List<String> defaultAllowedCorsOrigins;
+    private final List<String> defaultAllowedCorsHeaders;
+    private final boolean defaultAllowCredentials;
 
-    public CorsUtils(boolean corsEnabled, List<String> corsAllowedMethods, @NonNull List<String> allowedEndpoints) {
-        this.corsEnabled = corsEnabled;
-        this.allowedCorsHttpMethods = corsAllowedMethods;
+    public CorsUtils(
+            List<String> corsAllowedMethods,
+            boolean corsEnabled,
+            @NonNull List<String> allowedEndpoints,
+            @NonNull List<String> defaultAllowedCorsOrigins,
+            @NonNull List<String> defaultAllowedCorsHeaders,
+            boolean defaultAllowCredentials) {
+        this.defaultAllowedCorsHttpMethods = corsAllowedMethods;
+        this.gatewayCorsEnabled = corsEnabled;
         this.corsAllowedEndpoints = allowedEndpoints;
+        this.defaultAllowedCorsOrigins = defaultAllowedCorsOrigins;
+        this.defaultAllowedCorsHeaders = defaultAllowedCorsHeaders;
+        this.defaultAllowCredentials = defaultAllowCredentials;
     }
 
     public boolean isCorsEnabledForService(Map<String, String> metadata) {
-        String isCorsEnabledForService = metadata.get("apiml.corsEnabled");
+        var isCorsEnabledForService = metadata.get("apiml.corsEnabled");
         return Boolean.parseBoolean(isCorsEnabledForService);
     }
 
-    public void setCorsConfiguration(String serviceId, Map<String, String> metadata, TriConsumer<String, String, CorsConfiguration> entryMapper) {
-        if (corsEnabled) {
-            var corsConfiguration = setAllowedOriginsForService(serviceId, metadata);
+    public void setCorsConfiguration(String serviceId, Map<String, String> metadata, BiConsumer<String, CorsConfiguration> entryMapper) {
+        if (gatewayCorsEnabled) {
+            var corsConfiguration = createCorsConfigurationForService(serviceId, metadata);
             metadata.entrySet().stream()
                 .filter(entry -> gatewayRoutesPattern.matcher(entry.getKey()).find())
                 .forEach(entry ->
-                    entryMapper.accept(entry.getValue(), serviceId, corsConfiguration));
+                    entryMapper.accept(entry.getValue(), corsConfiguration));
+        } else {
+            log.debug("CORS is not enabled in Gateway");
         }
     }
 
-    private CorsConfiguration setAllowedOriginsForService(String serviceId, Map<String, String> metadata) {
+    private CorsConfiguration createCorsConfigurationForService(String serviceId, Map<String, String> metadata) {
         // Check if the configuration specifies allowed origins for this service
         var config = new CorsConfiguration();
         if (isCorsEnabledForService(metadata)) {
+
+            defaultAllowedCorsOrigins.forEach(config::addAllowedOrigin);
+
             var corsAllowedOriginsForService = metadata.get("apiml.corsAllowedOrigins");
-            if (corsAllowedOriginsForService == null || corsAllowedOriginsForService.isEmpty()) {
-                // Origins not specified: allow everything
-                log.debug("For service {}, set all as allowed origins", serviceId);
-                config.addAllowedOriginPattern(CorsConfiguration.ALL);
-            } else {
+            var allowedHeadersForService = metadata.get("apiml.corsAllowedHeaders");
+            var allowedCredentialsForService = metadata.get("apiml.corsAllowCredentials");
+            var allowedMethodsForService = metadata.get("apiml.corsAllowedMethods");
+
+            if (isNotBlank(corsAllowedOriginsForService)) {
                 // Origins specified: split by comma, add to whitelist
                 log.debug("For service {}, set [{}] as allowed origins", serviceId, Arrays.toString(corsAllowedOriginsForService.split(",")));
                 Arrays.stream(corsAllowedOriginsForService.split(","))
                     .forEach(config::addAllowedOrigin);
             }
-            config.setAllowCredentials(true);
-            config.setAllowedHeaders(Collections.singletonList(CorsConfiguration.ALL));
-            config.setAllowedMethods(allowedCorsHttpMethods);
+
+            if (isNotBlank(allowedCredentialsForService)) {
+                config.setAllowCredentials(Boolean.parseBoolean(allowedCredentialsForService));
+            } else {
+                config.setAllowCredentials(defaultAllowCredentials);
+            }
+
+            if (isNotBlank(allowedMethodsForService)) {
+                config.setAllowedMethods(Arrays.asList(allowedMethodsForService.split(",")));
+            } else {
+                config.setAllowedMethods(defaultAllowedCorsHttpMethods);
+            }
+
+            if (isNotBlank(allowedHeadersForService)) {
+                config.setAllowedHeaders(Arrays.asList(allowedHeadersForService.split(",")));
+            } else {
+                config.setAllowedHeaders(defaultAllowedCorsHeaders);
+            }
         } else {
+            config.setAllowedOrigins(defaultAllowedCorsOrigins);
+            config.setAllowedHeaders(defaultAllowedCorsHeaders);
+            config.setAllowCredentials(defaultAllowCredentials);
+            config.setAllowedMethods(defaultAllowedCorsHttpMethods);
             log.debug("CORS is not enabled for service {}, using defaults", serviceId);
         }
         return config;
     }
 
     public void registerDefaultCorsConfiguration(BiConsumer<String, CorsConfiguration> pathMapper) {
-        final CorsConfiguration config = new CorsConfiguration();
+        var config = new CorsConfiguration();
         List<String> pathsToEnable;
 
-        if (corsEnabled) {
+        config.setAllowedOrigins(defaultAllowedCorsOrigins);
+        if (gatewayCorsEnabled) {
             config.setAllowCredentials(true);
-            config.addAllowedOriginPattern(CorsConfiguration.ALL); //NOSONAR this is a replication of existing code
-            config.setAllowedHeaders(Collections.singletonList(CorsConfiguration.ALL));
-            config.setAllowedMethods(allowedCorsHttpMethods);
+            config.setAllowedHeaders(defaultAllowedCorsHeaders);
+            config.setAllowedMethods(defaultAllowedCorsHttpMethods);
             pathsToEnable = corsAllowedEndpoints;
         } else {
             pathsToEnable = Collections.singletonList("/**");

common-service-core/src/test/java/org/zowe/apiml/util/CorsUtilsTest.java

@@ -13,23 +13,28 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.junit.jupiter.MockitoExtension;
 
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
 import static org.junit.jupiter.api.Assertions.*;
 
+@ExtendWith(MockitoExtension.class)
 class CorsUtilsTest {
 
-    Map<String, String> metadata = new HashMap<>();
-    List<String> defaultCorsMethods = List.of("GET", "HEAD", "POST", "PATCH", "DELETE", "PUT", "OPTIONS");
+    private Map<String, String> metadata = new HashMap<>();
+    private List<String> defaultCorsMethods = List.of("GET", "HEAD", "POST", "PATCH", "DELETE", "PUT", "OPTIONS");
 
-    List<String> allowedEndpoints = List.of("/gateway/**");
+    private List<String> allowedEndpoints = List.of("/gateway/**");
 
     @BeforeEach
     void setup() {
+        metadata.clear();
         metadata.put("apiml.routes.v1.gateway", "api/v1");
         metadata.put("apiml.corsEnabled", "true");
     }
@@ -40,7 +45,19 @@ class GivenCorsEnabled {
         @Nested
         class givenDefaultCorsAllowedMethods {
 
-            CorsUtils corsUtils = new CorsUtils(true, defaultCorsMethods, allowedEndpoints);
+            private CorsUtils corsUtils;
+
+            @BeforeEach
+            void setUp() {
+                corsUtils = CorsUtils.builder()
+                    .gatewayCorsEnabled(true)
+                    .corsAllowedEndpoints(allowedEndpoints)
+                    .defaultAllowedCorsHttpMethods(defaultCorsMethods)
+                    .defaultAllowedCorsOrigins(Collections.emptyList())
+                    .defaultAllowedCorsHeaders(List.of("*"))
+                    .defaultAllowCredentials(true)
+                    .build();
+            }
 
             @Test
             void registerDefaultConfig() {
@@ -56,7 +73,7 @@ void registerDefaultConfig() {
             @Test
             void registerConfigForService() {
 
-                corsUtils.setCorsConfiguration("dclient", metadata, (path, serviceId, configuration) -> {
+                corsUtils.setCorsConfiguration("dclient", metadata, (path, configuration) -> {
                         assertEquals(metadata.get("apiml.routes.v1.gateway"), path);
                         assertNotNull(configuration.getAllowedHeaders());
                         assertEquals(1, configuration.getAllowedHeaders().size());
@@ -69,9 +86,10 @@ void registerConfigForService() {
             @Test
             void registerDefaultConfigForService() {
                 metadata.remove("apiml.corsEnabled");
-                corsUtils.setCorsConfiguration("dclient", metadata, (path, serviceId, configuration) -> {
+                corsUtils.setCorsConfiguration("dclient", metadata, (path, configuration) -> {
                         assertEquals(metadata.get("apiml.routes.v1.gateway"), path);
-                        assertNull(configuration.getAllowedMethods());
+                        assertTrue(configuration.getAllowCredentials());
+                        assertEquals(List.of("GET", "HEAD", "POST", "PATCH", "DELETE", "PUT", "OPTIONS"), configuration.getAllowedMethods());
                     }
                 );
             }
@@ -80,7 +98,7 @@ void registerDefaultConfigForService() {
             void registerConfigForServiceWithCustomOrigins() {
                 Map<String, String> customMetadata = new HashMap<>(metadata);
                 customMetadata.put("apiml.corsAllowedOrigins", "https://localhost:3000,http://hostname.com,https://anothehostname:3040");
-                corsUtils.setCorsConfiguration("dclient", customMetadata, (path, serviceId, configuration) -> {
+                corsUtils.setCorsConfiguration("dclient", customMetadata, (path, configuration) -> {
                         assertEquals(metadata.get("apiml.routes.v1.gateway"), path);
                         assertNotNull(configuration.getAllowedHeaders());
                         assertTrue(configuration.getAllowedOrigins().contains("https://localhost:3000"));
@@ -90,13 +108,25 @@ void registerConfigForServiceWithCustomOrigins() {
                     }
                 );
             }
+
         }
 
     }
 
     @Nested
     class GivenCorsDisabled {
-        CorsUtils corsUtils = new CorsUtils(false, null, Arrays.asList("/gateway/**", "/api-docs"));
+
+        private CorsUtils corsUtils;
+
+        @BeforeEach
+        void setUp() {
+            corsUtils = CorsUtils.builder()
+                .gatewayCorsEnabled(false)
+                .corsAllowedEndpoints(Arrays.asList("/gateway/**", "/api-docs"))
+                .defaultAllowedCorsOrigins(Collections.emptyList())
+                .defaultAllowedCorsHeaders(List.of("*"))
+                .build();
+        }
 
         @Test
         void registerEmptyDefaultConfig() {
@@ -109,12 +139,13 @@ void registerEmptyDefaultConfig() {
 
         @Test
         void registerEmptyConfigForService() {
-            corsUtils.setCorsConfiguration("dcclient", metadata, (path, serviceId, configuration) -> {
+            corsUtils.setCorsConfiguration("dcclient", metadata, (path, configuration) -> {
                     assertNull(configuration.getAllowedHeaders());
                     assertNull(configuration.getAllowedMethods());
                 }
             );
         }
+
     }
 
 }