Merge pull request #60 from groldan/forwarded_headers

Fix handling of forwarded headers and servlet context path for swagger ui

compose/compose.yml

@@ -36,13 +36,16 @@ services:
       - PG_USER=acl
       - PG_PASSWORD=acls3cr3t
       - SPRING_PROFILES_ACTIVE=logging_debug_requests
+      # uncomment for remote debugging
+      #- JAVA_OPTS=-Xdebug -agentlib:jdwp=transport=dt_socket,address=*:15005,server=y,suspend=n
     depends_on:
       acldb:
         condition: service_healthy
         required: true    
     ports:
       - 8080:8080
       - 8081:8081
+      - 15005:15005
     deploy:
       resources:
         limits:

compose/gateway-service.yml

@@ -1,6 +1,6 @@
 geoserver.base-path: ${geoserver_base_path:}
 
-targets.acl: http://10.0.0.71:8080
+targets.acl: http://acl:8080
 
 server:
   forward-headers-strategy: framework

src/artifacts/api/pom.xml

@@ -56,21 +56,7 @@
     <dependency>
       <groupId>org.springframework.boot</groupId>
       <artifactId>spring-boot-starter-web</artifactId>
-      <!--
-      <exclusions>
-        <exclusion>
-          <groupId>org.springframework.boot</groupId>
-          <artifactId>spring-boot-starter-tomcat</artifactId>
-        </exclusion>
-      </exclusions>
-      -->
     </dependency>
-    <!--
-    <dependency>
-      <groupId>org.springframework.boot</groupId>
-      <artifactId>spring-boot-starter-undertow</artifactId>
-    </dependency>
-    -->
     <dependency>
       <groupId>org.springframework.boot</groupId>
       <artifactId>spring-boot-starter-validation</artifactId>

src/artifacts/api/src/main/java/org/geoserver/acl/autoconfigure/springdoc/SpringDocAutoConfiguration.java

@@ -15,6 +15,7 @@
 import org.springdoc.core.providers.SpringWebProvider;
 import org.springdoc.webmvc.ui.SwaggerConfig;
 import org.springdoc.webmvc.ui.SwaggerWelcomeWebMvc;
+import org.springframework.beans.factory.annotation.Value;
 import org.springframework.boot.autoconfigure.AutoConfiguration;
 import org.springframework.context.annotation.Bean;
 import org.springframework.web.context.request.NativeWebRequest;
