From 6c198c3e94b972ceabc14f862b2b35faf1dd7457 Mon Sep 17 00:00:00 2001 From: Sergey Beryozkin Date: Mon, 5 Dec 2022 15:17:09 +0000 Subject: [PATCH] Do not support any Origin by default if CORS is enabled --- .../deployment/binder/UriTagCorsTest.java | 1 + .../resources/conf/cors-config.properties | 1 + .../src/test/resources/cors-config.properties | 3 ++- .../OpenApiHttpRootDefaultPathTestCase.java | 4 ++- .../vertx/http/cors/CORSSecurityTestCase.java | 1 + .../cors/CORSWildcardSecurityTestCase.java | 1 + .../resources/conf/cors-config.properties | 1 + .../vertx/http/runtime/cors/CORSConfig.java | 3 --- .../vertx/http/runtime/cors/CORSFilter.java | 25 +++++++++++++------ .../src/main/resources/application.properties | 1 + .../src/main/resources/application.properties | 3 ++- .../src/main/resources/application.properties | 1 + 12 files changed, 32 insertions(+), 13 deletions(-) diff --git a/extensions/micrometer/deployment/src/test/java/io/quarkus/micrometer/deployment/binder/UriTagCorsTest.java b/extensions/micrometer/deployment/src/test/java/io/quarkus/micrometer/deployment/binder/UriTagCorsTest.java index ab20fa9dda67b..a2daf07800176 100644 --- a/extensions/micrometer/deployment/src/test/java/io/quarkus/micrometer/deployment/binder/UriTagCorsTest.java +++ b/extensions/micrometer/deployment/src/test/java/io/quarkus/micrometer/deployment/binder/UriTagCorsTest.java @@ -24,6 +24,7 @@ public class UriTagCorsTest { .overrideConfigKey("quarkus.micrometer.binder.http-server.enabled", "true") .overrideConfigKey("quarkus.micrometer.binder.vertx.enabled", "true") .overrideConfigKey("quarkus.http.cors", "true") + .overrideConfigKey("quarkus.http.cors.origins", "*") .withApplicationRoot((jar) -> jar .addClasses(Util.class, VertxWebEndpoint.class, diff --git a/extensions/reactive-routes/deployment/src/test/resources/conf/cors-config.properties b/extensions/reactive-routes/deployment/src/test/resources/conf/cors-config.properties index 6cc822b80597c..10c86a915bd04 100644 --- a/extensions/reactive-routes/deployment/src/test/resources/conf/cors-config.properties +++ b/extensions/reactive-routes/deployment/src/test/resources/conf/cors-config.properties @@ -1,3 +1,4 @@ quarkus.http.cors=true +quarkus.http.cors.origins=* # whitespaces added to test that they are not taken into account config is parsed quarkus.http.cors.methods=GET, OPTIONS, POST diff --git a/extensions/resteasy-classic/resteasy/deployment/src/test/resources/cors-config.properties b/extensions/resteasy-classic/resteasy/deployment/src/test/resources/cors-config.properties index aa8d0000a7867..3f6f798ab006f 100644 --- a/extensions/resteasy-classic/resteasy/deployment/src/test/resources/cors-config.properties +++ b/extensions/resteasy-classic/resteasy/deployment/src/test/resources/cors-config.properties @@ -1 +1,2 @@ -quarkus.http.cors=true \ No newline at end of file +quarkus.http.cors=true +quarkus.http.cors.origins=* diff --git a/extensions/smallrye-openapi/deployment/src/test/java/io/quarkus/smallrye/openapi/test/vertx/OpenApiHttpRootDefaultPathTestCase.java b/extensions/smallrye-openapi/deployment/src/test/java/io/quarkus/smallrye/openapi/test/vertx/OpenApiHttpRootDefaultPathTestCase.java index 1f9b5e59f651c..45d6749767bb7 100644 --- a/extensions/smallrye-openapi/deployment/src/test/java/io/quarkus/smallrye/openapi/test/vertx/OpenApiHttpRootDefaultPathTestCase.java +++ b/extensions/smallrye-openapi/deployment/src/test/java/io/quarkus/smallrye/openapi/test/vertx/OpenApiHttpRootDefaultPathTestCase.java @@ -15,7 +15,9 @@ public class OpenApiHttpRootDefaultPathTestCase { static QuarkusUnitTest runner = new QuarkusUnitTest() .withApplicationRoot((jar) -> jar .addClasses(OpenApiRoute.class) - .addAsResource(new StringAsset("quarkus.http.root-path=/foo"), "application.properties")); + .addAsResource(new StringAsset("quarkus.http.root-path=/foo\n" + + "quarkus.http.cors=true\n" + + "quarkus.http.cors.origins=*"), "application.properties")); @Test public void testOpenApiPathAccessResource() { diff --git a/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/cors/CORSSecurityTestCase.java b/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/cors/CORSSecurityTestCase.java index 6b60a1b8ee620..8117bf00dbd57 100644 --- a/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/cors/CORSSecurityTestCase.java +++ b/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/cors/CORSSecurityTestCase.java @@ -22,6 +22,7 @@ public class CORSSecurityTestCase { private static final String APP_PROPS = "" + "quarkus.http.cors=true\n" + + "quarkus.http.cors.origins=*\n" + "quarkus.http.cors.methods=GET, OPTIONS, POST\n" + "quarkus.http.auth.basic=true\n" + "quarkus.http.auth.policy.r1.roles-allowed=test\n" + diff --git a/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/cors/CORSWildcardSecurityTestCase.java b/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/cors/CORSWildcardSecurityTestCase.java index 0170bc7186484..0cbda431190ef 100644 --- a/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/cors/CORSWildcardSecurityTestCase.java +++ b/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/cors/CORSWildcardSecurityTestCase.java @@ -22,6 +22,7 @@ public class CORSWildcardSecurityTestCase { private static final String APP_PROPS = "" + "quarkus.http.cors=true\n" + + "quarkus.http.cors.origins=*\n" + "quarkus.http.auth.basic=true\n" + "quarkus.http.auth.policy.r1.roles-allowed=test\n" + "quarkus.http.auth.permission.roles1.paths=/test\n" + diff --git a/extensions/vertx-http/deployment/src/test/resources/conf/cors-config.properties b/extensions/vertx-http/deployment/src/test/resources/conf/cors-config.properties index d9a2687cc383d..c782455a09e72 100644 --- a/extensions/vertx-http/deployment/src/test/resources/conf/cors-config.properties +++ b/extensions/vertx-http/deployment/src/test/resources/conf/cors-config.properties @@ -1,4 +1,5 @@ quarkus.http.cors=true +quarkus.http.cors.origins=* # whitespaces added to test that they are not taken into account config is parsed quarkus.http.cors.methods=GET, OPTIONS, POST quarkus.http.cors.access-control-allow-credentials=true diff --git a/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/cors/CORSConfig.java b/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/cors/CORSConfig.java index d98820d59c099..4b2a57fd22573 100644 --- a/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/cors/CORSConfig.java +++ b/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/cors/CORSConfig.java @@ -18,9 +18,6 @@ public class CORSConfig { * Comma separated list of valid URLs, e.g.: http://www.quarkus.io,http://localhost:3000 * In case an entry of the list is surrounded by forward slashes, * it is interpreted as a regular expression. - * The filter allows any origin if this is not set. - * - * default: returns any requested origin as valid */ @ConfigItem @ConvertWith(TrimmedStringConverter.class) diff --git a/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/cors/CORSFilter.java b/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/cors/CORSFilter.java index 05869dfbfb877..6de736d08af9f 100644 --- a/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/cors/CORSFilter.java +++ b/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/cors/CORSFilter.java @@ -23,18 +23,20 @@ public class CORSFilter implements Handler { // Must be static because the filter is created(deployed) at build time and runtime config is still not available final CORSConfig corsConfig; - final List allowedOriginsRegex; + private final boolean wildcardOrigin; + private final List allowedOriginsRegex; private final List configuredHttpMethods; public CORSFilter(CORSConfig corsConfig) { this.corsConfig = corsConfig; - this.allowedOriginsRegex = parseAllowedOriginsRegex(this.corsConfig.origins); - configuredHttpMethods = createConfiguredHttpMethods(this.corsConfig.methods); + this.wildcardOrigin = isOriginConfiguredWithWildcard(this.corsConfig.origins); + this.allowedOriginsRegex = this.wildcardOrigin ? List.of() : parseAllowedOriginsRegex(this.corsConfig.origins); + this.configuredHttpMethods = createConfiguredHttpMethods(this.corsConfig.methods); } private List createConfiguredHttpMethods(Optional> methods) { if (methods.isEmpty()) { - return Collections.emptyList(); + return List.of(); } List corsConfigMethods = methods.get(); List result = new ArrayList<>(corsConfigMethods.size()); @@ -53,6 +55,10 @@ public static boolean isConfiguredWithWildcard(Optional> optionalLi return list.isEmpty() || (list.size() == 1 && "*".equals(list.get(0))); } + private static boolean isOriginConfiguredWithWildcard(Optional> origins) { + return !origins.isEmpty() && origins.get().size() == 1 && "*".equals(origins.get().get(0)); + } + /** * Parse the provided allowed origins for any regexes * @@ -61,7 +67,7 @@ public static boolean isConfiguredWithWildcard(Optional> optionalLi */ public static List parseAllowedOriginsRegex(Optional> allowedOrigins) { if (allowedOrigins == null || !allowedOrigins.isPresent()) { - return Collections.emptyList(); + return List.of(); } // extract configured origins and find any Regular Expressions @@ -176,8 +182,13 @@ public void handle(RoutingContext event) { processRequestedHeaders(response, requestedHeaders); } - boolean allowsOrigin = isConfiguredWithWildcard(corsConfig.origins) || corsConfig.origins.get().contains(origin) - || isOriginAllowedByRegex(allowedOriginsRegex, origin) || isSameOrigin(request, origin); + boolean allowsOrigin = wildcardOrigin; + if (!allowsOrigin) { + allowsOrigin = !corsConfig.origins.isEmpty() + && (corsConfig.origins.get().contains(origin) + || isOriginAllowedByRegex(allowedOriginsRegex, origin) + || isSameOrigin(request, origin)); + } if (allowsOrigin) { response.headers().set(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN, origin); diff --git a/integration-tests/oidc-code-flow/src/main/resources/application.properties b/integration-tests/oidc-code-flow/src/main/resources/application.properties index c18b1c767ed39..aeacfc9106352 100644 --- a/integration-tests/oidc-code-flow/src/main/resources/application.properties +++ b/integration-tests/oidc-code-flow/src/main/resources/application.properties @@ -166,6 +166,7 @@ quarkus.http.auth.permission.post-logout.paths=/tenant-logout/post-logout quarkus.http.auth.permission.post-logout.policy=permit quarkus.http.cors=true +quarkus.http.cors.origins=* quarkus.http.auth.proactive=false quarkus.http.proxy.enable-forwarded-prefix=true quarkus.http.proxy.allow-forwarded=true diff --git a/integration-tests/oidc-tenancy/src/main/resources/application.properties b/integration-tests/oidc-tenancy/src/main/resources/application.properties index 85bc3b1ac7854..a60e78e2af704 100644 --- a/integration-tests/oidc-tenancy/src/main/resources/application.properties +++ b/integration-tests/oidc-tenancy/src/main/resources/application.properties @@ -1,4 +1,5 @@ quarkus.http.cors=true +quarkus.http.cors.origins=* quarkus.oidc.token-cache.max-size=3 @@ -115,4 +116,4 @@ quarkus.native.additional-build-args=-H:IncludeResources=.*\\.pem quarkus.log.category."io.quarkus.oidc.runtime.CodeAuthenticationMechanism".min-level=TRACE -quarkus.log.category."io.quarkus.oidc.runtime.CodeAuthenticationMechanism".level=TRACE \ No newline at end of file +quarkus.log.category."io.quarkus.oidc.runtime.CodeAuthenticationMechanism".level=TRACE diff --git a/integration-tests/oidc/src/main/resources/application.properties b/integration-tests/oidc/src/main/resources/application.properties index a82730a4bccf7..c9e1a6d9f3511 100644 --- a/integration-tests/oidc/src/main/resources/application.properties +++ b/integration-tests/oidc/src/main/resources/application.properties @@ -12,6 +12,7 @@ quarkus.oidc.tls.key-store-password=password quarkus.native.additional-build-args=-H:IncludeResources=.*\\.jks quarkus.http.cors=true +quarkus.http.cors.origins=* quarkus.http.auth.basic=true quarkus.security.users.embedded.enabled=true