From 5ac5d90f9709a631d9093058472c8680447ca6f3 Mon Sep 17 00:00:00 2001 From: Jorge Bornhausen Date: Sat, 28 Jun 2025 04:20:52 +0200 Subject: [PATCH] fix(tracing): implement redact for query, add default values --- .../commons/tracing/TracingConfiguration.java | 18 ++++++ .../commons/tracing/TracingRequestFilter.java | 42 ++++++++----- .../quarkus/commons/tracing/ActorTest.java | 2 +- .../commons/tracing/QueryParamTest.java | 54 ++++++++++++++++ .../quarkus/commons/tracing/RedactedTest.java | 61 +++++++++++++++++++ .../src/test/resources/application.yaml | 1 + 6 files changed, 162 insertions(+), 16 deletions(-) create mode 100644 quarkus-tracing-service/src/test/java/ch/phoenix/oss/quarkus/commons/tracing/QueryParamTest.java create mode 100644 quarkus-tracing-service/src/test/java/ch/phoenix/oss/quarkus/commons/tracing/RedactedTest.java diff --git a/quarkus-tracing-service/src/main/java/ch/phoenix/oss/quarkus/commons/tracing/TracingConfiguration.java b/quarkus-tracing-service/src/main/java/ch/phoenix/oss/quarkus/commons/tracing/TracingConfiguration.java index 46e2854..1093348 100644 --- a/quarkus-tracing-service/src/main/java/ch/phoenix/oss/quarkus/commons/tracing/TracingConfiguration.java +++ b/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 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> redact(); } @@ -31,9 +40,18 @@ public interface TracingConfiguration { Query query(); interface Query { + + Set 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> redact(); } } diff --git a/quarkus-tracing-service/src/main/java/ch/phoenix/oss/quarkus/commons/tracing/TracingRequestFilter.java b/quarkus-tracing-service/src/main/java/ch/phoenix/oss/quarkus/commons/tracing/TracingRequestFilter.java index 4a1cbd4..7bb2427 100644 --- a/quarkus-tracing-service/src/main/java/ch/phoenix/oss/quarkus/commons/tracing/TracingRequestFilter.java +++ b/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()); } + var redactedHeaders = configuration + .requestFilter() + .headers() + .redact() + .orElse(TracingConfiguration.RequestFilterConfiguration.Headers.DEFAULT_REDACTED); + requestContext.getHeaders().forEach((key, value) -> { var lowerCaseKey = key.toLowerCase(); var property = TracingConstants.REQUEST_HEADERS + '.' + lowerCaseKey; - if (configuration - .requestFilter() - .headers() - .redact() - .orElse(Set.of()) - .contains(lowerCaseKey)) { - tracingService.trace(property, "********"); + if (redactedHeaders.contains(lowerCaseKey)) { + tracingService.trace(property, REDACTED_VALUE); } else { tracingService.trace(property, joinStrings(value)); } }); - uriInfo.getQueryParameters() - .forEach((key, value) -> - tracingService.trace(TracingConstants.REQUEST_QUERY_PARAMS + '.' + key, joinStrings(value))); + var redactedQueryParams = configuration + .requestFilter() + .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.tracef( - "Incoming request: %s %s", method, uriInfo.getAbsolutePath().getRawPath()); - } + Log.debugf("Incoming request: %s %s", method, uriInfo.getAbsolutePath().getRawPath()); } private static String joinStrings(List value) { diff --git a/quarkus-tracing-service/src/test/java/ch/phoenix/oss/quarkus/commons/tracing/ActorTest.java b/quarkus-tracing-service/src/test/java/ch/phoenix/oss/quarkus/commons/tracing/ActorTest.java index 2190561..a173b72 100644 --- a/quarkus-tracing-service/src/test/java/ch/phoenix/oss/quarkus/commons/tracing/ActorTest.java +++ b/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")); diff --git a/quarkus-tracing-service/src/test/java/ch/phoenix/oss/quarkus/commons/tracing/QueryParamTest.java b/quarkus-tracing-service/src/test/java/ch/phoenix/oss/quarkus/commons/tracing/QueryParamTest.java new file mode 100644 index 0000000..7378040 --- /dev/null +++ b/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); + } +} diff --git a/quarkus-tracing-service/src/test/java/ch/phoenix/oss/quarkus/commons/tracing/RedactedTest.java b/quarkus-tracing-service/src/test/java/ch/phoenix/oss/quarkus/commons/tracing/RedactedTest.java new file mode 100644 index 0000000..34658e2 --- /dev/null +++ b/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); + } +} diff --git a/quarkus-tracing-service/src/test/resources/application.yaml b/quarkus-tracing-service/src/test/resources/application.yaml index 9d0943f..3c6bb6f 100644 --- a/quarkus-tracing-service/src/test/resources/application.yaml +++ b/quarkus-tracing-service/src/test/resources/application.yaml @@ -32,6 +32,7 @@ quarkus: headers: redact: - AUTHORIZATION + - X-SOMETHING-ELSE query: include-raw: true redact: -- 2.47.2