From cecf207ece09b79de27e0f45c10dcc69af2e7232 Mon Sep 17 00:00:00 2001 From: Thomas Iommi Date: Sat, 23 Nov 2024 19:30:31 +0100 Subject: [PATCH 1/8] fix: remove check on known routes to decide in a preflight request is ok --- .../java/io/micronaut/http/server/cors/CorsFilter.java | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/http-server/src/main/java/io/micronaut/http/server/cors/CorsFilter.java b/http-server/src/main/java/io/micronaut/http/server/cors/CorsFilter.java index dd49b81836a..906beab729c 100644 --- a/http-server/src/main/java/io/micronaut/http/server/cors/CorsFilter.java +++ b/http-server/src/main/java/io/micronaut/http/server/cors/CorsFilter.java @@ -89,7 +89,7 @@ public class CorsFilter implements Ordered, ConditionalFilter { /** * @param corsConfiguration The {@link CorsOriginConfiguration} instance * @param httpHostResolver HTTP Host resolver - * @deprecated use {@link CorsFilter(HttpServerConfiguration, HttpHostResolver, Router)} instead. + * @deprecated use {@link CorsFilter(HttpServerConfiguration, HttpHostResolver, Router)} instead. */ @Deprecated(since = "4.7", forRemoval = true) public CorsFilter(HttpServerConfiguration.CorsConfiguration corsConfiguration, @@ -494,20 +494,15 @@ private MutableHttpResponse handlePreflightRequest(@NonNull HttpRequest re @Nullable private boolean validatePreflightRequest(@NonNull HttpRequest request, - @NonNull CorsOriginConfiguration config) { + @NonNull CorsOriginConfiguration config) { Optional methodToMatchOptional = validateMethodToMatch(request, config); if (methodToMatchOptional.isEmpty()) { return false; } - HttpMethod methodToMatch = methodToMatchOptional.get(); if (!CorsUtil.isPreflightRequest(request)) { return false; } - List availableHttpMethods = router.findAny(request).stream().map(UriRouteMatch::getHttpMethod).toList(); - if (availableHttpMethods.stream().noneMatch(method -> method.equals(methodToMatch))) { - return false; - } if (!hasAllowedHeaders(request, config)) { return false; From e7fbe9a32b8239d954899e99906c08fc603a17f5 Mon Sep 17 00:00:00 2001 From: Thomas Iommi Date: Sat, 23 Nov 2024 19:35:02 +0100 Subject: [PATCH 2/8] test: preflight should not fail on unknown router paths --- .../io/micronaut/http/server/netty/cors/CorsVersionSpec.groovy | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/cors/CorsVersionSpec.groovy b/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/cors/CorsVersionSpec.groovy index 8046a351eb9..44fb91c0c04 100644 --- a/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/cors/CorsVersionSpec.groovy +++ b/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/cors/CorsVersionSpec.groovy @@ -95,8 +95,7 @@ class CorsVersionSpec extends Specification { client.exchange(request) then: - HttpClientResponseException ex = thrown() - ex.status == HttpStatus.FORBIDDEN + noExceptionThrown() } void "preflight for version routed from private network"() { From 3ae35be41bbb80cad0ef23e5c7cdddcef44a5d81 Mon Sep 17 00:00:00 2001 From: Thomas Iommi Date: Sat, 23 Nov 2024 19:36:43 +0100 Subject: [PATCH 3/8] test: previous test was failing for wrong reasons (unknown path, not cors config) --- .../io/micronaut/http/server/netty/cors/CorsVersionSpec.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/cors/CorsVersionSpec.groovy b/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/cors/CorsVersionSpec.groovy index 44fb91c0c04..058eed770e8 100644 --- a/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/cors/CorsVersionSpec.groovy +++ b/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/cors/CorsVersionSpec.groovy @@ -112,7 +112,7 @@ class CorsVersionSpec extends Specification { when: request = HttpRequest.OPTIONS("/new-not-allowed-from-private") - preflightHeaders(null, true).each { k, v -> request.header(k, v)} + preflightHeaders("x-api-version", true).each { k, v -> request.header(k, v)} client.exchange(request) then: From b5ad5afe7b986d74a935089652ecdceccedb4421 Mon Sep 17 00:00:00 2001 From: Thomas Iommi Date: Sat, 23 Nov 2024 19:37:28 +0100 Subject: [PATCH 4/8] test: preflight should not fail for unknown routes --- .../io/micronaut/http/server/netty/cors/CorsFilterSpec.groovy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/cors/CorsFilterSpec.groovy b/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/cors/CorsFilterSpec.groovy index fcef31fbdaf..c76e6e0c81c 100644 --- a/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/cors/CorsFilterSpec.groovy +++ b/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/cors/CorsFilterSpec.groovy @@ -301,7 +301,7 @@ class CorsFilterSpec extends Specification { @Property(name = "micronaut.server.cors.configurations.foo.allowed-origins", value = "http://www.foo.com") @Property(name = "micronaut.server.cors.configurations.foo.allowed-methods", value = "GET") - void "A preflight request is rejected for a non-existing route"() { + void "A preflight request is NOT rejected for a non-existing route if CORS configuration is valid"() { given: HttpRequest request = HttpRequest.OPTIONS("/doesnt-exists-route") .header(HttpHeaders.ORIGIN, 'http://www.foo.com') @@ -311,7 +311,7 @@ class CorsFilterSpec extends Specification { HttpResponse response = execute(request) then: - HttpStatus.FORBIDDEN == response.status() + HttpStatus.OK == response.status() } @Property(name = "micronaut.server.cors.configurations.foo.allowed-origins", value = "http://www.foo.com") From 473a2905e66fe670d4bb60ea5f70c5d93c51532b Mon Sep 17 00:00:00 2001 From: Thomas Iommi Date: Sat, 23 Nov 2024 19:37:56 +0100 Subject: [PATCH 5/8] test: aligns test with its description --- .../io/micronaut/http/server/netty/cors/CorsFilterSpec.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/cors/CorsFilterSpec.groovy b/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/cors/CorsFilterSpec.groovy index c76e6e0c81c..17dd8a271c3 100644 --- a/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/cors/CorsFilterSpec.groovy +++ b/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/cors/CorsFilterSpec.groovy @@ -315,7 +315,7 @@ class CorsFilterSpec extends Specification { } @Property(name = "micronaut.server.cors.configurations.foo.allowed-origins", value = "http://www.foo.com") - @Property(name = "micronaut.server.cors.configurations.foo.exposed-headers", value = "Foo-Header,Bar-Header") + @Property(name = "micronaut.server.cors.configurations.foo.allowed-methods", value = "GET") void "A preflight request is rejected for a route that does exist but doesn't handle the requested HTTP Method"() { given: HttpRequest request = HttpRequest.OPTIONS("/example") From 38f0a892b045be76dd0ca16e691810a888e5f3b4 Mon Sep 17 00:00:00 2001 From: Thomas Iommi Date: Sat, 23 Nov 2024 19:38:53 +0100 Subject: [PATCH 6/8] test: fixes test since preflight should not fail for unknown routes --- .../io/micronaut/http/server/netty/cors/NettyCorsSpec.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/cors/NettyCorsSpec.groovy b/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/cors/NettyCorsSpec.groovy index 91a5cd9f036..7111e04d308 100644 --- a/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/cors/NettyCorsSpec.groovy +++ b/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/cors/NettyCorsSpec.groovy @@ -273,7 +273,7 @@ class NettyCorsSpec extends AbstractMicronautSpec { }).blockFirst() expect: - response.code() == HttpStatus.FORBIDDEN.code + response.code() == HttpStatus.OK.code } void "test control headers are applied to error response routes"() { From 7bb04d7309eb1feb98f48526fcf45fbfc1b96b5c Mon Sep 17 00:00:00 2001 From: Thomas Iommi Date: Sat, 23 Nov 2024 19:49:05 +0100 Subject: [PATCH 7/8] test: removes redundant test --- .../http/server/netty/cors/CorsFilterSpec.groovy | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/cors/CorsFilterSpec.groovy b/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/cors/CorsFilterSpec.groovy index 17dd8a271c3..71618ce5f98 100644 --- a/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/cors/CorsFilterSpec.groovy +++ b/http-server-netty/src/test/groovy/io/micronaut/http/server/netty/cors/CorsFilterSpec.groovy @@ -314,21 +314,6 @@ class CorsFilterSpec extends Specification { HttpStatus.OK == response.status() } - @Property(name = "micronaut.server.cors.configurations.foo.allowed-origins", value = "http://www.foo.com") - @Property(name = "micronaut.server.cors.configurations.foo.allowed-methods", value = "GET") - void "A preflight request is rejected for a route that does exist but doesn't handle the requested HTTP Method"() { - given: - HttpRequest request = HttpRequest.OPTIONS("/example") - .header(HttpHeaders.ORIGIN, 'http://www.foo.com') - .header(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, 'POST') - - when: - HttpResponse response = execute(request) - - then: - HttpStatus.FORBIDDEN == response.status() - } - @Requires(property = "spec.name", value = "CorsFilterSpec") @Controller static class TestController{ From 4d8101be796351a436620900e8f435b76628f3fc Mon Sep 17 00:00:00 2001 From: Thomas Iommi Date: Sun, 24 Nov 2024 18:19:44 +0100 Subject: [PATCH 8/8] refactor: simplify conditions in the CORS preflight validation --- .../io/micronaut/http/server/cors/CorsFilter.java | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/http-server/src/main/java/io/micronaut/http/server/cors/CorsFilter.java b/http-server/src/main/java/io/micronaut/http/server/cors/CorsFilter.java index 906beab729c..1378edc5c7c 100644 --- a/http-server/src/main/java/io/micronaut/http/server/cors/CorsFilter.java +++ b/http-server/src/main/java/io/micronaut/http/server/cors/CorsFilter.java @@ -495,16 +495,9 @@ private MutableHttpResponse handlePreflightRequest(@NonNull HttpRequest re @Nullable private boolean validatePreflightRequest(@NonNull HttpRequest request, @NonNull CorsOriginConfiguration config) { - Optional methodToMatchOptional = validateMethodToMatch(request, config); - if (methodToMatchOptional.isEmpty()) { - return false; - } - - if (!CorsUtil.isPreflightRequest(request)) { - return false; - } - - if (!hasAllowedHeaders(request, config)) { + if (validateMethodToMatch(request, config).isEmpty() || + !CorsUtil.isPreflightRequest(request) || + !hasAllowedHeaders(request, config)) { return false; }