fix(tracing): implement redact for query, add default values

quarkus-tracing-service/src/main/java/ch/phoenix/oss/quarkus/commons/tracing/TracingConfiguration.java

@@ -3,6 +3,7 @@ package ch.phoenix.oss.quarkus.commons.tracing;
 import io.smallrye.config.ConfigMapping;
 import io.smallrye.config.WithConverter;
 import io.smallrye.config.WithDefault;
+import jakarta.ws.rs.core.HttpHeaders;
 import java.util.Optional;
 import java.util.Set;
 
@@ -17,6 +18,14 @@ public interface TracingConfiguration {
 
         interface Headers {
 
+            Set<String> DEFAULT_REDACTED = Set.of(HttpHeaders.AUTHORIZATION.toLowerCase());
+
+            /**
+             * Optional set of headers to redact when tracing. By default, redacts
+             * the 'Authorization' header.
+             *
+             * @return the set of headers to be redacted
+             */
             Optional<Set<@WithConverter(LowerCaseStringConverter.class) String>> redact();
         }
 
@@ -31,9 +40,18 @@ public interface TracingConfiguration {
         Query query();
 
         interface Query {
+
+            Set<String> DEFAULT_REDACTED = Set.of("access_token", "refresh_token", "apikey");
+
             @WithDefault("false")
             boolean includeRaw();
 
+            /**
+             * Optional set of query params to redact when tracing. By default, redacts
+             * the following params: 'access_token', 'refresh_token' and 'apikey'.
+             *
+             * @return the set of query params to be redacted
+             */
             Optional<Set<@WithConverter(LowerCaseStringConverter.class) String>> redact();
         }
     }

quarkus-tracing-service/src/main/java/ch/phoenix/oss/quarkus/commons/tracing/TracingRequestFilter.java

@@ -9,12 +9,13 @@ import jakarta.ws.rs.Path;
 import jakarta.ws.rs.container.ContainerRequestContext;
 import jakarta.ws.rs.container.ResourceInfo;
 import java.util.List;
-import java.util.Set;
 import org.jboss.resteasy.reactive.server.ServerRequestFilter;
 
 @Unremovable
 public class TracingRequestFilter {
 
+    private static final String REDACTED_VALUE = "********";
+
     private final RoutingContext routingContext;
     private final TracingService tracingService;
     private final SecurityIdentity securityIdentity;
@@ -57,24 +58,38 @@ public class TracingRequestFilter {
                     TracingConstants.REQUEST_PATH_RAW, uriInfo.getAbsolutePath().getRawPath());
         }
 
-        requestContext.getHeaders().forEach((key, value) -> {
+        var redactedHeaders = configuration
-            var lowerCaseKey = key.toLowerCase();
-            var property = TracingConstants.REQUEST_HEADERS + '.' + lowerCaseKey;
-            if (configuration
                 .requestFilter()
                 .headers()
                 .redact()
-                    .orElse(Set.of())
+                .orElse(TracingConfiguration.RequestFilterConfiguration.Headers.DEFAULT_REDACTED);
-                    .contains(lowerCaseKey)) {
+
-                tracingService.trace(property, "********");
+        requestContext.getHeaders().forEach((key, value) -> {
+            var lowerCaseKey = key.toLowerCase();
+            var property = TracingConstants.REQUEST_HEADERS + '.' + lowerCaseKey;
+            if (redactedHeaders.contains(lowerCaseKey)) {
+                tracingService.trace(property, REDACTED_VALUE);
             } else {
                 tracingService.trace(property, joinStrings(value));
             }
         });
 
-        uriInfo.getQueryParameters()
+        var redactedQueryParams = configuration
-                .forEach((key, value) ->
+                .requestFilter()
-                        tracingService.trace(TracingConstants.REQUEST_QUERY_PARAMS + '.' + key, joinStrings(value)));
+                .query()
+                .redact()
+                .orElse(TracingConfiguration.RequestFilterConfiguration.Query.DEFAULT_REDACTED);
+
+        uriInfo.getQueryParameters().forEach((key, value) -> {
+            var lowerCaseKey = key.toLowerCase();
+            var property = TracingConstants.REQUEST_QUERY_PARAMS + '.' + lowerCaseKey;
+
+            if (redactedQueryParams.contains(lowerCaseKey)) {
+                tracingService.trace(property, REDACTED_VALUE);
+            } else {
+                tracingService.trace(property, joinStrings(value));
+            }
+        });
 
         if (configuration.requestFilter().query().includeRaw()) {
             var rawQuery = uriInfo.getRequestUri().getRawQuery();
@@ -87,10 +102,7 @@ public class TracingRequestFilter {
                 TracingConstants.REQUEST_CLIENT_IP,
                 routingContext.request().connection().remoteAddress(true).hostAddress());
 
-        if (Log.isTraceEnabled()) {
+        Log.debugf("Incoming request: %s %s", method, uriInfo.getAbsolutePath().getRawPath());
-            Log.tracef(
-                    "Incoming request: %s %s", method, uriInfo.getAbsolutePath().getRawPath());
-        }
     }
 
     private static String joinStrings(List<String> value) {

quarkus-tracing-service/src/test/java/ch/phoenix/oss/quarkus/commons/tracing/ActorTest.java

@@ -34,7 +34,7 @@ public class ActorTest {
         verify(tracingService).trace("request.route", route);
         verify(tracingService).trace("request.headers.accept", "text/plain");
         verify(tracingService).trace("request.headers.accept-encoding", "gzip,deflate");
-        verify(tracingService).trace("request.headers.authorization", "Basic am9uOmRvZQ==");
+        verify(tracingService).trace("request.headers.authorization", "********");
         verify(tracingService).trace("request.headers.connection", "Keep-Alive");
         verify(tracingService).trace(eq("request.headers.host"), startsWith("localhost:"));
         verify(tracingService).trace(eq("request.headers.user-agent"), startsWith("Apache-HttpClient"));

quarkus-tracing-service/src/test/java/ch/phoenix/oss/quarkus/commons/tracing/QueryParamTest.java

@@ -0,0 +1,54 @@
+package ch.phoenix.oss.quarkus.commons.tracing;
+
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.ArgumentMatchers.startsWith;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+
+import io.quarkus.test.junit.QuarkusTest;
+import io.quarkus.test.junit.mockito.InjectSpy;
+import io.restassured.RestAssured;
+import io.restassured.http.ContentType;
+import org.junit.jupiter.api.Test;
+
+@QuarkusTest
+public class QueryParamTest {
+
+    @InjectSpy
+    TracingService tracingService;
+
+    @Test
+    void traceQueryParams() {
+        var route = "/authenticated";
+        RestAssured.given()
+                .auth()
+                .basic("jon", "doe")
+                .accept(ContentType.TEXT)
+                .header("X-SOMETHING-ELSE", "whatever")
+                .queryParam("access_token", "api123")
+                .queryParam("refresh_token", "refresh123")
+                .queryParam("apikey", "apikey123")
+                .queryParam("grant_type", "authorization_code")
+                .when()
+                .get(route)
+                .then()
+                .statusCode(200);
+
+        verify(tracingService).trace("actor", "jon");
+        verify(tracingService).trace("request.method", "GET");
+        verify(tracingService).trace("request.route", route);
+        verify(tracingService).trace("request.headers.accept", "text/plain");
+        verify(tracingService).trace("request.headers.accept-encoding", "gzip,deflate");
+        verify(tracingService).trace("request.headers.authorization", "********");
+        verify(tracingService).trace("request.headers.connection", "Keep-Alive");
+        verify(tracingService).trace(eq("request.headers.host"), startsWith("localhost:"));
+        verify(tracingService).trace(eq("request.headers.user-agent"), startsWith("Apache-HttpClient"));
+        verify(tracingService).trace("request.headers.x-something-else", "whatever");
+        verify(tracingService).trace("request.query.params.access_token", "********");
+        verify(tracingService).trace("request.query.params.refresh_token", "********");
+        verify(tracingService).trace("request.query.params.apikey", "********");
+        verify(tracingService).trace("request.query.params.grant_type", "authorization_code");
+        verify(tracingService).trace("request.client.ip", "127.0.0.1");
+        verifyNoMoreInteractions(tracingService);
+    }
+}

quarkus-tracing-service/src/test/java/ch/phoenix/oss/quarkus/commons/tracing/RedactedTest.java

@@ -0,0 +1,61 @@
+package ch.phoenix.oss.quarkus.commons.tracing;
+
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.ArgumentMatchers.startsWith;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+
+import io.quarkus.test.junit.QuarkusTest;
+import io.quarkus.test.junit.TestProfile;
+import io.quarkus.test.junit.mockito.InjectSpy;
+import io.restassured.RestAssured;
+import io.restassured.http.ContentType;
+import org.junit.jupiter.api.Test;
+
+@QuarkusTest
+@TestProfile(Test2Profile.class)
+public class RedactedTest {
+
+    @InjectSpy
+    TracingService tracingService;
+
+    @Test
+    void traceRedactedValues() {
+        var route = "/authenticated";
+        RestAssured.given()
+                .auth()
+                .basic("jon", "doe")
+                .accept(ContentType.TEXT)
+                .header("X-SOMETHING-ELSE", "whatever")
+                .queryParam("access_token", "api123")
+                .queryParam("refresh_token", "refresh123")
+                .queryParam("apikey", "apikey123")
+                .queryParam("grant_type", "authorization_code")
+                .when()
+                .get(route)
+                .then()
+                .statusCode(200);
+
+        verify(tracingService).trace("actor", "jon");
+        verify(tracingService).trace("request.method", "GET");
+        verify(tracingService).trace("request.route", route);
+        verify(tracingService).trace("request.path.raw", route);
+        verify(tracingService).trace("request.headers.accept", "text/plain");
+        verify(tracingService).trace("request.headers.accept-encoding", "gzip,deflate");
+        verify(tracingService).trace("request.headers.authorization", "********");
+        verify(tracingService).trace("request.headers.connection", "Keep-Alive");
+        verify(tracingService).trace(eq("request.headers.host"), startsWith("localhost:"));
+        verify(tracingService).trace(eq("request.headers.user-agent"), startsWith("Apache-HttpClient"));
+        verify(tracingService).trace("request.headers.x-something-else", "********");
+        verify(tracingService).trace("request.query.params.access_token", "********");
+        verify(tracingService).trace("request.query.params.refresh_token", "refresh123");
+        verify(tracingService).trace("request.query.params.apikey", "apikey123");
+        verify(tracingService).trace("request.query.params.grant_type", "authorization_code");
+        verify(tracingService)
+                .trace(
+                        "request.query.raw",
+                        "access_token=api123&refresh_token=refresh123&apikey=apikey123&grant_type=authorization_code");
+        verify(tracingService).trace("request.client.ip", "127.0.0.1");
+        verifyNoMoreInteractions(tracingService);
+    }
+}

quarkus-tracing-service/src/test/resources/application.yaml

@@ -32,6 +32,7 @@ quarkus:
           headers:
             redact:
               - AUTHORIZATION
+              - X-SOMETHING-ELSE
           query:
             include-raw: true
             redact: