fix: Fix IT testsuite following PR#104

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.
georchestra · May 28, 2024 · cb458e7 · cb458e7
1 parent 0b94599
commit cb458e7
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 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
@@ -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<String, Object> 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
@@ -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<String, Route> 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/.../src/test/java/org/georchestra/gateway/security/ResolveGeorchestraUserGlobalFilterIT.java b/.../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/...y/src/test/java/org/georchestra/gateway/security/accessrules/AccessRulesCustomizerIT.java b/...y/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();
     }