From 251aa499ed063e7cf83ddbaa9437dbcc483f818d Mon Sep 17 00:00:00 2001 From: Chris Cowan Date: Fri, 25 Sep 2015 11:27:58 -0700 Subject: [PATCH 1/6] Adding filtering to the logger - Closes #5036 - Add `applyFilterToKey()` - Add test for `applyFilterToKey()` - Add `filter` attribute to config for reporters - Add `this.filter` method to `LogFormat` class --- src/server/logging/LogFormat.js | 16 +++++- src/server/logging/LogReporter.js | 2 +- .../logging/__tests__/applyFilterToKey.js | 49 +++++++++++++++++++ src/server/logging/applyFilterToKey.js | 23 +++++++++ src/server/logging/index.js | 5 +- 5 files changed, 91 insertions(+), 4 deletions(-) create mode 100644 src/server/logging/__tests__/applyFilterToKey.js create mode 100644 src/server/logging/applyFilterToKey.js diff --git a/src/server/logging/LogFormat.js b/src/server/logging/LogFormat.js index db901d2ecc90..eb34d4fa4049 100644 --- a/src/server/logging/LogFormat.js +++ b/src/server/logging/LogFormat.js @@ -6,6 +6,7 @@ let ansicolors = require('ansicolors'); let stringify = require('json-stringify-safe'); let querystring = require('querystring'); let inspect = require('util').inspect; +let applyFilterToKey = require('./applyFilterToKey'); function serializeError(err) { return { @@ -24,16 +25,27 @@ let levelColor = function (code) { return ansicolors.red(code); }; + module.exports = class TransformObjStream extends Stream.Transform { - constructor() { + constructor(config) { super({ readableObjectMode: false, writableObjectMode: true }); + this.config = config; + } + + filter(data) { + if (this.config.filter) { + _.each(this.config.filter, (action, key) => { + applyFilterToKey(data, key, action); + }); + } + return data; } _transform(event, enc, next) { - var data = this.readEvent(event); + var data = this.filter(this.readEvent(event)); this.push(this.format(data) + '\n'); next(); } diff --git a/src/server/logging/LogReporter.js b/src/server/logging/LogReporter.js index 246e8164db60..ac098732a711 100644 --- a/src/server/logging/LogReporter.js +++ b/src/server/logging/LogReporter.js @@ -8,7 +8,7 @@ let LogFormatString = require('./LogFormatString'); module.exports = class KbnLogger { constructor(events, config) { this.squeeze = new Squeeze(events); - this.format = config.json ? new LogFormatJson() : new LogFormatString(); + this.format = config.json ? new LogFormatJson(config) : new LogFormatString(config); if (config.dest === 'stdout') { this.dest = process.stdout; diff --git a/src/server/logging/__tests__/applyFilterToKey.js b/src/server/logging/__tests__/applyFilterToKey.js new file mode 100644 index 000000000000..ef319a216297 --- /dev/null +++ b/src/server/logging/__tests__/applyFilterToKey.js @@ -0,0 +1,49 @@ +var applyFilterToKey = require('../applyFilterToKey'); +var expect = require('expect.js'); + +function fixture() { + return { + req: { + headers: { + authorization: 'Basic dskd939k2i' + } + } + }; +} + +describe('applyFilterToKey(obj, key, action)', function () { + + it('should remove a key from an object recursivly', function () { + var data = fixture(); + applyFilterToKey(data, 'authorization', 'remove'); + expect(data).to.eql({ + req: { headers: {} } + }); + }); + + it('should censor a key in an object recursivly', function () { + var data = fixture(); + applyFilterToKey(data, 'authorization', 'censor'); + expect(data).to.eql({ + req: { + headers: { + authorization: 'XXXXXXXXXXXXXXXX' + } + } + }); + }); + + it('should censor key with a RegEx in an object recursivly', function () { + var data = fixture(); + var regex = /([^\s]+)$/; + applyFilterToKey(data, 'authorization', regex); + expect(data).to.eql({ + req: { + headers: { + authorization: 'Basic XXXXXXXXXX' + } + } + }); + }); + +}); diff --git a/src/server/logging/applyFilterToKey.js b/src/server/logging/applyFilterToKey.js new file mode 100644 index 000000000000..6046ee867c68 --- /dev/null +++ b/src/server/logging/applyFilterToKey.js @@ -0,0 +1,23 @@ +function replacer(match, group) { + return (new Array(group.length + 1).join('X')); +} + +module.exports = function applyFilterToKey(obj, key, action) { + for (let k in obj) { + if (obj.hasOwnProperty(k)) { + let val = obj[k]; + if (typeof val === 'object') { + applyFilterToKey(val, key, action); + } else if (k === key) { + val = '' + val; + if (action === 'remove') delete obj[k]; + if (action === 'censor') { + obj[k] = val.replace(/./g, 'X'); + }; + if (action instanceof RegExp) { + obj[k] = val.replace(action, replacer); + } + } + } + } +}; diff --git a/src/server/logging/index.js b/src/server/logging/index.js index 8ac4bb4d7ef8..ee8c16956533 100644 --- a/src/server/logging/index.js +++ b/src/server/logging/index.js @@ -42,7 +42,10 @@ module.exports = function (kbnServer, server, config) { reporter: require('./LogReporter'), config: { json: config.get('logging.json'), - dest: config.get('logging.dest') + dest: config.get('logging.dest'), + filter: { + authorization: 'remove' + } }, events: _.transform(events, function (filtered, val, key) { // provide a string compatible way to remove events From d47481f9ef662d9fae6d073c8008d96fc3a3c857 Mon Sep 17 00:00:00 2001 From: Chris Cowan Date: Fri, 25 Sep 2015 11:44:48 -0700 Subject: [PATCH 2/6] Moving key match above recursion for removing branches --- src/server/logging/__tests__/applyFilterToKey.js | 8 ++++++++ src/server/logging/applyFilterToKey.js | 6 +++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/server/logging/__tests__/applyFilterToKey.js b/src/server/logging/__tests__/applyFilterToKey.js index ef319a216297..53a932f716e2 100644 --- a/src/server/logging/__tests__/applyFilterToKey.js +++ b/src/server/logging/__tests__/applyFilterToKey.js @@ -21,6 +21,14 @@ describe('applyFilterToKey(obj, key, action)', function () { }); }); + it('should remove an entire branch', function () { + var data = fixture(); + applyFilterToKey(data, 'headers', 'remove'); + expect(data).to.eql({ + req: { } + }); + }); + it('should censor a key in an object recursivly', function () { var data = fixture(); applyFilterToKey(data, 'authorization', 'censor'); diff --git a/src/server/logging/applyFilterToKey.js b/src/server/logging/applyFilterToKey.js index 6046ee867c68..06f7bae0e992 100644 --- a/src/server/logging/applyFilterToKey.js +++ b/src/server/logging/applyFilterToKey.js @@ -6,9 +6,7 @@ module.exports = function applyFilterToKey(obj, key, action) { for (let k in obj) { if (obj.hasOwnProperty(k)) { let val = obj[k]; - if (typeof val === 'object') { - applyFilterToKey(val, key, action); - } else if (k === key) { + if (k === key) { val = '' + val; if (action === 'remove') delete obj[k]; if (action === 'censor') { @@ -17,6 +15,8 @@ module.exports = function applyFilterToKey(obj, key, action) { if (action instanceof RegExp) { obj[k] = val.replace(action, replacer); } + } else if (typeof val === 'object') { + applyFilterToKey(val, key, action); } } } From 3902c380cd72c65c774e795dbae8cd1f02b41126 Mon Sep 17 00:00:00 2001 From: Chris Cowan Date: Sat, 26 Sep 2015 10:09:56 -0700 Subject: [PATCH 3/6] Fixing Edge Cases - Add ability to specify filters from CLI options - Add test and fix for removing branch with censor - Add change regex to use strings --- src/server/config/schema.js | 2 +- .../logging/__tests__/applyFilterToKey.js | 10 +++++++- src/server/logging/applyFilterToKey.js | 24 +++++++++++++------ src/server/logging/index.js | 8 +++++-- 4 files changed, 33 insertions(+), 11 deletions(-) diff --git a/src/server/config/schema.js b/src/server/config/schema.js index dc960c684189..69b50dc7aaf8 100644 --- a/src/server/config/schema.js +++ b/src/server/config/schema.js @@ -61,7 +61,7 @@ module.exports = Joi.object({ events: Joi.any().default({}), dest: Joi.string().default('stdout'), - + filter: Joi.any().default({}), json: Joi.boolean() .when('dest', { is: 'stdout', diff --git a/src/server/logging/__tests__/applyFilterToKey.js b/src/server/logging/__tests__/applyFilterToKey.js index 53a932f716e2..1d91b2b4d961 100644 --- a/src/server/logging/__tests__/applyFilterToKey.js +++ b/src/server/logging/__tests__/applyFilterToKey.js @@ -29,6 +29,14 @@ describe('applyFilterToKey(obj, key, action)', function () { }); }); + it('should remove an entire branch with censor', function () { + var data = fixture(); + applyFilterToKey(data, 'headers', 'censor'); + expect(data).to.eql({ + req: { } + }); + }); + it('should censor a key in an object recursivly', function () { var data = fixture(); applyFilterToKey(data, 'authorization', 'censor'); @@ -43,7 +51,7 @@ describe('applyFilterToKey(obj, key, action)', function () { it('should censor key with a RegEx in an object recursivly', function () { var data = fixture(); - var regex = /([^\s]+)$/; + var regex = '/([^\\s]+)$/'; applyFilterToKey(data, 'authorization', regex); expect(data).to.eql({ req: { diff --git a/src/server/logging/applyFilterToKey.js b/src/server/logging/applyFilterToKey.js index 06f7bae0e992..8a3ab9e5399d 100644 --- a/src/server/logging/applyFilterToKey.js +++ b/src/server/logging/applyFilterToKey.js @@ -7,13 +7,23 @@ module.exports = function applyFilterToKey(obj, key, action) { if (obj.hasOwnProperty(k)) { let val = obj[k]; if (k === key) { - val = '' + val; - if (action === 'remove') delete obj[k]; - if (action === 'censor') { - obj[k] = val.replace(/./g, 'X'); - }; - if (action instanceof RegExp) { - obj[k] = val.replace(action, replacer); + if (action === 'remove') { + delete obj[k]; + } + else if (action === 'censor' && typeof val === 'object') { + delete obj[key]; + } + else if (action === 'censor') { + obj[k] = ('' + val).replace(/./g, 'X'); + } + else if (/\/.+\//.test(action)) { + var matches = action.match(/\/(.+)\//); + try { + let regex = new RegExp(matches[1]); + obj[k] = ('' + val).replace(regex, replacer); + } catch (e) { + //meh + } } } else if (typeof val === 'object') { applyFilterToKey(val, key, action); diff --git a/src/server/logging/index.js b/src/server/logging/index.js index ee8c16956533..2763baa46208 100644 --- a/src/server/logging/index.js +++ b/src/server/logging/index.js @@ -43,9 +43,13 @@ module.exports = function (kbnServer, server, config) { config: { json: config.get('logging.json'), dest: config.get('logging.dest'), - filter: { + // I'm adding the default here because if you add another filter + // using the commandline it will remove authorization. I want users + // to have to explicitly set --logging.filter.authorization=none to + // have it show up int he logs. + filter: _.defaults(config.get('logging.filter'), { authorization: 'remove' - } + }) }, events: _.transform(events, function (filtered, val, key) { // provide a string compatible way to remove events From 2b419875ba2948e94fe13f250bdd45d343358bf4 Mon Sep 17 00:00:00 2001 From: Chris Cowan Date: Sat, 26 Sep 2015 10:34:51 -0700 Subject: [PATCH 4/6] Refactoring the regex codes --- src/server/logging/applyFilterToKey.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/server/logging/applyFilterToKey.js b/src/server/logging/applyFilterToKey.js index 8a3ab9e5399d..c3486e34b6f8 100644 --- a/src/server/logging/applyFilterToKey.js +++ b/src/server/logging/applyFilterToKey.js @@ -18,11 +18,9 @@ module.exports = function applyFilterToKey(obj, key, action) { } else if (/\/.+\//.test(action)) { var matches = action.match(/\/(.+)\//); - try { + if (matches) { let regex = new RegExp(matches[1]); obj[k] = ('' + val).replace(regex, replacer); - } catch (e) { - //meh } } } else if (typeof val === 'object') { From a69b0271307c5e57d08f693aaf7c5ae712330aac Mon Sep 17 00:00:00 2001 From: spalger Date: Wed, 30 Sep 2015 03:53:04 +0200 Subject: [PATCH 5/6] [server/log] simplify the log messages before filtering `applyFilterToKey()` used to choke on circular structures by simply recursing through the object passed in. When we create a circular structure though, we generally provide a simplified version of the object via `obj#toJSON()`. This makes logging those objects simple, and now the filter will use the same mechanism before attempting to itterate the object. --- src/server/logging/LogFormat.js | 10 ++-- .../logging/__tests__/applyFilterToKey.js | 51 +++++++++++++++++-- src/server/logging/applyFilterToKey.js | 19 ++++++- 3 files changed, 66 insertions(+), 14 deletions(-) diff --git a/src/server/logging/LogFormat.js b/src/server/logging/LogFormat.js index eb34d4fa4049..9bc7be017176 100644 --- a/src/server/logging/LogFormat.js +++ b/src/server/logging/LogFormat.js @@ -6,7 +6,7 @@ let ansicolors = require('ansicolors'); let stringify = require('json-stringify-safe'); let querystring = require('querystring'); let inspect = require('util').inspect; -let applyFilterToKey = require('./applyFilterToKey'); +let applyFiltersToKeys = require('./applyFilterToKey').applyFiltersToKeys; function serializeError(err) { return { @@ -36,12 +36,8 @@ module.exports = class TransformObjStream extends Stream.Transform { } filter(data) { - if (this.config.filter) { - _.each(this.config.filter, (action, key) => { - applyFilterToKey(data, key, action); - }); - } - return data; + if (!this.config.filter) return data; + return applyFiltersToKeys(data, this.config.filter); } _transform(event, enc, next) { diff --git a/src/server/logging/__tests__/applyFilterToKey.js b/src/server/logging/__tests__/applyFilterToKey.js index 1d91b2b4d961..776a12138ff8 100644 --- a/src/server/logging/__tests__/applyFilterToKey.js +++ b/src/server/logging/__tests__/applyFilterToKey.js @@ -1,4 +1,5 @@ var applyFilterToKey = require('../applyFilterToKey'); +var applyFiltersToKeys = applyFilterToKey.applyFiltersToKeys; var expect = require('expect.js'); function fixture() { @@ -15,7 +16,7 @@ describe('applyFilterToKey(obj, key, action)', function () { it('should remove a key from an object recursivly', function () { var data = fixture(); - applyFilterToKey(data, 'authorization', 'remove'); + data = applyFilterToKey(data, 'authorization', 'remove'); expect(data).to.eql({ req: { headers: {} } }); @@ -23,7 +24,7 @@ describe('applyFilterToKey(obj, key, action)', function () { it('should remove an entire branch', function () { var data = fixture(); - applyFilterToKey(data, 'headers', 'remove'); + data = applyFilterToKey(data, 'headers', 'remove'); expect(data).to.eql({ req: { } }); @@ -31,7 +32,7 @@ describe('applyFilterToKey(obj, key, action)', function () { it('should remove an entire branch with censor', function () { var data = fixture(); - applyFilterToKey(data, 'headers', 'censor'); + data = applyFilterToKey(data, 'headers', 'censor'); expect(data).to.eql({ req: { } }); @@ -39,7 +40,7 @@ describe('applyFilterToKey(obj, key, action)', function () { it('should censor a key in an object recursivly', function () { var data = fixture(); - applyFilterToKey(data, 'authorization', 'censor'); + data = applyFilterToKey(data, 'authorization', 'censor'); expect(data).to.eql({ req: { headers: { @@ -52,7 +53,7 @@ describe('applyFilterToKey(obj, key, action)', function () { it('should censor key with a RegEx in an object recursivly', function () { var data = fixture(); var regex = '/([^\\s]+)$/'; - applyFilterToKey(data, 'authorization', regex); + data = applyFilterToKey(data, 'authorization', regex); expect(data).to.eql({ req: { headers: { @@ -62,4 +63,44 @@ describe('applyFilterToKey(obj, key, action)', function () { }); }); + it('uses the JSON version of objects for serializaion', function () { + var data = applyFilterToKey({ + a: { + b: 1, + toJSON: () => ({ c: 10 }) + } + }, 'c', 'censor'); + + expect(data).to.eql({ + a: { + c: 'XX' + } + }); + }); + + describe('applyFiltersToKeys(obj, actionsByKey)', function () { + it('applies applyFilterToKey() for each key+prop in actionsByKey', function () { + var data = applyFiltersToKeys({ + a: { + b: { + c: 1 + }, + d: { + e: 'foobar' + } + } + }, { + b: 'remove', + e: 'censor', + }); + + expect(data).to.eql({ + a: { + d: { + e: 'XXXXXX', + }, + }, + }); + }); + }); }); diff --git a/src/server/logging/applyFilterToKey.js b/src/server/logging/applyFilterToKey.js index c3486e34b6f8..df399eb86732 100644 --- a/src/server/logging/applyFilterToKey.js +++ b/src/server/logging/applyFilterToKey.js @@ -1,8 +1,12 @@ +function toPojo(obj) { + return JSON.parse(JSON.stringify(obj)); +} + function replacer(match, group) { return (new Array(group.length + 1).join('X')); } -module.exports = function applyFilterToKey(obj, key, action) { +function apply(obj, key, action) { for (let k in obj) { if (obj.hasOwnProperty(k)) { let val = obj[k]; @@ -24,8 +28,19 @@ module.exports = function applyFilterToKey(obj, key, action) { } } } else if (typeof val === 'object') { - applyFilterToKey(val, key, action); + val = apply(val, key, action); } } } + return obj; +} + +module.exports = function applyFilterToKey(obj, key, action) { + return apply(toPojo(obj), key, action); +}; + +module.exports.applyFiltersToKeys = function (obj, actionsByKey) { + return Object.keys(actionsByKey).reduce((output, key) => { + return apply(output, key, actionsByKey[key]); + }, toPojo(obj)); }; From 18c4338e7a2be494b6446885185b0660d93cc62a Mon Sep 17 00:00:00 2001 From: Chris Cowan Date: Wed, 7 Oct 2015 18:29:51 -0700 Subject: [PATCH 6/6] Refactoring to simpliefy things a bit --- src/server/logging/LogFormat.js | 2 +- .../logging/__tests__/applyFilterToKey.js | 106 ------------------ .../logging/__tests__/applyFiltersToKeys.js | 39 +++++++ ...lyFilterToKey.js => applyFiltersToKeys.js} | 6 +- 4 files changed, 41 insertions(+), 112 deletions(-) delete mode 100644 src/server/logging/__tests__/applyFilterToKey.js create mode 100644 src/server/logging/__tests__/applyFiltersToKeys.js rename src/server/logging/{applyFilterToKey.js => applyFiltersToKeys.js} (85%) diff --git a/src/server/logging/LogFormat.js b/src/server/logging/LogFormat.js index 9bc7be017176..18832199fa91 100644 --- a/src/server/logging/LogFormat.js +++ b/src/server/logging/LogFormat.js @@ -6,7 +6,7 @@ let ansicolors = require('ansicolors'); let stringify = require('json-stringify-safe'); let querystring = require('querystring'); let inspect = require('util').inspect; -let applyFiltersToKeys = require('./applyFilterToKey').applyFiltersToKeys; +let applyFiltersToKeys = require('./applyFiltersToKeys'); function serializeError(err) { return { diff --git a/src/server/logging/__tests__/applyFilterToKey.js b/src/server/logging/__tests__/applyFilterToKey.js deleted file mode 100644 index 776a12138ff8..000000000000 --- a/src/server/logging/__tests__/applyFilterToKey.js +++ /dev/null @@ -1,106 +0,0 @@ -var applyFilterToKey = require('../applyFilterToKey'); -var applyFiltersToKeys = applyFilterToKey.applyFiltersToKeys; -var expect = require('expect.js'); - -function fixture() { - return { - req: { - headers: { - authorization: 'Basic dskd939k2i' - } - } - }; -} - -describe('applyFilterToKey(obj, key, action)', function () { - - it('should remove a key from an object recursivly', function () { - var data = fixture(); - data = applyFilterToKey(data, 'authorization', 'remove'); - expect(data).to.eql({ - req: { headers: {} } - }); - }); - - it('should remove an entire branch', function () { - var data = fixture(); - data = applyFilterToKey(data, 'headers', 'remove'); - expect(data).to.eql({ - req: { } - }); - }); - - it('should remove an entire branch with censor', function () { - var data = fixture(); - data = applyFilterToKey(data, 'headers', 'censor'); - expect(data).to.eql({ - req: { } - }); - }); - - it('should censor a key in an object recursivly', function () { - var data = fixture(); - data = applyFilterToKey(data, 'authorization', 'censor'); - expect(data).to.eql({ - req: { - headers: { - authorization: 'XXXXXXXXXXXXXXXX' - } - } - }); - }); - - it('should censor key with a RegEx in an object recursivly', function () { - var data = fixture(); - var regex = '/([^\\s]+)$/'; - data = applyFilterToKey(data, 'authorization', regex); - expect(data).to.eql({ - req: { - headers: { - authorization: 'Basic XXXXXXXXXX' - } - } - }); - }); - - it('uses the JSON version of objects for serializaion', function () { - var data = applyFilterToKey({ - a: { - b: 1, - toJSON: () => ({ c: 10 }) - } - }, 'c', 'censor'); - - expect(data).to.eql({ - a: { - c: 'XX' - } - }); - }); - - describe('applyFiltersToKeys(obj, actionsByKey)', function () { - it('applies applyFilterToKey() for each key+prop in actionsByKey', function () { - var data = applyFiltersToKeys({ - a: { - b: { - c: 1 - }, - d: { - e: 'foobar' - } - } - }, { - b: 'remove', - e: 'censor', - }); - - expect(data).to.eql({ - a: { - d: { - e: 'XXXXXX', - }, - }, - }); - }); - }); -}); diff --git a/src/server/logging/__tests__/applyFiltersToKeys.js b/src/server/logging/__tests__/applyFiltersToKeys.js new file mode 100644 index 000000000000..bb900f3fa2fb --- /dev/null +++ b/src/server/logging/__tests__/applyFiltersToKeys.js @@ -0,0 +1,39 @@ +var applyFiltersToKeys = require('../applyFiltersToKeys'); +var expect = require('expect.js'); + +describe('applyFiltersToKeys(obj, actionsByKey)', function () { + it('applies for each key+prop in actionsByKey', function () { + var data = applyFiltersToKeys({ + a: { + b: { + c: 1 + }, + d: { + e: 'foobar' + } + }, + req: { + headers: { + authorization: 'Basic dskd939k2i' + } + } + }, { + b: 'remove', + e: 'censor', + authorization: '/([^\\s]+)$/' + }); + + expect(data).to.eql({ + a: { + d: { + e: 'XXXXXX', + }, + }, + req: { + headers: { + authorization: 'Basic XXXXXXXXXX' + } + } + }); + }); +}); diff --git a/src/server/logging/applyFilterToKey.js b/src/server/logging/applyFiltersToKeys.js similarity index 85% rename from src/server/logging/applyFilterToKey.js rename to src/server/logging/applyFiltersToKeys.js index df399eb86732..860bdd38b76f 100644 --- a/src/server/logging/applyFilterToKey.js +++ b/src/server/logging/applyFiltersToKeys.js @@ -35,11 +35,7 @@ function apply(obj, key, action) { return obj; } -module.exports = function applyFilterToKey(obj, key, action) { - return apply(toPojo(obj), key, action); -}; - -module.exports.applyFiltersToKeys = function (obj, actionsByKey) { +module.exports = function (obj, actionsByKey) { return Object.keys(actionsByKey).reduce((output, key) => { return apply(output, key, actionsByKey[key]); }, toPojo(obj));