ref: Allow to extract user IP address without req.user present (#2467)

Author: kamilogorek

packages/node/src/handlers.ts

@@ -174,42 +174,28 @@ const DEFAULT_USER_KEYS = ['id', 'username', 'email'];
 
 /** JSDoc */
 function extractUserData(
-  req: {
-    ip?: string;
-    connection?: {
-      remoteAddress?: string;
-    };
-    user?: {
-      [key: string]: any;
-    };
+  user: {
+    [key: string]: any;
   },
   keys: boolean | string[],
 ): { [key: string]: any } {
-  const user: { [key: string]: any } = {};
+  const extractedUser: { [key: string]: any } = {};
   const attributes = Array.isArray(keys) ? keys : DEFAULT_USER_KEYS;
 
   attributes.forEach(key => {
-    if (req.user && key in req.user) {
-      user[key] = req.user[key];
+    if (user && key in user) {
+      extractedUser[key] = user[key];
     }
   });
 
-  // client ip:
-  //   node: req.connection.remoteAddress
-  //   express, koa: req.ip
-  const ip = req.ip || (req.connection && req.connection.remoteAddress);
-
-  if (ip) {
-    user.ip_address = ip;
-  }
-
-  return user;
+  return extractedUser;
 }
 
 /**
  * Options deciding what parts of the request to use when enhancing an event
  */
 interface ParseRequestOptions {
+  ip?: boolean;
   request?: boolean | string[];
   serverName?: boolean;
   transaction?: boolean | TransactionTypes;
@@ -229,11 +215,19 @@ export function parseRequest(
   event: Event,
   req: {
     [key: string]: any;
+    user?: {
+      [key: string]: any;
+    };
+    ip?: string;
+    connection?: {
+      remoteAddress?: string;
+    };
   },
   options?: ParseRequestOptions,
 ): Event {
   // tslint:disable-next-line:no-parameter-reassignment
   options = {
+    ip: false,
     request: true,
     serverName: true,
     transaction: true,
@@ -260,11 +254,28 @@ export function parseRequest(
     event.server_name = global.process.env.SENTRY_NAME || os.hostname();
   }
 
-  if (options.user && req.user) {
-    event.user = {
-      ...event.user,
-      ...extractUserData(req, options.user),
-    };
+  if (options.user) {
+    const extractedUser = req.user ? extractUserData(req.user, options.user) : {};
+
+    if (Object.keys(extractedUser)) {
+      event.user = {
+        ...event.user,
+        ...extractedUser,
+      };
+    }
+  }
+
+  // client ip:
+  //   node: req.connection.remoteAddress
+  //   express, koa: req.ip
+  if (options.ip) {
+    const ip = req.ip || (req.connection && req.connection.remoteAddress);
+    if (ip) {
+      event.user = {
+        ...event.user,
+        ip_address: ip,
+      };
+    }
   }
 
   if (options.transaction && !event.transaction) {

packages/node/test/handlers.test.ts

@@ -22,52 +22,66 @@ describe('parseRequest', () => {
     const DEFAULT_USER_KEYS = ['id', 'username', 'email'];
     const CUSTOM_USER_KEYS = ['custom_property'];
 
-    test('parseRequest.user only contains the default properties from the user', done => {
-      const fakeEvent: Event = {};
-      const parsedRequest: Event = parseRequest(fakeEvent, mockReq);
-      const userKeys = Object.keys(parsedRequest.user);
-
-      expect(userKeys).toEqual(DEFAULT_USER_KEYS);
-      expect(userKeys).not.toEqual(expect.arrayContaining(CUSTOM_USER_KEYS));
-      done();
+    test('parseRequest.user only contains the default properties from the user', () => {
+      const parsedRequest: Event = parseRequest({}, mockReq, {
+        user: DEFAULT_USER_KEYS,
+      });
+      expect(Object.keys(parsedRequest.user as any[])).toEqual(DEFAULT_USER_KEYS);
     });
 
-    test('parseRequest.user only contains the custom properties specified in the options.user array', done => {
-      const options = {
+    test('parseRequest.user only contains the custom properties specified in the options.user array', () => {
+      const parsedRequest: Event = parseRequest({}, mockReq, {
         user: CUSTOM_USER_KEYS,
-      };
-      const fakeEvent: Event = {};
-      const parsedRequest: Event = parseRequest(fakeEvent, mockReq, options);
-      const userKeys = Object.keys(parsedRequest.user);
+      });
+      expect(Object.keys(parsedRequest.user as any[])).toEqual(CUSTOM_USER_KEYS);
+    });
+  });
 
-      expect(userKeys).toEqual(CUSTOM_USER_KEYS);
-      expect(userKeys).not.toEqual(expect.arrayContaining(DEFAULT_USER_KEYS));
-      done();
+  describe('parseRequest.ip property', () => {
+    test('can be extracted from req.ip', () => {
+      const parsedRequest: Event = parseRequest(
+        {},
+        {
+          ...mockReq,
+          ip: '123',
+        },
+        {
+          ip: true,
+        },
+      );
+      expect(parsedRequest.user!.ip_address).toEqual('123');
+    });
+
+    test('can extract from req.connection.remoteAddress', () => {
+      const parsedRequest: Event = parseRequest(
+        {},
+        {
+          ...mockReq,
+          connection: {
+            remoteAddress: '321',
+          },
+        },
+        {
+          ip: true,
+        },
+      );
+      expect(parsedRequest.user!.ip_address).toEqual('321');
     });
   });
 
   describe('parseRequest.request properties', () => {
-    test('parseRequest.request only contains the default set of properties from the request', done => {
+    test('parseRequest.request only contains the default set of properties from the request', () => {
       const DEFAULT_REQUEST_PROPERTIES = ['cookies', 'data', 'headers', 'method', 'query_string', 'url'];
-      const fakeEvent: Event = {};
-      const parsedRequest: Event = parseRequest(fakeEvent, mockReq, {});
-      expect(Object.keys(parsedRequest.request)).toEqual(DEFAULT_REQUEST_PROPERTIES);
-      done();
+      const parsedRequest: Event = parseRequest({}, mockReq, {});
+      expect(Object.keys(parsedRequest.request as any[])).toEqual(DEFAULT_REQUEST_PROPERTIES);
     });
 
-    test('parseRequest.request only contains the specified properties in the options.request array', done => {
-      const EXCLUDED_PROPERTIES = ['cookies', 'method'];
+    test('parseRequest.request only contains the specified properties in the options.request array', () => {
       const INCLUDED_PROPERTIES = ['data', 'headers', 'query_string', 'url'];
-      const options = {
+      const parsedRequest: Event = parseRequest({}, mockReq, {
         request: INCLUDED_PROPERTIES,
-      };
-      const fakeEvent: Event = {};
-      const parsedRequest: Event = parseRequest(fakeEvent, mockReq, options);
-      const requestKeys = Object.keys(parsedRequest.request);
-
-      expect(requestKeys).toEqual(INCLUDED_PROPERTIES);
-      expect(requestKeys).not.toEqual(expect.arrayContaining(EXCLUDED_PROPERTIES));
-      done();
+      });
+      expect(Object.keys(parsedRequest.request as any[])).toEqual(['data', 'headers', 'query_string', 'url']);
     });
   });
 });