From a0d6c1cf7c2d1bd651645bc9ebf29a73b20a1aff Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Wed, 22 Jul 2026 09:21:42 +0200 Subject: [PATCH] feat(user): Apply user information collection policy Gate automatic user and device identity enrichment across core, Android, and Spring integrations with the Data Collection policy. Preserve legacy sendDefaultPii and Android installation identity behavior when Data Collection is absent. Co-Authored-By: Claude --- .../ApplicationExitInfoEventProcessor.java | 7 ++- .../core/DefaultAndroidEventProcessor.java | 8 ++- .../sentry/android/core/DeviceInfoUtil.java | 3 +- .../android/core/InternalSentrySdk.java | 3 +- .../ApplicationExitInfoEventProcessorTest.kt | 55 +++++++++++++++++++ .../core/DefaultAndroidEventProcessorTest.kt | 28 ++++++++++ .../sentry/android/core/DeviceInfoUtilTest.kt | 18 ++++++ .../android/core/InternalSentrySdkTest.kt | 23 ++++++++ .../HttpServletRequestSentryUserProvider.java | 2 +- .../io/sentry/spring7/SentryUserFilter.java | 2 +- .../SpringSecuritySentryUserProvider.java | 2 +- ...ttpServletRequestSentryUserProviderTest.kt | 37 +++++++++++++ .../io/sentry/spring7/SentryUserFilterTest.kt | 37 ++++++++++++- .../SpringSecuritySentryUserProviderTest.kt | 21 ++++++- .../HttpServletRequestSentryUserProvider.java | 2 +- .../spring/jakarta/SentryUserFilter.java | 2 +- .../SpringSecuritySentryUserProvider.java | 2 +- ...ttpServletRequestSentryUserProviderTest.kt | 37 +++++++++++++ .../spring/jakarta/SentryUserFilterTest.kt | 35 +++++++++++- .../SpringSecuritySentryUserProviderTest.kt | 21 ++++++- .../HttpServletRequestSentryUserProvider.java | 2 +- .../io/sentry/spring/SentryUserFilter.java | 2 +- .../SpringSecuritySentryUserProvider.java | 2 +- ...ttpServletRequestSentryUserProviderTest.kt | 37 +++++++++++++ .../io/sentry/spring/SentryUserFilterTest.kt | 35 +++++++++++- .../SpringSecuritySentryUserProviderTest.kt | 21 ++++++- sentry/api/sentry.api | 1 + .../io/sentry/DataCollectionResolver.java | 4 ++ .../java/io/sentry/MainEventProcessor.java | 2 +- .../src/main/java/io/sentry/TraceContext.java | 11 ---- .../io/sentry/DataCollectionResolverTest.kt | 11 ++++ .../java/io/sentry/MainEventProcessorTest.kt | 34 ++++++++++++ 32 files changed, 471 insertions(+), 36 deletions(-) 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)