diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ApplicationExitInfoEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/ApplicationExitInfoEventProcessor.java index 2eca0e68b5b..62b32dd76ce 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ApplicationExitInfoEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ApplicationExitInfoEventProcessor.java @@ -568,10 +568,10 @@ 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. - if (user.getId() == null) { + if (user.getId() == null && options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) { user.setId(getDeviceId()); } - if (user.getIpAddress() == null && options.isSendDefaultPii()) { + if (user.getIpAddress() == null && options.getDataCollectionResolver().isUserInfo()) { user.setIpAddress(IpAddressUtils.DEFAULT_IP_ADDRESS); } } @@ -635,7 +635,8 @@ private void setDevice(final @NotNull SentryBaseEvent event) { device.setScreenDpi(displayMetrics.densityDpi); } - if (device.getId() == null) { + if (device.getId() == null + && options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) { device.setId(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 83f892573e4..520706b352c 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 @@ -175,10 +175,10 @@ 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. - if (user.getId() == null) { + if (user.getId() == null && options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) { user.setId(Installation.id(context)); } - if (user.getIpAddress() == null && options.isSendDefaultPii()) { + if (user.getIpAddress() == null && options.getDataCollectionResolver().isUserInfo()) { user.setIpAddress(IpAddressUtils.DEFAULT_IP_ADDRESS); } } @@ -374,7 +374,9 @@ private void setAppExtras(final @NotNull App app, final @NotNull Hint hint) { */ public @NotNull User getDefaultUser(final @NotNull Context context) { final @NotNull User user = new User(); - user.setId(Installation.id(context)); + if (options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) { + user.setId(Installation.id(context)); + } return user; } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/DeviceInfoUtil.java b/sentry-android-core/src/main/java/io/sentry/android/core/DeviceInfoUtil.java index 63b88c0e440..d988cbd090e 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/DeviceInfoUtil.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/DeviceInfoUtil.java @@ -130,7 +130,8 @@ public Device collectDeviceInformation( device.setBootTime(getBootTime()); device.setTimezone(getTimeZone()); - if (device.getId() == null) { + if (device.getId() == null + && options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) { device.setId(getDeviceId()); } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java b/sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java index 2779f803a69..822a65727d0 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java @@ -99,7 +99,8 @@ public static Map serializeScope( user = new User(); scope.setUser(user); } - if (user.getId() == null) { + if (user.getId() == null + && options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) { try { user.setId(Installation.id(context)); } catch (RuntimeException e) { diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ApplicationExitInfoEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ApplicationExitInfoEventProcessorTest.kt index e7583429910..7eaa269f39f 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ApplicationExitInfoEventProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ApplicationExitInfoEventProcessorTest.kt @@ -228,6 +228,26 @@ class ApplicationExitInfoEventProcessorTest { assertEquals(SentryBaseEvent.DEFAULT_PLATFORM, processed.platform) } + @Test + fun `when user info is disabled, does not set device id`() { + fixture.options.dataCollection.setUserInfo(false) + val hint = HintUtils.createWithTypeCheckHint(AbnormalExitHint()) + + val processed = processEvent(hint) + + assertNull(processed.contexts.device!!.id) + } + + @Test + fun `when user info is enabled, sets device id`() { + fixture.options.dataCollection.setUserInfo(true) + val hint = HintUtils.createWithTypeCheckHint(AbnormalExitHint()) + + val processed = processEvent(hint, isSendDefaultPii = false) + + assertNotNull(processed.contexts.device!!.id) + } + @Test fun `when backfillable event is not enrichable, sets OS`() { val hint = HintUtils.createWithTypeCheckHint(BackfillableHint(shouldEnrich = false)) @@ -336,6 +356,28 @@ class ApplicationExitInfoEventProcessorTest { assertNull(processed.user!!.ipAddress) } + @Test + fun `when user info is disabled, does not backfill automatic user data`() { + fixture.options.dataCollection.setUserInfo(false) + val hint = HintUtils.createWithTypeCheckHint(BackfillableHint()) + val processed = processEvent(hint, isSendDefaultPii = true, populateScopeCache = true) + + assertEquals("bot", processed.user!!.username) + assertEquals("bot@me.com", processed.user!!.id) + assertNull(processed.user!!.ipAddress) + } + + @Test + fun `when user info is enabled, backfills automatic user data`() { + fixture.options.dataCollection.setUserInfo(true) + val hint = HintUtils.createWithTypeCheckHint(BackfillableHint()) + val processed = processEvent(hint, isSendDefaultPii = false, populateScopeCache = true) + + assertEquals("bot", processed.user!!.username) + assertEquals("bot@me.com", processed.user!!.id) + assertEquals("{{auto}}", processed.user!!.ipAddress) + } + @Test fun `when backfillable event is enrichable, backfills serialized options data`() { val hint = HintUtils.createWithTypeCheckHint(BackfillableHint()) @@ -435,6 +477,19 @@ class ApplicationExitInfoEventProcessorTest { assertEquals(Installation.deviceId, processed!!.user!!.id) } + @Test + fun `when user info is disabled, does not set installation id for missing user id`() { + fixture.options.dataCollection.setUserInfo(false) + val hint = HintUtils.createWithTypeCheckHint(BackfillableHint()) + val original = SentryEvent() + val processor = fixture.getSut(tmpDir) + fixture.persistOptions(USER_FILENAME, User()) + + val processed = processor.process(original, hint) + + assertNull(processed!!.user!!.id) + } + @Test fun `when event has some fields set, does not override them`() { val hint = HintUtils.createWithTypeCheckHint(BackfillableHint()) 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 091a75e1295..fbcd20b99fb 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 @@ -285,6 +285,34 @@ class DefaultAndroidEventProcessorTest { assertNotNull(event.user) { assertEquals("{{auto}}", it.ipAddress) } } + @Test + fun `when user info is disabled, does not set automatic user data`() { + fixture.options.dataCollection.setUserInfo(false) + val sut = fixture.getSut(context, isSendDefaultPii = true) + val event = SentryEvent().apply { user = User() } + + sut.process(event, Hint()) + + assertNotNull(event.user) { + assertNull(it.id) + assertNull(it.ipAddress) + } + } + + @Test + fun `when user info is enabled, sets automatic user data`() { + fixture.options.dataCollection.setUserInfo(true) + val sut = fixture.getSut(context, isSendDefaultPii = false) + val event = SentryEvent().apply { user = User() } + + sut.process(event, Hint()) + + assertNotNull(event.user) { + assertNotNull(it.id) + assertEquals("{{auto}}", it.ipAddress) + } + } + @Test fun `when event has ip address set, keeps original ip address`() { val sut = fixture.getSut(context) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/DeviceInfoUtilTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/DeviceInfoUtilTest.kt index faf993e1610..49c828b551e 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/DeviceInfoUtilTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/DeviceInfoUtilTest.kt @@ -53,6 +53,24 @@ class DeviceInfoUtilTest { assertNotNull(deviceInfo.memorySize) } + @Test + fun `does not set device id when user info is disabled`() { + val options = SentryAndroidOptions().apply { dataCollection.setUserInfo(false) } + val deviceInfo = + DeviceInfoUtil.getInstance(context, options).collectDeviceInformation(false, false) + + assertNull(deviceInfo.id) + } + + @Test + fun `sets device id when user info is enabled`() { + val options = SentryAndroidOptions().apply { dataCollection.setUserInfo(true) } + val deviceInfo = + DeviceInfoUtil.getInstance(context, options).collectDeviceInformation(false, false) + + assertNotNull(deviceInfo.id) + } + @Test fun `sets default timezone`() { val deviceInfoUtil = DeviceInfoUtil.getInstance(context, SentryAndroidOptions()) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/InternalSentrySdkTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/InternalSentrySdkTest.kt index 5917d44d11d..8c552a8b633 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/InternalSentrySdkTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/InternalSentrySdkTest.kt @@ -38,6 +38,7 @@ import java.util.concurrent.atomic.AtomicReference import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFalse import kotlin.test.assertNotEquals import kotlin.test.assertNotNull import kotlin.test.assertNull @@ -326,6 +327,28 @@ class InternalSentrySdkTest { assertTrue((serializedScope["user"] as Map<*, *>).containsKey("id")) } + @Test + fun `serializeScope does not provide fallback user id when user info is disabled`() { + val options = SentryAndroidOptions().apply { dataCollection.setUserInfo(false) } + val scope = Scope(options) + scope.user = null + + val serializedScope = InternalSentrySdk.serializeScope(context, options, scope) + + assertFalse((serializedScope["user"] as Map<*, *>).containsKey("id")) + } + + @Test + fun `serializeScope provides fallback user id when user info is enabled`() { + val options = SentryAndroidOptions().apply { dataCollection.setUserInfo(true) } + val scope = Scope(options) + scope.user = null + + val serializedScope = InternalSentrySdk.serializeScope(context, options, scope) + + assertTrue((serializedScope["user"] as Map<*, *>).containsKey("id")) + } + @Test fun `serializeScope does not override user-id`() { val options = SentryAndroidOptions() diff --git a/sentry-spring-7/src/main/java/io/sentry/spring7/HttpServletRequestSentryUserProvider.java b/sentry-spring-7/src/main/java/io/sentry/spring7/HttpServletRequestSentryUserProvider.java index 54ad7602ae0..44ae584ec17 100644 --- a/sentry-spring-7/src/main/java/io/sentry/spring7/HttpServletRequestSentryUserProvider.java +++ b/sentry-spring-7/src/main/java/io/sentry/spring7/HttpServletRequestSentryUserProvider.java @@ -23,7 +23,7 @@ public HttpServletRequestSentryUserProvider(final @NotNull SentryOptions options @Override public @Nullable User provideUser() { - if (options.isSendDefaultPii()) { + if (options.getDataCollectionResolver().isUserInfo()) { final RequestAttributes requestAttributes = RequestContextHolder.getRequestAttributes(); if (requestAttributes instanceof ServletRequestAttributes) { final ServletRequestAttributes servletRequestAttributes = diff --git a/sentry-spring-7/src/main/java/io/sentry/spring7/SentryUserFilter.java b/sentry-spring-7/src/main/java/io/sentry/spring7/SentryUserFilter.java index b7e226929a5..da9d2f0f77e 100644 --- a/sentry-spring-7/src/main/java/io/sentry/spring7/SentryUserFilter.java +++ b/sentry-spring-7/src/main/java/io/sentry/spring7/SentryUserFilter.java @@ -46,7 +46,7 @@ protected void doFilterInternal( for (final SentryUserProvider provider : sentryUserProviders) { apply(user, provider.provideUser()); } - if (scopes.getOptions().isSendDefaultPii()) { + if (scopes.getOptions().getDataCollectionResolver().isUserInfo()) { if (IpAddressUtils.isDefault(user.getIpAddress())) { // unset {{auto}} as it would set the server's ip address as a user ip address user.setIpAddress(null); diff --git a/sentry-spring-7/src/main/java/io/sentry/spring7/SpringSecuritySentryUserProvider.java b/sentry-spring-7/src/main/java/io/sentry/spring7/SpringSecuritySentryUserProvider.java index 164a43c5bd2..ff3f118898a 100644 --- a/sentry-spring-7/src/main/java/io/sentry/spring7/SpringSecuritySentryUserProvider.java +++ b/sentry-spring-7/src/main/java/io/sentry/spring7/SpringSecuritySentryUserProvider.java @@ -22,7 +22,7 @@ public SpringSecuritySentryUserProvider(final @NotNull SentryOptions options) { @Override public @Nullable User provideUser() { - if (options.isSendDefaultPii()) { + if (options.getDataCollectionResolver().isUserInfo()) { final SecurityContext context = SecurityContextHolder.getContext(); if (context != null && context.getAuthentication() != null) { final User user = new User(); diff --git a/sentry-spring-7/src/test/kotlin/io/sentry/spring7/HttpServletRequestSentryUserProviderTest.kt b/sentry-spring-7/src/test/kotlin/io/sentry/spring7/HttpServletRequestSentryUserProviderTest.kt index 5254270a05c..16bddd0c27e 100644 --- a/sentry-spring-7/src/test/kotlin/io/sentry/spring7/HttpServletRequestSentryUserProviderTest.kt +++ b/sentry-spring-7/src/test/kotlin/io/sentry/spring7/HttpServletRequestSentryUserProviderTest.kt @@ -45,6 +45,43 @@ class HttpServletRequestSentryUserProviderTest { assertEquals("janesmith", result.username) } + @Test + fun `when user info is disabled, does not attach user data`() { + val principal = mock() + whenever(principal.name).thenReturn("janesmith") + val request = MockHttpServletRequest() + request.userPrincipal = principal + RequestContextHolder.setRequestAttributes(ServletRequestAttributes(request)) + + val options = + SentryOptions().apply { + isSendDefaultPii = true + dataCollection.setUserInfo(false) + } + val result = HttpServletRequestSentryUserProvider(options).provideUser() + + assertNull(result) + } + + @Test + fun `when user info is enabled, attaches user data`() { + val principal = mock() + whenever(principal.name).thenReturn("janesmith") + val request = MockHttpServletRequest() + request.userPrincipal = principal + RequestContextHolder.setRequestAttributes(ServletRequestAttributes(request)) + + val options = + SentryOptions().apply { + isSendDefaultPii = false + dataCollection.setUserInfo(true) + } + val result = HttpServletRequestSentryUserProvider(options).provideUser() + + assertNotNull(result) + assertEquals("janesmith", result.username) + } + @Test fun `when sendDefaultPii is set to false, does not attach user data Sentry Event`() { val principal = mock() diff --git a/sentry-spring-7/src/test/kotlin/io/sentry/spring7/SentryUserFilterTest.kt b/sentry-spring-7/src/test/kotlin/io/sentry/spring7/SentryUserFilterTest.kt index 6284e8241ae..92327456e13 100644 --- a/sentry-spring-7/src/test/kotlin/io/sentry/spring7/SentryUserFilterTest.kt +++ b/sentry-spring-7/src/test/kotlin/io/sentry/spring7/SentryUserFilterTest.kt @@ -23,9 +23,14 @@ class SentryUserFilterTest { fun getSut( isSendDefaultPii: Boolean = false, + userInfo: Boolean? = null, userProviders: List, ): SentryUserFilter { - val options = SentryOptions().apply { this.isSendDefaultPii = isSendDefaultPii } + val options = + SentryOptions().apply { + this.isSendDefaultPii = isSendDefaultPii + userInfo?.let { dataCollection.setUserInfo(it) } + } whenever(scopes.options).thenReturn(options) return SentryUserFilter(scopes, userProviders) } @@ -76,7 +81,7 @@ class SentryUserFilterTest { } @Test - fun `merges user#others with existing user#others set on SentryEvent`() { + fun `merges user#data with existing user#data set on SentryEvent`() { val filter = fixture.getSut( userProviders = @@ -118,6 +123,34 @@ class SentryUserFilterTest { verify(fixture.scopes).setUser(check { assertNull(it.ipAddress) }) } + @Test + fun `when user info is disabled, preserves auto ip from a custom provider`() { + val filter = + fixture.getSut( + isSendDefaultPii = true, + userInfo = false, + userProviders = listOf(SentryUserProvider { User().apply { ipAddress = "{{auto}}" } }), + ) + + filter.doFilter(fixture.request, fixture.response, fixture.chain) + + verify(fixture.scopes).setUser(check { assertEquals("{{auto}}", it.ipAddress) }) + } + + @Test + fun `when user info is enabled, removes auto ip from a custom provider`() { + val filter = + fixture.getSut( + isSendDefaultPii = false, + userInfo = true, + userProviders = listOf(SentryUserProvider { User().apply { ipAddress = "{{auto}}" } }), + ) + + filter.doFilter(fixture.request, fixture.response, fixture.chain) + + verify(fixture.scopes).setUser(check { assertNull(it.ipAddress) }) + } + private fun assertEquals(user1: User, user2: User) { assertEquals(user1.username, user2.username) assertEquals(user1.id, user2.id) diff --git a/sentry-spring-7/src/test/kotlin/io/sentry/spring7/SpringSecuritySentryUserProviderTest.kt b/sentry-spring-7/src/test/kotlin/io/sentry/spring7/SpringSecuritySentryUserProviderTest.kt index 6330405999c..ca931ce3b8f 100644 --- a/sentry-spring-7/src/test/kotlin/io/sentry/spring7/SpringSecuritySentryUserProviderTest.kt +++ b/sentry-spring-7/src/test/kotlin/io/sentry/spring7/SpringSecuritySentryUserProviderTest.kt @@ -16,8 +16,13 @@ class SpringSecuritySentryUserProviderTest { fun getSut( isSendDefaultPii: Boolean = true, username: String? = null, + userInfo: Boolean? = null, ): SpringSecuritySentryUserProvider { - val options = SentryOptions().apply { this.isSendDefaultPii = isSendDefaultPii } + val options = + SentryOptions().apply { + this.isSendDefaultPii = isSendDefaultPii + userInfo?.let { dataCollection.setUserInfo(it) } + } val securityContext = mock() if (username != null) { val authentication = mock() @@ -47,6 +52,20 @@ class SpringSecuritySentryUserProviderTest { assertNull(user) } + @Test + fun `when user info is disabled, returns null even if sendDefaultPii is true`() { + val provider = fixture.getSut(isSendDefaultPii = true, username = "name", userInfo = false) + + assertNull(provider.provideUser()) + } + + @Test + fun `when user info is enabled, returns user even if sendDefaultPii is false`() { + val provider = fixture.getSut(isSendDefaultPii = false, username = "name", userInfo = true) + + assertNotNull(provider.provideUser()) { assertEquals("name", it.username) } + } + @Test fun `when send default pii is set to true and security context is not set, returns null`() { val provider = fixture.getSut(true) diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/HttpServletRequestSentryUserProvider.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/HttpServletRequestSentryUserProvider.java index 6174da0dc5f..b7f4646b4a8 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/HttpServletRequestSentryUserProvider.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/HttpServletRequestSentryUserProvider.java @@ -23,7 +23,7 @@ public HttpServletRequestSentryUserProvider(final @NotNull SentryOptions options @Override public @Nullable User provideUser() { - if (options.isSendDefaultPii()) { + if (options.getDataCollectionResolver().isUserInfo()) { final RequestAttributes requestAttributes = RequestContextHolder.getRequestAttributes(); if (requestAttributes instanceof ServletRequestAttributes) { final ServletRequestAttributes servletRequestAttributes = diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryUserFilter.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryUserFilter.java index 31cc73a3468..23a77f79f0d 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryUserFilter.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryUserFilter.java @@ -46,7 +46,7 @@ protected void doFilterInternal( for (final SentryUserProvider provider : sentryUserProviders) { apply(user, provider.provideUser()); } - if (scopes.getOptions().isSendDefaultPii()) { + if (scopes.getOptions().getDataCollectionResolver().isUserInfo()) { if (IpAddressUtils.isDefault(user.getIpAddress())) { // unset {{auto}} as it would set the server's ip address as a user ip address user.setIpAddress(null); diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SpringSecuritySentryUserProvider.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SpringSecuritySentryUserProvider.java index d36bc4bf2b0..c3f55166c30 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SpringSecuritySentryUserProvider.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SpringSecuritySentryUserProvider.java @@ -22,7 +22,7 @@ public SpringSecuritySentryUserProvider(final @NotNull SentryOptions options) { @Override public @Nullable User provideUser() { - if (options.isSendDefaultPii()) { + if (options.getDataCollectionResolver().isUserInfo()) { final SecurityContext context = SecurityContextHolder.getContext(); if (context != null && context.getAuthentication() != null) { final User user = new User(); diff --git a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/HttpServletRequestSentryUserProviderTest.kt b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/HttpServletRequestSentryUserProviderTest.kt index f2cce25574d..f3cf07525a2 100644 --- a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/HttpServletRequestSentryUserProviderTest.kt +++ b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/HttpServletRequestSentryUserProviderTest.kt @@ -45,6 +45,43 @@ class HttpServletRequestSentryUserProviderTest { assertEquals("janesmith", result.username) } + @Test + fun `when user info is disabled, does not attach user data`() { + val principal = mock() + whenever(principal.name).thenReturn("janesmith") + val request = MockHttpServletRequest() + request.userPrincipal = principal + RequestContextHolder.setRequestAttributes(ServletRequestAttributes(request)) + + val options = + SentryOptions().apply { + isSendDefaultPii = true + dataCollection.setUserInfo(false) + } + val result = HttpServletRequestSentryUserProvider(options).provideUser() + + assertNull(result) + } + + @Test + fun `when user info is enabled, attaches user data`() { + val principal = mock() + whenever(principal.name).thenReturn("janesmith") + val request = MockHttpServletRequest() + request.userPrincipal = principal + RequestContextHolder.setRequestAttributes(ServletRequestAttributes(request)) + + val options = + SentryOptions().apply { + isSendDefaultPii = false + dataCollection.setUserInfo(true) + } + val result = HttpServletRequestSentryUserProvider(options).provideUser() + + assertNotNull(result) + assertEquals("janesmith", result.username) + } + @Test fun `when sendDefaultPii is set to false, does not attach user data Sentry Event`() { val principal = mock() diff --git a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/SentryUserFilterTest.kt b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/SentryUserFilterTest.kt index c790f3e9997..15a7bf377cd 100644 --- a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/SentryUserFilterTest.kt +++ b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/SentryUserFilterTest.kt @@ -23,9 +23,14 @@ class SentryUserFilterTest { fun getSut( isSendDefaultPii: Boolean = false, + userInfo: Boolean? = null, userProviders: List, ): SentryUserFilter { - val options = SentryOptions().apply { this.isSendDefaultPii = isSendDefaultPii } + val options = + SentryOptions().apply { + this.isSendDefaultPii = isSendDefaultPii + userInfo?.let { dataCollection.setUserInfo(it) } + } whenever(scopes.options).thenReturn(options) return SentryUserFilter(scopes, userProviders) } @@ -118,6 +123,34 @@ class SentryUserFilterTest { verify(fixture.scopes).setUser(check { assertNull(it.ipAddress) }) } + @Test + fun `when user info is disabled, preserves auto ip from a custom provider`() { + val filter = + fixture.getSut( + isSendDefaultPii = true, + userInfo = false, + userProviders = listOf(SentryUserProvider { User().apply { ipAddress = "{{auto}}" } }), + ) + + filter.doFilter(fixture.request, fixture.response, fixture.chain) + + verify(fixture.scopes).setUser(check { assertEquals("{{auto}}", it.ipAddress) }) + } + + @Test + fun `when user info is enabled, removes auto ip from a custom provider`() { + val filter = + fixture.getSut( + isSendDefaultPii = false, + userInfo = true, + userProviders = listOf(SentryUserProvider { User().apply { ipAddress = "{{auto}}" } }), + ) + + filter.doFilter(fixture.request, fixture.response, fixture.chain) + + verify(fixture.scopes).setUser(check { assertNull(it.ipAddress) }) + } + private fun assertEquals(user1: User, user2: User) { assertEquals(user1.username, user2.username) assertEquals(user1.id, user2.id) diff --git a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/SpringSecuritySentryUserProviderTest.kt b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/SpringSecuritySentryUserProviderTest.kt index 80f8efc9ce2..8bd503c3180 100644 --- a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/SpringSecuritySentryUserProviderTest.kt +++ b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/SpringSecuritySentryUserProviderTest.kt @@ -16,8 +16,13 @@ class SpringSecuritySentryUserProviderTest { fun getSut( isSendDefaultPii: Boolean = true, username: String? = null, + userInfo: Boolean? = null, ): SpringSecuritySentryUserProvider { - val options = SentryOptions().apply { this.isSendDefaultPii = isSendDefaultPii } + val options = + SentryOptions().apply { + this.isSendDefaultPii = isSendDefaultPii + userInfo?.let { dataCollection.setUserInfo(it) } + } val securityContext = mock() if (username != null) { val authentication = mock() @@ -47,6 +52,20 @@ class SpringSecuritySentryUserProviderTest { assertNull(user) } + @Test + fun `when user info is disabled, returns null even if sendDefaultPii is true`() { + val provider = fixture.getSut(isSendDefaultPii = true, username = "name", userInfo = false) + + assertNull(provider.provideUser()) + } + + @Test + fun `when user info is enabled, returns user even if sendDefaultPii is false`() { + val provider = fixture.getSut(isSendDefaultPii = false, username = "name", userInfo = true) + + assertNotNull(provider.provideUser()) { assertEquals("name", it.username) } + } + @Test fun `when send default pii is set to true and security context is not set, returns null`() { val provider = fixture.getSut(true) diff --git a/sentry-spring/src/main/java/io/sentry/spring/HttpServletRequestSentryUserProvider.java b/sentry-spring/src/main/java/io/sentry/spring/HttpServletRequestSentryUserProvider.java index c24d2c2ff10..9951e6961dd 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/HttpServletRequestSentryUserProvider.java +++ b/sentry-spring/src/main/java/io/sentry/spring/HttpServletRequestSentryUserProvider.java @@ -23,7 +23,7 @@ public HttpServletRequestSentryUserProvider(final @NotNull SentryOptions options @Override public @Nullable User provideUser() { - if (options.isSendDefaultPii()) { + if (options.getDataCollectionResolver().isUserInfo()) { final RequestAttributes requestAttributes = RequestContextHolder.getRequestAttributes(); if (requestAttributes instanceof ServletRequestAttributes) { final ServletRequestAttributes servletRequestAttributes = diff --git a/sentry-spring/src/main/java/io/sentry/spring/SentryUserFilter.java b/sentry-spring/src/main/java/io/sentry/spring/SentryUserFilter.java index e0b4e9c1ba8..18e1c0d2875 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/SentryUserFilter.java +++ b/sentry-spring/src/main/java/io/sentry/spring/SentryUserFilter.java @@ -46,7 +46,7 @@ protected void doFilterInternal( for (final SentryUserProvider provider : sentryUserProviders) { apply(user, provider.provideUser()); } - if (scopes.getOptions().isSendDefaultPii()) { + if (scopes.getOptions().getDataCollectionResolver().isUserInfo()) { if (IpAddressUtils.isDefault(user.getIpAddress())) { // unset {{auto}} as it would set the server's ip address as a user ip address user.setIpAddress(null); diff --git a/sentry-spring/src/main/java/io/sentry/spring/SpringSecuritySentryUserProvider.java b/sentry-spring/src/main/java/io/sentry/spring/SpringSecuritySentryUserProvider.java index 23b7820ad94..ef361b0b1f6 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/SpringSecuritySentryUserProvider.java +++ b/sentry-spring/src/main/java/io/sentry/spring/SpringSecuritySentryUserProvider.java @@ -22,7 +22,7 @@ public SpringSecuritySentryUserProvider(final @NotNull SentryOptions options) { @Override public @Nullable User provideUser() { - if (options.isSendDefaultPii()) { + if (options.getDataCollectionResolver().isUserInfo()) { final SecurityContext context = SecurityContextHolder.getContext(); if (context != null && context.getAuthentication() != null) { final User user = new User(); diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/HttpServletRequestSentryUserProviderTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/HttpServletRequestSentryUserProviderTest.kt index 46027a1c09f..3f3cc08bdcd 100644 --- a/sentry-spring/src/test/kotlin/io/sentry/spring/HttpServletRequestSentryUserProviderTest.kt +++ b/sentry-spring/src/test/kotlin/io/sentry/spring/HttpServletRequestSentryUserProviderTest.kt @@ -45,6 +45,43 @@ class HttpServletRequestSentryUserProviderTest { assertEquals("janesmith", result.username) } + @Test + fun `when user info is disabled, does not attach user data`() { + val principal = mock() + whenever(principal.name).thenReturn("janesmith") + val request = MockHttpServletRequest() + request.userPrincipal = principal + RequestContextHolder.setRequestAttributes(ServletRequestAttributes(request)) + + val options = + SentryOptions().apply { + isSendDefaultPii = true + dataCollection.setUserInfo(false) + } + val result = HttpServletRequestSentryUserProvider(options).provideUser() + + assertNull(result) + } + + @Test + fun `when user info is enabled, attaches user data`() { + val principal = mock() + whenever(principal.name).thenReturn("janesmith") + val request = MockHttpServletRequest() + request.userPrincipal = principal + RequestContextHolder.setRequestAttributes(ServletRequestAttributes(request)) + + val options = + SentryOptions().apply { + isSendDefaultPii = false + dataCollection.setUserInfo(true) + } + val result = HttpServletRequestSentryUserProvider(options).provideUser() + + assertNotNull(result) + assertEquals("janesmith", result.username) + } + @Test fun `when sendDefaultPii is set to false, does not attach user data Sentry Event`() { val principal = mock() diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/SentryUserFilterTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/SentryUserFilterTest.kt index f545e605560..07283bd5b95 100644 --- a/sentry-spring/src/test/kotlin/io/sentry/spring/SentryUserFilterTest.kt +++ b/sentry-spring/src/test/kotlin/io/sentry/spring/SentryUserFilterTest.kt @@ -23,9 +23,14 @@ class SentryUserFilterTest { fun getSut( isSendDefaultPii: Boolean = false, + userInfo: Boolean? = null, userProviders: List, ): SentryUserFilter { - val options = SentryOptions().apply { this.isSendDefaultPii = isSendDefaultPii } + val options = + SentryOptions().apply { + this.isSendDefaultPii = isSendDefaultPii + userInfo?.let { dataCollection.setUserInfo(it) } + } whenever(scopes.options).thenReturn(options) return SentryUserFilter(scopes, userProviders) } @@ -118,6 +123,34 @@ class SentryUserFilterTest { verify(fixture.scopes).setUser(check { assertNull(it.ipAddress) }) } + @Test + fun `when user info is disabled, preserves auto ip from a custom provider`() { + val filter = + fixture.getSut( + isSendDefaultPii = true, + userInfo = false, + userProviders = listOf(SentryUserProvider { User().apply { ipAddress = "{{auto}}" } }), + ) + + filter.doFilter(fixture.request, fixture.response, fixture.chain) + + verify(fixture.scopes).setUser(check { assertEquals("{{auto}}", it.ipAddress) }) + } + + @Test + fun `when user info is enabled, removes auto ip from a custom provider`() { + val filter = + fixture.getSut( + isSendDefaultPii = false, + userInfo = true, + userProviders = listOf(SentryUserProvider { User().apply { ipAddress = "{{auto}}" } }), + ) + + filter.doFilter(fixture.request, fixture.response, fixture.chain) + + verify(fixture.scopes).setUser(check { assertNull(it.ipAddress) }) + } + private fun assertEquals(user1: User, user2: User) { assertEquals(user1.username, user2.username) assertEquals(user1.id, user2.id) diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/SpringSecuritySentryUserProviderTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/SpringSecuritySentryUserProviderTest.kt index 3fa443658d3..7ba3ea787e9 100644 --- a/sentry-spring/src/test/kotlin/io/sentry/spring/SpringSecuritySentryUserProviderTest.kt +++ b/sentry-spring/src/test/kotlin/io/sentry/spring/SpringSecuritySentryUserProviderTest.kt @@ -16,8 +16,13 @@ class SpringSecuritySentryUserProviderTest { fun getSut( isSendDefaultPii: Boolean = true, username: String? = null, + userInfo: Boolean? = null, ): SpringSecuritySentryUserProvider { - val options = SentryOptions().apply { this.isSendDefaultPii = isSendDefaultPii } + val options = + SentryOptions().apply { + this.isSendDefaultPii = isSendDefaultPii + userInfo?.let { dataCollection.setUserInfo(it) } + } val securityContext = mock() if (username != null) { val authentication = mock() @@ -47,6 +52,20 @@ class SpringSecuritySentryUserProviderTest { assertNull(user) } + @Test + fun `when user info is disabled, returns null even if sendDefaultPii is true`() { + val provider = fixture.getSut(isSendDefaultPii = true, username = "name", userInfo = false) + + assertNull(provider.provideUser()) + } + + @Test + fun `when user info is enabled, returns user even if sendDefaultPii is false`() { + val provider = fixture.getSut(isSendDefaultPii = false, username = "name", userInfo = true) + + assertNotNull(provider.provideUser()) { assertEquals("name", it.username) } + } + @Test fun `when send default pii is set to true and security context is not set, returns null`() { val provider = fixture.getSut(true) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 30f28e17d66..55d5a34e3ba 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -436,6 +436,7 @@ public final class io/sentry/DataCollectionResolver { public fun isOutgoingResponseBody ()Z public fun isOutgoingResponseBodyWithLegacyBodyGate ()Z public fun isUserInfo ()Z + public fun isUserInfoWithLegacyAlways ()Z } public final class io/sentry/DateUtils { diff --git a/sentry/src/main/java/io/sentry/DataCollectionResolver.java b/sentry/src/main/java/io/sentry/DataCollectionResolver.java index 16d27b68781..78da42e8e1d 100644 --- a/sentry/src/main/java/io/sentry/DataCollectionResolver.java +++ b/sentry/src/main/java/io/sentry/DataCollectionResolver.java @@ -27,6 +27,10 @@ public boolean isUserInfo() { return explicitOrSendDefaultPii(options.getDataCollection().getUserInfo(), true); } + public boolean isUserInfoWithLegacyAlways() { + return explicitOrDefault(options.getDataCollection().getUserInfo(), true, true); + } + public boolean isDatabaseQueryData() { return explicitOrSendDefaultPii(options.getDataCollection().getDatabaseQueryData(), true); } diff --git a/sentry/src/main/java/io/sentry/MainEventProcessor.java b/sentry/src/main/java/io/sentry/MainEventProcessor.java index d84c9e47be8..d72783cfe9c 100644 --- a/sentry/src/main/java/io/sentry/MainEventProcessor.java +++ b/sentry/src/main/java/io/sentry/MainEventProcessor.java @@ -206,7 +206,7 @@ private void mergeUser(final @NotNull SentryBaseEvent event) { user = new User(); event.setUser(user); } - if (user.getIpAddress() == null && options.isSendDefaultPii()) { + if (user.getIpAddress() == null && options.getDataCollectionResolver().isUserInfo()) { user.setIpAddress(IpAddressUtils.DEFAULT_IP_ADDRESS); } } diff --git a/sentry/src/main/java/io/sentry/TraceContext.java b/sentry/src/main/java/io/sentry/TraceContext.java index b10954f5285..1bb5508f85b 100644 --- a/sentry/src/main/java/io/sentry/TraceContext.java +++ b/sentry/src/main/java/io/sentry/TraceContext.java @@ -1,7 +1,6 @@ package io.sentry; import io.sentry.protocol.SentryId; -import io.sentry.protocol.User; import io.sentry.vendor.gson.stream.JsonToken; import java.io.IOException; import java.util.Map; @@ -81,16 +80,6 @@ public final class TraceContext implements JsonUnknown, JsonSerializable { this.sampleRand = sampleRand; } - @SuppressWarnings("UnusedMethod") - private static @Nullable String getUserId( - final @NotNull SentryOptions options, final @Nullable User user) { - if (options.isSendDefaultPii() && user != null) { - return user.getId(); - } - - return null; - } - public @NotNull SentryId getTraceId() { return traceId; } diff --git a/sentry/src/test/java/io/sentry/DataCollectionResolverTest.kt b/sentry/src/test/java/io/sentry/DataCollectionResolverTest.kt index d84df7ae6a7..89823d6c3ff 100644 --- a/sentry/src/test/java/io/sentry/DataCollectionResolverTest.kt +++ b/sentry/src/test/java/io/sentry/DataCollectionResolverTest.kt @@ -55,6 +55,17 @@ class DataCollectionResolverTest { assertThat(options.dataCollectionResolver.isUserInfo).isTrue() } + @Test + fun `user info legacy always variant preserves collection when namespace is absent`() { + val options = SentryOptions().apply { isSendDefaultPii = false } + + assertThat(options.dataCollectionResolver.isUserInfoWithLegacyAlways).isTrue() + + options.dataCollection.setUserInfo(false) + + assertThat(options.dataCollectionResolver.isUserInfoWithLegacyAlways).isFalse() + } + @Test fun `omitted booleans use data collection defaults once namespace is explicit`() { val options = SentryOptions().apply { isSendDefaultPii = false } diff --git a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt index fe5c835c90f..643b850e86e 100644 --- a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt +++ b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt @@ -1,5 +1,6 @@ package io.sentry +import com.google.common.truth.Truth.assertThat import io.sentry.hints.AbnormalExit import io.sentry.hints.ApplyScopeData import io.sentry.protocol.DebugMeta @@ -321,6 +322,39 @@ class MainEventProcessorTest { assertNotNull(event.user) { assertNull(it.ipAddress) } } + @Test + fun `when user info is disabled, do not enrich ip address if sendDefaultPii is true`() { + fixture.sentryOptions.dataCollection.setUserInfo(false) + val sut = fixture.getSut(sendDefaultPii = true) + val event = SentryEvent() + + sut.process(event, Hint()) + + assertThat(event.user?.ipAddress).isNull() + } + + @Test + fun `when user info is enabled, enrich ip address if sendDefaultPii is false`() { + fixture.sentryOptions.dataCollection.setUserInfo(true) + val sut = fixture.getSut(sendDefaultPii = false) + val event = SentryEvent() + + sut.process(event, Hint()) + + assertThat(event.user?.ipAddress).isEqualTo("{{auto}}") + } + + @Test + fun `when another data collection setting is configured, omitted user info uses its default`() { + fixture.sentryOptions.dataCollection.cookies = KeyValueCollectionBehavior.off() + val sut = fixture.getSut(sendDefaultPii = false) + val event = SentryEvent() + + sut.process(event, Hint()) + + assertThat(event.user?.ipAddress).isEqualTo("{{auto}}") + } + @Test fun `when event has ip address set, keeps original ip address`() { val sut = fixture.getSut(sendDefaultPii = true)