Skip to content
Draft
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -568,10 +568,10 @@ private void mergeUser(final @NotNull SentryBaseEvent event) {
}

// userId should be set even if event is Cached as the userId is static and won't change anyway.
if (user.getId() == null) {
if (user.getId() == null && options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) {
user.setId(getDeviceId());
}
if (user.getIpAddress() == null && options.isSendDefaultPii()) {
if (user.getIpAddress() == null && options.getDataCollectionResolver().isUserInfo()) {
user.setIpAddress(IpAddressUtils.DEFAULT_IP_ADDRESS);
}
}
Expand Down Expand Up @@ -635,7 +635,8 @@ private void setDevice(final @NotNull SentryBaseEvent event) {
device.setScreenDpi(displayMetrics.densityDpi);
}

if (device.getId() == null) {
if (device.getId() == null
&& options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) {
device.setId(getDeviceId());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,10 +175,10 @@ private void mergeUser(final @NotNull SentryBaseEvent event) {
}

// userId should be set even if event is Cached as the userId is static and won't change anyway.
if (user.getId() == null) {
if (user.getId() == null && options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) {
user.setId(Installation.id(context));
}
if (user.getIpAddress() == null && options.isSendDefaultPii()) {
if (user.getIpAddress() == null && options.getDataCollectionResolver().isUserInfo()) {
user.setIpAddress(IpAddressUtils.DEFAULT_IP_ADDRESS);
}
}
Expand Down Expand Up @@ -374,7 +374,9 @@ private void setAppExtras(final @NotNull App app, final @NotNull Hint hint) {
*/
public @NotNull User getDefaultUser(final @NotNull Context context) {
final @NotNull User user = new User();
user.setId(Installation.id(context));
if (options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) {
user.setId(Installation.id(context));
}
return user;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,8 @@ public Device collectDeviceInformation(
device.setBootTime(getBootTime());
device.setTimezone(getTimeZone());

if (device.getId() == null) {
if (device.getId() == null
&& options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) {
device.setId(getDeviceId());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ public static Map<String, Object> serializeScope(
user = new User();
scope.setUser(user);
}
if (user.getId() == null) {
if (user.getId() == null
&& options.getDataCollectionResolver().isUserInfoWithLegacyAlways()) {
try {
user.setId(Installation.id(context));
} catch (RuntimeException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,26 @@ class ApplicationExitInfoEventProcessorTest {
assertEquals(SentryBaseEvent.DEFAULT_PLATFORM, processed.platform)
}

@Test
fun `when user info is disabled, does not set device id`() {
fixture.options.dataCollection.setUserInfo(false)
val hint = HintUtils.createWithTypeCheckHint(AbnormalExitHint())

val processed = processEvent(hint)

assertNull(processed.contexts.device!!.id)
}

@Test
fun `when user info is enabled, sets device id`() {
fixture.options.dataCollection.setUserInfo(true)
val hint = HintUtils.createWithTypeCheckHint(AbnormalExitHint())

val processed = processEvent(hint, isSendDefaultPii = false)

assertNotNull(processed.contexts.device!!.id)
}

@Test
fun `when backfillable event is not enrichable, sets OS`() {
val hint = HintUtils.createWithTypeCheckHint(BackfillableHint(shouldEnrich = false))
Expand Down Expand Up @@ -336,6 +356,28 @@ class ApplicationExitInfoEventProcessorTest {
assertNull(processed.user!!.ipAddress)
}

@Test
fun `when user info is disabled, does not backfill automatic user data`() {
fixture.options.dataCollection.setUserInfo(false)
val hint = HintUtils.createWithTypeCheckHint(BackfillableHint())
val processed = processEvent(hint, isSendDefaultPii = true, populateScopeCache = true)

assertEquals("bot", processed.user!!.username)
assertEquals("bot@me.com", processed.user!!.id)
assertNull(processed.user!!.ipAddress)
}

@Test
fun `when user info is enabled, backfills automatic user data`() {
fixture.options.dataCollection.setUserInfo(true)
val hint = HintUtils.createWithTypeCheckHint(BackfillableHint())
val processed = processEvent(hint, isSendDefaultPii = false, populateScopeCache = true)

assertEquals("bot", processed.user!!.username)
assertEquals("bot@me.com", processed.user!!.id)
assertEquals("{{auto}}", processed.user!!.ipAddress)
}

@Test
fun `when backfillable event is enrichable, backfills serialized options data`() {
val hint = HintUtils.createWithTypeCheckHint(BackfillableHint())
Expand Down Expand Up @@ -435,6 +477,19 @@ class ApplicationExitInfoEventProcessorTest {
assertEquals(Installation.deviceId, processed!!.user!!.id)
}

@Test
fun `when user info is disabled, does not set installation id for missing user id`() {
fixture.options.dataCollection.setUserInfo(false)
val hint = HintUtils.createWithTypeCheckHint(BackfillableHint())
val original = SentryEvent()
val processor = fixture.getSut(tmpDir)
fixture.persistOptions(USER_FILENAME, User())

val processed = processor.process(original, hint)

assertNull(processed!!.user!!.id)
}

@Test
fun `when event has some fields set, does not override them`() {
val hint = HintUtils.createWithTypeCheckHint(BackfillableHint())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,34 @@ class DefaultAndroidEventProcessorTest {
assertNotNull(event.user) { assertEquals("{{auto}}", it.ipAddress) }
}

@Test
fun `when user info is disabled, does not set automatic user data`() {
fixture.options.dataCollection.setUserInfo(false)
val sut = fixture.getSut(context, isSendDefaultPii = true)
val event = SentryEvent().apply { user = User() }

sut.process(event, Hint())

assertNotNull(event.user) {
assertNull(it.id)
assertNull(it.ipAddress)
}
}

@Test
fun `when user info is enabled, sets automatic user data`() {
fixture.options.dataCollection.setUserInfo(true)
val sut = fixture.getSut(context, isSendDefaultPii = false)
val event = SentryEvent().apply { user = User() }

sut.process(event, Hint())

assertNotNull(event.user) {
assertNotNull(it.id)
assertEquals("{{auto}}", it.ipAddress)
}
}

@Test
fun `when event has ip address set, keeps original ip address`() {
val sut = fixture.getSut(context)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,24 @@ class DeviceInfoUtilTest {
assertNotNull(deviceInfo.memorySize)
}

@Test
fun `does not set device id when user info is disabled`() {
val options = SentryAndroidOptions().apply { dataCollection.setUserInfo(false) }
val deviceInfo =
DeviceInfoUtil.getInstance(context, options).collectDeviceInformation(false, false)

assertNull(deviceInfo.id)
}

@Test
fun `sets device id when user info is enabled`() {
val options = SentryAndroidOptions().apply { dataCollection.setUserInfo(true) }
val deviceInfo =
DeviceInfoUtil.getInstance(context, options).collectDeviceInformation(false, false)

assertNotNull(deviceInfo.id)
}

@Test
fun `sets default timezone`() {
val deviceInfoUtil = DeviceInfoUtil.getInstance(context, SentryAndroidOptions())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import java.util.concurrent.atomic.AtomicReference
import kotlin.test.BeforeTest
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertNotEquals
import kotlin.test.assertNotNull
import kotlin.test.assertNull
Expand Down Expand Up @@ -326,6 +327,28 @@ class InternalSentrySdkTest {
assertTrue((serializedScope["user"] as Map<*, *>).containsKey("id"))
}

@Test
fun `serializeScope does not provide fallback user id when user info is disabled`() {
val options = SentryAndroidOptions().apply { dataCollection.setUserInfo(false) }
val scope = Scope(options)
scope.user = null

val serializedScope = InternalSentrySdk.serializeScope(context, options, scope)

assertFalse((serializedScope["user"] as Map<*, *>).containsKey("id"))
}

@Test
fun `serializeScope provides fallback user id when user info is enabled`() {
val options = SentryAndroidOptions().apply { dataCollection.setUserInfo(true) }
val scope = Scope(options)
scope.user = null

val serializedScope = InternalSentrySdk.serializeScope(context, options, scope)

assertTrue((serializedScope["user"] as Map<*, *>).containsKey("id"))
}

@Test
fun `serializeScope does not override user-id`() {
val options = SentryAndroidOptions()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public HttpServletRequestSentryUserProvider(final @NotNull SentryOptions options

@Override
public @Nullable User provideUser() {
if (options.isSendDefaultPii()) {
if (options.getDataCollectionResolver().isUserInfo()) {
final RequestAttributes requestAttributes = RequestContextHolder.getRequestAttributes();
if (requestAttributes instanceof ServletRequestAttributes) {
final ServletRequestAttributes servletRequestAttributes =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ protected void doFilterInternal(
for (final SentryUserProvider provider : sentryUserProviders) {
apply(user, provider.provideUser());
}
if (scopes.getOptions().isSendDefaultPii()) {
if (scopes.getOptions().getDataCollectionResolver().isUserInfo()) {
if (IpAddressUtils.isDefault(user.getIpAddress())) {
// unset {{auto}} as it would set the server's ip address as a user ip address
user.setIpAddress(null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public SpringSecuritySentryUserProvider(final @NotNull SentryOptions options) {

@Override
public @Nullable User provideUser() {
if (options.isSendDefaultPii()) {
if (options.getDataCollectionResolver().isUserInfo()) {
final SecurityContext context = SecurityContextHolder.getContext();
if (context != null && context.getAuthentication() != null) {
final User user = new User();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,43 @@ class HttpServletRequestSentryUserProviderTest {
assertEquals("janesmith", result.username)
}

@Test
fun `when user info is disabled, does not attach user data`() {
val principal = mock<Principal>()
whenever(principal.name).thenReturn("janesmith")
val request = MockHttpServletRequest()
request.userPrincipal = principal
RequestContextHolder.setRequestAttributes(ServletRequestAttributes(request))

val options =
SentryOptions().apply {
isSendDefaultPii = true
dataCollection.setUserInfo(false)
}
val result = HttpServletRequestSentryUserProvider(options).provideUser()

assertNull(result)
}

@Test
fun `when user info is enabled, attaches user data`() {
val principal = mock<Principal>()
whenever(principal.name).thenReturn("janesmith")
val request = MockHttpServletRequest()
request.userPrincipal = principal
RequestContextHolder.setRequestAttributes(ServletRequestAttributes(request))

val options =
SentryOptions().apply {
isSendDefaultPii = false
dataCollection.setUserInfo(true)
}
val result = HttpServletRequestSentryUserProvider(options).provideUser()

assertNotNull(result)
assertEquals("janesmith", result.username)
}

@Test
fun `when sendDefaultPii is set to false, does not attach user data Sentry Event`() {
val principal = mock<Principal>()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,14 @@ class SentryUserFilterTest {

fun getSut(
isSendDefaultPii: Boolean = false,
userInfo: Boolean? = null,
userProviders: List<SentryUserProvider>,
): SentryUserFilter {
val options = SentryOptions().apply { this.isSendDefaultPii = isSendDefaultPii }
val options =
SentryOptions().apply {
this.isSendDefaultPii = isSendDefaultPii
userInfo?.let { dataCollection.setUserInfo(it) }
}
whenever(scopes.options).thenReturn(options)
return SentryUserFilter(scopes, userProviders)
}
Expand Down Expand Up @@ -76,7 +81,7 @@ class SentryUserFilterTest {
}

@Test
fun `merges user#others with existing user#others set on SentryEvent`() {
fun `merges user#data with existing user#data set on SentryEvent`() {
val filter =
fixture.getSut(
userProviders =
Expand Down Expand Up @@ -118,6 +123,34 @@ class SentryUserFilterTest {
verify(fixture.scopes).setUser(check { assertNull(it.ipAddress) })
}

@Test
fun `when user info is disabled, preserves auto ip from a custom provider`() {
val filter =
fixture.getSut(
isSendDefaultPii = true,
userInfo = false,
userProviders = listOf(SentryUserProvider { User().apply { ipAddress = "{{auto}}" } }),
)

filter.doFilter(fixture.request, fixture.response, fixture.chain)

verify(fixture.scopes).setUser(check { assertEquals("{{auto}}", it.ipAddress) })
}

@Test
fun `when user info is enabled, removes auto ip from a custom provider`() {
val filter =
fixture.getSut(
isSendDefaultPii = false,
userInfo = true,
userProviders = listOf(SentryUserProvider { User().apply { ipAddress = "{{auto}}" } }),
)

filter.doFilter(fixture.request, fixture.response, fixture.chain)

verify(fixture.scopes).setUser(check { assertNull(it.ipAddress) })
}

private fun assertEquals(user1: User, user2: User) {
assertEquals(user1.username, user2.username)
assertEquals(user1.id, user2.id)
Expand Down
Loading
Loading