Merge pull request #177 from georchestra/strip-basic-auth-but-not-bearer

strip basic-authentication authorization http headers, but not bearer-related ones

docs/custom_filters.adoc

@@ -77,7 +77,38 @@ spring:
 === RemoveSecurityHeadersGatewayFilter
 
 Removes any incoming `sec-\*` request header to prevent impersonation. Valid `sec-*` headers ought to be
-computed by the filter chain and appended to proxied requests.
+computed by the filter chain and appended to proxied requests. This filter also removes the
+`Authorization: basic [...]` http headers, which is meant to authenticate geOrchestra users and
+should not be sent to proxified webapps.
+
+Technically, this filter is an instance of the `RemoveHeadersGatewayFilter` (see next section)
+instanciated with the following regex:
+
+```
+"(?i)(sec-.*|Authorization)"
+```
+
+=== RemoveHeadersGatewayFilter
+
+This filter is a more generic version of the previous one, which allows to pass a custom regular
+expression as parameter.
+
+Considering you want to strip Basic-authentication http headers, but keep the bearer-related ones
+proxified to underlying webapps, one could use the following configuration:
+
+```
+spring:
+  cloud:
+    gateway:
+      routes:
+      - id: proxified-service
+        uri: http://proxified-service:8080
+        predicates:
+        - Path=/proxified-service/**
+        filters:
+        # Strip basic authentication headers but keep bearer ones
+        - RemoveHeaders=(?i)^(sec-.*|Authorization:(?!\s*Bearer\s*$))
+```
 
 === AddSecHeadersGatewayFilter
 

gateway/src/main/java/org/georchestra/gateway/filter/headers/HeaderFiltersConfiguration.java

@@ -95,4 +95,5 @@ RemoveHeadersGatewayFilterFactory removeHeadersGatewayFilterFactory() {
     RemoveSecurityHeadersGatewayFilterFactory removeSecurityHeadersGatewayFilterFactory() {
         return new RemoveSecurityHeadersGatewayFilterFactory();
     }
+
 }

gateway/src/main/java/org/georchestra/gateway/filter/headers/RemoveHeadersGatewayFilterFactory.java

@@ -21,6 +21,7 @@
 import java.util.Arrays;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
 import java.util.regex.Pattern;
 
