Skip to content

Only send {{auto}} ip-adress if sendDefaultPii is enabled (8.x.x) #4072

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

## Unreleased

### Behavioural Changes

- The user ip-address is now only set to `"{{auto}}"` if sendDefaultPii is enabled ([#4072](https://github.com/getsentry/sentry-java/pull/4072))
- This change gives you control over IP address collection directly on the client

### Fixes

- Do not set the exception group marker when there is a suppressed exception ([#4056](https://github.com/getsentry/sentry-java/pull/4056))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,7 @@ private void mergeUser(final @NotNull SentryBaseEvent event) {
if (user.getId() == null) {
user.setId(getDeviceId());
}
if (user.getIpAddress() == null) {
if (user.getIpAddress() == null && options.isSendDefaultPii()) {
user.setIpAddress(IpAddressUtils.DEFAULT_IP_ADDRESS);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ private void mergeUser(final @NotNull SentryBaseEvent event) {
if (user.getId() == null) {
user.setId(Installation.id(context));
}
if (user.getIpAddress() == null) {
if (user.getIpAddress() == null && options.isSendDefaultPii()) {
user.setIpAddress(IpAddressUtils.DEFAULT_IP_ADDRESS);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,18 +85,20 @@ class AnrV2EventProcessorTest {
lateinit var context: Context
val options = SentryAndroidOptions().apply {
setLogger(NoOpLogger.getInstance())
isSendDefaultPii = true
}

fun getSut(
dir: TemporaryFolder,
currentSdk: Int = Build.VERSION_CODES.LOLLIPOP,
populateScopeCache: Boolean = false,
populateOptionsCache: Boolean = false,
replayErrorSampleRate: Double? = null
replayErrorSampleRate: Double? = null,
isSendDefaultPii: Boolean = true
): AnrV2EventProcessor {
options.cacheDirPath = dir.newFolder().absolutePath
options.environment = "release"
options.isSendDefaultPii = isSendDefaultPii

whenever(buildInfo.sdkInfoVersion).thenReturn(currentSdk)
whenever(buildInfo.isEmulator).thenReturn(true)

Expand Down Expand Up @@ -278,6 +280,7 @@ class AnrV2EventProcessorTest {
// user
assertEquals("bot", processed.user!!.username)
assertEquals("[email protected]", processed.user!!.id)
assertEquals("{{auto}}", processed.user!!.ipAddress)
// trace
assertEquals("ui.load", processed.contexts.trace!!.operation)
// tags
Expand All @@ -304,6 +307,13 @@ class AnrV2EventProcessorTest {
assertEquals("Google Chrome", processed.contexts.browser!!.name)
}

@Test
fun `when backfillable event is enrichable, does not backfill user ip`() {
val hint = HintUtils.createWithTypeCheckHint(BackfillableHint())
val processed = processEvent(hint, isSendDefaultPii = false, populateScopeCache = true)
assertNull(processed.user!!.ipAddress)
}

@Test
fun `when backfillable event is enrichable, backfills serialized options data`() {
val hint = HintUtils.createWithTypeCheckHint(BackfillableHint())
Expand Down Expand Up @@ -617,14 +627,16 @@ class AnrV2EventProcessorTest {
hint: Hint,
populateScopeCache: Boolean = false,
populateOptionsCache: Boolean = false,
isSendDefaultPii: Boolean = true,
configureEvent: SentryEvent.() -> Unit = {}
): SentryEvent {
val original = SentryEvent().apply(configureEvent)

val processor = fixture.getSut(
tmpDir,
populateScopeCache = populateScopeCache,
populateOptionsCache = populateOptionsCache
populateOptionsCache = populateOptionsCache,
isSendDefaultPii = isSendDefaultPii
)
return processor.process(original, hint)!!
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ class DefaultAndroidEventProcessorTest {
lateinit var sentryTracer: SentryTracer

fun getSut(context: Context): DefaultAndroidEventProcessor {
options.isSendDefaultPii = isSendDefaultPii
whenever(scopes.options).thenReturn(options)
sentryTracer = SentryTracer(TransactionContext("", ""), scopes)
return DefaultAndroidEventProcessor(context, buildInfo, options)
Expand Down Expand Up @@ -284,8 +285,20 @@ class DefaultAndroidEventProcessorTest {
}

@Test
fun `when event user data does not have ip address set, sets {{auto}} as the ip address`() {
val sut = fixture.getSut(context)
fun `when event user data does not have ip address set, sets no ip address if sendDefaultPii is false`() {
val sut = fixture.getSut(context, isSendDefaultPii = false)
val event = SentryEvent().apply {
user = User()
}
sut.process(event, Hint())
assertNotNull(event.user) {
assertNull(it.ipAddress)
}
}

@Test
fun `when event user data does not have ip address set, sets {{auto}} if sendDefaultPii is true`() {
val sut = fixture.getSut(context, isSendDefaultPii = true)
val event = SentryEvent().apply {
user = User()
}
Expand Down
2 changes: 1 addition & 1 deletion sentry/src/main/java/io/sentry/MainEventProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ private void mergeUser(final @NotNull SentryBaseEvent event) {
user = new User();
event.setUser(user);
}
if (user.getIpAddress() == null) {
if (user.getIpAddress() == null && options.isSendDefaultPii()) {
user.setIpAddress(IpAddressUtils.DEFAULT_IP_ADDRESS);
}
}
Expand Down
10 changes: 10 additions & 0 deletions sentry/src/test/java/io/sentry/MainEventProcessorTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,16 @@ class MainEventProcessorTest {
}
}

@Test
fun `when event does not have ip address set, do not enrich ip address if sendDefaultPii is false`() {
val sut = fixture.getSut(sendDefaultPii = false)
val event = SentryEvent()
sut.process(event, Hint())
assertNotNull(event.user) {
assertNull(it.ipAddress)
}
}

@Test
fun `when event has ip address set, keeps original ip address`() {
val sut = fixture.getSut(sendDefaultPii = true)
Expand Down
Loading