From 7def4c008332040f228c730af09deac8b2d32b3a Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Fri, 28 Aug 2020 09:29:01 +0200 Subject: [PATCH 1/5] Add PiiEventProcessor to filter out personal identifiable information from the events. --- .../io/sentry/core/PiiEventProcessor.java | 33 +++++++++++ .../java/io/sentry/core/SentryOptions.java | 12 ++++ .../io/sentry/core/PiiEventProcessorTest.kt | 56 +++++++++++++++++++ 3 files changed, 101 insertions(+) create mode 100644 sentry-core/src/main/java/io/sentry/core/PiiEventProcessor.java create mode 100644 sentry-core/src/test/java/io/sentry/core/PiiEventProcessorTest.kt diff --git a/sentry-core/src/main/java/io/sentry/core/PiiEventProcessor.java b/sentry-core/src/main/java/io/sentry/core/PiiEventProcessor.java new file mode 100644 index 000000000..b55922b7a --- /dev/null +++ b/sentry-core/src/main/java/io/sentry/core/PiiEventProcessor.java @@ -0,0 +1,33 @@ +package io.sentry.core; + +import io.sentry.core.protocol.User; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +/** + * Removes personal identifiable information from {@link SentryEvent} if {@link + * SentryOptions#isSendDefaultPii()} is set to false. + */ +@ApiStatus.Internal +final class PiiEventProcessor implements EventProcessor { + private final @NotNull SentryOptions options; + + PiiEventProcessor(final @NotNull SentryOptions options) { + this.options = options; + } + + @Override + public SentryEvent process(SentryEvent event, @Nullable Object hint) { + if (!options.isSendDefaultPii()) { + final User user = event.getUser(); + if (user != null) { + user.setUsername(null); + user.setIpAddress(null); + user.setEmail(null); + event.setUser(user); + } + } + return event; + } +} diff --git a/sentry-core/src/main/java/io/sentry/core/SentryOptions.java b/sentry-core/src/main/java/io/sentry/core/SentryOptions.java index c3246f8d3..7f36ffd41 100644 --- a/sentry-core/src/main/java/io/sentry/core/SentryOptions.java +++ b/sentry-core/src/main/java/io/sentry/core/SentryOptions.java @@ -210,6 +210,9 @@ public class SentryOptions { /** SdkVersion object that contains the Sentry Client Name and its version */ private @Nullable SdkVersion sdkVersion; + /** whether to send personal identifiable information along with events */ + private boolean sendDefaultPii = false; + /** * Adds an event processor * @@ -996,6 +999,14 @@ public void setSdkVersion(final @Nullable SdkVersion sdkVersion) { this.sdkVersion = sdkVersion; } + public boolean isSendDefaultPii() { + return sendDefaultPii; + } + + public void setSendDefaultPii(boolean sendDefaultPii) { + this.sendDefaultPii = sendDefaultPii; + } + /** The BeforeSend callback */ public interface BeforeSendCallback { @@ -1036,6 +1047,7 @@ public SentryOptions() { integrations.add(new ShutdownHookIntegration()); eventProcessors.add(new MainEventProcessor(this)); + eventProcessors.add(new PiiEventProcessor(this)); setSentryClientName(BuildConfig.SENTRY_JAVA_SDK_NAME + "/" + BuildConfig.VERSION_NAME); setSdkVersion(createSdkVersion()); diff --git a/sentry-core/src/test/java/io/sentry/core/PiiEventProcessorTest.kt b/sentry-core/src/test/java/io/sentry/core/PiiEventProcessorTest.kt new file mode 100644 index 000000000..a94e649a4 --- /dev/null +++ b/sentry-core/src/test/java/io/sentry/core/PiiEventProcessorTest.kt @@ -0,0 +1,56 @@ +package io.sentry.core + +import io.sentry.core.protocol.User +import kotlin.test.Test +import kotlin.test.assertNotNull +import kotlin.test.assertNull + +class PiiEventProcessorTest { + + private class Fixture { + val user = with(User()) { + this.id = "some-id" + this.username = "john.doe" + this.email = "john.doe@example.com" + this.ipAddress = "66.249.73.223" + this + } + + fun getSut(sendPii: Boolean) = + PiiEventProcessor(with(SentryOptions()) { + this.isSendDefaultPii = sendPii + this + }) + } + + private val fixture = Fixture() + + @Test + fun `when sendDefaultPii is set to false, removes user data from events`() { + val eventProcessor = fixture.getSut(sendPii = false) + val event = SentryEvent() + event.user = fixture.user + + eventProcessor.process(event, null) + + assertNotNull(event.user.id) + assertNull(event.user.email) + assertNull(event.user.username) + assertNull(event.user.ipAddress) + } + + @Test + fun `when sendDefaultPii is set to true, does not remove user data from events`() { + val eventProcessor = fixture.getSut(sendPii = true) + val event = SentryEvent() + event.user = fixture.user + + eventProcessor.process(event, null) + + assertNotNull(event.user) + assertNotNull(event.user.id) + assertNotNull(event.user.email) + assertNotNull(event.user.username) + assertNotNull(event.user.ipAddress) + } +} From 670a5d260e770bedaad79d38a9ea0bccd34c1ae5 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Fri, 28 Aug 2020 10:24:31 +0200 Subject: [PATCH 2/5] Remove sensitive headers from events. --- .../io/sentry/core/PiiEventProcessor.java | 21 ++++++++-- .../io/sentry/core/PiiEventProcessorTest.kt | 42 +++++++++++++++++++ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/sentry-core/src/main/java/io/sentry/core/PiiEventProcessor.java b/sentry-core/src/main/java/io/sentry/core/PiiEventProcessor.java index b55922b7a..52168e500 100644 --- a/sentry-core/src/main/java/io/sentry/core/PiiEventProcessor.java +++ b/sentry-core/src/main/java/io/sentry/core/PiiEventProcessor.java @@ -1,7 +1,10 @@ package io.sentry.core; +import io.sentry.core.protocol.Request; import io.sentry.core.protocol.User; -import org.jetbrains.annotations.ApiStatus; +import io.sentry.core.util.CollectionUtils; +import io.sentry.core.util.Objects; +import java.util.Map; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -9,12 +12,11 @@ * Removes personal identifiable information from {@link SentryEvent} if {@link * SentryOptions#isSendDefaultPii()} is set to false. */ -@ApiStatus.Internal final class PiiEventProcessor implements EventProcessor { private final @NotNull SentryOptions options; PiiEventProcessor(final @NotNull SentryOptions options) { - this.options = options; + this.options = Objects.requireNonNull(options, "The options object is required"); } @Override @@ -27,6 +29,19 @@ public SentryEvent process(SentryEvent event, @Nullable Object hint) { user.setEmail(null); event.setUser(user); } + final Request request = event.getRequest(); + if (request != null) { + final Map headers = CollectionUtils.shallowCopy(request.getHeaders()); + if (headers != null) { + headers.remove("X-FORWARDED-FOR"); + headers.remove("Authorization"); + headers.remove("Cookies"); + request.setHeaders(headers); + } + if (request.getCookies() != null) { + request.setCookies(null); + } + } } return event; } diff --git a/sentry-core/src/test/java/io/sentry/core/PiiEventProcessorTest.kt b/sentry-core/src/test/java/io/sentry/core/PiiEventProcessorTest.kt index a94e649a4..b45432366 100644 --- a/sentry-core/src/test/java/io/sentry/core/PiiEventProcessorTest.kt +++ b/sentry-core/src/test/java/io/sentry/core/PiiEventProcessorTest.kt @@ -1,5 +1,6 @@ package io.sentry.core +import io.sentry.core.protocol.Request import io.sentry.core.protocol.User import kotlin.test.Test import kotlin.test.assertNotNull @@ -16,6 +17,17 @@ class PiiEventProcessorTest { this } + val request = with(Request()) { + this.headers = mutableMapOf( + "X-FORWARDED-FOR" to "66.249.73.223", + "Authorization" to "token", + "Cookies" to "some-cookies", + "Safe-Header" to "some-value" + ) + this.cookies = "some-cookies" + this + } + fun getSut(sendPii: Boolean) = PiiEventProcessor(with(SentryOptions()) { this.isSendDefaultPii = sendPii @@ -53,4 +65,34 @@ class PiiEventProcessorTest { assertNotNull(event.user.username) assertNotNull(event.user.ipAddress) } + + @Test + fun `when sendDefaultPii is set to false, removes user identifiable request headers data from events`() { + val eventProcessor = fixture.getSut(sendPii = false) + val event = SentryEvent() + event.request = fixture.request + + eventProcessor.process(event, null) + + assertNotNull(event.request.headers) + assertNull(event.request.headers["Authorization"]) + assertNull(event.request.headers["X-FORWARDED-FOR"]) + assertNull(event.request.headers["Cookies"]) + assertNotNull(event.request.headers["Safe-Header"]) + } + + @Test + fun `when sendDefaultPii is set to true, does not remove user identifiable request headers data from events`() { + val eventProcessor = fixture.getSut(sendPii = true) + val event = SentryEvent() + event.request = fixture.request + + eventProcessor.process(event, null) + + assertNotNull(event.request.headers) + assertNotNull(event.request.headers["Authorization"]) + assertNotNull(event.request.headers["X-FORWARDED-FOR"]) + assertNotNull(event.request.headers["Cookies"]) + assertNotNull(event.request.headers["Safe-Header"]) + } } From 3db0435d8be915a5ba57b17e75f45e6a9d9424bd Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Fri, 28 Aug 2020 10:49:23 +0200 Subject: [PATCH 3/5] Make sure that PiiEventProcessor runs as the last one. --- .../main/java/io/sentry/core/SentryClient.java | 4 ++++ .../main/java/io/sentry/core/SentryOptions.java | 10 +++++++++- .../test/java/io/sentry/core/SentryClientTest.kt | 15 +++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/sentry-core/src/main/java/io/sentry/core/SentryClient.java b/sentry-core/src/main/java/io/sentry/core/SentryClient.java index e65aac25b..5e7ce3045 100644 --- a/sentry-core/src/main/java/io/sentry/core/SentryClient.java +++ b/sentry-core/src/main/java/io/sentry/core/SentryClient.java @@ -92,6 +92,10 @@ public SentryClient(final @NotNull SentryOptions options, @Nullable Connection c } } + if (event != null) { + event = options.getPiiEventProcessor().process(event, hint); + } + if (event == null) { return SentryId.EMPTY_ID; } diff --git a/sentry-core/src/main/java/io/sentry/core/SentryOptions.java b/sentry-core/src/main/java/io/sentry/core/SentryOptions.java index 7f36ffd41..045ace87e 100644 --- a/sentry-core/src/main/java/io/sentry/core/SentryOptions.java +++ b/sentry-core/src/main/java/io/sentry/core/SentryOptions.java @@ -31,6 +31,8 @@ public class SentryOptions { */ private final @NotNull List eventProcessors = new CopyOnWriteArrayList<>(); + private final @NotNull EventProcessor piiEventProcessor; + /** * Code that provides middlewares, bindings or hooks into certain frameworks or environments, * along with code that inserts those bindings and activates them. @@ -1007,6 +1009,11 @@ public void setSendDefaultPii(boolean sendDefaultPii) { this.sendDefaultPii = sendDefaultPii; } + @NotNull + public EventProcessor getPiiEventProcessor() { + return piiEventProcessor; + } + /** The BeforeSend callback */ public interface BeforeSendCallback { @@ -1047,7 +1054,8 @@ public SentryOptions() { integrations.add(new ShutdownHookIntegration()); eventProcessors.add(new MainEventProcessor(this)); - eventProcessors.add(new PiiEventProcessor(this)); + // piiEventProcessor is set outside of eventProcessors to make sure that it runs last + piiEventProcessor = new PiiEventProcessor(this); setSentryClientName(BuildConfig.SENTRY_JAVA_SDK_NAME + "/" + BuildConfig.VERSION_NAME); setSdkVersion(createSdkVersion()); diff --git a/sentry-core/src/test/java/io/sentry/core/SentryClientTest.kt b/sentry-core/src/test/java/io/sentry/core/SentryClientTest.kt index 08cb4531d..c55f8e436 100644 --- a/sentry-core/src/test/java/io/sentry/core/SentryClientTest.kt +++ b/sentry-core/src/test/java/io/sentry/core/SentryClientTest.kt @@ -440,6 +440,21 @@ class SentryClientTest { verify(processor).process(eq(event), anyOrNull()) } + @Test + fun `pii event processor is the last event processor applied`() { + val processor = EventProcessor { event, hint -> + event.user = User() + event.user.email = "john.doe@example.com" + event + } + fixture.sentryOptions.addEventProcessor(processor) + + val event = SentryEvent() + + fixture.getSut().captureEvent(event) + assertNull(event.user.email) + } + @Test fun `when captureSession and no release is set, do nothing`() { fixture.getSut().captureSession(createSession("")) From 2c76507896d1c5c436c3313905a6e500ac1746fa Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Fri, 28 Aug 2020 10:52:51 +0200 Subject: [PATCH 4/5] Polish. --- .../main/java/io/sentry/core/PiiEventProcessor.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/sentry-core/src/main/java/io/sentry/core/PiiEventProcessor.java b/sentry-core/src/main/java/io/sentry/core/PiiEventProcessor.java index 52168e500..678ce54d7 100644 --- a/sentry-core/src/main/java/io/sentry/core/PiiEventProcessor.java +++ b/sentry-core/src/main/java/io/sentry/core/PiiEventProcessor.java @@ -4,6 +4,8 @@ import io.sentry.core.protocol.User; import io.sentry.core.util.CollectionUtils; import io.sentry.core.util.Objects; +import java.util.Arrays; +import java.util.List; import java.util.Map; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -13,6 +15,9 @@ * SentryOptions#isSendDefaultPii()} is set to false. */ final class PiiEventProcessor implements EventProcessor { + private static final List SENSITIVE_HEADERS = + Arrays.asList("X-FORWARDED-FOR", "Authorization", "Cookies"); + private final @NotNull SentryOptions options; PiiEventProcessor(final @NotNull SentryOptions options) { @@ -33,9 +38,9 @@ public SentryEvent process(SentryEvent event, @Nullable Object hint) { if (request != null) { final Map headers = CollectionUtils.shallowCopy(request.getHeaders()); if (headers != null) { - headers.remove("X-FORWARDED-FOR"); - headers.remove("Authorization"); - headers.remove("Cookies"); + for (String sensitiveHeader : SENSITIVE_HEADERS) { + headers.remove(sensitiveHeader); + } request.setHeaders(headers); } if (request.getCookies() != null) { From 3d18b0373c1e94900c6d21abe4c285ac5b823b8f Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Fri, 28 Aug 2020 14:30:03 +0200 Subject: [PATCH 5/5] Polish. --- .../java/io/sentry/core/PiiEventProcessorTest.kt | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/sentry-core/src/test/java/io/sentry/core/PiiEventProcessorTest.kt b/sentry-core/src/test/java/io/sentry/core/PiiEventProcessorTest.kt index b45432366..5c00fc0d5 100644 --- a/sentry-core/src/test/java/io/sentry/core/PiiEventProcessorTest.kt +++ b/sentry-core/src/test/java/io/sentry/core/PiiEventProcessorTest.kt @@ -10,27 +10,27 @@ class PiiEventProcessorTest { private class Fixture { val user = with(User()) { - this.id = "some-id" - this.username = "john.doe" - this.email = "john.doe@example.com" - this.ipAddress = "66.249.73.223" + id = "some-id" + username = "john.doe" + email = "john.doe@example.com" + ipAddress = "66.249.73.223" this } val request = with(Request()) { - this.headers = mutableMapOf( + headers = mutableMapOf( "X-FORWARDED-FOR" to "66.249.73.223", "Authorization" to "token", "Cookies" to "some-cookies", "Safe-Header" to "some-value" ) - this.cookies = "some-cookies" + cookies = "some-cookies" this } fun getSut(sendPii: Boolean) = PiiEventProcessor(with(SentryOptions()) { - this.isSendDefaultPii = sendPii + isSendDefaultPii = sendPii this }) }