feat(core)!: Stop setting user in requestDataIntegration (#14898)

This was an express-specific, rather undocumented behavior, and also
conflicted with our privacy-by-default stance.

Starting in v9, you'll need to manually call `Sentry.setUser()` e.g. in
a middleware to set the user on Sentry events.

Docs for this are already pending:
getsentry/sentry-docs#12224

Extracted this out of
#14806

Author: mydea

dev-packages/node-integration-tests/suites/express/requestUser/test.ts

@@ -5,28 +5,21 @@ describe('express user handling', () => {
     cleanupChildProcesses();
   });
 
-  test('picks user from request', done => {
+  test('ignores user from request', done => {
+    expect.assertions(2);
+
     createRunner(__dirname, 'server.js')
       .expect({
-        event: {
-          user: {
-            id: '1',
-            email: '[email protected]',
-          },
-          exception: {
-            values: [
-              {
-                value: 'error_1',
-              },
-            ],
-          },
+        event: event => {
+          expect(event.user).toBeUndefined();
+          expect(event.exception?.values?.[0]?.value).toBe('error_1');
         },
       })
       .start(done)
       .makeRequest('get', '/test1', { expectError: true });
   });
 
-  test('setUser overwrites user from request', done => {
+  test('using setUser in middleware works', done => {
     createRunner(__dirname, 'server.js')
       .expect({
         event: {

docs/migration/v8-to-v9.md

@@ -84,6 +84,8 @@ In v9, an `undefined` value will be treated the same as if the value is not defi
 
 - When `skipOpenTelemetrySetup: true` is configured, `httpIntegration({ spans: false })` will be configured by default. This means that you no longer have to specify this yourself in this scenario. With this change, no spans are emitted once `skipOpenTelemetrySetup: true` is configured, without any further configuration being needed.
 
+- The `requestDataIntegration` will no longer automatically set the user from `request.user`. This is an express-specific, undocumented behavior, and also conflicts with our privacy-by-default strategy. Starting in v9, you'll need to manually call `Sentry.setUser()` e.g. in a middleware to set the user on Sentry events.
+
 ### `@sentry/browser`
 
 - The `captureUserFeedback` method has been removed. Use `captureFeedback` instead and update the `comments` field to `message`.
@@ -128,6 +130,8 @@ Sentry.init({
 });
 ```
 
+- The `DEFAULT_USER_INCLUDES` constant has been removed.
+
 ### `@sentry/react`
 
 - The `wrapUseRoutes` method has been removed. Use `wrapUseRoutesV6` or `wrapUseRoutesV7` instead depending on what version of react router you are using.

packages/astro/src/index.server.ts

@@ -31,7 +31,6 @@ export {
   cron,
   dataloaderIntegration,
   dedupeIntegration,
-  DEFAULT_USER_INCLUDES,
   defaultStackParser,
   endSession,
   expressErrorHandler,

packages/aws-serverless/src/index.ts

@@ -42,7 +42,6 @@ export {
   flush,
   close,
   getSentryRelease,
-  DEFAULT_USER_INCLUDES,
   createGetModuleFromFilename,
   anrIntegration,
   disableAnrDetectionForCallback,

packages/bun/src/index.ts

@@ -62,7 +62,6 @@ export {
   flush,
   close,
   getSentryRelease,
-  DEFAULT_USER_INCLUDES,
   createGetModuleFromFilename,
   anrIntegration,
   disableAnrDetectionForCallback,

packages/core/src/integrations/requestdata.ts

@@ -13,13 +13,6 @@ export type RequestDataIntegrationOptions = {
     ip?: boolean;
     query_string?: boolean;
     url?: boolean;
-    user?:
-      | boolean
-      | {
-          id?: boolean;
-          username?: boolean;
-          email?: boolean;
-        };
   };
 };
 
@@ -31,11 +24,6 @@ const DEFAULT_OPTIONS = {
     ip: false,
     query_string: true,
     url: true,
-    user: {
-      id: true,
-      username: true,
-      email: true,
-    },
   },
   transactionNamingScheme: 'methodPath' as const,
 };
@@ -49,14 +37,6 @@ const _requestDataIntegration = ((options: RequestDataIntegrationOptions = {}) =
     include: {
       ...DEFAULT_OPTIONS.include,
       ...options.include,
-      user:
-        options.include && typeof options.include.user === 'boolean'
-          ? options.include.user
-          : {
-              ...DEFAULT_OPTIONS.include.user,
-              // Unclear why TS still thinks `options.include.user` could be a boolean at this point
-              ...((options.include || {}).user as Record<string, boolean>),
-            },
     },
   };
 
@@ -69,16 +49,12 @@ const _requestDataIntegration = ((options: RequestDataIntegrationOptions = {}) =
       // that's happened, it will be easier to add this logic in without worrying about unexpected side effects.)
 
       const { sdkProcessingMetadata = {} } = event;
-      const { request, normalizedRequest, ipAddress } = sdkProcessingMetadata;
+      const { normalizedRequest, ipAddress } = sdkProcessingMetadata;
 
       const addRequestDataOptions = convertReqDataIntegrationOptsToAddReqDataOpts(_options);
 
-      // If this is set, it takes precedence over the plain request object
       if (normalizedRequest) {
-        // Some other data is not available in standard HTTP requests, but can sometimes be augmented by e.g. Express or Next.js
-        const user = request ? request.user : undefined;
-
-        addNormalizedRequestDataToEvent(event, normalizedRequest, { ipAddress, user }, addRequestDataOptions);
+        addNormalizedRequestDataToEvent(event, normalizedRequest, { ipAddress }, addRequestDataOptions);
         return event;
       }
 
@@ -99,7 +75,7 @@ function convertReqDataIntegrationOptsToAddReqDataOpts(
   integrationOptions: Required<RequestDataIntegrationOptions>,
 ): AddRequestDataToEventOptions {
   const {
-    include: { ip, user, ...requestOptions },
+    include: { ip, ...requestOptions },
   } = integrationOptions;
 
   const requestIncludeKeys: string[] = ['method'];
@@ -109,25 +85,9 @@ function convertReqDataIntegrationOptsToAddReqDataOpts(
     }
   }
 
-  let addReqDataUserOpt;
-  if (user === undefined) {
-    addReqDataUserOpt = true;
-  } else if (typeof user === 'boolean') {
-    addReqDataUserOpt = user;
-  } else {
-    const userIncludeKeys: string[] = [];
-    for (const [key, value] of Object.entries(user)) {
-      if (value) {
-        userIncludeKeys.push(key);
-      }
-    }
-    addReqDataUserOpt = userIncludeKeys;
-  }
-
   return {
     include: {
       ip,
-      user: addReqDataUserOpt,
       request: requestIncludeKeys.length !== 0 ? requestIncludeKeys : undefined,
     },
   };

packages/core/src/utils-hoist/index.ts

@@ -66,7 +66,6 @@ export type { PromiseBuffer } from './promisebuffer';
 
 // TODO: Remove requestdata export once equivalent integration is used everywhere
 export {
-  DEFAULT_USER_INCLUDES,
   addNormalizedRequestDataToEvent,
   winterCGHeadersToDict,
   winterCGRequestToRequestData,

packages/core/src/utils-hoist/requestdata.ts

@@ -2,18 +2,15 @@ import type { Event, PolymorphicRequest, RequestEventData, WebFetchHeaders, WebF
 
 import { parseCookie } from './cookie';
 import { DEBUG_BUILD } from './debug-build';
-import { isPlainObject } from './is';
 import { logger } from './logger';
 import { dropUndefinedKeys } from './object';
 import { getClientIPAddress, ipHeaderNames } from './vendor/getIpAddress';
 
 const DEFAULT_INCLUDES = {
   ip: false,
   request: true,
-  user: true,
 };
 const DEFAULT_REQUEST_INCLUDES = ['cookies', 'data', 'headers', 'method', 'query_string', 'url'];
-export const DEFAULT_USER_INCLUDES = ['id', 'username', 'email'];
 
 /**
  * Options deciding what parts of the request to use when enhancing an event
@@ -23,7 +20,6 @@ export type AddRequestDataToEventOptions = {
   include?: {
     ip?: boolean;
     request?: boolean | Array<(typeof DEFAULT_REQUEST_INCLUDES)[number]>;
-    user?: boolean | Array<(typeof DEFAULT_USER_INCLUDES)[number]>;
   };
 
   /** Injected platform-specific dependencies */
@@ -39,24 +35,6 @@ export type AddRequestDataToEventOptions = {
   };
 };
 
-function extractUserData(
-  user: {
-    [key: string]: unknown;
-  },
-  keys: boolean | string[],
-): { [key: string]: unknown } {
-  const extractedUser: { [key: string]: unknown } = {};
-  const attributes = Array.isArray(keys) ? keys : DEFAULT_USER_INCLUDES;
-
-  attributes.forEach(key => {
-    if (user && key in user) {
-      extractedUser[key] = user[key];
-    }
-  });
-
-  return extractedUser;
-}
-
 /**
  * Add already normalized request data to an event.
  * This mutates the passed in event.
@@ -65,7 +43,7 @@ export function addNormalizedRequestDataToEvent(
   event: Event,
   req: RequestEventData,
   // This is non-standard data that is not part of the regular HTTP request
-  additionalData: { ipAddress?: string; user?: Record<string, unknown> },
+  additionalData: { ipAddress?: string },
   options: AddRequestDataToEventOptions,
 ): void {
   const include = {
@@ -87,20 +65,6 @@ export function addNormalizedRequestDataToEvent(
     };
   }
 
-  if (include.user) {
-    const extractedUser =
-      additionalData.user && isPlainObject(additionalData.user)
-        ? extractUserData(additionalData.user, include.user)
-        : {};
-
-    if (Object.keys(extractedUser).length) {
-      event.user = {
-        ...extractedUser,
-        ...event.user,
-      };
-    }
-  }
-
   if (include.ip) {
     const ip = (req.headers && getClientIPAddress(req.headers)) || additionalData.ipAddress;
     if (ip) {

packages/core/test/lib/integrations/requestdata.test.ts

@@ -59,13 +59,13 @@ describe('`RequestData` integration', () => {
 
   describe('option conversion', () => {
     it('leaves `ip` and `user` at top level of `include`', () => {
-      const requestDataEventProcessor = initWithRequestDataIntegrationOptions({ include: { ip: false, user: true } });
+      const requestDataEventProcessor = initWithRequestDataIntegrationOptions({ include: { ip: false } });
 
       void requestDataEventProcessor(event, {});
       expect(addNormalizedRequestDataToEventSpy).toHaveBeenCalled();
       const passedOptions = addNormalizedRequestDataToEventSpy.mock.calls[0]?.[3];
 
-      expect(passedOptions?.include).toEqual(expect.objectContaining({ ip: false, user: true }));
+      expect(passedOptions?.include).toEqual(expect.objectContaining({ ip: false }));
     });
 
     it('moves `true` request keys into `request` include, but omits `false` ones', async () => {
@@ -80,18 +80,5 @@ describe('`RequestData` integration', () => {
       expect(passedOptions?.include?.request).toEqual(expect.arrayContaining(['data']));
       expect(passedOptions?.include?.request).not.toEqual(expect.arrayContaining(['cookies']));
     });
-
-    it('moves `true` user keys into `user` include, but omits `false` ones', async () => {
-      const requestDataEventProcessor = initWithRequestDataIntegrationOptions({
-        include: { user: { id: true, email: false } },
-      });
-
-      void requestDataEventProcessor(event, {});
-
-      const passedOptions = addNormalizedRequestDataToEventSpy.mock.calls[0]?.[3];
-
-      expect(passedOptions?.include?.user).toEqual(expect.arrayContaining(['id']));
-      expect(passedOptions?.include?.user).not.toEqual(expect.arrayContaining(['email']));
-    });
   });
 });

packages/google-cloud-serverless/src/index.ts

@@ -42,7 +42,6 @@ export {
   flush,
   close,
   getSentryRelease,
-  DEFAULT_USER_INCLUDES,
   createGetModuleFromFilename,
   anrIntegration,
   disableAnrDetectionForCallback,

packages/node/src/index.ts

@@ -54,8 +54,6 @@ export { cron } from './cron';
 
 export type { NodeOptions } from './types';
 
-export { DEFAULT_USER_INCLUDES } from '@sentry/core';
-
 export {
   // This needs exporting so the NodeClient can be used without calling init
   setOpenTelemetryContextAsyncContextStrategy as setNodeAsyncContextStrategy,

packages/remix/src/index.server.ts

@@ -34,7 +34,6 @@ export {
   createTransport,
   cron,
   dedupeIntegration,
-  DEFAULT_USER_INCLUDES,
   defaultStackParser,
   endSession,
   expressErrorHandler,

packages/solidstart/src/server/index.ts

@@ -26,7 +26,6 @@ export {
   createTransport,
   cron,
   dedupeIntegration,
-  DEFAULT_USER_INCLUDES,
   defaultStackParser,
   endSession,
   expressErrorHandler,

packages/sveltekit/src/server/index.ts

@@ -26,7 +26,6 @@ export {
   createTransport,
   cron,
   dedupeIntegration,
-  DEFAULT_USER_INCLUDES,
   defaultStackParser,
   endSession,
   expressErrorHandler,