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,6 @@
 logging:
     level:
-        ROOT: INFO
-        #org.springframework: DEBUG
-        #org.springframework.boot.autoconfigure.web: DEBUG
+        ROOT: DEBUG
         org.springframework.cloud.gateway: DEBUG
         org.springframework.http.server.reactive: DEBUG
         org.springframework.web.reactive: DEBUG
@@ -43,7 +41,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,70 +22,113 @@
 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.setAllowCredentials(true);
-            config.addAllowedOriginPattern(CorsConfiguration.ALL); //NOSONAR this is a replication of existing code
-            config.setAllowedHeaders(Collections.singletonList(CorsConfiguration.ALL));
-            config.setAllowedMethods(allowedCorsHttpMethods);
+        config.setAllowedOrigins(defaultAllowedCorsOrigins);
+        config.setAllowCredentials(true);
+        config.setAllowedHeaders(defaultAllowedCorsHeaders);
+        config.setAllowedMethods(defaultAllowedCorsHttpMethods);
+
+        if (gatewayCorsEnabled) {
+            // When gateway has CORS handling enabled, defaults go to the /gateway/** endpoints plus any routes that southbound services register. If a service does not register its routes with apiml.corsEnabled metadata entry, the behaviour is really not recommended as there is no CORS configuration set for the service (if the service receives requests with Origin header)
             pathsToEnable = corsAllowedEndpoints;
         } else {
+            // When gateway has CORS handling disabled, all endpoints use Gateway defaults.
             pathsToEnable = Collections.singletonList("/**");
         }
         pathsToEnable.forEach(path -> pathMapper.accept(path, config));