fix: auto ip guarded by sendDefaultPii (#2726)

* fix auto ip * fix default IP address guarded by sendDefaultPii * remove null ip assertion, this will be set by default later * update CHANGELOG * fix tests * fix test * Add behavioral changes header * fix spelling * Update sentry_client.dart * format * add more assertions
getsentry · Feb 19, 2025 · f304748 · f304748
1 parent e11265d
commit f304748
Show file tree

Hide file tree

Showing 4 changed files with 133 additions and 13 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,11 @@
 
 ## Unreleased
 
+### Behavioral changes
+
+- ⚠️ Auto IP assignment for `SentryUser` is now guarded by `sendDefaultPii` ([#2726](https://github.com/getsentry/sentry-dart/pull/2726))
+  - If you rely on Sentry automatically processing the IP address of the user, set `options.sendDefaultPii = true` or manually set the IP address of the `SentryUser` to `{{auto}}`
+
 ### Features
 
 - Disable `ScreenshotIntegration`, `WidgetsBindingIntegration` and `SentryWidget` in multi-view apps #2366 ([#2366](https://github.com/getsentry/sentry-dart/pull/2366))

diff --git a/dart/lib/src/sentry_client.dart b/dart/lib/src/sentry_client.dart
@@ -35,6 +35,9 @@ import 'version.dart';
 /// to true.
 const _defaultIpAddress = '{{auto}}';
 
+@visibleForTesting
+String get defaultIpAddress => _defaultIpAddress;
+
 /// Logs crash reports and events to the Sentry.io service.
 class SentryClient {
   final SentryOptions _options;
@@ -304,12 +307,18 @@ class SentryClient {
   }
 
   SentryEvent _createUserOrSetDefaultIpAddress(SentryEvent event) {
-    var user = event.user;
-    if (user == null) {
-      return event.copyWith(user: SentryUser(ipAddress: _defaultIpAddress));
-    } else if (event.user?.ipAddress == null) {
-      return event.copyWith(user: user.copyWith(ipAddress: _defaultIpAddress));
+    final user = event.user;
+    final effectiveIpAddress =
+        user?.ipAddress ?? (_options.sendDefaultPii ? _defaultIpAddress : null);
+
+    if (effectiveIpAddress != null) {
+      final updatedUser = user == null
+          ? SentryUser(ipAddress: effectiveIpAddress)
+          : user.copyWith(ipAddress: effectiveIpAddress);
+
+      return event.copyWith(user: updatedUser);
     }
+
     return event;
   }
 

diff --git a/dart/test/sentry_client_test.dart b/dart/test/sentry_client_test.dart
@@ -1003,16 +1003,17 @@ void main() {
     });
   });
 
-  group('SentryClient: sets user & user ip', () {
+  group('SentryClient: user & user ip', () {
     late Fixture fixture;
 
     setUp(() {
       fixture = Fixture();
     });
 
-    test('event has no user', () async {
+    test('event has no user and sendDefaultPii = true', () async {
       final client = fixture.getSut(sendDefaultPii: true);
       var fakeEvent = SentryEvent();
+      expect(fakeEvent.user, isNull);
 
       await client.captureEvent(fakeEvent);
 
@@ -1021,12 +1022,27 @@ void main() {
 
       expect(fixture.transport.envelopes.length, 1);
       expect(capturedEvent.user, isNotNull);
-      expect(capturedEvent.user?.ipAddress, '{{auto}}');
+      expect(capturedEvent.user?.ipAddress, defaultIpAddress);
+    });
+
+    test('event has no user and sendDefaultPii = false', () async {
+      final client = fixture.getSut(sendDefaultPii: false);
+      var fakeEvent = SentryEvent();
+      expect(fakeEvent.user, isNull);
+
+      await client.captureEvent(fakeEvent);
+
+      final capturedEnvelope = fixture.transport.envelopes.first;
+      final capturedEvent = await eventFromEnvelope(capturedEnvelope);
+
+      expect(fixture.transport.envelopes.length, 1);
+      expect(capturedEvent.user, isNull);
     });
 
     test('event has a user with IP address', () async {
       final client = fixture.getSut(sendDefaultPii: true);
 
+      expect(fakeEvent.user?.ipAddress, isNotNull);
       await client.captureEvent(fakeEvent);
 
       final capturedEnvelope = fixture.transport.envelopes.first;
@@ -1040,10 +1056,31 @@ void main() {
       expect(capturedEvent.user?.email, fakeEvent.user!.email);
     });
 
-    test('event has a user without IP address', () async {
+    test('event has a user without IP address and sendDefaultPii = true',
+        () async {
       final client = fixture.getSut(sendDefaultPii: true);
 
       final event = fakeEvent.copyWith(user: fakeUser);
+      expect(event.user?.ipAddress, isNull);
+
+      await client.captureEvent(event);
+
+      final capturedEnvelope = fixture.transport.envelopes.first;
+      final capturedEvent = await eventFromEnvelope(capturedEnvelope);
+
+      expect(fixture.transport.envelopes.length, 1);
+      expect(capturedEvent.user, isNotNull);
+      expect(capturedEvent.user?.ipAddress, defaultIpAddress);
+      expect(capturedEvent.user?.id, fakeUser.id);
+      expect(capturedEvent.user?.email, fakeUser.email);
+    });
+
+    test('event has a user without IP address and sendDefaultPii = false',
+        () async {
+      final client = fixture.getSut(sendDefaultPii: false);
+
+      final event = fakeEvent.copyWith(user: fakeUser);
+      expect(event.user?.ipAddress, isNull);
 
       await client.captureEvent(event);
 
@@ -1052,7 +1089,7 @@ void main() {
 
       expect(fixture.transport.envelopes.length, 1);
       expect(capturedEvent.user, isNotNull);
-      expect(capturedEvent.user?.ipAddress, '{{auto}}');
+      expect(capturedEvent.user?.ipAddress, isNull);
       expect(capturedEvent.user?.id, fakeUser.id);
       expect(capturedEvent.user?.email, fakeUser.email);
     });

diff --git a/flutter/test/integrations/load_contexts_integration_test.dart b/flutter/test/integrations/load_contexts_integration_test.dart
@@ -3,9 +3,9 @@ library flutter_test;
 
 import 'package:flutter_test/flutter_test.dart';
 import 'package:mockito/mockito.dart';
+import 'package:sentry/src/sentry_tracer.dart';
 import 'package:sentry_flutter/sentry_flutter.dart';
 import 'package:sentry_flutter/src/integrations/load_contexts_integration.dart';
-import 'package:sentry/src/sentry_tracer.dart';
 
 import 'fixture.dart';
 
@@ -86,9 +86,10 @@ void main() {
     });
 
     test(
-        'apply default IP to user during captureEvent after loading context if ip is null',
+        'apply default IP to user during captureEvent after loading context if ip is null and sendDefaultPii is true',
         () async {
       fixture.options.enableScopeSync = true;
+      fixture.options.sendDefaultPii = true;
 
       const expectedIp = '{{auto}}';
       String? actualIp;
@@ -118,9 +119,42 @@ void main() {
     });
 
     test(
-        'apply default IP to user during captureTransaction after loading context if ip is null',
+        'does not apply default IP to user during captureEvent after loading context if ip is null and sendDefaultPii is false',
+        () async {
+      fixture.options.enableScopeSync = true;
+      // sendDefaultPii false is by default
+
+      String? actualIp;
+
+      const expectedId = '1';
+      String? actualId;
+
+      fixture.options.beforeSend = (event, hint) {
+        actualIp = event.user?.ipAddress;
+        actualId = event.user?.id;
+        return event;
+      };
+
+      final options = fixture.options;
+
+      final user = SentryUser(id: expectedId);
+      when(fixture.binding.loadContexts())
+          .thenAnswer((_) async => {'user': user.toJson()});
+
+      final client = SentryClient(options);
+      final event = SentryEvent();
+
+      await client.captureEvent(event);
+
+      expect(actualIp, isNull);
+      expect(actualId, expectedId);
+    });
+
+    test(
+        'apply default IP to user during captureTransaction after loading context if ip is null and sendDefaultPii is true',
         () async {
       fixture.options.enableScopeSync = true;
+      fixture.options.sendDefaultPii = true;
 
       const expectedIp = '{{auto}}';
       String? actualIp;
@@ -151,5 +185,40 @@ void main() {
       expect(actualIp, expectedIp);
       expect(actualId, expectedId);
     });
+
+    test(
+        'does not apply default IP to user during captureTransaction after loading context if ip is null and sendDefaultPii is false',
+        () async {
+      fixture.options.enableScopeSync = true;
+      // sendDefaultPii false is by default
+
+      String? actualIp;
+
+      const expectedId = '1';
+      String? actualId;
+
+      fixture.options.beforeSendTransaction = (transaction) {
+        actualIp = transaction.user?.ipAddress;
+        actualId = transaction.user?.id;
+        return transaction;
+      };
+
+      final options = fixture.options;
+
+      final user = SentryUser(id: expectedId);
+      when(fixture.binding.loadContexts())
+          .thenAnswer((_) async => {'user': user.toJson()});
+
+      final client = SentryClient(options);
+      final tracer =
+          SentryTracer(SentryTransactionContext('name', 'op'), fixture.hub);
+      final transaction = SentryTransaction(tracer);
+
+      // ignore: invalid_use_of_internal_member
+      await client.captureTransaction(transaction);
+
+      expect(actualIp, isNull);
+      expect(actualId, expectedId);
+    });
   });
 }