From 6ceb5c5fdfdc6d1b312479f07ab75c923d6a11fc Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Fri, 21 Jul 2023 08:16:34 +0200 Subject: [PATCH 1/2] Set {{auto}} ip-address, regardless of PII settings For disabling: The project configuration has a toggle to disable storing the IP address. --- .../android/core/AnrV2EventProcessor.java | 32 +++++-------------- .../core/DefaultAndroidEventProcessor.java | 27 +++++++--------- .../android/core/AnrV2EventProcessorTest.kt | 7 ++++ .../core/DefaultAndroidEventProcessorTest.kt | 25 +++++++++++++++ .../java/io/sentry/MainEventProcessor.java | 15 ++++----- .../java/io/sentry/MainEventProcessorTest.kt | 15 ++------- 6 files changed, 60 insertions(+), 61 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2EventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2EventProcessor.java index 6a4b7edfa1..81422ea514 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2EventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2EventProcessor.java @@ -475,35 +475,19 @@ private void setExceptions(final @NotNull SentryEvent event, final @NotNull Obje } private void mergeUser(final @NotNull SentryBaseEvent event) { - if (options.isSendDefaultPii()) { - if (event.getUser() == null) { - final User user = new User(); - user.setIpAddress(IpAddressUtils.DEFAULT_IP_ADDRESS); - event.setUser(user); - } else if (event.getUser().getIpAddress() == null) { - event.getUser().setIpAddress(IpAddressUtils.DEFAULT_IP_ADDRESS); - } + @Nullable User user = event.getUser(); + if (user == null) { + user = new User(); + event.setUser(user); } // userId should be set even if event is Cached as the userId is static and won't change anyway. - final User user = event.getUser(); - if (user == null) { - event.setUser(getDefaultUser()); - } else if (user.getId() == null) { + if (user.getId() == null) { user.setId(getDeviceId()); } - } - - /** - * Sets the default user which contains only the userId. - * - * @return the User object - */ - private @NotNull User getDefaultUser() { - User user = new User(); - user.setId(getDeviceId()); - - return user; + if (user.getIpAddress() == null) { + user.setIpAddress(IpAddressUtils.DEFAULT_IP_ADDRESS); + } } private @Nullable String getDeviceId() { diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java index 8d88c6674a..446443f544 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java @@ -20,6 +20,7 @@ import io.sentry.DateUtils; import io.sentry.EventProcessor; import io.sentry.Hint; +import io.sentry.IpAddressUtils; import io.sentry.SentryBaseEvent; import io.sentry.SentryEvent; import io.sentry.SentryLevel; @@ -165,13 +166,19 @@ private boolean shouldApplyScopeData( } private void mergeUser(final @NotNull SentryBaseEvent event) { - // userId should be set even if event is Cached as the userId is static and won't change anyway. - final User user = event.getUser(); + @Nullable User user = event.getUser(); if (user == null) { - event.setUser(getDefaultUser()); - } else if (user.getId() == null) { + user = new User(); + event.setUser(user); + } + + // userId should be set even if event is Cached as the userId is static and won't change anyway. + if (user.getId() == null) { user.setId(getDeviceId()); } + if (user.getIpAddress() == null) { + user.setIpAddress(IpAddressUtils.DEFAULT_IP_ADDRESS); + } } private void setDevice( @@ -728,18 +735,6 @@ private void setAppPackageInfo(final @NotNull App app, final @NotNull PackageInf } } - /** - * Sets the default user which contains only the userId. - * - * @return the User object - */ - public @NotNull User getDefaultUser() { - User user = new User(); - user.setId(getDeviceId()); - - return user; - } - private @Nullable String getDeviceId() { try { return Installation.id(context); diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AnrV2EventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AnrV2EventProcessorTest.kt index 69598a0e1e..d6a05ddd2c 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AnrV2EventProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AnrV2EventProcessorTest.kt @@ -406,6 +406,10 @@ class AnrV2EventProcessorTest { debugMeta = DebugMeta().apply { images = listOf(DebugImage().apply { type = DebugImage.PROGUARD; uuid = "uuid1" }) } + user = User().apply { + id = "42" + ipAddress = "2.4.8.16" + } } assertEquals("NotAndroid", processed.platform) @@ -427,6 +431,9 @@ class AnrV2EventProcessorTest { assertEquals(2, processed.debugMeta!!.images!!.size) assertEquals("uuid1", processed.debugMeta!!.images!![0].uuid) assertEquals("uuid", processed.debugMeta!!.images!![1].uuid) + + assertEquals("42", processed.user!!.id) + assertEquals("2.4.8.16", processed.user!!.ipAddress) } @Test diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt index 8747409cce..65d86c8cfa 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt @@ -295,6 +295,31 @@ class DefaultAndroidEventProcessorTest { } } + @Test + fun `when event user data does not have ip address set, sets {{auto}} as the ip address`() { + val sut = fixture.getSut(context) + val event = SentryEvent().apply { + user = User() + } + sut.process(event, Hint()) + assertNotNull(event.user) { + assertEquals("{{auto}}", it.ipAddress) + } + } + + @Test + fun `when event has ip address set, keeps original ip address`() { + val sut = fixture.getSut(context) + val event = SentryEvent() + event.user = User().apply { + ipAddress = "192.168.0.1" + } + sut.process(event, Hint()) + assertNotNull(event.user) { + assertEquals("192.168.0.1", it.ipAddress) + } + } + @Test fun `Executor service should be called on ctor`() { val sut = fixture.getSut(context) diff --git a/sentry/src/main/java/io/sentry/MainEventProcessor.java b/sentry/src/main/java/io/sentry/MainEventProcessor.java index d046855958..abbf21c84e 100644 --- a/sentry/src/main/java/io/sentry/MainEventProcessor.java +++ b/sentry/src/main/java/io/sentry/MainEventProcessor.java @@ -220,14 +220,13 @@ private void setTags(final @NotNull SentryBaseEvent event) { } private void mergeUser(final @NotNull SentryBaseEvent event) { - if (options.isSendDefaultPii()) { - if (event.getUser() == null) { - final User user = new User(); - user.setIpAddress(IpAddressUtils.DEFAULT_IP_ADDRESS); - event.setUser(user); - } else if (event.getUser().getIpAddress() == null) { - event.getUser().setIpAddress(IpAddressUtils.DEFAULT_IP_ADDRESS); - } + @Nullable User user = event.getUser(); + if (user == null) { + user = new User(); + event.setUser(user); + } + if (user.getIpAddress() == null) { + user.setIpAddress(IpAddressUtils.DEFAULT_IP_ADDRESS); } } diff --git a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt index 4e6b4f3d99..bbdd252ce3 100644 --- a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt +++ b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt @@ -297,7 +297,7 @@ class MainEventProcessorTest { } @Test - fun `when event does not have ip address set and sendDefaultPii is set to true, sets {{auto}} as the ip address`() { + fun `when event does not have ip address set, sets {{auto}} as the ip address`() { val sut = fixture.getSut(sendDefaultPii = true) val event = SentryEvent() sut.process(event, Hint()) @@ -307,7 +307,7 @@ class MainEventProcessorTest { } @Test - fun `when event has ip address set and sendDefaultPii is set to true, keeps original ip address`() { + fun `when event has ip address set, keeps original ip address`() { val sut = fixture.getSut(sendDefaultPii = true) val event = SentryEvent() event.user = User().apply { @@ -319,17 +319,6 @@ class MainEventProcessorTest { } } - @Test - fun `when event does not have ip address set and sendDefaultPii is set to false, does not set ip address`() { - val sut = fixture.getSut(sendDefaultPii = false) - val event = SentryEvent() - event.user = User() - sut.process(event, Hint()) - assertNotNull(event.user) { - assertNull(it.ipAddress) - } - } - @Test fun `when event has environment set, does not overwrite environment`() { val sut = fixture.getSut(environment = null) From 0809e9560d329c714a1e1f688d374dd7eadf4560 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Fri, 21 Jul 2023 09:41:10 +0200 Subject: [PATCH 2/2] Add Changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2376590a8f..409af905f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ Breaking changes: - Capture failed HTTP requests by default ([#2794](https://github.com/getsentry/sentry-java/pull/2794)) +- Set ip_address to {{auto}} by default, even if sendDefaultPII is disabled ([#2860](https://github.com/getsentry/sentry-java/pull/2860)) + - Instead use the "Prevent Storing of IP Addresses" option in the "Security & Privacy" project settings on sentry.io ### Fixes