@@ -27,128 +28,82 @@
 @Slf4j(topic = "org.geoserver.acl.autoconfigure.springdoc")
 public class SpringDocAutoConfiguration {
 
+    private @Value("${server.servlet.context-path:/}") String servletContextPath;
+
     @Bean
     SpringDocHomeRedirectController homeRedirectController(NativeWebRequest req) {
-        return new SpringDocHomeRedirectController(req);
+        return new SpringDocHomeRedirectController(req, servletContextPath);
     }
 
     @Bean
-    ServerBaseUrlCustomizer xForwardedPrefixAwareServerBaseUrlCustomizer(NativeWebRequest req) {
-        return new XForwardedPrefixBaseUrlCustomizer(req);
+    ServerBaseUrlCustomizer xForwardedPrefixAwareServerBaseUrlCustomizer() {
+        return new ServletContextSuffixingBaseUrlCustomizer(servletContextPath);
     }
 
     /**
-     * Override the one defined in {@link SwaggerConfig} to apply the{@literal X-Forwarded-Prefix}
-     * request header prefix to the swagger ui config urls
+     * Override the one defined in {@link SwaggerConfig} to append the servlet-context path suffix
+     * to URLs if they don't have it
      */
     @Bean
     SwaggerWelcomeWebMvc xForwardedPrefixAwareSwaggerWelcome(
             SwaggerUiConfigProperties swaggerUiConfig,
             SpringDocConfigProperties springDocConfigProperties,
             SwaggerUiConfigParameters swaggerUiConfigParameters,
-            SpringWebProvider springWebProvider,
-            NativeWebRequest nativeWebRequest) {
-        return new XForwardedPrefixAwareSwaggerWelcomeWebMvc(
+            SpringWebProvider springWebProvider) {
+        return new ServletContextSuffixingSwaggerWelcomeWebMvc(
                 swaggerUiConfig,
                 springDocConfigProperties,
                 swaggerUiConfigParameters,
                 springWebProvider,
-                nativeWebRequest);
+                servletContextPath);
     }
 
-    /**
-     * Springdoc {@link ServerBaseUrlCustomizer} to apply the {@literal X-Forwarded-Prefix} request
-     * header prefix to the base server url presented in the swagger-
-     */
     @RequiredArgsConstructor
-    static class XForwardedPrefixBaseUrlCustomizer implements ServerBaseUrlCustomizer {
-        private final @NonNull NativeWebRequest req;
+    static class ServletContextSuffixingBaseUrlCustomizer implements ServerBaseUrlCustomizer {
+        private final @NonNull String servletContextPath;
 
         @Override
         public String customize(String serverBaseUrl) {
-            return customizeUrl(serverBaseUrl, req);
+            String url = serverBaseUrl;
+            String path = URI.create(serverBaseUrl).getPath();
+            if (path.endsWith("/")) path = path.substring(0, path.length() - 1);
+            if (!path.endsWith(servletContextPath)) {
+                UriComponentsBuilder builder = UriComponentsBuilder.fromHttpUrl(serverBaseUrl);
+                builder.path(servletContextPath);
+                url = builder.build().toString();
+            }
+
+            return url;
         }
     }
 
-    static class XForwardedPrefixAwareSwaggerWelcomeWebMvc extends SwaggerWelcomeWebMvc {
+    static class ServletContextSuffixingSwaggerWelcomeWebMvc extends SwaggerWelcomeWebMvc {
 
-        private final NativeWebRequest nativeWebRequest;
+        private final @NonNull String servletContextPath;
 
-        public XForwardedPrefixAwareSwaggerWelcomeWebMvc(
+        public ServletContextSuffixingSwaggerWelcomeWebMvc(
                 SwaggerUiConfigProperties swaggerUiConfig,
                 SpringDocConfigProperties springDocConfigProperties,
                 SwaggerUiConfigParameters swaggerUiConfigParameters,
                 SpringWebProvider springWebProvider,
-                NativeWebRequest nativeWebRequest) {
+                String contextPath) {
             super(
                     swaggerUiConfig,
                     springDocConfigProperties,
                     swaggerUiConfigParameters,
                     springWebProvider);
-            this.nativeWebRequest = nativeWebRequest;
-        }
-
-        @Override
-        protected String buildApiDocUrl() {
-            var url = super.buildApiDocUrl();
-            url = applyForwardedPrefix(url, nativeWebRequest);
-            log.debug("buildApiDocUrl: {}", url);
-            return url;
-        }
-
-        @Override
-        protected String buildSwaggerConfigUrl() {
-            var url = super.buildSwaggerConfigUrl();
-            url = applyForwardedPrefix(url, nativeWebRequest);
-            log.debug("buildSwaggerConfigUrl: {}", url);
-            return url;
+            this.servletContextPath = contextPath;
         }
 
         @Override
         protected String buildUrl(String contextPath, final String docsUrl) {
-            var url = super.buildUrl(contextPath, docsUrl);
-            url = applyForwardedPrefix(url, nativeWebRequest);
-            log.debug("buildUrl({}, {}): {}", contextPath, docsUrl, url);
-            return url;
-        }
+            String realContextPath = contextPath;
 
-        @Override
-        protected String buildUrlWithContextPath(String swaggerUiUrl) {
-            var url = super.buildUrlWithContextPath(swaggerUiUrl);
-            url = applyForwardedPrefix(url, nativeWebRequest);
-            log.debug("buildUrlWithContextPath({}): {}", swaggerUiUrl, url);
+            if (!realContextPath.endsWith(this.servletContextPath))
+                realContextPath += this.servletContextPath;
+            var url = super.buildUrl(realContextPath, docsUrl);
+            log.debug("buildUrl({}, {}): {}", contextPath, docsUrl, url);
             return url;
         }
     }
-
-    private static String applyForwardedPrefix(String path, NativeWebRequest req) {
-        String prefix = getFirstHeader(req, "X-Forwarded-Prefix");
-        if (null != prefix && !path.startsWith(prefix)) {
-            return prefix + path;
-        }
-        return path;
-    }
-
-    private static String getFirstHeader(NativeWebRequest req, String headerName) {
-        String[] headerValues = req.getHeaderValues(headerName);
-        final String value;
-        if (null != headerValues && headerValues.length > 0) {
-            value = headerValues[0];
-        } else {
-            value = null;
-        }
-        return value;
-    }
-
-    /**
-     * Applies the {@literal X-Forwarded-Prefix} header prefix to a full URL, if provided in the
-     * request
-     */
-    static String customizeUrl(String url, NativeWebRequest req) {
-        String path = URI.create(url).getPath();
-        UriComponentsBuilder builder = UriComponentsBuilder.fromHttpUrl(url);
-        String prefixedPath = applyForwardedPrefix(path, req);
-        builder.replacePath(prefixedPath);
-        return builder.build().toString();
-    }
 }

src/artifacts/api/src/main/java/org/geoserver/acl/autoconfigure/springdoc/SpringDocHomeRedirectController.java

@@ -10,7 +10,8 @@
 import org.springframework.stereotype.Controller;
 import org.springframework.web.bind.annotation.GetMapping;
 import org.springframework.web.context.request.NativeWebRequest;
-import org.springframework.web.util.UriComponentsBuilder;
+
+import java.net.URI;
 
 import javax.servlet.http.HttpServletRequest;
 
@@ -19,14 +20,19 @@
 class SpringDocHomeRedirectController {
 
     private final @NonNull NativeWebRequest req;
+    private final @NonNull String servletContextPath;
 
-    @GetMapping(value = "/")
+    @GetMapping(value = {"", "/"})
     public String redirectToSwaggerUI() {
-        String url = ((HttpServletRequest) req.getNativeRequest()).getRequestURL().toString();
-        UriComponentsBuilder builder = UriComponentsBuilder.fromHttpUrl(url);
-        builder.path("openapi/swagger-ui/index.html");
-        String fullUrl = builder.build().toString();
-        String xForwardedPrefixUrl = SpringDocAutoConfiguration.customizeUrl(fullUrl, req);
-        return "redirect:" + xForwardedPrefixUrl;
+        var target = "/openapi/swagger-ui/index.html";
+        URI url =
+                URI.create(
+                        ((HttpServletRequest) req.getNativeRequest()).getRequestURL().toString());
+        var path = url.getPath();
+        if (path != null) {
+            if (path.endsWith("/")) path = path.substring(0, path.length() - 1);
+            if (!path.endsWith(servletContextPath)) target = servletContextPath + target;
+        }
+        return "redirect:%s".formatted(target);
     }
 }

src/artifacts/api/src/main/resources/application.yml

@@ -13,7 +13,7 @@ server:
   port: 8080
   servlet.context-path: /acl
   # Let spring-boot's ForwardedHeaderFilter take care of reflecting the client-originated protocol and address in the HttpServletRequest  
-  forward-headers-strategy: native
+  forward-headers-strategy: framework
   error:
     # one of never, always, on_trace_param (deprecated), on_param
     include-stacktrace: on-param
@@ -25,6 +25,7 @@ server:
     - application/json
     - application/x-jackson-smile
   tomcat:
+    use-relative-redirects: true
     # Maximum number of connections that the server accepts and processes at any given time.
     # Once the limit has been reached, the operating system may still accept connections based on the "acceptCount" property.
     max-connections: ${tomcat.max.connections:8192}

src/artifacts/api/src/test/java/org/geoserver/acl/app/AbstractAccesControlListApplicationTest.java

@@ -109,11 +109,17 @@ protected void login(String user, String pwd) {
         client = client.withBasicAuth(user, pwd);
     }
 
-    protected <T> ResponseEntity<T> get(String url, Class<T> responseType) {
-        HttpHeaders headers = new HttpHeaders();
-        headers.setAccept(List.of(MediaType.APPLICATION_JSON));
+    protected <T> ResponseEntity<T> get(String path, Class<T> responseType) {
+        return get(path, responseType, new HttpHeaders());
+    }
+
+    protected <T> ResponseEntity<T> get(String path, Class<T> responseType, HttpHeaders headers) {
+        if (headers.getAccept().isEmpty()) {
+            headers.setAccept(List.of(MediaType.APPLICATION_JSON));
+        }
         HttpEntity<String> entity = new HttpEntity<>(headers);
 
+        var url = fullUrl(path);
         return client.exchange(url, HttpMethod.GET, entity, responseType);
     }
 
@@ -122,13 +128,20 @@ private ResponseEntity<Rule> createRule(String json) {
     }
 
     protected <T> ResponseEntity<T> post(
-            String url, String requestBodyJson, Class<T> responseType, Object... urlVariables) {
+            String path, String requestBodyJson, Class<T> responseType, Object... urlVariables) {
 
         HttpHeaders headers = new HttpHeaders();
         headers.setContentType(MediaType.APPLICATION_JSON);
         headers.setAccept(List.of(MediaType.APPLICATION_JSON));
         HttpEntity<String> entity = new HttpEntity<>(requestBodyJson, headers);
 
+        var url = fullUrl(path);
         return client.postForEntity(url, entity, responseType, urlVariables);
     }
+
+    private String fullUrl(String path) {
+        String rootUri = client.getRootUri();
+        assertThat(rootUri).endsWith("/acl");
+        return rootUri + path;
+    }
 }

src/artifacts/api/src/test/java/org/geoserver/acl/app/AccesControlListApplicationTest.java

@@ -4,17 +4,37 @@
  */
 package org.geoserver.acl.app;
 
-import static org.junit.jupiter.api.Assertions.*;
+import static org.assertj.core.api.Assertions.assertThat;
 
-import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
 import org.springframework.boot.test.context.SpringBootTest;
 import org.springframework.boot.test.context.SpringBootTest.WebEnvironment;
+import org.springframework.http.HttpHeaders;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.ResponseEntity;
 import org.springframework.test.context.ActiveProfiles;
 
 @SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT)
 @ActiveProfiles("dev")
 class AccesControlListApplicationTest extends AbstractAccesControlListApplicationTest {
 
-    @BeforeEach
-    void setUp() throws Exception {}
+    @Test
+    void rootRedirectsToSwaggerUI() {
+        String expected = "/acl/openapi/swagger-ui/index.html";
+
+        ResponseEntity<String> response = get("/", String.class);
+        assertThat(response.getStatusCode()).isEqualTo(HttpStatus.FOUND);
+        assertThat(response.getHeaders().get("Location")).containsExactly(expected);
+    }
+
+    @Test
+    void rootRedirectsToSwaggerUIWithXForwardedHeaders() {
+        var headers = new HttpHeaders();
+        headers.add("X-Forwarded-Prefix", "/geoserver/cloud");
+
+        String expected = "/geoserver/cloud/acl/openapi/swagger-ui/index.html";
+        ResponseEntity<String> response = get("/", String.class, headers);
+        assertThat(response.getStatusCode()).isEqualTo(HttpStatus.SEE_OTHER);
+        assertThat(response.getHeaders().get("Location")).containsExactly(expected);
+    }
 }