diff --git a/CHANGELOG.md b/CHANGELOG.md index ade1671912..c90455ab48 100644 --- 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 index dc59eea643..bef31c1e9f 100644 --- 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 index 170dc9d36a..ede2330686 100644 --- 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 index caed2f3293..33be3e56af 100644 --- 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); + }); }); }