Skip to content

Commit

Permalink
Feat: Send default pii (#360)
Browse files Browse the repository at this point in the history
* Feature: Send Default Pii

* Changelog

* Fix test

* Fix lint

* Update CHANGELOG.md

* Sync default pii to Android

* Sync default pii to iOS and bump Sentry Cocoa

* Remove event processor

* Only send non null field of user

* Fix tests

* PR feedback

* Improve tests with fixture

* Improve TestFixture

* changelog
  • Loading branch information
ueman authored Mar 16, 2021
1 parent 4c92c45 commit ae21d7e
Show file tree
Hide file tree
Showing 10 changed files with 140 additions and 10 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
* Fix: event.origin and event.environment tags have wrong value for iOS (#365)
* Fix: Fix deprecated `registrar.messenger` call in `SentryFlutterWeb` (#364)
* Fix: Enable breadcrumb recording mechanism based on platform (#366)
* Feat: Send default PII options (#360)
* Bump: sentry-cocoa to v6.2.1 (#360)

## Breaking Changes:

Expand Down
10 changes: 5 additions & 5 deletions dart/lib/src/protocol/sentry_user.dart
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,11 @@ class SentryUser {
/// Produces a [Map] that can be serialized to JSON.
Map<String, dynamic> toJson() {
return <String, dynamic>{
'id': id,
'username': username,
'email': email,
'ip_address': ipAddress,
'extras': extras,
if (id != null) 'id': id,
if (username != null) 'username': username,
if (email != null) 'email': email,
if (ipAddress != null) 'ip_address': ipAddress,
if (extras?.isNotEmpty ?? false) 'extras': extras,
};
}

Expand Down
29 changes: 27 additions & 2 deletions dart/lib/src/sentry_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ import 'transport/http_transport.dart';
import 'transport/noop_transport.dart';
import 'version.dart';

/// Default value for [User.ipAddress]. It gets set when an event does not have
/// a user and IP address. Only applies if [SentryOptions.sendDefaultPii] is set
/// to true.
const _defaultIpAddress = '{{auto}}';

/// Logs crash reports and events to the Sentry.io service.
class SentryClient {
final SentryOptions _options;
Expand Down Expand Up @@ -80,9 +85,10 @@ class SentryClient {
return _sentryId;
}

if (_options.beforeSend != null) {
final beforeSend = _options.beforeSend;
if (beforeSend != null) {
try {
preparedEvent = _options.beforeSend!(preparedEvent, hint: hint);
preparedEvent = beforeSend(preparedEvent, hint: hint);
} catch (err) {
_options.logger(
SentryLevel.error,
Expand Down Expand Up @@ -111,6 +117,8 @@ class SentryClient {
platform: event.platform ?? sdkPlatform,
);

event = _applyDefaultPii(event);

if (event.exception != null) return event;

if (event.throwableMechanism != null) {
Expand All @@ -132,6 +140,23 @@ class SentryClient {
return event;
}

/// This modifies the users IP address according
/// to [SentryOptions.sendDefaultPii].
SentryEvent _applyDefaultPii(SentryEvent event) {
if (!_options.sendDefaultPii) {
return event;
}
var user = event.user;
if (user == null) {
user = SentryUser(ipAddress: _defaultIpAddress);
return event.copyWith(user: user);
} else if (event.user?.ipAddress == null) {
return event.copyWith(user: user.copyWith(ipAddress: _defaultIpAddress));
}

return event;
}

/// Reports the [throwable] and optionally its [stackTrace] to Sentry.io.
Future<SentryId> captureException(
dynamic throwable, {
Expand Down
5 changes: 3 additions & 2 deletions dart/lib/src/sentry_options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,10 @@ class SentryOptions {
/// When enabled, all the threads are automatically attached to all logged events (Android).
bool attachThreads = false;

// TODO: Scope observers, enableScopeSync
/// Whether to send personal identifiable information along with events
bool sendDefaultPii = false;

// TODO: sendDefaultPii
// TODO: Scope observers, enableScopeSync

SentryOptions({this.dsn}) {
sdk.addPackage('pub:sentry', sdkVersion);
Expand Down
26 changes: 26 additions & 0 deletions dart/test/protocol/user_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,32 @@ void main() {
expect('ipAddress1', copy.ipAddress);
expect({'key1': 'value1'}, copy.extras);
});

test('toJson only serialises non-null values', () {
var data = SentryUser(
id: 'id',
);

var json = data.toJson();

expect(json.containsKey('id'), true);
expect(json.containsKey('username'), false);
expect(json.containsKey('email'), false);
expect(json.containsKey('ip_address'), false);
expect(json.containsKey('extras'), false);

data = SentryUser(
ipAddress: 'ip',
);

json = data.toJson();

expect(json.containsKey('id'), false);
expect(json.containsKey('username'), false);
expect(json.containsKey('email'), false);
expect(json.containsKey('ip_address'), true);
expect(json.containsKey('extras'), false);
});
}

SentryUser _generate() => SentryUser(
Expand Down
70 changes: 70 additions & 0 deletions dart/test/sentry_client_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,66 @@ void main() {
});
});

group('SentryClient: apply default pii', () {
late Fixture fixture;

setUp(() {
fixture = Fixture();
});

test('sendDefaultPii is disabled', () async {
final transport = MockTransport();
final client = fixture.getSut(false, transport);

await client.captureEvent(fakeEvent);

expect(transport.events.first.user, fakeEvent.user);
});

test('sendDefaultPii is enabled and event has no user', () async {
final transport = MockTransport();
final client = fixture.getSut(true, transport);
var fakeEvent = SentryEvent();

await client.captureEvent(fakeEvent);

expect(transport.events.length, 1);
expect(transport.events.first.user, isNotNull);
expect(transport.events.first.user?.ipAddress, '{{auto}}');
});

test('sendDefaultPii is enabled and event has a user with IP address',
() async {
final transport = MockTransport();
final client = fixture.getSut(true, transport);

await client.captureEvent(fakeEvent);

expect(transport.events.length, 1);
expect(transport.events.first.user, isNotNull);
// fakeEvent has a user which is not null
expect(transport.events.first.user?.ipAddress, fakeEvent.user!.ipAddress);
expect(transport.events.first.user?.id, fakeEvent.user!.id);
expect(transport.events.first.user?.email, fakeEvent.user!.email);
});

test('sendDefaultPii is enabled and event has a user without IP address',
() async {
final transport = MockTransport();
final client = fixture.getSut(true, transport);

final event = fakeEvent.copyWith(user: fakeUser);

await client.captureEvent(event);

expect(transport.events.length, 1);
expect(transport.events.first.user, isNotNull);
expect(transport.events.first.user?.ipAddress, '{{auto}}');
expect(transport.events.first.user?.id, fakeUser.id);
expect(transport.events.first.user?.email, fakeUser.email);
});
});

group('SentryClient sampling', () {
var options = SentryOptions(dsn: fakeDsn);

Expand Down Expand Up @@ -551,3 +611,13 @@ SentryEvent beforeSendCallback(SentryEvent event, {dynamic hint}) {
SentryEvent? eventProcessorDropEvent(SentryEvent event, {dynamic hint}) {
return null;
}

class Fixture {
/// Test Fixture for tests with [SentryOptions.sendDefaultPii]
SentryClient getSut(bool sendDefaultPii, Transport transport) {
var options = SentryOptions(dsn: fakeDsn);
options.sendDefaultPii = sendDefaultPii;
options.transport = transport;
return SentryClient(options);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ class SentryFlutterPlugin : FlutterPlugin, MethodCallHandler {
}
}
args.getIfNotNull<Boolean>("anrEnabled") { options.isAnrEnabled = it }
args.getIfNotNull<Boolean>("sendDefaultPii") { options.isSendDefaultPii = it }

val nativeCrashHandling = (args["enableNativeCrashHandling"] as? Boolean) ?: true
// nativeCrashHandling has priority over anrEnabled
Expand Down
4 changes: 4 additions & 0 deletions flutter/ios/Classes/SwiftSentryFlutterPlugin.swift
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,10 @@ public class SwiftSentryFlutterPlugin: NSObject, FlutterPlugin {
if let maxBreadcrumbs = arguments["maxBreadcrumbs"] as? UInt {
options.maxBreadcrumbs = maxBreadcrumbs
}

if let sendDefaultPii = arguments["sendDefaultPii"] as? Bool {
options.sendDefaultPii = sendDefaultPii
}
}

private func logLevelFrom(diagnosticLevel: String) -> SentryLogLevel {
Expand Down
2 changes: 1 addition & 1 deletion flutter/ios/sentry_flutter.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Sentry SDK for Flutter with support to native through sentry-cocoa.
s.source = { :git => "https://github.com/getsentry/sentry-dart.git",
:tag => s.version.to_s }
s.source_files = 'Classes/**/*'
s.dependency 'Sentry', '~> 6.1.4'
s.dependency 'Sentry', '~> 6.2.1'
s.dependency 'Flutter'
s.platform = :ios, '9.0'

Expand Down
1 change: 1 addition & 0 deletions flutter/lib/src/default_integrations.dart
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ class NativeSdkIntegration extends Integration<SentryFlutterOptions> {
'anrTimeoutIntervalMillis': options.anrTimeoutIntervalMillis,
'enableAutoNativeBreadcrumbs': options.enableAutoNativeBreadcrumbs,
'cacheDirSize': options.cacheDirSize,
'sendDefaultPii': options.sendDefaultPii,
});

options.sdk.addIntegration('nativeSdkIntegration');
Expand Down

0 comments on commit ae21d7e

Please sign in to comment.