fix(sdk): Set {{auto}} if user.ip_address is undefined and sendDefaultPii: true (#4466)

Author: krystofwoldrich

CHANGELOG.md

@@ -40,7 +40,7 @@ Version 7 of the SDK is compatible with Sentry self-hosted versions 24.4.2 or hi
 
 ### Major Changes
 
-- `ip addresses` is only collected when `sendDefaultPii`: `true`
+- Set `{{auto}}` if `user.ip_address` is `undefined` and `sendDefaultPii: true` ([#4466](https://github.com/getsentry/sentry-react-native/pull/4466))
 - Exceptions from `captureConsoleIntegration` are now marked as handled: true by default
 - `shutdownTimeout` moved from `core` to `@sentry/react-native`
 - `hasTracingEnabled` was renamed to `hasSpansEnabled`

packages/core/src/js/client.ts

@@ -10,7 +10,14 @@ import type {
   TransportMakeRequestResponse,
   UserFeedback,
 } from '@sentry/core';
-import { BaseClient, dateTimestampInSeconds, logger, SentryError } from '@sentry/core';
+import {
+  addAutoIpAddressToSession,
+  addAutoIpAddressToUser,
+  BaseClient,
+  dateTimestampInSeconds,
+  logger,
+  SentryError,
+} from '@sentry/core';
 import { Alert } from 'react-native';
 
 import { getDevServer } from './integrations/debugsymbolicatorutils';
@@ -48,6 +55,11 @@ export class ReactNativeClient extends BaseClient<ReactNativeClientOptions> {
     super(options);
 
     this._outcomesBuffer = [];
+
+    if (options.sendDefaultPii === true) {
+      this.on('postprocessEvent', addAutoIpAddressToUser);
+      this.on('beforeSendSession', addAutoIpAddressToSession);
+    }
   }
 
   /**

packages/core/test/client.test.ts

@@ -2,8 +2,21 @@ import * as mockedtimetodisplaynative from './tracing/mockedtimetodisplaynative'
 jest.mock('../src/js/tracing/timetodisplaynative', () => mockedtimetodisplaynative);
 
 import { defaultStackParser } from '@sentry/browser';
-import type { Envelope, Event, Outcome, Transport, TransportMakeRequestResponse } from '@sentry/core';
-import { rejectedSyncPromise, SentryError } from '@sentry/core';
+import type {
+  Envelope,
+  Event,
+  Outcome,
+  SessionAggregates,
+  Transport,
+  TransportMakeRequestResponse,
+} from '@sentry/core';
+import {
+  addAutoIpAddressToSession,
+  addAutoIpAddressToUser,
+  makeSession,
+  rejectedSyncPromise,
+  SentryError,
+} from '@sentry/core';
 import * as RN from 'react-native';
 
 import { ReactNativeClient } from '../src/js/client';
@@ -625,6 +638,206 @@ describe('Tests ReactNativeClient', () => {
       client.recordDroppedEvent('before_send', 'error');
     }
   });
+
+  describe('ipAddress', () => {
+    let mockTransportSend: jest.Mock;
+    let client: ReactNativeClient;
+
+    beforeEach(() => {
+      mockTransportSend = jest.fn(() => Promise.resolve());
+      client = new ReactNativeClient({
+        ...DEFAULT_OPTIONS,
+        dsn: EXAMPLE_DSN,
+        transport: () => ({
+          send: mockTransportSend,
+          flush: jest.fn(),
+        }),
+        sendDefaultPii: true,
+      });
+    });
+
+    test('preserves ip_address null', () => {
+      client.captureEvent({
+        user: {
+          ip_address: null,
+        },
+      });
+
+      expect(mockTransportSend.mock.calls[0][firstArg][envelopeItems][0][envelopeItemPayload].user).toEqual(
+        expect.objectContaining({ ip_address: null }),
+      );
+    });
+
+    test('preserves ip_address value if set', () => {
+      client.captureEvent({
+        user: {
+          ip_address: '203.45.167.89',
+        },
+      });
+
+      expect(mockTransportSend.mock.calls[0][firstArg][envelopeItems][0][envelopeItemPayload].user).toEqual(
+        expect.objectContaining({ ip_address: '203.45.167.89' }),
+      );
+    });
+
+    test('adds ip_address {{auto}} to user if set to undefined', () => {
+      client.captureEvent({
+        user: {
+          ip_address: undefined,
+        },
+      });
+
+      expect(mockTransportSend.mock.calls[0][firstArg][envelopeItems][0][envelopeItemPayload].user).toEqual(
+        expect.objectContaining({ ip_address: '{{auto}}' }),
+      );
+    });
+
+    test('adds ip_address {{auto}} to user if not set', () => {
+      client.captureEvent({
+        user: {},
+      });
+
+      expect(mockTransportSend.mock.calls[0][firstArg][envelopeItems][0][envelopeItemPayload].user).toEqual(
+        expect.objectContaining({ ip_address: '{{auto}}' }),
+      );
+    });
+
+    test('adds ip_address {{auto}} to undefined user', () => {
+      client.captureEvent({});
+
+      expect(mockTransportSend.mock.calls[0][firstArg][envelopeItems][0][envelopeItemPayload].user).toEqual(
+        expect.objectContaining({ ip_address: '{{auto}}' }),
+      );
+    });
+
+    test('does not add ip_address {{auto}} to undefined user if sendDefaultPii is false', () => {
+      const { client, onSpy } = createClientWithSpy({
+        transport: () => ({
+          send: mockTransportSend,
+          flush: jest.fn(),
+        }),
+        sendDefaultPii: false,
+      });
+
+      client.captureEvent({});
+
+      expect(onSpy).not.toHaveBeenCalledWith('postprocessEvent', addAutoIpAddressToUser);
+      expect(
+        mockTransportSend.mock.calls[0][firstArg][envelopeItems][0][envelopeItemPayload].user?.ip_address,
+      ).toBeUndefined();
+    });
+
+    test('uses ip address hooks if sendDefaultPii is true', () => {
+      const { onSpy } = createClientWithSpy({
+        sendDefaultPii: true,
+      });
+
+      expect(onSpy).toHaveBeenCalledWith('postprocessEvent', addAutoIpAddressToUser);
+      expect(onSpy).toHaveBeenCalledWith('beforeSendSession', addAutoIpAddressToSession);
+    });
+
+    test('does not add ip_address {{auto}} to session if sendDefaultPii is false', () => {
+      const { client, onSpy } = createClientWithSpy({
+        release: 'test', // required for sessions to be sent
+        transport: () => ({
+          send: mockTransportSend,
+          flush: jest.fn(),
+        }),
+        sendDefaultPii: false,
+      });
+
+      const session = makeSession();
+      session.ipAddress = undefined;
+      client.captureSession(session);
+
+      expect(onSpy).not.toHaveBeenCalledWith('beforeSendSession', addAutoIpAddressToSession);
+      expect(
+        mockTransportSend.mock.calls[0][firstArg][envelopeItems][0][envelopeItemPayload].attrs.ip_address,
+      ).toBeUndefined();
+    });
+
+    test('does not add ip_address {{auto}} to session aggregate if sendDefaultPii is false', () => {
+      const { client, onSpy } = createClientWithSpy({
+        release: 'test', // required for sessions to be sent
+        transport: () => ({
+          send: mockTransportSend,
+          flush: jest.fn(),
+        }),
+        sendDefaultPii: false,
+      });
+
+      const session: SessionAggregates = {
+        aggregates: [],
+      };
+      client.sendSession(session);
+
+      expect(onSpy).not.toHaveBeenCalledWith('beforeSendSession', addAutoIpAddressToSession);
+      expect(
+        mockTransportSend.mock.calls[0][firstArg][envelopeItems][0][envelopeItemPayload].attrs.ip_address,
+      ).toBeUndefined();
+    });
+
+    test('does not overwrite session aggregate ip_address if already set', () => {
+      const { client } = createClientWithSpy({
+        release: 'test', // required for sessions to be sent
+        transport: () => ({
+          send: mockTransportSend,
+          flush: jest.fn(),
+        }),
+        sendDefaultPii: true,
+      });
+
+      const session: SessionAggregates = {
+        aggregates: [],
+        attrs: {
+          ip_address: '123.45.67.89',
+        },
+      };
+      client.sendSession(session);
+
+      expect(mockTransportSend.mock.calls[0][firstArg][envelopeItems][0][envelopeItemPayload].attrs.ip_address).toBe(
+        '123.45.67.89',
+      );
+    });
+
+    test('does add ip_address {{auto}} to session if sendDefaultPii is true', () => {
+      const { client } = createClientWithSpy({
+        release: 'test', // required for sessions to be sent
+        transport: () => ({
+          send: mockTransportSend,
+          flush: jest.fn(),
+        }),
+        sendDefaultPii: true,
+      });
+
+      const session = makeSession();
+      session.ipAddress = undefined;
+      client.captureSession(session);
+
+      expect(mockTransportSend.mock.calls[0][firstArg][envelopeItems][0][envelopeItemPayload].attrs.ip_address).toBe(
+        '{{auto}}',
+      );
+    });
+
+    test('does not overwrite session  ip_address if already set', () => {
+      const { client } = createClientWithSpy({
+        release: 'test', // required for sessions to be sent
+        transport: () => ({
+          send: mockTransportSend,
+          flush: jest.fn(),
+        }),
+        sendDefaultPii: true,
+      });
+
+      const session = makeSession();
+      session.ipAddress = '123.45.67.89';
+      client.captureSession(session);
+
+      expect(mockTransportSend.mock.calls[0][firstArg][envelopeItems][0][envelopeItemPayload].attrs.ip_address).toBe(
+        '123.45.67.89',
+      );
+    });
+  });
 });
 
 function mockedOptions(options: Partial<ReactNativeClientOptions>): ReactNativeClientOptions {
@@ -638,3 +851,23 @@ function mockedOptions(options: Partial<ReactNativeClientOptions>): ReactNativeC
     ...options,
   };
 }
+
+function createClientWithSpy(options: Partial<ReactNativeClientOptions>) {
+  const onSpy = jest.fn();
+  class SpyClient extends ReactNativeClient {
+    public on(hook: string, callback: unknown): () => void {
+      onSpy(hook, callback);
+      // @ts-expect-error - the public interface doesn't allow string and unknown
+      return super.on(hook, callback);
+    }
+  }
+
+  return {
+    client: new SpyClient({
+      ...DEFAULT_OPTIONS,
+      dsn: EXAMPLE_DSN,
+      ...options,
+    }),
+    onSpy,
+  };
+}