From c3b729e72514a4a6523f6238551e3e95697522ff Mon Sep 17 00:00:00 2001 From: Guillaume Smet Date: Wed, 23 Oct 2024 18:41:45 +0200 Subject: [PATCH 1/3] Take regexps into account when setting access-control-allow-credentials We used to only consider exact matches which looks like an oversight. Fixes #43736 --- .../quarkus/vertx/http/cors/CORSRegexTestCase.java | 12 ++++++++---- .../quarkus/vertx/http/runtime/cors/CORSFilter.java | 9 ++++----- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/cors/CORSRegexTestCase.java b/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/cors/CORSRegexTestCase.java index e21e9aae7413f..555e6e6cff086 100644 --- a/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/cors/CORSRegexTestCase.java +++ b/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/cors/CORSRegexTestCase.java @@ -22,7 +22,8 @@ public void corsRegexValidOriginTest() { .when() .get("/test").then() .statusCode(200) - .header("Access-Control-Allow-Origin", "https://asdf.domain.com"); + .header("Access-Control-Allow-Origin", "https://asdf.domain.com") + .header("Access-Control-Allow-Credentials", "true"); } @Test @@ -31,7 +32,8 @@ public void corsRegexValidOrigin2Test() { .when() .get("/test").then() .statusCode(200) - .header("Access-Control-Allow-Origin", "https://abc-123.app.mydomain.com"); + .header("Access-Control-Allow-Origin", "https://abc-123.app.mydomain.com") + .header("Access-Control-Allow-Credentials", "true"); } @Test @@ -40,7 +42,8 @@ public void corsRegexInvalidOriginTest() { .when() .get("/test").then() .statusCode(403) - .header("Access-Control-Allow-Origin", nullValue()); + .header("Access-Control-Allow-Origin", nullValue()) + .header("Access-Control-Allow-Credentials", nullValue()); } @Test @@ -49,6 +52,7 @@ public void corsRegexInvalidOrigin2Test() { .when() .get("/test").then() .statusCode(403) - .header("Access-Control-Allow-Origin", nullValue()); + .header("Access-Control-Allow-Origin", nullValue()) + .header("Access-Control-Allow-Credentials", nullValue()); } } 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 b6def745a683c..f97cd1d47482a 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 @@ -140,11 +140,11 @@ public void handle(RoutingContext event) { //for both normal and preflight requests we need to check the origin boolean allowsOrigin = wildcardOrigin; + boolean originMatches = corsConfig.origins.isPresent() && + (corsConfig.origins.get().contains(origin) || isOriginAllowedByRegex(allowedOriginsRegex, origin)); if (!allowsOrigin) { if (corsConfig.origins.isPresent()) { - allowsOrigin = corsConfig.origins.get().contains(origin) - || isOriginAllowedByRegex(allowedOriginsRegex, origin) - || isSameOrigin(request, origin); + allowsOrigin = originMatches || isSameOrigin(request, origin); } else { allowsOrigin = isSameOrigin(request, origin); } @@ -154,8 +154,7 @@ public void handle(RoutingContext event) { response.setStatusCode(403); response.setStatusMessage("CORS Rejected - Invalid origin"); } else { - boolean allowCredentials = corsConfig.accessControlAllowCredentials - .orElse(corsConfig.origins.isPresent() && corsConfig.origins.get().contains(origin)); + boolean allowCredentials = corsConfig.accessControlAllowCredentials.orElse(originMatches); response.headers().set(HttpHeaders.ACCESS_CONTROL_ALLOW_CREDENTIALS, String.valueOf(allowCredentials)); response.headers().set(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN, origin); } From 29e9cc67f1e15d2a5c5d44b9ec15b629e9175ec8 Mon Sep 17 00:00:00 2001 From: Guillaume Smet Date: Tue, 12 Nov 2024 15:46:24 +0100 Subject: [PATCH 2/3] Also consider /.*/ a wildcard in CORSFilter --- .../io/quarkus/vertx/http/runtime/cors/CORSFilter.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) 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 f97cd1d47482a..85cc320581cde 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 @@ -83,7 +83,13 @@ public static boolean isConfiguredWithWildcard(Optional> optionalLi } private static boolean isOriginConfiguredWithWildcard(Optional> origins) { - return !origins.isEmpty() && origins.get().size() == 1 && "*".equals(origins.get().get(0)); + if (origins.isEmpty() || origins.get().size() != 1) { + return false; + } + + String origin = origins.get().get(0); + + return "*".equals(origin) || "/.*/".equals(origin); } /** From cd280566a69354f5ccd2c2015742606f1b4a3a91 Mon Sep 17 00:00:00 2001 From: Sergey Beryozkin Date: Tue, 12 Nov 2024 23:39:32 +0000 Subject: [PATCH 3/3] Add CORSRegexWildcardTest --- .../http/cors/CORSRegexWildcardTestCase.java | 27 +++++++++++++++++++ .../cors/CORSWildcardSecurityTestCase.java | 26 +++++++++++------- .../conf/cors-regex-wildcard.properties | 2 ++ .../vertx/http/runtime/cors/CORSFilter.java | 2 +- 4 files changed, 46 insertions(+), 11 deletions(-) create mode 100644 extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/cors/CORSRegexWildcardTestCase.java create mode 100644 extensions/vertx-http/deployment/src/test/resources/conf/cors-regex-wildcard.properties diff --git a/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/cors/CORSRegexWildcardTestCase.java b/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/cors/CORSRegexWildcardTestCase.java new file mode 100644 index 0000000000000..c9e27b6f42d5c --- /dev/null +++ b/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/cors/CORSRegexWildcardTestCase.java @@ -0,0 +1,27 @@ +package io.quarkus.vertx.http.cors; + +import static io.restassured.RestAssured.given; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.test.QuarkusUnitTest; + +public class CORSRegexWildcardTestCase { + + @RegisterExtension + static QuarkusUnitTest runner = new QuarkusUnitTest() + .withApplicationRoot((jar) -> jar + .addClasses(BeanRegisteringRoute.class) + .addAsResource("conf/cors-regex-wildcard.properties", "application.properties")); + + @Test + public void corsRegexValidOriginTest() { + given().header("Origin", "https://asdf.domain.com") + .when() + .get("/test").then() + .statusCode(200) + .header("Access-Control-Allow-Origin", "https://asdf.domain.com") + .header("Access-Control-Allow-Credentials", "false"); + } +} 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 e364866570885..b2161f7f434dd 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 @@ -57,7 +57,8 @@ public void corsPreflightTest() { .statusCode(200) .header("Access-Control-Allow-Origin", origin) .header("Access-Control-Allow-Methods", methods) - .header("Access-Control-Allow-Headers", headers); + .header("Access-Control-Allow-Headers", headers) + .header("Access-Control-Allow-Credentials", "false"); given().header("Origin", origin) .header("Access-Control-Request-Method", methods) @@ -68,7 +69,8 @@ public void corsPreflightTest() { .statusCode(200) .header("Access-Control-Allow-Origin", origin) .header("Access-Control-Allow-Methods", methods) - .header("Access-Control-Allow-Headers", headers); + .header("Access-Control-Allow-Headers", headers) + .header("Access-Control-Allow-Credentials", "false"); given().header("Origin", origin) .header("Access-Control-Request-Method", methods) @@ -79,7 +81,8 @@ public void corsPreflightTest() { .statusCode(200) .header("Access-Control-Allow-Origin", origin) .header("Access-Control-Allow-Methods", methods) - .header("Access-Control-Allow-Headers", headers); + .header("Access-Control-Allow-Headers", headers) + .header("Access-Control-Allow-Credentials", "false"); given().header("Origin", origin) .header("Access-Control-Request-Method", methods) @@ -90,20 +93,20 @@ public void corsPreflightTest() { .statusCode(200) .header("Access-Control-Allow-Origin", origin) .header("Access-Control-Allow-Methods", methods) - .header("Access-Control-Allow-Headers", headers); + .header("Access-Control-Allow-Headers", headers) + .header("Access-Control-Allow-Credentials", "false"); } @Test @DisplayName("Handles a direct CORS request correctly") public void corsNoPreflightTest() { String origin = "http://custom.origin.quarkus"; - String methods = "GET, POST"; - String headers = "X-Custom"; given().header("Origin", origin) .when() .get("/test").then() .statusCode(401) - .header("Access-Control-Allow-Origin", origin); + .header("Access-Control-Allow-Origin", origin) + .header("Access-Control-Allow-Credentials", "false"); given().header("Origin", origin) .when() @@ -111,20 +114,23 @@ public void corsNoPreflightTest() { .get("/test").then() .statusCode(200) .header("Access-Control-Allow-Origin", origin) - .body(Matchers.equalTo("test:/test")); + .body(Matchers.equalTo("test:/test")) + .header("Access-Control-Allow-Credentials", "false"); given().header("Origin", origin) .when() .auth().basic("test", "wrongpassword") .get("/test").then() .statusCode(401) - .header("Access-Control-Allow-Origin", origin); + .header("Access-Control-Allow-Origin", origin) + .header("Access-Control-Allow-Credentials", "false"); given().header("Origin", origin) .when() .auth().basic("user", "user") .get("/test").then() .statusCode(403) - .header("Access-Control-Allow-Origin", origin); + .header("Access-Control-Allow-Origin", origin) + .header("Access-Control-Allow-Credentials", "false"); } } diff --git a/extensions/vertx-http/deployment/src/test/resources/conf/cors-regex-wildcard.properties b/extensions/vertx-http/deployment/src/test/resources/conf/cors-regex-wildcard.properties new file mode 100644 index 0000000000000..d6c772b5b05cb --- /dev/null +++ b/extensions/vertx-http/deployment/src/test/resources/conf/cors-regex-wildcard.properties @@ -0,0 +1,2 @@ +quarkus.http.cors=true +quarkus.http.cors.origins=/.*/ \ No newline at end of file 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 85cc320581cde..9ecd59b373ec5 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 @@ -146,7 +146,7 @@ public void handle(RoutingContext event) { //for both normal and preflight requests we need to check the origin boolean allowsOrigin = wildcardOrigin; - boolean originMatches = corsConfig.origins.isPresent() && + boolean originMatches = !wildcardOrigin && corsConfig.origins.isPresent() && (corsConfig.origins.get().contains(origin) || isOriginAllowedByRegex(allowedOriginsRegex, origin)); if (!allowsOrigin) { if (corsConfig.origins.isPresent()) {