Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hide the value of sensitive query parameters in log #8242

Merged
merged 6 commits into from
Apr 6, 2024

Conversation

MinhDang685
Copy link
Contributor

@MinhDang685 MinhDang685 commented Feb 6, 2024

For logging interceptor: add an option to hide the value of sensitive query parameters in the request and response URL.
Inspired by the implementation of header redaction in okhttp3.logging.HttpLoggingInterceptor

To use
Call the method redactQueryParams(the list of query parameters to hide value, separated by commas) from a okhttp3.logging.HttpLoggingInterceptor instance. For example, to hide the value of "user" and "password" query parameters, use interceptor.redactQueryParams("user", "password").

  • Original URL: http://localhost:8000/api/login?user=user1&authentication=basic&password=confidential_password"
  • With the method applied: http://localhost:8000/api/login?user=██&authentication=basic&password=██"

@MinhDang685 MinhDang685 changed the title Add an option to hide the value of sensitive query parameters. Hide the value of sensitive query parameters in logging Feb 6, 2024
@MinhDang685 MinhDang685 changed the title Hide the value of sensitive query parameters in logging Hide the value of sensitive query parameters in log Feb 6, 2024
@yschimke
Copy link
Collaborator

yschimke commented Feb 10, 2024

This is against the resolution of #7476

But I continue to believe that HttpLoggingInterceptor is not designed for production use. Speculative logging is too noisy. For development I love Chucker; for production I’d like something that does spans like Datadog or Chronosphere.

As it's a functional PR, I'm inclined to accept it.

Will wait for a final take by @swankjesse

Any follow up work could be wasted, so feel free to wait for a clearer signal.

Copy link
Collaborator

@swankjesse swankjesse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me. I’ve got some naming changes I’d like you to make before I merge it!

@yschimke yschimke added the android Relates to usage specifically on Android label Apr 6, 2024
@@ -903,6 +903,101 @@ class HttpLoggingInterceptorTest {
.assertNoMoreLogs()
}

@Suppress("INVISIBLE_MEMBER", "INVISIBLE_REFERENCE")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MinhDang685 we run these tests in android also, which means they need to workaround internal methods. So I added this.

@yschimke yschimke merged commit b4614e4 into square:master Apr 6, 2024
22 checks passed
@MinhDang685 MinhDang685 deleted the add-redact-query-params branch May 20, 2024 06:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
android Relates to usage specifically on Android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants