From 8f9302be7468381f8a1ad6dcf26140fd2db85138 Mon Sep 17 00:00:00 2001 From: Xumk Date: Wed, 16 Sep 2020 18:42:23 +0300 Subject: [PATCH] Support '*' in CORS filter configuration --- docs/src/main/asciidoc/http-reference.adoc | 14 +- ...estCase.java => CORSSecurityTestCase.java} | 2 +- .../cors/CORSWildcardSecurityTestCase.java | 149 +++++++++++++++++ .../CORSWildcardStarSecurityTestCase.java | 153 ++++++++++++++++++ .../vertx/http/runtime/cors/CORSConfig.java | 3 +- .../vertx/http/runtime/cors/CORSFilter.java | 24 ++- .../http/runtime/cors/CORSFilterTest.java | 25 +++ 7 files changed, 354 insertions(+), 16 deletions(-) rename extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/cors/{CoresSecurityTestCase.java => CORSSecurityTestCase.java} (99%) create mode 100644 extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/cors/CORSWildcardSecurityTestCase.java create mode 100644 extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/cors/CORSWildcardStarSecurityTestCase.java create mode 100644 extensions/vertx-http/runtime/src/test/java/io/quarkus/vertx/http/runtime/cors/CORSFilterTest.java diff --git a/docs/src/main/asciidoc/http-reference.adoc b/docs/src/main/asciidoc/http-reference.adoc index 0ef217e7987cf..f01ae61f1e31e 100644 --- a/docs/src/main/asciidoc/http-reference.adoc +++ b/docs/src/main/asciidoc/http-reference.adoc @@ -176,13 +176,13 @@ following properties will be applied before passing the request on to its actual [cols="() { + @Override + public JavaArchive get() { + return ShrinkWrap.create(JavaArchive.class) + .addClasses(TestIdentityProvider.class, TestIdentityController.class, PathHandler.class) + .addAsResource(new StringAsset(APP_PROPS), "application.properties"); + } + }); + + @BeforeAll + public static void setup() { + TestIdentityController.resetRoles().add("test", "test", "test").add("user", "user", "user"); + } + + @Test + @DisplayName("Handles a preflight CORS request correctly") + public void corsPreflightTest() { + String origin = "http://custom.origin.quarkus"; + String methods = "GET,POST"; + String headers = "X-Custom"; + given().header("Origin", origin) + .header("Access-Control-Request-Method", methods) + .header("Access-Control-Request-Headers", headers) + .when() + .options("/test").then() + .statusCode(200) + .header("Access-Control-Allow-Origin", origin) + .header("Access-Control-Allow-Methods", methods) + .header("Access-Control-Allow-Headers", headers); + + given().header("Origin", origin) + .header("Access-Control-Request-Method", methods) + .header("Access-Control-Request-Headers", headers) + .when() + .auth().basic("test", "test") + .options("/test").then() + .statusCode(200) + .header("Access-Control-Allow-Origin", origin) + .header("Access-Control-Allow-Methods", methods) + .header("Access-Control-Allow-Headers", headers); + + given().header("Origin", origin) + .header("Access-Control-Request-Method", methods) + .header("Access-Control-Request-Headers", headers) + .when() + .auth().basic("test", "wrongpassword") + .options("/test").then() + .statusCode(200) + .header("Access-Control-Allow-Origin", origin) + .header("Access-Control-Allow-Methods", methods) + .header("Access-Control-Allow-Headers", headers); + + given().header("Origin", origin) + .header("Access-Control-Request-Method", methods) + .header("Access-Control-Request-Headers", headers) + .when() + .auth().basic("user", "user") + .options("/test").then() + .statusCode(200) + .header("Access-Control-Allow-Origin", origin) + .header("Access-Control-Allow-Methods", methods) + .header("Access-Control-Allow-Headers", headers); + } + + @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) + .header("Access-Control-Request-Method", methods) + .header("Access-Control-Request-Headers", headers) + .when() + .log().headers() + .get("/test").then() + .statusCode(401) + .header("Access-Control-Allow-Origin", origin) + .header("Access-Control-Allow-Methods", methods) + .header("Access-Control-Allow-Headers", headers); + + given().header("Origin", origin) + .header("Access-Control-Request-Method", methods) + .header("Access-Control-Request-Headers", headers) + .when() + .auth().basic("test", "test") + .log().headers() + .get("/test").then() + .statusCode(200) + .header("Access-Control-Allow-Origin", origin) + .header("Access-Control-Allow-Methods", methods) + .header("Access-Control-Allow-Headers", headers) + .body(Matchers.equalTo("test:/test")); + + given().header("Origin", origin) + .header("Access-Control-Request-Method", methods) + .header("Access-Control-Request-Headers", headers) + .when() + .auth().basic("test", "wrongpassword") + .log().headers() + .get("/test").then() + .statusCode(401) + .header("Access-Control-Allow-Origin", origin) + .header("Access-Control-Allow-Methods", methods) + .header("Access-Control-Allow-Headers", headers); + + given().header("Origin", origin) + .header("Access-Control-Request-Method", methods) + .header("Access-Control-Request-Headers", headers) + .when() + .auth().basic("user", "user") + .log().headers() + .get("/test").then() + .statusCode(403) + .header("Access-Control-Allow-Origin", origin) + .header("Access-Control-Allow-Methods", methods) + .header("Access-Control-Allow-Headers", headers); + } +} diff --git a/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/cors/CORSWildcardStarSecurityTestCase.java b/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/cors/CORSWildcardStarSecurityTestCase.java new file mode 100644 index 0000000000000..51498f4e75e11 --- /dev/null +++ b/extensions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/cors/CORSWildcardStarSecurityTestCase.java @@ -0,0 +1,153 @@ +package io.quarkus.vertx.http.cors; + +import static io.restassured.RestAssured.given; + +import java.util.function.Supplier; + +import org.hamcrest.Matchers; +import org.jboss.shrinkwrap.api.ShrinkWrap; +import org.jboss.shrinkwrap.api.asset.StringAsset; +import org.jboss.shrinkwrap.api.spec.JavaArchive; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.security.test.utils.TestIdentityController; +import io.quarkus.security.test.utils.TestIdentityProvider; +import io.quarkus.test.QuarkusUnitTest; +import io.quarkus.vertx.http.security.PathHandler; + +public class CORSWildcardStarSecurityTestCase { + + private static final String APP_PROPS = "" + + "quarkus.http.cors=true\n" + + "quarkus.http.cors.methods=*\n" + + "quarkus.http.cors.origins=*\n" + + "quarkus.http.cors.headers=*\n" + + "quarkus.http.auth.basic=true\n" + + "quarkus.http.auth.policy.r1.roles-allowed=test\n" + + "quarkus.http.auth.permission.roles1.paths=/test\n" + + "quarkus.http.auth.permission.roles1.policy=r1\n"; + + @RegisterExtension + static QuarkusUnitTest test = new QuarkusUnitTest().setArchiveProducer(new Supplier() { + @Override + public JavaArchive get() { + return ShrinkWrap.create(JavaArchive.class) + .addClasses(TestIdentityProvider.class, TestIdentityController.class, PathHandler.class) + .addAsResource(new StringAsset(APP_PROPS), "application.properties"); + } + }); + + @BeforeAll + public static void setup() { + TestIdentityController.resetRoles().add("test", "test", "test").add("user", "user", "user"); + } + + @Test + @DisplayName("Handles a preflight CORS request correctly") + public void corsPreflightTest() { + String origin = "http://custom.origin.quarkus"; + String methods = "GET,POST,OPTIONS,DELETE"; + String headers = "X-Custom,B-Custom,Test-Headers"; + given().header("Origin", origin) + .header("Access-Control-Request-Method", methods) + .header("Access-Control-Request-Headers", headers) + .when() + .options("/test").then() + .statusCode(200) + .header("Access-Control-Allow-Origin", origin) + .header("Access-Control-Allow-Methods", methods) + .header("Access-Control-Allow-Headers", headers); + + given().header("Origin", origin) + .header("Access-Control-Request-Method", methods) + .header("Access-Control-Request-Headers", headers) + .when() + .auth().basic("test", "test") + .options("/test").then() + .statusCode(200) + .header("Access-Control-Allow-Origin", origin) + .header("Access-Control-Allow-Methods", methods) + .header("Access-Control-Allow-Headers", headers); + + given().header("Origin", origin) + .header("Access-Control-Request-Method", methods) + .header("Access-Control-Request-Headers", headers) + .when() + .auth().basic("test", "wrongpassword") + .options("/test").then() + .statusCode(200) + .header("Access-Control-Allow-Origin", origin) + .header("Access-Control-Allow-Methods", methods) + .header("Access-Control-Allow-Headers", headers); + + given().header("Origin", origin) + .header("Access-Control-Request-Method", methods) + .header("Access-Control-Request-Headers", headers) + .when() + .auth().basic("user", "user") + .options("/test").then() + .statusCode(200) + .header("Access-Control-Allow-Origin", origin) + .header("Access-Control-Allow-Methods", methods) + .header("Access-Control-Allow-Headers", headers); + } + + @Test + @DisplayName("Handles a direct CORS request correctly") + public void corsNoPreflightTest() { + String origin = "http://custom.origin.quarkus"; + String methods = "GET,POST,OPTIONS,DELETE"; + String headers = "X-Custom,B-Custom,Test-Headers"; + given().header("Origin", origin) + .header("Access-Control-Request-Method", methods) + .header("Access-Control-Request-Headers", headers) + .when() + .log().headers() + .get("/test").then() + .statusCode(401) + .header("Access-Control-Allow-Origin", origin) + .header("Access-Control-Allow-Methods", methods) + .header("Access-Control-Allow-Headers", headers); + + given().header("Origin", origin) + .header("Access-Control-Request-Method", methods) + .header("Access-Control-Request-Headers", headers) + .when() + .auth().basic("test", "test") + .log().headers() + .get("/test").then() + .statusCode(200) + .header("Access-Control-Allow-Origin", origin) + .header("Access-Control-Allow-Methods", methods) + .header("Access-Control-Allow-Headers", headers) + .body(Matchers.equalTo("test:/test")); + + given().header("Origin", origin) + .header("Access-Control-Request-Method", methods) + .header("Access-Control-Request-Headers", headers) + .when() + .auth().basic("test", "wrongpassword") + .log().headers() + .get("/test").then() + .statusCode(401) + .header("Access-Control-Allow-Origin", origin) + .header("Access-Control-Allow-Methods", methods) + .header("Access-Control-Allow-Headers", headers); + + given().header("Origin", origin) + .header("Access-Control-Request-Method", methods) + .header("Access-Control-Request-Headers", headers) + .when() + .auth().basic("user", "user") + .log().headers() + .get("/test").then() + .statusCode(403) + .header("Access-Control-Allow-Origin", origin) + .header("Access-Control-Allow-Methods", methods) + .header("Access-Control-Allow-Headers", headers); + } + +} 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 5e290e6b6ed28..36a9a5bb7c4ef 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 @@ -8,7 +8,6 @@ import io.quarkus.runtime.annotations.ConfigItem; import io.quarkus.runtime.annotations.ConvertWith; import io.quarkus.runtime.configuration.TrimmedStringConverter; -import io.vertx.core.http.HttpMethod; @ConfigGroup public class CORSConfig { @@ -34,7 +33,7 @@ public class CORSConfig { * default: returns any requested method as valid */ @ConfigItem - public Optional> methods; + public Optional> methods; /** * HTTP headers allowed for CORS 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 29cee851b47d7..a24bc50588c02 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 @@ -6,6 +6,7 @@ import java.util.Objects; import java.util.Optional; import java.util.regex.Pattern; +import java.util.stream.Collectors; import io.vertx.core.Handler; import io.vertx.core.http.HttpHeaders; @@ -26,8 +27,17 @@ public CORSFilter(CORSConfig corsConfig) { this.corsConfig = corsConfig; } + public static boolean isConfiguredWithWildcard(Optional> optionalList) { + if (optionalList == null || !optionalList.isPresent()) { + return true; + } + + List list = optionalList.get(); + return list.isEmpty() || (list.size() == 1 && "*".equals(list.get(0))); + } + private void processRequestedHeaders(HttpServerResponse response, String allowHeadersValue) { - if (!corsConfig.headers.isPresent()) { + if (isConfiguredWithWildcard(corsConfig.headers)) { response.headers().set(HttpHeaders.ACCESS_CONTROL_ALLOW_HEADERS, allowHeadersValue); } else { List requestedHeaders = new ArrayList<>(); @@ -49,7 +59,7 @@ private void processRequestedHeaders(HttpServerResponse response, String allowHe } private void processMethods(HttpServerResponse response, String allowMethodsValue) { - if (!corsConfig.methods.isPresent()) { + if (isConfiguredWithWildcard(corsConfig.methods)) { response.headers().set(HttpHeaders.ACCESS_CONTROL_ALLOW_METHODS, allowMethodsValue); } else { List requestedMethods = new ArrayList<>(); @@ -58,7 +68,9 @@ private void processMethods(HttpServerResponse response, String allowMethodsValu } List validRequestedMethods = new ArrayList<>(); - for (HttpMethod configMethod : corsConfig.methods.get()) { + List methods = corsConfig.methods.get().stream().map(String::trim).map(HttpMethod::valueOf) + .collect(Collectors.toList()); + for (HttpMethod configMethod : methods) { if (requestedMethods.contains(configMethod.name().toLowerCase())) { validRequestedMethods.add(configMethod.name()); } @@ -91,7 +103,7 @@ public void handle(RoutingContext event) { processRequestedHeaders(response, requestedHeaders); } - boolean allowsOrigin = !corsConfig.origins.isPresent() || corsConfig.origins.get().contains(origin); + boolean allowsOrigin = isConfiguredWithWildcard(corsConfig.origins) || corsConfig.origins.get().contains(origin); if (allowsOrigin) { response.headers().set(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN, origin); @@ -105,7 +117,7 @@ public void handle(RoutingContext event) { final Optional> exposedHeaders = corsConfig.exposedHeaders; - if (exposedHeaders.isPresent()) { + if (!isConfiguredWithWildcard(exposedHeaders)) { response.headers().set(HttpHeaders.ACCESS_CONTROL_EXPOSE_HEADERS, String.join(",", exposedHeaders.orElse(Collections.emptyList()))); } @@ -121,4 +133,4 @@ public void handle(RoutingContext event) { } } } -} +} \ No newline at end of file diff --git a/extensions/vertx-http/runtime/src/test/java/io/quarkus/vertx/http/runtime/cors/CORSFilterTest.java b/extensions/vertx-http/runtime/src/test/java/io/quarkus/vertx/http/runtime/cors/CORSFilterTest.java new file mode 100644 index 0000000000000..d94e005e57d35 --- /dev/null +++ b/extensions/vertx-http/runtime/src/test/java/io/quarkus/vertx/http/runtime/cors/CORSFilterTest.java @@ -0,0 +1,25 @@ +package io.quarkus.vertx.http.runtime.cors; + +import static io.quarkus.vertx.http.runtime.cors.CORSFilter.isConfiguredWithWildcard; + +import java.util.Arrays; +import java.util.Collections; +import java.util.Optional; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +public class CORSFilterTest { + + @Test + public void isConfiguredWithWildcardTest() { + Assertions.assertTrue(isConfiguredWithWildcard(Optional.empty())); + Assertions.assertTrue(isConfiguredWithWildcard(Optional.of(Collections.EMPTY_LIST))); + Assertions.assertTrue(isConfiguredWithWildcard(Optional.of(Collections.singletonList("*")))); + + Assertions.assertFalse(isConfiguredWithWildcard(Optional.of(Arrays.asList("PUT", "GET", "POST")))); + Assertions.assertFalse(isConfiguredWithWildcard(Optional.of(Arrays.asList("http://localhost:8080/", "*")))); + Assertions.assertFalse(isConfiguredWithWildcard(Optional.of(Collections.singletonList("http://localhost:8080/")))); + } + +}