From 2c38f18a33a3d1e37994cdbcf4aa1397a241ab66 Mon Sep 17 00:00:00 2001 From: Justin Wind Date: Fri, 25 Mar 2022 15:55:59 -0700 Subject: [PATCH] split logger into separate module with minor improvements --- package-lock.json | 10 ++-- package.json | 1 + src/logger.js | 91 ----------------------------------- src/logger/data-sanitizers.js | 25 ++++++++++ src/logger/index.js | 13 +++++ test/src/logger.js | 40 ++++++--------- 6 files changed, 61 insertions(+), 119 deletions(-) delete mode 100644 src/logger.js create mode 100644 src/logger/data-sanitizers.js create mode 100644 src/logger/index.js diff --git a/package-lock.json b/package-lock.json index 249034b..eb88815 100644 --- a/package-lock.json +++ b/package-lock.json @@ -534,6 +534,10 @@ "microformats-parser": "^1.4.1" } }, + "@squeep/logger-json-console": { + "version": "git+https://git.squeep.com/squeep-logger-json-console#229dafe0003708b9fad190b4c0fc68136efd4f8c", + "from": "git+https://git.squeep.com/squeep-logger-json-console#v1.0.0" + }, "@squeep/mystery-box": { "version": "git+https://git.squeep.com/squeep-mystery-box/#c47d2327a3a642792e9521677cd5f551e9aa7bfb", "from": "git+https://git.squeep.com/squeep-mystery-box/#v1.0.4" @@ -2248,9 +2252,9 @@ } }, "minimist": { - "version": "1.2.5", - "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.5.tgz", - "integrity": "sha512-FM9nNUYrRBAELZQT3xeZQ7fmMOBg6nWNmJKTcgsJeaLstP/UODVpGsr5OhXhhXg6f+qtJ8uiZ+PUxkDWcgIXLw==" + "version": "1.2.6", + "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.6.tgz", + "integrity": "sha512-Jsjnk4bw3YJqYzbdyBiNsPWHPfO++UGG749Cxs6peCu5Xg4nrena6OVxOYxrQTqww0Jmwt+Ref8rggumkTLz9Q==" }, "minipass": { "version": "3.1.6", diff --git a/package.json b/package.json index 188f2d6..c8d3399 100644 --- a/package.json +++ b/package.json @@ -35,6 +35,7 @@ "@squeep/api-dingus": "git+https://git.squeep.com/squeep-api-dingus/#v1.2.5", "@squeep/authentication-module": "git+https://git.squeep.com/squeep-authentication-module/#v1.2.1", "@squeep/html-template-helper": "git+https://git.squeep.com/squeep-html-template-helper#v1.0.4", + "@squeep/logger-json-console": "git+https://git.squeep.com/squeep-logger-json-console#v1.0.0", "@squeep/web-linking": "git+https://git.squeep.com/squeep-web-linking/#v1.0.4", "axios": "^0.26.1", "feedparser": "^2.2.10", diff --git a/src/logger.js b/src/logger.js deleted file mode 100644 index 00edfb8..0000000 --- a/src/logger.js +++ /dev/null @@ -1,91 +0,0 @@ -'use strict'; - -/** - * Log as JSON to stdout/stderr. - */ - -const common = require('./common'); - -// This is uncomfortable, but is the simplest way to let logging work for BigInts. -// TODO: revisit with better solution -BigInt.prototype.toJSON = function() { - return this.toString(); -} - -// Also uncomfortable, but let us log Errors reasonably. -Object.defineProperty(Error.prototype, 'toJSON', { - configurable: true, - writable: true, // Required to let Axios override on its own Errors - value: function () { - const result = {}; - const dupKey = function (key) { - // eslint-disable-next-line security/detect-object-injection - result[key] = this[key]; - }; - Object.getOwnPropertyNames(this) - // .filter((prop) => !(prop in [])) - .forEach(dupKey, this); - return result; - }, -}); - -class Logger { - /** - * Wrap backend calls with payload normalization. - * @param {Object} options - * @param {*} backend Console style interface - * @param {Object} options.logger - * @param {String} options.logger.ignoreBelowLevel minimum level to log - * @param {String} options.nodeId - */ - constructor(options, backend = console) { - const logLevels = Object.keys(common.nullLogger); - const ignoreBelowLevel = options && options.logger && options.logger.ignoreBelowLevel || 'debug'; - this.nodeId = options.nodeId; - - if (!logLevels.includes(ignoreBelowLevel)) { - throw new RangeError(`unrecognized minimum log level '${ignoreBelowLevel}'`); - } - const ignoreLevelIdx = logLevels.indexOf(ignoreBelowLevel); - logLevels.forEach((level) => { - // eslint-disable-next-line security/detect-object-injection - this[level] = (logLevels.indexOf(level) > ignoreLevelIdx) ? - common.nop : - this.levelTemplateFn(backend, level); - }); - } - - levelTemplateFn(backend, level) { - // eslint-disable-next-line security/detect-object-injection - if (!(level in backend) || typeof backend[level] !== 'function') { - return common.nop; - } - - // eslint-disable-next-line security/detect-object-injection - return (...args) => backend[level](this.payload(level, ...args)); - } - - payload(level, scope, message, data, ...other) { - // Try to keep credentials out of logs. - // This approach feels sort of jank, but it's better than nothing, for now. - if (data && data.ctx && data.ctx.parsedBody && data.ctx.parsedBody.credential) { - // Create copy of data - data = JSON.parse(JSON.stringify(data)); - data.ctx.parsedBody.credential = '*'.repeat(data.ctx.parsedBody.credential.length); - } - - const now = new Date(); - return JSON.stringify({ - nodeId: this.nodeId, - timestamp: now.toISOString(), - timestampMs: now.getTime(), - level: level, - scope: scope || '[unknown]', - message: message || '', - data: data || {}, - ...(other.length && { other }), - }); - } -} - -module.exports = Logger; diff --git a/src/logger/data-sanitizers.js b/src/logger/data-sanitizers.js new file mode 100644 index 0000000..a6b444b --- /dev/null +++ b/src/logger/data-sanitizers.js @@ -0,0 +1,25 @@ +'use strict'; + +/** + * Scrub credential from POST login body data. + * @param {Object} data + * @param {Boolean} sanitize + * @returns {Boolean} + */ +function sanitizePostCredential(data, sanitize = true) { + let unclean = false; + + const credentialLength = data && data.ctx && data.ctx.parsedBody && data.ctx.parsedBody.credential && data.ctx.parsedBody.credential.length; + if (credentialLength) { + unclean = true; + } + if (unclean && sanitize) { + data.ctx.parsedBody.credential = '*'.repeat(credentialLength); + } + + return unclean; +} + +module.exports = { + sanitizePostCredential, +}; \ No newline at end of file diff --git a/src/logger/index.js b/src/logger/index.js new file mode 100644 index 0000000..7944cf6 --- /dev/null +++ b/src/logger/index.js @@ -0,0 +1,13 @@ +'use strict'; + +const BaseLogger = require('@squeep/logger-json-console'); +const dataSanitizers = require('./data-sanitizers'); + +class Logger extends BaseLogger { + constructor(options, ...args) { + super(options, ...args); + Array.prototype.push.apply(this.dataSanitizers, Object.values(dataSanitizers)); + } +} + +module.exports = Logger; \ No newline at end of file diff --git a/test/src/logger.js b/test/src/logger.js index fc602aa..47017dd 100644 --- a/test/src/logger.js +++ b/test/src/logger.js @@ -2,6 +2,7 @@ 'use strict'; const assert = require('assert'); +const sinon = require('sinon'); // eslint-disable-line node/no-unpublished-require const Logger = require('../../src/logger'); const Config = require('../../config'); @@ -11,46 +12,32 @@ describe('Logger', function () { beforeEach(function () { config = new Config('test'); + logger = new Logger(config); + Object.keys(Logger.nullLogger).forEach((level) => sinon.stub(logger.backend, level)); }); - it('logs', function () { - logger = new Logger(config); - logger.info('testScope', 'message', { baz: 'quux' }, { foo: 1 }, 'more other'); + afterEach(function () { + sinon.restore(); }); - it('stubs missing levels', function () { - const backend = {}; - logger = new Logger(config, backend); - assert.strictEqual(typeof logger.info, 'function'); + it('logs', function () { + logger.info('testScope', 'message', { baz: 'quux' }, { foo: 1 }, 'more other'); + assert(logger.backend.info.called); }); it('logs BigInts', function () { - logger = new Logger(config); logger.info('testScope', 'message', { aBigInteger: BigInt(2) }); + assert(logger.backend.info.called); + assert(logger.backend.info.args[0][0].includes('"2"')); }); it('logs Errors', function () { - logger = new Logger(config); logger.error('testScope', 'message', { e: new Error('an error') }); - }); - - it('covers config error', function () { - config.logger.ignoreBelowLevel = 'not a level'; - try { - logger = new Logger(config); - assert.fail('expected RangeError here'); - } catch (e) { - assert(e instanceof RangeError); - } - }); - - it('covers empty fields', function () { - logger = new Logger(config); - logger.info(); + assert(logger.backend.error.called); + assert(logger.backend.error.args[0][0].includes('an error')); }); it('masks credentials', function () { - logger = new Logger(config); logger.info('testScope', 'message', { ctx: { parsedBody: { @@ -59,6 +46,9 @@ describe('Logger', function () { }, }, }); + assert(logger.backend.info.called); + assert(logger.backend.info.args[0][0].includes('"username"')); + assert(logger.backend.info.args[0][0].includes('"********"')); }); }); // Logger -- 2.43.2