From a186023560ab5509e486b53b5db6aad690407ad8 Mon Sep 17 00:00:00 2001 From: davidpatrick Date: Sat, 7 Sep 2019 17:16:11 -0700 Subject: [PATCH] Sanitize Error Request Data Sanitizes sensitive error request data --- package.json | 2 +- src/auth/OAuthAuthenticator.js | 2 + src/errors.js | 47 ++++++++++ test/auth/oauth.tests.js | 13 +++ test/errors.tests.js | 93 +++++++++++++++++++ .../management-token-provider.tests.js | 4 +- yarn.lock | 8 +- 7 files changed, 162 insertions(+), 7 deletions(-) create mode 100644 src/errors.js create mode 100644 test/errors.tests.js diff --git a/package.json b/package.json index 5cb714155..9d3fdf185 100644 --- a/package.json +++ b/package.json @@ -36,7 +36,7 @@ "lru-memoizer": "^2.0.1", "object.assign": "^4.1.0", "request": "^2.88.0", - "rest-facade": "^1.11.0", + "rest-facade": "^1.12.0", "retry": "^0.12.0" }, "devDependencies": { diff --git a/src/auth/OAuthAuthenticator.js b/src/auth/OAuthAuthenticator.js index 31a841bd5..bbd3fefd2 100644 --- a/src/auth/OAuthAuthenticator.js +++ b/src/auth/OAuthAuthenticator.js @@ -4,6 +4,7 @@ var sanitizeArguments = require('../utils').sanitizeArguments; var ArgumentError = require('rest-facade').ArgumentError; var RestClient = require('rest-facade').Client; +var SanitizedError = require('../errors').SanitizedError; var OAUthWithIDTokenValidation = require('./OAUthWithIDTokenValidation'); function getParamsFromOptions(options) { @@ -48,6 +49,7 @@ var OAuthAuthenticator = function(options) { * @type {Object} */ var clientOptions = { + errorCustomizer: SanitizedError, errorFormatter: { message: 'message', name: 'error' }, headers: options.headers }; diff --git a/src/errors.js b/src/errors.js new file mode 100644 index 000000000..02837d6d0 --- /dev/null +++ b/src/errors.js @@ -0,0 +1,47 @@ +var util = require('util'); + +/** + * @module errors + */ +var errors = (module.exports = {}); + +/** + * Given a response request error, sanitize sensitive data. + * + * @method sanitizeErrorRequestData + * @memberOf module:errors + */ +errors.sanitizeErrorRequestData = function(error) { + if (!error.response || !error.response.request || !error.response.request._data) { + return error; + } + + Object.keys(error.response.request._data).forEach(function(key) { + if (key.toLowerCase().match('password|secret')) { + error.response.request._data[key] = '[SANITIZED]'; + } + }); + + return error; +}; + +/** + * Given an Api Error, modify the original error and sanitize + * sensitive information using sanitizeErrorRequestData + * + * @method SanitizedError + * @memberOf module:errors + */ +var SanitizedError = function(name, message, status, requestInfo, originalError) { + this.name = name || this.constructor.name || this.constructor.prototype.name || ''; + this.message = message || ''; + this.statusCode = status || (originalError && originalError.code); + this.requestInfo = Object.assign({}, requestInfo); + this.originalError = errors.sanitizeErrorRequestData(originalError); + + Error.captureStackTrace(this, this.constructor); +}; + +util.inherits(SanitizedError, Error); + +errors.SanitizedError = SanitizedError; diff --git a/test/auth/oauth.tests.js b/test/auth/oauth.tests.js index 3cfe3419d..47bfa7e37 100644 --- a/test/auth/oauth.tests.js +++ b/test/auth/oauth.tests.js @@ -807,6 +807,19 @@ describe('OAuthAuthenticator', function() { expect(request.isDone()).to.be.true; }); }); + + it('should sanitize sensitive request data from errors', function() { + nock.cleanAll(); + + var request = nock(API_URL) + .post(path) + .reply(401); + + return this.authenticator.clientCredentialsGrant(options).catch(function(err) { + const originalRequestData = err.originalError.response.request._data; + expect(originalRequestData.client_secret).to.not.equal(CLIENT_SECRET); + }); + }); }); describe('#authorizationCodeGrant', function() { diff --git a/test/errors.tests.js b/test/errors.tests.js new file mode 100644 index 000000000..604dba670 --- /dev/null +++ b/test/errors.tests.js @@ -0,0 +1,93 @@ +var expect = require('chai').expect; + +var errors = require('../src/errors'); + +describe('Errors', function() { + describe('sanitizeErrorRequestData', function() { + describe('when passed in error is missing request data', function() { + var error = { response: { request: {} } }; + var sanitizedError = errors.sanitizeErrorRequestData(error); + + it('should return error', function() { + expect(sanitizedError).to.equal(error); + }); + }); + + describe('when passed in error has request data', function() { + const error = { + response: { + request: { + _data: { + DATA_SECRET: 'secret', + USER_PASSWORD: 'password', + USER_NAME: 'username' + } + } + } + }; + const sanitizedError = errors.sanitizeErrorRequestData(error); + const sanitizedData = sanitizedError.response.request._data; + + it('should return [SANITIZED] for DATA_SECRET', function() { + expect(sanitizedData.DATA_SECRET).to.equal('[SANITIZED]'); + }); + it('should return [SANITIZED] for DATA_SECRET', function() { + expect(sanitizedData.DATA_SECRET).to.equal('[SANITIZED]'); + }); + it('should return original value for USER_NAME', function() { + expect(sanitizedData.USER_NAME).to.equal(sanitizedData.USER_NAME); + }); + }); + }); + + describe('SanitizedError', function() { + var name = 'ErrorName'; + var message = 'message'; + var status = 500; + var requestInfo = { keyA: 'a', keyB: 'b' }; + var originalError = { response: { request: { _data: { secret: 'secretpassword' } } } }; + var sanitizedError = new errors.SanitizedError( + name, + message, + status, + requestInfo, + originalError + ); + + it('should be an instance of the builtin Error', function() { + expect(sanitizedError).to.be.an.instanceof(Error); + }); + + it('should be an instance of its class', function() { + expect(sanitizedError).to.be.an.instanceof(errors.SanitizedError); + }); + + it('should have a name', function() { + expect(sanitizedError.name).to.eql(name); + }); + + it('should have a message', function() { + expect(sanitizedError.message).to.eql(message); + }); + + it('should have a statusCode', function() { + expect(sanitizedError.statusCode).to.eql(status); + }); + + it('should have request info', function() { + expect(sanitizedError.requestInfo).to.deep.eql(requestInfo); + }); + + it('should have the original error', function() { + expect(sanitizedError.originalError).to.eql(originalError); + }); + + it('should sanitize the original error sensitive information', function() { + expect(sanitizedError.originalError.response.request._data.secret).to.eql('[SANITIZED]'); + }); + + it('should have a stack with the message and location the error was created', function() { + expect(sanitizedError.stack).to.exist; + }); + }); +}); diff --git a/test/management/management-token-provider.tests.js b/test/management/management-token-provider.tests.js index e75a5fd89..f486d188b 100644 --- a/test/management/management-token-provider.tests.js +++ b/test/management/management-token-provider.tests.js @@ -2,7 +2,7 @@ var expect = require('chai').expect; var nock = require('nock'); var assign = Object.assign || require('object.assign'); var ArgumentError = require('rest-facade').ArgumentError; -var APIError = require('rest-facade').APIError; +var SanitizedError = require('../../src/errors').SanitizedError; var ManagementTokenProvider = require('../../src/management/ManagementTokenProvider'); @@ -162,7 +162,7 @@ describe('ManagementTokenProvider', function() { .reply(401); client.getAccessToken().catch(function(err) { - expect(err).to.exist.to.be.an.instanceOf(APIError); + expect(err).to.exist.to.be.an.instanceOf(SanitizedError); expect(err.statusCode).to.be.equal(401); done(); nock.cleanAll(); diff --git a/yarn.lock b/yarn.lock index 8a6693270..b8b8e6fe9 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4289,10 +4289,10 @@ resolve@^1.10.0, resolve@^1.11.1: dependencies: path-parse "^1.0.6" -rest-facade@^1.11.0: - version "1.11.0" - resolved "https://registry.yarnpkg.com/rest-facade/-/rest-facade-1.11.0.tgz#96c99f3051ae3ea87600f90227d5ca7e1496f91d" - integrity sha512-/LtQhugC4zRc+3qDF9gvyQCMbejeviYCPCpTT1Dj9/ty+eCLeUX1iUOR//iLGJvg3wikQShvphRB1MOufEmB0w== +rest-facade@^1.12.0: + version "1.12.0" + resolved "https://registry.yarnpkg.com/rest-facade/-/rest-facade-1.12.0.tgz#e6a25958b34bab2323af5a953c0135ac2684d371" + integrity sha512-LsrzWP+aAy8LWwAi1ZshecO7nOWWZB9bB4gJTwXLl5DS40D5vmmdupKvqEcz8m0GUybLI3uJX8bofJtSrs5Acw== dependencies: change-case "^2.3.0" deepmerge "^3.2.0"