From def864ad5603f269a9e4f8f64dabef438b30fd5d Mon Sep 17 00:00:00 2001 From: Lewis Ellis Date: Wed, 22 Feb 2017 14:05:31 -0800 Subject: [PATCH] Adds parseUser option to control user parsing behavior --- docs/config.rst | 23 ++++++++++ lib/client.js | 3 +- lib/parsers.js | 102 +++++++++++++++++------------------------- test/raven.parsers.js | 62 ++++++++++++++++++++----- 4 files changed, 119 insertions(+), 71 deletions(-) diff --git a/docs/config.rst b/docs/config.rst index 1897e18..21d89bf 100644 --- a/docs/config.rst +++ b/docs/config.rst @@ -73,6 +73,29 @@ Those configuration options are documented below: extra: {planet: {name: 'Earth'}} } +.. describe:: parseUser + + Controls how Raven tries to parse user context when parsing a request object. + + An array of strings will serve as a whitelist for fields to grab from ``req.user``. + ``true`` will collect all keys from ``req.user``. ``false`` will collect nothing. + + Defaults to ``['id', 'username', 'email']``. + + Alternatively, a function can be provided for fully custom parsing: + + .. code-block:: javascript + + { + parseUser: function (req) { + // custom user parsing logic + return { + username: req.specialUserField.username, + id: req.specialUserField.getId() + }; + } + } + .. describe:: dataCallback A function that allows mutation of the data payload right before being diff --git a/lib/client.js b/lib/client.js index e694952..7b58acf 100644 --- a/lib/client.js +++ b/lib/client.js @@ -71,6 +71,7 @@ extend(Raven.prototype, { this.loggerName = options.logger || ''; this.dataCallback = options.dataCallback; this.shouldSendCallback = options.shouldSendCallback; + this.parseUser = options.parseUser; if (!this.dsn) { utils.consoleAlert('no DSN provided, error reporting disabled'); @@ -407,7 +408,7 @@ extend(Raven.prototype, { // skip anything not marked as an internal server error if (status < 500) return next(err); - var kwargs = parsers.parseRequest(req); + var kwargs = parsers.parseRequest(req, self.parseUser); var eventId = self.captureException(err, kwargs); res.sentry = eventId; return next(err); diff --git a/lib/parsers.js b/lib/parsers.js index e2cfeaf..3236cd8 100644 --- a/lib/parsers.js +++ b/lib/parsers.js @@ -53,74 +53,48 @@ module.exports.parseError = function parseError(err, kwargs, cb) { }); }; -module.exports.parseRequest = function parseRequest(req, kwargs) { - kwargs = kwargs || {}; +module.exports.parseRequest = function parseRequest(req, parseUser) { + var kwargs = {}; // headers: - // - // node: req.headers - // express: req.headers + // node, express: req.headers // koa: req.header - // var headers = req.headers || req.header || {}; // method: - // - // node: req.method - // express: req.method - // koa: req.method - // + // node, express, koa: req.method var method = req.method; // host: - // - // node: req.headers.host // express: req.hostname in > 4 and req.host in < 4 // koa: req.host - // + // node: req.headers.host var host = req.hostname || req.host || headers.host || ''; // protocol: - // // node: - // express: req.protocol - // koa: req.protocol - // + // express, koa: req.protocol var protocol = req.protocol === 'https' || req.secure || (req.socket || {}).encrypted ? 'https' : 'http'; // url (including path and query string): - // - // node: req.originalUrl - // express: req.originalUrl + // node, express: req.originalUrl // koa: req.url - // var originalUrl = req.originalUrl || req.url; // absolute url - var url = protocol + '://' + host + originalUrl; + var absoluteUrl = protocol + '://' + host + originalUrl; - // query string - // + // query string: // node: req.url (raw) - // express: req.query - // koa: req.query - // + // express, koa: req.query var query = req.query || urlParser.parse(originalUrl || '', true).query; // cookies: - // - // node: req.headers.cookie - // express: req.headers.cookie - // koa: req.headers.cookie - // + // node, express, koa: req.headers.cookie var cookies = cookie.parse(headers.cookie || ''); // body data: - // - // node: req.body - // express: req.body - // koa: req.body - // + // node, express, koa: req.body var data = req.body; if (['GET', 'HEAD'].indexOf(method) === -1) { if (typeof data === 'undefined') { @@ -128,19 +102,11 @@ module.exports.parseRequest = function parseRequest(req, kwargs) { } } - if (data && {}.toString.call(data) !== '[object String]') { + if (data && typeof data !== 'string' && {}.toString.call(data) !== '[object String]') { // Make sure the request body is a string data = stringify(data); } - // client ip: - // - // node: req.connection.remoteAddress - // express: req.ip - // koa: req.ip - // - var ip = req.ip || (req.connection || {}).remoteAddress; - // http interface var http = { method: method, @@ -148,28 +114,44 @@ module.exports.parseRequest = function parseRequest(req, kwargs) { headers: headers, cookies: cookies, data: data, - url: url + url: absoluteUrl }; // expose http interface kwargs.request = http; - // user - // - // typically found on req.user according to Express and Passport - - var user = {}; - if (!kwargs.user) { - if (req.user) { - // shallow copy is okay because we are only modifying top-level - // object (req.user) - for (var key in req.user) { - if ({}.hasOwnProperty.call(req.user, key)) { - user[key] = req.user[key]; + // user: typically found on req.user in express/passport patterns + // five cases for parseUser value: + // absent: grab only id, username, email from req.user + // false: capture nothing + // true: capture all keys from req.user + // array: provided whitelisted keys to grab from req.user + // function :: req -> user: custom parsing function + if (parseUser == null) parseUser = ['id', 'username', 'email']; + if (parseUser) { + var user = {}; + if (typeof parseUser === 'function') { + user = parseUser(req); + } else if (req.user) { + if (parseUser === true) { + for (var key in req.user) { + if ({}.hasOwnProperty.call(req.user, key)) { + user[key] = req.user[key]; + } } + } else { + parseUser.forEach(function (fieldName) { + if ({}.hasOwnProperty.call(req.user, fieldName)) { + user[fieldName] = req.user[fieldName]; + } + }); } } + // client ip: + // node: req.connection.remoteAddress + // express, koa: req.ip + var ip = req.ip || req.connection && req.connection.remoteAddress; if (ip) { user.ip_address = ip; } diff --git a/test/raven.parsers.js b/test/raven.parsers.js index 304316e..e7ded4e 100644 --- a/test/raven.parsers.js +++ b/test/raven.parsers.js @@ -416,22 +416,27 @@ describe('raven.parsers', function () { }); }); - it('should NOT assign req.user if already present in kwargs', function () { + it('should add ip address to user if available', function () { var mockReq = { method: 'POST', hostname: 'example.org', url: '/some/path?key=value', + ip: '127.0.0.1', user: { username: 'janedoe', email: 'hello@janedoe.com' } }; - var parsed = raven.parsers.parseRequest(mockReq, { user: {} }); - parsed.should.have.property('user', {}); + var parsed = raven.parsers.parseRequest(mockReq); + parsed.should.have.property('user', { + username: 'janedoe', + email: 'hello@janedoe.com', + ip_address: '127.0.0.1' + }); }); - it('should add ip address to user if available', function () { + describe('parseUser option', function () { var mockReq = { method: 'POST', hostname: 'example.org', @@ -439,15 +444,52 @@ describe('raven.parsers', function () { ip: '127.0.0.1', user: { username: 'janedoe', - email: 'hello@janedoe.com' + email: 'hello@janedoe.com', + random: 'random' } }; - var parsed = raven.parsers.parseRequest(mockReq); - parsed.should.have.property('user', { - username: 'janedoe', - email: 'hello@janedoe.com', - ip_address: '127.0.0.1' + it('should limit to default whitelisted fields', function () { + var parsed = raven.parsers.parseRequest(mockReq); + parsed.should.have.property('user', { + username: 'janedoe', + email: 'hello@janedoe.com', + ip_address: '127.0.0.1' + }); + }); + + it('should limit to provided whitelisted fields array', function () { + var parsed = raven.parsers.parseRequest(mockReq, ['username', 'random']); + parsed.should.have.property('user', { + username: 'janedoe', + random: 'random', + ip_address: '127.0.0.1' + }); + }); + + it('should parse all fields when passed true', function () { + var parsed = raven.parsers.parseRequest(mockReq, true); + parsed.should.have.property('user', { + username: 'janedoe', + email: 'hello@janedoe.com', + random: 'random', + ip_address: '127.0.0.1' + }); + }); + + it('should parse nothing when passed false', function () { + var parsed = raven.parsers.parseRequest(mockReq, false); + parsed.should.not.have.property('user'); + }); + + it('should take a custom parsing function', function () { + var parsed = raven.parsers.parseRequest(mockReq, function (req) { + return { user: req.user.username }; + }); + parsed.should.have.property('user', { + user: 'janedoe', + ip_address: '127.0.0.1' + }); }); }); });