@@ -76,11 +77,10 @@ public List<String> shortcutFieldOrder() {
     public GatewayFilter apply(RegExConfig regexConfig) {
         return (exchange, chain) -> {
             final RegExConfig config = regexConfig;// == null ? DEFAULT_SECURITY_HEADERS_CONFIG : regexConfig;
-            HttpHeaders incoming = exchange.getRequest().getHeaders();
-            if (config.anyMatches(incoming)) {
-                ServerHttpRequest request = exchange.getRequest().mutate().headers(config::removeMatching).build();
-                exchange = exchange.mutate().request(request).build();
-            }
+
+            ServerHttpRequest request = exchange.getRequest().mutate().headers(config::removeMatching).build();
+            exchange = exchange.mutate().request(request).build();
+
             return chain.filter(exchange);
         };
     }
@@ -107,17 +107,21 @@ private Pattern pattern() {
             return compiled;
         }
 
-        boolean matches(@NonNull String headerName) {
-            return pattern().matcher(headerName).matches();
+        boolean anyMatches(@NonNull HttpHeaders httpHeaders) {
+            return httpHeaders.keySet().stream().anyMatch(h -> this.matches(h, httpHeaders.get(h)));
+        }
+
+        boolean matches(@NonNull String headerNameOrTuple) {
+            return pattern().matcher(headerNameOrTuple).matches();
         }
 
-        boolean anyMatches(@NonNull HttpHeaders headers) {
-            return headers.keySet().stream().anyMatch(this::matches);
+        boolean matches(@NonNull String headerName, List<String> values) {
+            return values.stream().map(value -> "%s: %s".formatted(headerName, value)).anyMatch(this::matches);
         }
 
         void removeMatching(@NonNull HttpHeaders headers) {
-            new HashSet<>(headers.keySet()).stream()//
-                    .filter(this::matches)//
+            List.copyOf(headers.entrySet()).stream().filter(e -> matches(e.getKey()))//
+                    .filter(e -> matches(e.getKey(), e.getValue())).map(Map.Entry::getKey)//
                     .peek(name -> log.trace("Removing header {}", name))//
                     .forEach(headers::remove);
         }

gateway/src/main/resources/application.yml

@@ -48,7 +48,7 @@ spring:
       default-filters:
       - SecureHeaders # add security-related HTTP headers to responses sent from the gateway to clients. See https://blog.appcanary.com/2017/http-security-headers.html
       - TokenRelay # propagates OAuth2 access tokens from incoming requests to downstream services
-      - RemoveSecurityHeaders # removing incoming sec-* headers to prevent impresionation
+      - RemoveSecurityHeaders # removing incoming sec-* headers to prevent impersonation
       - AddSecHeaders # append resolved sec-* headers to proxied requests based on the currently authenticated user
       - PreserveHostHeader # ensure that the original Host header from the incoming HTTP request is preserved and passed along to the downstream service
       - ApplicationError # use gateway's custom error pages when a downstream request returns an error code

gateway/src/test/java/org/georchestra/gateway/filter/headers/RemoveHeadersGatewayFilterFactoryTest.java

@@ -106,10 +106,37 @@ void testIsSecurityHeader() {
         assertTrue(secHeadersConfig.matches("SEC-USER"));
         assertTrue(secHeadersConfig.matches("sec-org"));
         assertTrue(secHeadersConfig.matches("SEC-ORG"));
-        assertTrue(secHeadersConfig.matches("authoriZation"), "Basic Auth header shoulud should be filtered");
+        assertTrue(secHeadersConfig.matches("authoriZation"), "Basic Auth header should should be filtered");
         assertFalse(secHeadersConfig.matches("SECORG"));
     }
 
+    @Test
+    void testHeaderFilterOnHeaderNameAndValues() {
+        RegExConfig headersConfig = new RemoveHeadersGatewayFilterFactory.RegExConfig(
+                "(?i)^(sec-.*|Authorization:(?!\s*Bearer\s*$))"
+        );
+        GatewayFilter toTest = new RemoveHeadersGatewayFilterFactory().apply(headersConfig);
+        HttpHeaders headers = headers(
+                "Authorization", "Basic YWRtaW46ZWNlZXdhZGVpY2hhZTJhaWZhZVF1YWkyb29OZ2llN3U=",
+                "Authorization", "Bearer ahlai7Eer1Vuz8ThaiY4ahbohdeish7a",
+                "sec-roles", "ROLE_ADMINISTRATOR",
+                "sec-username", "testadmin"
+        );
+        MockServerHttpRequest request = MockServerHttpRequest.get("/").headers(headers).build();
+        ServerWebExchange exchange = MockServerWebExchange.from(request);
+        GatewayFilterChain chain = mock(GatewayFilterChain.class);
+        ArgumentCaptor<ServerWebExchange> filterArgCaptor = ArgumentCaptor.forClass(ServerWebExchange.class);
+
+        toTest.filter(exchange, chain);
+
+        verify(chain).filter(filterArgCaptor.capture());
+        ServerWebExchange mutatedExchange = filterArgCaptor.getValue();
+        HttpHeaders mutatedHeaders = mutatedExchange.getRequest().getHeaders();
+        HttpHeaders expected = headers("Authorization", "Bearer ahlai7Eer1Vuz8ThaiY4ahbohdeish7a");
+        assertEquals(expected, mutatedHeaders);
+
+    }
+
     private HttpHeaders headers(String... kvp) {
         assertTrue(kvp.length % 2 == 0);
         HttpHeaders headers = new HttpHeaders();