Skip to content

Commit d9d0356

Browse files
authored
Merge pull request #5812 from getsentry/feat/data-collection-user-info
feat(user): [Data Collection 14] Apply user information policy
2 parents bb73d62 + 0800ca5 commit d9d0356

32 files changed

Lines changed: 471 additions & 36 deletions

‎sentry-android-core/src/main/java/io/sentry/android/core/ApplicationExitInfoEventProcessor.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -678,10 +678,10 @@ private void mergeUser(final @NotNull SentryBaseEvent event) {
678678
}
679679

680680
// userId should be set even if event is Cached as the userId is static and won't change anyway.
681-
if (user.getId() == null) {
681+
if (user.getId() == null && options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) {
682682
user.setId(getDeviceId());
683683
}
684-
if (user.getIpAddress() == null && options.isSendDefaultPii()) {
684+
if (user.getIpAddress() == null && options.getDataCollectionResolver().isUserInfo()) {
685685
user.setIpAddress(IpAddressUtils.DEFAULT_IP_ADDRESS);
686686
}
687687
}
@@ -745,7 +745,8 @@ private void setDevice(final @NotNull SentryBaseEvent event) {
745745
device.setScreenDpi(displayMetrics.densityDpi);
746746
}
747747

748-
if (device.getId() == null) {
748+
if (device.getId() == null
749+
&& options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) {
749750
device.setId(getDeviceId());
750751
}
751752

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -175,10 +175,10 @@ private void mergeUser(final @NotNull SentryBaseEvent event) {
175175
}
176176

177177
// userId should be set even if event is Cached as the userId is static and won't change anyway.
178-
if (user.getId() == null) {
178+
if (user.getId() == null && options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) {
179179
user.setId(Installation.id(context));
180180
}
181-
if (user.getIpAddress() == null && options.isSendDefaultPii()) {
181+
if (user.getIpAddress() == null && options.getDataCollectionResolver().isUserInfo()) {
182182
user.setIpAddress(IpAddressUtils.DEFAULT_IP_ADDRESS);
183183
}
184184
}
@@ -374,7 +374,9 @@ private void setAppExtras(final @NotNull App app, final @NotNull Hint hint) {
374374
*/
375375
public @NotNull User getDefaultUser(final @NotNull Context context) {
376376
final @NotNull User user = new User();
377-
user.setId(Installation.id(context));
377+
if (options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) {
378+
user.setId(Installation.id(context));
379+
}
378380
return user;
379381
}
380382

‎sentry-android-core/src/main/java/io/sentry/android/core/DeviceInfoUtil.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,8 @@ public Device collectDeviceInformation(
130130
device.setBootTime(getBootTime());
131131
device.setTimezone(getTimeZone());
132132

133-
if (device.getId() == null) {
133+
if (device.getId() == null
134+
&& options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) {
134135
device.setId(getDeviceId());
135136
}
136137

‎sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,8 @@ public static Map<String, Object> serializeScope(
100100
user = new User();
101101
scope.setUser(user);
102102
}
103-
if (user.getId() == null) {
103+
if (user.getId() == null
104+
&& options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) {
104105
try {
105106
user.setId(Installation.id(context));
106107
} catch (RuntimeException e) {

‎sentry-android-core/src/test/java/io/sentry/android/core/ApplicationExitInfoEventProcessorTest.kt‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,26 @@ class ApplicationExitInfoEventProcessorTest {
233233
assertEquals(SentryBaseEvent.DEFAULT_PLATFORM, processed.platform)
234234
}
235235

236+
@Test
237+
fun `when user info is disabled, does not set device id`() {
238+
fixture.options.dataCollection.setUserInfo(false)
239+
val hint = HintUtils.createWithTypeCheckHint(AbnormalExitHint())
240+
241+
val processed = processEvent(hint)
242+
243+
assertNull(processed.contexts.device!!.id)
244+
}
245+
246+
@Test
247+
fun `when user info is enabled, sets device id`() {
248+
fixture.options.dataCollection.setUserInfo(true)
249+
val hint = HintUtils.createWithTypeCheckHint(AbnormalExitHint())
250+
251+
val processed = processEvent(hint, isSendDefaultPii = false)
252+
253+
assertNotNull(processed.contexts.device!!.id)
254+
}
255+
236256
@Test
237257
fun `when backfillable event is not enrichable, sets OS`() {
238258
val hint = HintUtils.createWithTypeCheckHint(BackfillableHint(shouldEnrich = false))
@@ -356,6 +376,28 @@ class ApplicationExitInfoEventProcessorTest {
356376
assertNull(processed.user!!.ipAddress)
357377
}
358378

379+
@Test
380+
fun `when user info is disabled, does not backfill automatic user data`() {
381+
fixture.options.dataCollection.setUserInfo(false)
382+
val hint = HintUtils.createWithTypeCheckHint(BackfillableHint())
383+
val processed = processEvent(hint, isSendDefaultPii = true, populateScopeCache = true)
384+
385+
assertEquals("bot", processed.user!!.username)
386+
assertEquals("bot@me.com", processed.user!!.id)
387+
assertNull(processed.user!!.ipAddress)
388+
}
389+
390+
@Test
391+
fun `when user info is enabled, backfills automatic user data`() {
392+
fixture.options.dataCollection.setUserInfo(true)
393+
val hint = HintUtils.createWithTypeCheckHint(BackfillableHint())
394+
val processed = processEvent(hint, isSendDefaultPii = false, populateScopeCache = true)
395+
396+
assertEquals("bot", processed.user!!.username)
397+
assertEquals("bot@me.com", processed.user!!.id)
398+
assertEquals("{{auto}}", processed.user!!.ipAddress)
399+
}
400+
359401
@Test
360402
fun `when backfillable event is enrichable, backfills serialized options data`() {
361403
val hint = HintUtils.createWithTypeCheckHint(BackfillableHint())
@@ -640,6 +682,19 @@ class ApplicationExitInfoEventProcessorTest {
640682
assertEquals(Installation.deviceId, processed!!.user!!.id)
641683
}
642684

685+
@Test
686+
fun `when user info is disabled, does not set installation id for missing user id`() {
687+
fixture.options.dataCollection.setUserInfo(false)
688+
val hint = HintUtils.createWithTypeCheckHint(BackfillableHint())
689+
val original = SentryEvent()
690+
val processor = fixture.getSut(tmpDir)
691+
fixture.persistOptions(USER_FILENAME, User())
692+
693+
val processed = processor.process(original, hint)
694+
695+
assertNull(processed!!.user!!.id)
696+
}
697+
643698
@Test
644699
fun `when event has some fields set, does not override them`() {
645700
val hint = HintUtils.createWithTypeCheckHint(BackfillableHint())

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,34 @@ class DefaultAndroidEventProcessorTest {
285285
assertNotNull(event.user) { assertEquals("{{auto}}", it.ipAddress) }
286286
}
287287

288+
@Test
289+
fun `when user info is disabled, does not set automatic user data`() {
290+
fixture.options.dataCollection.setUserInfo(false)
291+
val sut = fixture.getSut(context, isSendDefaultPii = true)
292+
val event = SentryEvent().apply { user = User() }
293+
294+
sut.process(event, Hint())
295+
296+
assertNotNull(event.user) {
297+
assertNull(it.id)
298+
assertNull(it.ipAddress)
299+
}
300+
}
301+
302+
@Test
303+
fun `when user info is enabled, sets automatic user data`() {
304+
fixture.options.dataCollection.setUserInfo(true)
305+
val sut = fixture.getSut(context, isSendDefaultPii = false)
306+
val event = SentryEvent().apply { user = User() }
307+
308+
sut.process(event, Hint())
309+
310+
assertNotNull(event.user) {
311+
assertNotNull(it.id)
312+
assertEquals("{{auto}}", it.ipAddress)
313+
}
314+
}
315+
288316
@Test
289317
fun `when event has ip address set, keeps original ip address`() {
290318
val sut = fixture.getSut(context)

‎sentry-android-core/src/test/java/io/sentry/android/core/DeviceInfoUtilTest.kt‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,24 @@ class DeviceInfoUtilTest {
5353
assertNotNull(deviceInfo.memorySize)
5454
}
5555

56+
@Test
57+
fun `does not set device id when user info is disabled`() {
58+
val options = SentryAndroidOptions().apply { dataCollection.setUserInfo(false) }
59+
val deviceInfo =
60+
DeviceInfoUtil.getInstance(context, options).collectDeviceInformation(false, false)
61+
62+
assertNull(deviceInfo.id)
63+
}
64+
65+
@Test
66+
fun `sets device id when user info is enabled`() {
67+
val options = SentryAndroidOptions().apply { dataCollection.setUserInfo(true) }
68+
val deviceInfo =
69+
DeviceInfoUtil.getInstance(context, options).collectDeviceInformation(false, false)
70+
71+
assertNotNull(deviceInfo.id)
72+
}
73+
5674
@Test
5775
fun `sets default timezone`() {
5876
val deviceInfoUtil = DeviceInfoUtil.getInstance(context, SentryAndroidOptions())

‎sentry-android-core/src/test/java/io/sentry/android/core/InternalSentrySdkTest.kt‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import java.util.concurrent.atomic.AtomicReference
4040
import kotlin.test.BeforeTest
4141
import kotlin.test.Test
4242
import kotlin.test.assertEquals
43+
import kotlin.test.assertFalse
4344
import kotlin.test.assertNotEquals
4445
import kotlin.test.assertNotNull
4546
import kotlin.test.assertNull
@@ -354,6 +355,28 @@ class InternalSentrySdkTest {
354355
assertTrue((serializedScope["user"] as Map<*, *>).containsKey("id"))
355356
}
356357

358+
@Test
359+
fun `serializeScope does not provide fallback user id when user info is disabled`() {
360+
val options = SentryAndroidOptions().apply { dataCollection.setUserInfo(false) }
361+
val scope = Scope(options)
362+
scope.user = null
363+
364+
val serializedScope = InternalSentrySdk.serializeScope(context, options, scope)
365+
366+
assertFalse((serializedScope["user"] as Map<*, *>).containsKey("id"))
367+
}
368+
369+
@Test
370+
fun `serializeScope provides fallback user id when user info is enabled`() {
371+
val options = SentryAndroidOptions().apply { dataCollection.setUserInfo(true) }
372+
val scope = Scope(options)
373+
scope.user = null
374+
375+
val serializedScope = InternalSentrySdk.serializeScope(context, options, scope)
376+
377+
assertTrue((serializedScope["user"] as Map<*, *>).containsKey("id"))
378+
}
379+
357380
@Test
358381
fun `serializeScope does not override user-id`() {
359382
val options = SentryAndroidOptions()

‎sentry-spring-7/src/main/java/io/sentry/spring7/HttpServletRequestSentryUserProvider.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ public HttpServletRequestSentryUserProvider(final @NotNull SentryOptions options
2323

2424
@Override
2525
public @Nullable User provideUser() {
26-
if (options.isSendDefaultPii()) {
26+
if (options.getDataCollectionResolver().isUserInfo()) {
2727
final RequestAttributes requestAttributes = RequestContextHolder.getRequestAttributes();
2828
if (requestAttributes instanceof ServletRequestAttributes) {
2929
final ServletRequestAttributes servletRequestAttributes =

‎sentry-spring-7/src/main/java/io/sentry/spring7/SentryUserFilter.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ protected void doFilterInternal(
4646
for (final SentryUserProvider provider : sentryUserProviders) {
4747
apply(user, provider.provideUser());
4848
}
49-
if (scopes.getOptions().isSendDefaultPii()) {
49+
if (scopes.getOptions().getDataCollectionResolver().isUserInfo()) {
5050
if (IpAddressUtils.isDefault(user.getIpAddress())) {
5151
// unset {{auto}} as it would set the server's ip address as a user ip address
5252
user.setIpAddress(null);

0 commit comments

Comments
 (0)