From 0b94599fdacf8950424cdb8efb4ddfc3dcd64d69 Mon Sep 17 00:00:00 2001 From: Emilien Devos <121870973+edevosc2c@users.noreply.github.com> Date: Wed, 13 Mar 2024 15:32:58 +0100 Subject: [PATCH 1/3] preserve host header + set forward-headers-strategy FRAMEWORK --- gateway/src/main/resources/application.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gateway/src/main/resources/application.yml b/gateway/src/main/resources/application.yml index fdb21a7d..9ff47941 100644 --- a/gateway/src/main/resources/application.yml +++ b/gateway/src/main/resources/application.yml @@ -15,6 +15,7 @@ server: ssl: enabled: false #TODO: configure SSL with a self-signed certificate + forward-headers-strategy: FRAMEWORK spring: config: @@ -52,6 +53,7 @@ spring: - RemoveSecurityHeaders # AddSecHeaders appends sec-* headers to proxied requests based on the currently authenticated user - AddSecHeaders + - PreserveHostHeader - ApplicationError global-filter: websocket-routing: From cb458e76e15a225c3cfec58eb4d9c45cc7877878 Mon Sep 17 00:00:00 2001 From: Pierre Mauduit Date: Tue, 20 Feb 2024 15:29:42 +0100 Subject: [PATCH 2/3] fix: Fix IT testsuite following PR#104 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Following #104, the `Host` http header became mandatory, else throwing a NPE in Netty's code when reaching the NettyRoutingFilter code: ``` java.lang.NullPointerException: value at io.netty.util.internal.ObjectUtil.checkNotNull(ObjectUtil.java:39) Suppressed: The stacktrace has been enhanced by Reactor, refer to additional information below: Error has been observed at the following site(s): *__checkpoint ⇢ org.springframework.cloud.gateway.filter.WeightCalculatorWebFilter [DefaultWebFilterChain] *__checkpoint ⇢ org.springframework.security.web.server.authorization.AuthorizationWebFilter [DefaultWebFilterChain] *__checkpoint ⇢ org.springframework.security.web.server.authorization.ExceptionTranslationWebFilter [DefaultWebFilterChain] *__checkpoint ⇢ org.springframework.security.web.server.authentication.logout.LogoutWebFilter [DefaultWebFilterChain] *__checkpoint ⇢ org.springframework.security.web.server.savedrequest.ServerRequestCacheWebFilter [DefaultWebFilterChain] *__checkpoint ⇢ org.springframework.security.web.server.context.SecurityContextServerWebExchangeWebFilter [DefaultWebFilterChain] *__checkpoint ⇢ org.springframework.security.web.server.authentication.AuthenticationWebFilter [DefaultWebFilterChain] *__checkpoint ⇢ org.springframework.security.web.server.context.ReactorContextWebFilter [DefaultWebFilterChain] *__checkpoint ⇢ org.springframework.security.web.server.header.HttpHeaderWriterWebFilter [DefaultWebFilterChain] *__checkpoint ⇢ org.springframework.security.config.web.server.ServerHttpSecurity$ServerWebExchangeReactorContextWebFilter [DefaultWebFilterChain] *__checkpoint ⇢ org.springframework.security.web.server.WebFilterChainProxy [DefaultWebFilterChain] *__checkpoint ⇢ org.springframework.boot.actuate.metrics.web.reactive.server.MetricsWebFilter [DefaultWebFilterChain] *__checkpoint ⇢ org.springframework.security.test.web.reactive.server.SecurityMockServerConfigurers$MutatorFilter [DefaultWebFilterChain] *__checkpoint ⇢ HTTP GET "/path/..." [ExceptionHandlingWebHandler] Original Stack Trace: at io.netty.util.internal.ObjectUtil.checkNotNull(ObjectUtil.java:39) at io.netty.handler.codec.DefaultHeaders.fromObject(DefaultHeaders.java:1117) at io.netty.handler.codec.DefaultHeaders.addObject(DefaultHeaders.java:364) at io.netty.handler.codec.http.DefaultHttpHeaders.add(DefaultHttpHeaders.java:115) at org.springframework.cloud.gateway.filter.NettyRoutingFilter.lambda$filter$0(NettyRoutingFilter.java:139) at reactor.netty.http.client.HttpClient.headers(HttpClient.java:1038) at org.springframework.cloud.gateway.filter.NettyRoutingFilter.filter(NettyRoutingFilter.java:133) at org.springframework.cloud.gateway.handler.FilteringWebHandler$GatewayFilterAdapter.filter(FilteringWebHandler.java:137) at org.springframework.cloud.gateway.filter.OrderedGatewayFilter.filter(OrderedGatewayFilter.java:44) at org.springframework.cloud.gateway.handler.FilteringWebHandler$DefaultGatewayFilterChain.lambda$filter$0(FilteringWebHandler.java:117) ``` Tests: `mvn clean verify` on gateway subdir went fine 3 times in a row. Notes: * It might deserve a trace in the documentation that the `Host` header is now mandatory, somehow ? * I did not have to patch every WebTestClient calls strangely enough, only the one expecting a `200 - OK` return code. --- .../app/CustomErrorAttributes.java | 2 +- .../GeorchestraGatewayApplicationTests.java | 11 ++++-- .../ResolveGeorchestraUserGlobalFilterIT.java | 3 ++ .../accessrules/AccessRulesCustomizerIT.java | 35 +++++++++++-------- 4 files changed, 33 insertions(+), 18 deletions(-) diff --git a/gateway/src/main/java/org/georchestra/gateway/autoconfigure/app/CustomErrorAttributes.java b/gateway/src/main/java/org/georchestra/gateway/autoconfigure/app/CustomErrorAttributes.java index dcae1733..d5838e4f 100644 --- a/gateway/src/main/java/org/georchestra/gateway/autoconfigure/app/CustomErrorAttributes.java +++ b/gateway/src/main/java/org/georchestra/gateway/autoconfigure/app/CustomErrorAttributes.java @@ -47,7 +47,7 @@ * {@link java.net.UnknownHostException} and {@link java.net.ConnectException} * to {@link HttpStatus#SERVICE_UNAVAILABLE} */ -class CustomErrorAttributes extends DefaultErrorAttributes { +public class CustomErrorAttributes extends DefaultErrorAttributes { @Override public Map getErrorAttributes(ServerRequest request, ErrorAttributeOptions options) { diff --git a/gateway/src/test/java/org/georchestra/gateway/app/GeorchestraGatewayApplicationTests.java b/gateway/src/test/java/org/georchestra/gateway/app/GeorchestraGatewayApplicationTests.java index a7591c5a..6d5cde6e 100644 --- a/gateway/src/test/java/org/georchestra/gateway/app/GeorchestraGatewayApplicationTests.java +++ b/gateway/src/test/java/org/georchestra/gateway/app/GeorchestraGatewayApplicationTests.java @@ -29,6 +29,7 @@ import java.util.function.Function; import java.util.stream.Collectors; +import org.georchestra.gateway.autoconfigure.app.CustomErrorAttributes; import org.georchestra.gateway.autoconfigure.app.ErrorCustomizerAutoConfiguration; import org.georchestra.gateway.security.ldap.extended.GeorchestraUserNamePasswordAuthenticationToken; import org.junit.jupiter.api.Test; @@ -39,6 +40,7 @@ import org.springframework.boot.test.context.SpringBootTest.WebEnvironment; import org.springframework.cloud.gateway.route.Route; import org.springframework.cloud.gateway.route.RouteLocator; +import org.springframework.context.ApplicationContext; import org.springframework.core.env.Environment; import org.springframework.http.HttpStatus; import org.springframework.security.core.Authentication; @@ -59,6 +61,8 @@ class GeorchestraGatewayApplicationTests { private @Autowired GeorchestraGatewayApplication application; private @Autowired WebTestClient testClient; + private @Autowired ApplicationContext context; + @Test void contextLoadsFromDatadir() { assertEquals("src/test/resources/test-datadir", env.getProperty("georchestra.datadir")); @@ -109,12 +113,13 @@ void makeSureWhoamiDoesNotProvideAnySensitiveInfo() { void errorCustomizerReturnsServiceUnavailableInsteadOfServerError() { Map routesById = routeLocator.getRoutes() .collect(Collectors.toMap(Route::getId, Function.identity())).block(); - + assertThat(context.getBean(CustomErrorAttributes.class)).isNotNull(); Route testRoute = routesById.get("unknownHostRoute"); assertNotNull(testRoute); assertThat(testRoute.getUri()).isEqualTo(URI.create("http://not.a.valid.host:80")); - testClient.get().uri("/path/to/unavailable/service").exchange().expectStatus() - .isEqualTo(HttpStatus.SERVICE_UNAVAILABLE); + testClient.get().uri("/path/to/unavailable/service")// + .header("Host", "localhost")// + .exchange().expectStatus().isEqualTo(HttpStatus.SERVICE_UNAVAILABLE); } } diff --git a/gateway/src/test/java/org/georchestra/gateway/security/ResolveGeorchestraUserGlobalFilterIT.java b/gateway/src/test/java/org/georchestra/gateway/security/ResolveGeorchestraUserGlobalFilterIT.java index 238e2395..f1e8ed9a 100644 --- a/gateway/src/test/java/org/georchestra/gateway/security/ResolveGeorchestraUserGlobalFilterIT.java +++ b/gateway/src/test/java/org/georchestra/gateway/security/ResolveGeorchestraUserGlobalFilterIT.java @@ -68,6 +68,7 @@ protected void doStart() { assertNotNull(context.getBean(JsonPayloadHeadersContributor.class)); testClient.get().uri("/echo/")// + .header("Host", "localhost")// .header("Authorization", "Basic dGVzdGFkbWluOnRlc3RhZG1pbg==") // testadmin:testadmin .exchange()// .expectStatus()// @@ -81,6 +82,7 @@ protected void doStart() { gatewayConfig.getDefaultHeaders().setJsonOrganization(Optional.of(false)); testClient.get().uri("/echo/")// + .header("Host", "localhost")// .header("Authorization", "Basic dGVzdGFkbWluOnRlc3RhZG1pbg==") // testadmin:testadmin .exchange()// .expectStatus()// @@ -96,6 +98,7 @@ protected void doStart() { gatewayConfig.getDefaultHeaders().setJsonOrganization(Optional.of(true)); testClient.get().uri("/echo/")// + .header("Host", "localhost")// .header("Authorization", "Basic dGVzdGFkbWluOnRlc3RhZG1pbg==") // testadmin:testadmin .exchange()// .expectStatus()// diff --git a/gateway/src/test/java/org/georchestra/gateway/security/accessrules/AccessRulesCustomizerIT.java b/gateway/src/test/java/org/georchestra/gateway/security/accessrules/AccessRulesCustomizerIT.java index 7ee5ed25..5877a8f1 100644 --- a/gateway/src/test/java/org/georchestra/gateway/security/accessrules/AccessRulesCustomizerIT.java +++ b/gateway/src/test/java/org/georchestra/gateway/security/accessrules/AccessRulesCustomizerIT.java @@ -19,14 +19,10 @@ package org.georchestra.gateway.security.accessrules; -import static com.github.tomakehurst.wiremock.client.WireMock.equalTo; -import static com.github.tomakehurst.wiremock.client.WireMock.get; -import static com.github.tomakehurst.wiremock.client.WireMock.noContent; -import static com.github.tomakehurst.wiremock.client.WireMock.ok; -import static com.github.tomakehurst.wiremock.client.WireMock.urlMatching; - -import java.net.URI; - +import com.github.tomakehurst.wiremock.core.WireMockConfiguration; +import com.github.tomakehurst.wiremock.junit5.WireMockExtension; +import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo; +import lombok.extern.slf4j.Slf4j; import org.georchestra.gateway.app.GeorchestraGatewayApplication; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -40,11 +36,9 @@ import org.springframework.test.context.DynamicPropertySource; import org.springframework.test.web.reactive.server.WebTestClient; -import com.github.tomakehurst.wiremock.core.WireMockConfiguration; -import com.github.tomakehurst.wiremock.junit5.WireMockExtension; -import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo; +import java.net.URI; -import lombok.extern.slf4j.Slf4j; +import static com.github.tomakehurst.wiremock.client.WireMock.*; /** * Integration tests for {@link AccessRulesCustomizer} for the access rules in @@ -133,8 +127,12 @@ private static void mockServiceTarget(DynamicPropertyRegistry registry, String s .withHeader("sec-proxy", equalTo("true"))// .willReturn(ok())); - testClient.get().uri("/header").exchange().expectStatus().isOk(); - testClient.get().uri("/header/img/logo.png").exchange().expectStatus().isOk(); + testClient.get().uri("/header")// + .header("Host", "localhost")// + .exchange().expectStatus().isOk(); + testClient.get().uri("/header/img/logo.png")// + .header("Host", "localhost")// + .exchange().expectStatus().isOk(); } /** @@ -172,10 +170,12 @@ private static void mockServiceTarget(DynamicPropertyRegistry registry, String s mockService.stubFor(get(urlMatching("/import(/.*)?")).willReturn(ok())); testClient.get().uri("/import")// + .header("Host", "localhost")// .exchange()// .expectStatus().isOk(); testClient.get().uri("/import/any/thing")// + .header("Host", "localhost")// .exchange()// .expectStatus().isOk(); } @@ -216,10 +216,12 @@ private static void mockServiceTarget(DynamicPropertyRegistry registry, String s mockService.stubFor(get(urlMatching("/analytics(/.*)?")).willReturn(ok())); testClient.get().uri("/analytics")// + .header("Host", "localhost")// .exchange()// .expectStatus().isOk(); testClient.get().uri("/analytics/any/thing")// + .header("Host", "localhost")// .exchange()// .expectStatus().isOk(); } @@ -264,10 +266,12 @@ void testGlobalAccessRule() { mockService.stubFor(get(urlMatching("/mapstore(/.*)?")).willReturn(ok())); testClient.get().uri("/mapstore")// + .header("Host", "localhost")// .exchange()// .expectStatus().isOk(); testClient.get().uri("/mapstore/any/thing")// + .header("Host", "localhost")// .exchange()// .expectStatus().isOk(); } @@ -281,6 +285,7 @@ void testQueryParamAuthentication_forbidden_when_anonymous() { .expectStatus().is3xxRedirection(); testClient.get().uri("/header")// + .header("Host", "localhost")// .exchange()// .expectStatus().isOk(); } @@ -291,10 +296,12 @@ void testQueryParamAuthentication_authorized_if_logged_in() { mockService.stubFor(get(urlMatching("/header(.*)?")).willReturn(ok())); testClient.get().uri("/header?login")// + .header("Host", "localhost")// .exchange()// .expectStatus().isOk(); testClient.get().uri("/header")// + .header("Host", "localhost")// .exchange()// .expectStatus().isOk(); } From 6d35e43cb8d93ea6f08700a480059d631c785875 Mon Sep 17 00:00:00 2001 From: Florian Necas Date: Tue, 28 May 2024 15:34:12 +0200 Subject: [PATCH 3/3] fix: update host header for testSecOrgName --- .../gateway/security/ResolveGeorchestraUserGlobalFilterIT.java | 1 + 1 file changed, 1 insertion(+) diff --git a/gateway/src/test/java/org/georchestra/gateway/security/ResolveGeorchestraUserGlobalFilterIT.java b/gateway/src/test/java/org/georchestra/gateway/security/ResolveGeorchestraUserGlobalFilterIT.java index f1e8ed9a..1e0ecf53 100644 --- a/gateway/src/test/java/org/georchestra/gateway/security/ResolveGeorchestraUserGlobalFilterIT.java +++ b/gateway/src/test/java/org/georchestra/gateway/security/ResolveGeorchestraUserGlobalFilterIT.java @@ -110,6 +110,7 @@ protected void doStart() { public @Test void testSecOrgnamePresent() { testClient.get().uri("/echo/")// + .header("Host", "localhost")// .header("Authorization", "Basic dGVzdGFkbWluOnRlc3RhZG1pbg==") // testadmin:testadmin .exchange()// .expectStatus()//