From b4614e466ce1217be72c1075b60fd36ec7382df5 Mon Sep 17 00:00:00 2001 From: Minh Date: Sat, 6 Apr 2024 16:14:38 +0700 Subject: [PATCH] Hide the value of sensitive query parameters in log (#8242) Add option to redact sensitive query params. --- .../api/logging-interceptor.api | 1 + .../okhttp3/logging/HttpLoggingInterceptor.kt | 28 +++++- .../logging/HttpLoggingInterceptorTest.kt | 98 +++++++++++++++++++ 3 files changed, 125 insertions(+), 2 deletions(-) diff --git a/okhttp-logging-interceptor/api/logging-interceptor.api b/okhttp-logging-interceptor/api/logging-interceptor.api index 264c2e88f..b6b4d7e94 100644 --- a/okhttp-logging-interceptor/api/logging-interceptor.api +++ b/okhttp-logging-interceptor/api/logging-interceptor.api @@ -8,6 +8,7 @@ public final class okhttp3/logging/HttpLoggingInterceptor : okhttp3/Interceptor public fun intercept (Lokhttp3/Interceptor$Chain;)Lokhttp3/Response; public final fun level (Lokhttp3/logging/HttpLoggingInterceptor$Level;)V public final fun redactHeader (Ljava/lang/String;)V + public final fun redactQueryParams ([Ljava/lang/String;)V public final fun setLevel (Lokhttp3/logging/HttpLoggingInterceptor$Level;)Lokhttp3/logging/HttpLoggingInterceptor; } diff --git a/okhttp-logging-interceptor/src/main/kotlin/okhttp3/logging/HttpLoggingInterceptor.kt b/okhttp-logging-interceptor/src/main/kotlin/okhttp3/logging/HttpLoggingInterceptor.kt index 5d03adb5c..99bcfc58a 100644 --- a/okhttp-logging-interceptor/src/main/kotlin/okhttp3/logging/HttpLoggingInterceptor.kt +++ b/okhttp-logging-interceptor/src/main/kotlin/okhttp3/logging/HttpLoggingInterceptor.kt @@ -22,6 +22,7 @@ import java.nio.charset.Charset import java.util.TreeSet import java.util.concurrent.TimeUnit import okhttp3.Headers +import okhttp3.HttpUrl import okhttp3.Interceptor import okhttp3.OkHttpClient import okhttp3.Response @@ -46,6 +47,8 @@ class HttpLoggingInterceptor ) : Interceptor { @Volatile private var headersToRedact = emptySet() + @Volatile private var queryParamsNameToRedact = emptySet() + @set:JvmName("level") @Volatile var level = Level.NONE @@ -132,6 +135,13 @@ class HttpLoggingInterceptor headersToRedact = newHeadersToRedact } + fun redactQueryParams(vararg name: String) { + val newQueryParamsNameToRedact = TreeSet(String.CASE_INSENSITIVE_ORDER) + newQueryParamsNameToRedact += queryParamsNameToRedact + newQueryParamsNameToRedact.addAll(name) + queryParamsNameToRedact = newQueryParamsNameToRedact + } + /** * Sets the level and returns this. * @@ -168,7 +178,7 @@ class HttpLoggingInterceptor val connection = chain.connection() var requestStartMessage = - ("--> ${request.method} ${request.url}${if (connection != null) " " + connection.protocol() else ""}") + ("--> ${request.method} ${redactUrl(request.url)}${if (connection != null) " " + connection.protocol() else ""}") if (!logHeaders && requestBody != null) { requestStartMessage += " (${requestBody.contentLength()}-byte body)" } @@ -251,7 +261,7 @@ class HttpLoggingInterceptor buildString { append("<-- ${response.code}") if (response.message.isNotEmpty()) append(" ${response.message}") - append(" ${response.request.url} (${tookMs}ms") + append(" ${redactUrl(response.request.url)} (${tookMs}ms") if (!logHeaders) append(", $bodySize body") append(")") }, @@ -312,6 +322,20 @@ class HttpLoggingInterceptor return response } + internal fun redactUrl(url: HttpUrl): String { + if (queryParamsNameToRedact.isEmpty() || url.querySize == 0) { + return url.toString() + } + return url.newBuilder().query(null).apply { + for (i in 0 until url.querySize) { + val parameterName = url.queryParameterName(i) + val newValue = if (parameterName in queryParamsNameToRedact) "██" else url.queryParameterValue(i) + + addEncodedQueryParameter(parameterName, newValue) + } + }.toString() + } + private fun logHeader( headers: Headers, i: Int, diff --git a/okhttp-logging-interceptor/src/test/java/okhttp3/logging/HttpLoggingInterceptorTest.kt b/okhttp-logging-interceptor/src/test/java/okhttp3/logging/HttpLoggingInterceptorTest.kt index bc93ac55d..0c8549a17 100644 --- a/okhttp-logging-interceptor/src/test/java/okhttp3/logging/HttpLoggingInterceptorTest.kt +++ b/okhttp-logging-interceptor/src/test/java/okhttp3/logging/HttpLoggingInterceptorTest.kt @@ -903,6 +903,104 @@ class HttpLoggingInterceptorTest { .assertNoMoreLogs() } + @Suppress("INVISIBLE_MEMBER", "INVISIBLE_REFERENCE") + @Test + fun sensitiveQueryParamsAreRedacted() { + url = server.url("/api/login?user=test_user&authentication=basic&password=confidential_password") + val networkInterceptor = + HttpLoggingInterceptor(networkLogs).setLevel( + Level.BASIC, + ) + networkInterceptor.redactQueryParams("user", "passWord") + + val applicationInterceptor = + HttpLoggingInterceptor(applicationLogs).setLevel( + Level.BASIC, + ) + applicationInterceptor.redactQueryParams("user", "PassworD") + + client = + OkHttpClient.Builder() + .addNetworkInterceptor(networkInterceptor) + .addInterceptor(applicationInterceptor) + .build() + server.enqueue( + MockResponse.Builder() + .build(), + ) + val response = + client + .newCall( + request() + .build(), + ) + .execute() + response.body.close() + val redactedUrl = networkInterceptor.redactUrl(url) + val redactedUrlPattern = redactedUrl.replace("?", """\?""") + applicationLogs + .assertLogEqual("--> GET $redactedUrl") + .assertLogMatch(Regex("""<-- 200 OK $redactedUrlPattern \(\d+ms, \d+-byte body\)""")) + .assertNoMoreLogs() + networkLogs + .assertLogEqual("--> GET $redactedUrl http/1.1") + .assertLogMatch(Regex("""<-- 200 OK $redactedUrlPattern \(\d+ms, \d+-byte body\)""")) + .assertNoMoreLogs() + } + + @Suppress("INVISIBLE_MEMBER", "INVISIBLE_REFERENCE") + @Test + fun preserveQueryParamsAfterRedacted() { + url = + server.url( + """/api/login? + |user=test_user& + |authentication=basic& + |password=confidential_password& + |authentication=rather simple login method + """.trimMargin(), + ) + val networkInterceptor = + HttpLoggingInterceptor(networkLogs).setLevel( + Level.BASIC, + ) + networkInterceptor.redactQueryParams("user", "passWord") + + val applicationInterceptor = + HttpLoggingInterceptor(applicationLogs).setLevel( + Level.BASIC, + ) + applicationInterceptor.redactQueryParams("user", "PassworD") + + client = + OkHttpClient.Builder() + .addNetworkInterceptor(networkInterceptor) + .addInterceptor(applicationInterceptor) + .build() + server.enqueue( + MockResponse.Builder() + .build(), + ) + val response = + client + .newCall( + request() + .build(), + ) + .execute() + response.body.close() + val redactedUrl = networkInterceptor.redactUrl(url) + val redactedUrlPattern = redactedUrl.replace("?", """\?""") + applicationLogs + .assertLogEqual("--> GET $redactedUrl") + .assertLogMatch(Regex("""<-- 200 OK $redactedUrlPattern \(\d+ms, \d+-byte body\)""")) + .assertNoMoreLogs() + networkLogs + .assertLogEqual("--> GET $redactedUrl http/1.1") + .assertLogMatch(Regex("""<-- 200 OK $redactedUrlPattern \(\d+ms, \d+-byte body\)""")) + .assertNoMoreLogs() + } + @Test fun duplexRequestsAreNotLogged() { platform.assumeHttp2Support()