Send user.ip_address = {{auto}} when sendDefaultPii is true

CHANGELOG.md

@@ -12,6 +12,7 @@
 * Fix: Initialize Logback after context refreshes (#1129)
 * Ref: Return NoOpTransaction instead of null (#1126)
 * Fix: Do not crash when passing null values to @Nullable methods, eg User and Scope
+* Enhancement: Send user.ip_address = {{auto}} when sendDefaultPii is true
 * Fix: Resolving dashed properties from external configuration
 * Feat: Read `uncaught.handler.enabled` property from the external configuration 
 

sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java

@@ -149,8 +149,11 @@ public DefaultAndroidEventProcessor(
     }
 
     // userId should be set even if event is Cached as the userId is static and won't change anyway.
-    if (event.getUser() == null) {
+    final User user = event.getUser();
+    if (user == null) {
       event.setUser(getDefaultUser());
+    } else if (user.getId() == null) {
+      user.setId(getDeviceId());
     }
 
     if (event.getContexts().getDevice() == null) {

sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt

@@ -182,9 +182,10 @@ class DefaultAndroidEventProcessorTest {
     }
 
     @Test
-    fun `When user is already set, do not overwrite it`() {
+    fun `When user with id is already set, do not overwrite it`() {
         val processor = DefaultAndroidEventProcessor(context, fixture.options.logger, fixture.buildInfo)
         val user = User()
+        user.id = "user-id"
         var event = SentryEvent().apply {
             setUser(user)
         }
@@ -193,6 +194,18 @@ class DefaultAndroidEventProcessorTest {
         assertSame(user, event.user)
     }
 
+    @Test
+    fun `When user without id is set, user id is applied`() {
+        val processor = DefaultAndroidEventProcessor(context, fixture.options.logger, fixture.buildInfo)
+        val user = User()
+        var event = SentryEvent().apply {
+            setUser(user)
+        }
+        event = processor.process(event, null)
+        assertNotNull(event.user)
+        assertNotNull(event.user.id)
+    }
+
     @Test
     fun `Executor service should be called on ctor`() {
         val processor = DefaultAndroidEventProcessor(context, fixture.options.logger, fixture.buildInfo)

sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java

@@ -80,7 +80,7 @@ static class HubConfiguration {
         sentryUserProviders.forEach(
             sentryUserProvider ->
                 options.addEventProcessor(
-                    new SentryUserProviderEventProcessor(sentryUserProvider)));
+                    new SentryUserProviderEventProcessor(options, sentryUserProvider)));
         transportGate.ifAvailable(options::setTransportGate);
         transport.ifAvailable(options::setTransport);
         inAppPackagesResolver.resolveInAppIncludes().forEach(options::addInAppInclude);

sentry-spring/api/sentry-spring.api

@@ -53,7 +53,7 @@ public abstract interface class io/sentry/spring/SentryUserProvider {
 }
 
 public final class io/sentry/spring/SentryUserProviderEventProcessor : io/sentry/EventProcessor {
-	public fun <init> (Lio/sentry/spring/SentryUserProvider;)V
+	public fun <init> (Lio/sentry/SentryOptions;Lio/sentry/spring/SentryUserProvider;)V
 	public fun getSentryUserProvider ()Lio/sentry/spring/SentryUserProvider;
 	public fun process (Lio/sentry/SentryEvent;Ljava/lang/Object;)Lio/sentry/SentryEvent;
 }

sentry-spring/src/main/java/io/sentry/spring/SentryInitBeanPostProcessor.java

@@ -28,7 +28,7 @@ public Object postProcessAfterInitialization(
             .forEach(
                 sentryUserProvider ->
                     options.addEventProcessor(
-                        new SentryUserProviderEventProcessor(sentryUserProvider)));
+                        new SentryUserProviderEventProcessor(options, sentryUserProvider)));
       }
       Sentry.init(options);
     }

sentry-spring/src/main/java/io/sentry/spring/SentryUserProviderEventProcessor.java

@@ -1,7 +1,9 @@
 package io.sentry.spring;
 
 import io.sentry.EventProcessor;
+import io.sentry.MainEventProcessor;
 import io.sentry.SentryEvent;
+import io.sentry.SentryOptions;
 import io.sentry.protocol.User;
 import io.sentry.util.Objects;
 import java.util.Map;
@@ -12,9 +14,12 @@
 import org.jetbrains.annotations.Nullable;
 
 public final class SentryUserProviderEventProcessor implements EventProcessor {
+  private final @NotNull SentryOptions options;
   private final @NotNull SentryUserProvider sentryUserProvider;
 
-  public SentryUserProviderEventProcessor(final @NotNull SentryUserProvider sentryUserProvider) {
+  public SentryUserProviderEventProcessor(
+      final @NotNull SentryOptions options, final @NotNull SentryUserProvider sentryUserProvider) {
+    this.options = Objects.requireNonNull(options, "options is required");
     this.sentryUserProvider =
         Objects.requireNonNull(sentryUserProvider, "sentryUserProvider is required");
   }
@@ -39,6 +44,14 @@ public SentryEvent process(final @NotNull SentryEvent event, final @Nullable Obj
       }
       event.setUser(existingUser);
     }
+    if (options.isSendDefaultPii()) {
+      final User existingUser = event.getUser();
+      if (existingUser != null
+          && MainEventProcessor.DEFAULT_IP_ADDRESS.equals(existingUser.getIpAddress())) {
+        // unset {{auto}} as it would set the server's ip address as a user ip address
+        existingUser.setIpAddress(null);
+      }
+    }
     return event;
   }
 

sentry-spring/src/test/kotlin/io/sentry/spring/SentryUserProviderEventProcessorTest.kt

@@ -1,17 +1,29 @@
 package io.sentry.spring
 
 import io.sentry.SentryEvent
+import io.sentry.SentryOptions
 import io.sentry.protocol.User
 import kotlin.test.Test
 import kotlin.test.assertEquals
 import kotlin.test.assertNotNull
+import kotlin.test.assertNull
 
 class SentryUserProviderEventProcessorTest {
 
+    class Fixture {
+
+        fun getSut(isSendDefaultPii: Boolean = false, userProvider: () -> User?): SentryUserProviderEventProcessor {
+            val options = SentryOptions()
+            options.isSendDefaultPii = isSendDefaultPii
+            return SentryUserProviderEventProcessor(options, userProvider)
+        }
+    }
+
+    private val fixture = Fixture()
+
     @Test
     fun `when event user is null, provider user data is set`() {
-
-        val processor = SentryUserProviderEventProcessor {
+        val processor = fixture.getSut {
             val user = User()
             user.username = "john.doe"
             user.id = "user-id"
@@ -35,7 +47,7 @@ class SentryUserProviderEventProcessorTest {
 
     @Test
     fun `when event user is empty, provider user data is set`() {
-        val processor = SentryUserProviderEventProcessor {
+        val processor = fixture.getSut {
             val user = User()
             user.username = "john.doe"
             user.id = "user-id"
@@ -60,7 +72,7 @@ class SentryUserProviderEventProcessorTest {
 
     @Test
     fun `when processor returns empty User, user data is not changed`() {
-        val processor = SentryUserProviderEventProcessor {
+        val processor = fixture.getSut {
             val user = User()
             user
         }
@@ -86,7 +98,7 @@ class SentryUserProviderEventProcessorTest {
 
     @Test
     fun `when processor returns null, user data is not changed`() {
-        val processor = SentryUserProviderEventProcessor {
+        val processor = fixture.getSut {
             null
         }
 
@@ -111,7 +123,7 @@ class SentryUserProviderEventProcessorTest {
 
     @Test
     fun `merges user#others with existing user#others set on SentryEvent`() {
-        val processor = SentryUserProviderEventProcessor {
+        val processor = fixture.getSut {
             val user = User()
             user.others = mapOf("key" to "value")
             user
@@ -127,4 +139,53 @@ class SentryUserProviderEventProcessorTest {
         assertNotNull(result.user)
         assertEquals(mapOf("key" to "value", "new-key" to "new-value"), result.user.others)
     }
+
+    @Test
+    fun `when isSendDefaultPii is true and user is not set, user remains null`() {
+        val processor = fixture.getSut(isSendDefaultPii = true) {
+            null
+        }
+
+        val event = SentryEvent()
+        event.user = null
+
+        val result = processor.process(event, null)
+
+        assertNotNull(result)
+        assertNull(result.user)
+    }
+
+    @Test
+    fun `when isSendDefaultPii is true and user is set with custom ip address, user ip is unchanged`() {
+        val processor = fixture.getSut(isSendDefaultPii = true) {
+            null
+        }
+
+        val event = SentryEvent()
+        val user = User()
+        user.ipAddress = "192.168.0.1"
+        event.user = user
+
+        val result = processor.process(event, null)
+
+        assertNotNull(result)
+        assertEquals(user.ipAddress, result.user.ipAddress)
+    }
+
+    @Test
+    fun `when isSendDefaultPii is true and user is set with {{auto}} ip address, user ip is set to null`() {
+        val processor = fixture.getSut(isSendDefaultPii = true) {
+            null
+        }
+
+        val event = SentryEvent()
+        val user = User()
+        user.ipAddress = "{{auto}}"
+        event.user = user
+
+        val result = processor.process(event, null)
+
+        assertNotNull(result)
+        assertNull(result.user.ipAddress)
+    }
 }

sentry/api/sentry.api

@@ -311,6 +311,7 @@ public abstract interface class io/sentry/Integration {
 }
 
 public final class io/sentry/MainEventProcessor : io/sentry/EventProcessor {
+	public static final field DEFAULT_IP_ADDRESS Ljava/lang/String;
 	public fun process (Lio/sentry/SentryEvent;Ljava/lang/Object;)Lio/sentry/SentryEvent;
 }
 

sentry/src/main/java/io/sentry/MainEventProcessor.java

@@ -1,6 +1,7 @@
 package io.sentry;
 
 import io.sentry.protocol.SentryException;
+import io.sentry.protocol.User;
 import io.sentry.util.ApplyScopeUtils;
 import io.sentry.util.Objects;
 import java.util.ArrayList;
@@ -13,6 +14,12 @@
 @ApiStatus.Internal
 public final class MainEventProcessor implements EventProcessor {
 
+  /**
+   * Default value for {@link User#getIpAddress()} set when event does not have user and ip address
+   * set and when {@link SentryOptions#isSendDefaultPii()} is set to true.
+   */
+  public static final String DEFAULT_IP_ADDRESS = "{{auto}}";
+
   /**
    * Default value for {@link SentryEvent#getEnvironment()} set when both event and {@link
    * SentryOptions} do not have the environment field set.
@@ -123,5 +130,14 @@ private void processNonCachedEvent(final @NotNull SentryEvent event) {
         event.setThreads(sentryThreadFactory.getCurrentThread());
       }
     }
+    if (options.isSendDefaultPii()) {
+      if (event.getUser() == null) {
+        final User user = new User();
+        user.setIpAddress(DEFAULT_IP_ADDRESS);
+        event.setUser(user);
+      } else if (event.getUser().getIpAddress() == null) {
+        event.getUser().setIpAddress(DEFAULT_IP_ADDRESS);
+      }
+    }
   }
 }

sentry/src/test/java/io/sentry/MainEventProcessorTest.kt

@@ -4,6 +4,7 @@ import com.nhaarman.mockitokotlin2.mock
 import io.sentry.hints.ApplyScopeData
 import io.sentry.hints.Cached
 import io.sentry.protocol.SdkVersion
+import io.sentry.protocol.User
 import java.lang.RuntimeException
 import kotlin.test.Test
 import kotlin.test.assertEquals
@@ -25,10 +26,13 @@ class MainEventProcessorTest {
                 version = "1.2.3"
             }
         }
-        fun getSut(attachThreads: Boolean = true, attachStackTrace: Boolean = true, environment: String? = "environment", tags: Map<String, String> = emptyMap()): MainEventProcessor {
+        fun getSut(attachThreads: Boolean = true, attachStackTrace: Boolean = true, environment: String? = "environment", tags: Map<String, String> = emptyMap(), sendDefaultPii: Boolean? = null): MainEventProcessor {
             sentryOptions.isAttachThreads = attachThreads
             sentryOptions.isAttachStacktrace = attachStackTrace
             sentryOptions.environment = environment
+            if (sendDefaultPii != null) {
+                sentryOptions.isSendDefaultPii = sendDefaultPii
+            }
             tags.forEach { sentryOptions.setTag(it.key, it.value) }
             return MainEventProcessor(sentryOptions)
         }
@@ -191,6 +195,36 @@ class MainEventProcessorTest {
         assertEquals("production", event.environment)
     }
 
+    @Test
+    fun `when event does not have ip address set and sendDefaultPii is set to true, sets "{{auto}}" as the ip address`() {
+        val sut = fixture.getSut(sendDefaultPii = true)
+        val event = SentryEvent()
+        sut.process(event, null)
+        assertNotNull(event.user)
+        assertEquals("{{auto}}", event.user.ipAddress)
+    }
+
+    @Test
+    fun `when event has ip address set and sendDefaultPii is set to true, keeps original ip address`() {
+        val sut = fixture.getSut(sendDefaultPii = true)
+        val event = SentryEvent()
+        event.user = User()
+        event.user.ipAddress = "192.168.0.1"
+        sut.process(event, null)
+        assertNotNull(event.user)
+        assertEquals("192.168.0.1", event.user.ipAddress)
+    }
+
+    @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, null)
+        assertNotNull(event.user)
+        assertNull(event.user.ipAddress)
+    }
+
     @Test
     fun `when event has environment set, does not overwrite environment`() {
         val sut = fixture.getSut(environment = null)