Set ip_address = {{auto}} by default (#2860)

getsentry · Jul 24, 2023 · 19fca04 · 19fca04
1 parent 3838091
commit 19fca04
Show file tree

Hide file tree

Showing 7 changed files with 62 additions and 61 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,8 @@
 Breaking changes:
 - Capture failed HTTP requests by default ([#2794](https://github.com/getsentry/sentry-java/pull/2794))
 - Reduce flush timeout to 4s on Android to avoid ANRs ([#2858](https://github.com/getsentry/sentry-java/pull/2858))
+- 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
 

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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)