Skip to content

Commit 8f84a18

Browse files
kamilogorekkamilogorek
authored
ref: Allow to extract user IP address without req.user present (getsentry#2467)
1 parent 7040313 commit 8f84a18

File tree

2 files changed

+84
-59
lines changed

2 files changed

+84
-59
lines changed

‎packages/node/src/handlers.ts

+37-26
Original file line numberDiff line numberDiff line change
@@ -174,42 +174,28 @@ const DEFAULT_USER_KEYS = ['id', 'username', 'email'];
174174

175175
/** JSDoc */
176176
function extractUserData(
177-
req: {
178-
ip?: string;
179-
connection?: {
180-
remoteAddress?: string;
181-
};
182-
user?: {
183-
[key: string]: any;
184-
};
177+
user: {
178+
[key: string]: any;
185179
},
186180
keys: boolean | string[],
187181
): { [key: string]: any } {
188-
const user: { [key: string]: any } = {};
182+
const extractedUser: { [key: string]: any } = {};
189183
const attributes = Array.isArray(keys) ? keys : DEFAULT_USER_KEYS;
190184

191185
attributes.forEach(key => {
192-
if (req.user && key in req.user) {
193-
user[key] = req.user[key];
186+
if (user && key in user) {
187+
extractedUser[key] = user[key];
194188
}
195189
});
196190

197-
// client ip:
198-
// node: req.connection.remoteAddress
199-
// express, koa: req.ip
200-
const ip = req.ip || (req.connection && req.connection.remoteAddress);
201-
202-
if (ip) {
203-
user.ip_address = ip;
204-
}
205-
206-
return user;
191+
return extractedUser;
207192
}
208193

209194
/**
210195
* Options deciding what parts of the request to use when enhancing an event
211196
*/
212197
interface ParseRequestOptions {
198+
ip?: boolean;
213199
request?: boolean | string[];
214200
serverName?: boolean;
215201
transaction?: boolean | TransactionTypes;
@@ -229,11 +215,19 @@ export function parseRequest(
229215
event: Event,
230216
req: {
231217
[key: string]: any;
218+
user?: {
219+
[key: string]: any;
220+
};
221+
ip?: string;
222+
connection?: {
223+
remoteAddress?: string;
224+
};
232225
},
233226
options?: ParseRequestOptions,
234227
): Event {
235228
// tslint:disable-next-line:no-parameter-reassignment
236229
options = {
230+
ip: false,
237231
request: true,
238232
serverName: true,
239233
transaction: true,
@@ -260,11 +254,28 @@ export function parseRequest(
260254
event.server_name = global.process.env.SENTRY_NAME || os.hostname();
261255
}
262256

263-
if (options.user && req.user) {
264-
event.user = {
265-
...event.user,
266-
...extractUserData(req, options.user),
267-
};
257+
if (options.user) {
258+
const extractedUser = req.user ? extractUserData(req.user, options.user) : {};
259+
260+
if (Object.keys(extractedUser)) {
261+
event.user = {
262+
...event.user,
263+
...extractedUser,
264+
};
265+
}
266+
}
267+
268+
// client ip:
269+
// node: req.connection.remoteAddress
270+
// express, koa: req.ip
271+
if (options.ip) {
272+
const ip = req.ip || (req.connection && req.connection.remoteAddress);
273+
if (ip) {
274+
event.user = {
275+
...event.user,
276+
ip_address: ip,
277+
};
278+
}
268279
}
269280

270281
if (options.transaction && !event.transaction) {

‎packages/node/test/handlers.test.ts

+47-33
Original file line numberDiff line numberDiff line change
@@ -22,52 +22,66 @@ describe('parseRequest', () => {
2222
const DEFAULT_USER_KEYS = ['id', 'username', 'email'];
2323
const CUSTOM_USER_KEYS = ['custom_property'];
2424

25-
test('parseRequest.user only contains the default properties from the user', done => {
26-
const fakeEvent: Event = {};
27-
const parsedRequest: Event = parseRequest(fakeEvent, mockReq);
28-
const userKeys = Object.keys(parsedRequest.user);
29-
30-
expect(userKeys).toEqual(DEFAULT_USER_KEYS);
31-
expect(userKeys).not.toEqual(expect.arrayContaining(CUSTOM_USER_KEYS));
32-
done();
25+
test('parseRequest.user only contains the default properties from the user', () => {
26+
const parsedRequest: Event = parseRequest({}, mockReq, {
27+
user: DEFAULT_USER_KEYS,
28+
});
29+
expect(Object.keys(parsedRequest.user as any[])).toEqual(DEFAULT_USER_KEYS);
3330
});
3431

35-
test('parseRequest.user only contains the custom properties specified in the options.user array', done => {
36-
const options = {
32+
test('parseRequest.user only contains the custom properties specified in the options.user array', () => {
33+
const parsedRequest: Event = parseRequest({}, mockReq, {
3734
user: CUSTOM_USER_KEYS,
38-
};
39-
const fakeEvent: Event = {};
40-
const parsedRequest: Event = parseRequest(fakeEvent, mockReq, options);
41-
const userKeys = Object.keys(parsedRequest.user);
35+
});
36+
expect(Object.keys(parsedRequest.user as any[])).toEqual(CUSTOM_USER_KEYS);
37+
});
38+
});
4239

43-
expect(userKeys).toEqual(CUSTOM_USER_KEYS);
44-
expect(userKeys).not.toEqual(expect.arrayContaining(DEFAULT_USER_KEYS));
45-
done();
40+
describe('parseRequest.ip property', () => {
41+
test('can be extracted from req.ip', () => {
42+
const parsedRequest: Event = parseRequest(
43+
{},
44+
{
45+
...mockReq,
46+
ip: '123',
47+
},
48+
{
49+
ip: true,
50+
},
51+
);
52+
expect(parsedRequest.user!.ip_address).toEqual('123');
53+
});
54+
55+
test('can extract from req.connection.remoteAddress', () => {
56+
const parsedRequest: Event = parseRequest(
57+
{},
58+
{
59+
...mockReq,
60+
connection: {
61+
remoteAddress: '321',
62+
},
63+
},
64+
{
65+
ip: true,
66+
},
67+
);
68+
expect(parsedRequest.user!.ip_address).toEqual('321');
4669
});
4770
});
4871

4972
describe('parseRequest.request properties', () => {
50-
test('parseRequest.request only contains the default set of properties from the request', done => {
73+
test('parseRequest.request only contains the default set of properties from the request', () => {
5174
const DEFAULT_REQUEST_PROPERTIES = ['cookies', 'data', 'headers', 'method', 'query_string', 'url'];
52-
const fakeEvent: Event = {};
53-
const parsedRequest: Event = parseRequest(fakeEvent, mockReq, {});
54-
expect(Object.keys(parsedRequest.request)).toEqual(DEFAULT_REQUEST_PROPERTIES);
55-
done();
75+
const parsedRequest: Event = parseRequest({}, mockReq, {});
76+
expect(Object.keys(parsedRequest.request as any[])).toEqual(DEFAULT_REQUEST_PROPERTIES);
5677
});
5778

58-
test('parseRequest.request only contains the specified properties in the options.request array', done => {
59-
const EXCLUDED_PROPERTIES = ['cookies', 'method'];
79+
test('parseRequest.request only contains the specified properties in the options.request array', () => {
6080
const INCLUDED_PROPERTIES = ['data', 'headers', 'query_string', 'url'];
61-
const options = {
81+
const parsedRequest: Event = parseRequest({}, mockReq, {
6282
request: INCLUDED_PROPERTIES,
63-
};
64-
const fakeEvent: Event = {};
65-
const parsedRequest: Event = parseRequest(fakeEvent, mockReq, options);
66-
const requestKeys = Object.keys(parsedRequest.request);
67-
68-
expect(requestKeys).toEqual(INCLUDED_PROPERTIES);
69-
expect(requestKeys).not.toEqual(expect.arrayContaining(EXCLUDED_PROPERTIES));
70-
done();
83+
});
84+
expect(Object.keys(parsedRequest.request as any[])).toEqual(['data', 'headers', 'query_string', 'url']);
7185
});
7286
});
7387
});

0 commit comments

Comments
 (0)