feat: strip sensitive parameters if specified

dadi/index.js

@@ -3,6 +3,7 @@
  */
 const bunyan = require('bunyan') // underlying logger
 const KinesisStream = require('aws-kinesis-writable') // kinesis
+const LogFilter = require('@dadi/log-filter')
 const mkdirp = require('mkdirp') // recursive mkdir
 const moment = require('moment') // datestamps and timing
 const path = require('path')
@@ -196,11 +197,14 @@ const self = (module.exports = {
         clientIpAddress = getClientIpAddress(req.headers['x-forwarded-for'])
       }
 
+      let logFilter = new LogFilter(req, self.options.filter || [])
+      let requestPath = logFilter.filterPath()
+
       let accessRecord =
         `${clientIpAddress || ''}` +
         ` -` +
         ` ${moment().format()}` +
-        ` ${req.method} ${req.url} HTTP/ ${req.httpVersion}` +
+        ` ${req.method} ${requestPath} HTTP/ ${req.httpVersion}` +
         ` ${res.statusCode}` +
         ` ${
           res.getHeader('content-length') ? res.getHeader('content-length') : ''
@@ -214,7 +218,7 @@ const self = (module.exports = {
       // log the request method and url, and the duration
       self.info(
         { module: 'router' },
-        `${req.method} ${req.url} ${res.statusCode} ${duration}ms`
+        `${req.method} ${requestPath} ${res.statusCode} ${duration}ms`
       )
 
       if (trackRequestCount) stats.requests++

package.json

@@ -34,6 +34,7 @@
   "author": "DADI <team@dadi.cloud>",
   "license": "ISC",
   "dependencies": {
+    "@dadi/log-filter": "^0.0.1",
     "aws-kinesis-writable": "^2.0.0",
     "bunyan": "^1.8.5",
     "kinesis": "^1.2.2",
@@ -46,7 +47,6 @@
     "coveralls": "^3.0.1",
     "greenkeeper-postpublish": "^1.0.1",
     "istanbul": "^0.4.5",
-    "istanbul-cobertura-badger": "^1.2.1",
     "jasmine": "^2.5.2",
     "lint-staged": "^7.2.0",
     "mocha": "^5.2.0",

test/requestLogger.js

@@ -15,6 +15,7 @@ function generateMockRequestAndResponse (statusCode, forwarded, ip, url) {
       remoteAddress: ip || '8.8.8.8'
     },
     headers: {
+      host: 'http://0.0.0.0',
       referer: 'http://google.com',
       'user-agent':
         'Mozilla/5.0 (Windows NT x.y; WOW64; rv:10.0) Gecko/20100101 Firefox/10.0'
@@ -77,7 +78,7 @@ describe('Request Logger', function () {
     memstream.on('data', function (chunk) {
       let output = JSON.parse(chunk.toString())
       chunks++
-      if (output.name === 'dadi.test') {
+      if (output.name === 'dadi-test') {
         assert(
           output.msg.indexOf('GET /test') !== -1,
           'contains method and path'
@@ -109,7 +110,7 @@ describe('Request Logger', function () {
     memstream.on('data', function (chunk) {
       let output = JSON.parse(chunk.toString())
       chunks++
-      if (output.name === 'dadi.test') {
+      if (output.name === 'dadi-test') {
         assert(
           output.msg.indexOf('GET /test') !== -1,
           'contains method and path'
@@ -141,7 +142,7 @@ describe('Request Logger', function () {
     memstream.on('data', function (chunk) {
       let output = JSON.parse(chunk.toString())
       chunks++
-      if (output.name === 'dadi.test') {
+      if (output.name === 'dadi-test') {
         assert(
           output.msg.indexOf('GET /test') !== -1,
           'contains method and path'
@@ -174,7 +175,8 @@ describe('Request Logger', function () {
     memstream.on('data', function (chunk) {
       let output = JSON.parse(chunk.toString())
       chunks++
-      if (output.name === 'dadi.test') {
+
+      if (output.name === 'dadi-test') {
         assert(
           output.msg.indexOf('GET /test') !== -1,
           'contains method and path'
@@ -199,4 +201,108 @@ describe('Request Logger', function () {
     })
     logger.requestLogger(testHttp.req, testHttp.res, testHttp.next) // fire
   })
+
+  describe('Filtered parameters', function () {
+    beforeEach(function (done) {
+    // our logger is a singleton, but we need a clean instance
+      delete require.cache[require.resolve('./../dadi/index.js')]
+      logger = require('./../dadi/index.js')
+      memstream = new MemoryStream() // save ourselves from the fs rabbit hole
+
+      done()
+    })
+
+    it('should remove sensitive parameters from querystring when writing to log', function (done) {
+      logger.init(
+        {
+          accessLog: {
+            enabled: true
+          },
+          enabled: true,
+          filename: 'test',
+          level: 'trace',
+          path: 'log/',
+          stream: memstream,
+          filter: ['password']
+        },
+        null,
+        'test'
+      )
+
+      let testHttp = generateMockRequestAndResponse()
+      let chunks = 0
+
+      testHttp.req.url = testHttp.req.url + '?username=ed&password=octopus'
+
+      memstream.on('data', function (chunk) {
+        let output = JSON.parse(chunk.toString())
+        chunks++
+
+        if (output.name === 'dadi-test') {
+          assert(
+            output.msg.indexOf('octopus') === -1,
+            'path contains unfiltered parameters'
+          )
+        } else if (output.name === 'access') {
+          assert(
+            output.msg.indexOf('octopus') === -1,
+            'path contains unfiltered parameters'
+          )
+        }
+
+        if (chunks >= 2) {
+          memstream.removeAllListeners('data')
+          return done() // only finish after accesslog and info
+        }
+      })
+
+      logger.requestLogger(testHttp.req, testHttp.res, testHttp.next) // fire
+    })
+
+    it('should not remove unspecified parameters from querystring when writing to log', function (done) {
+      logger.init(
+        {
+          accessLog: {
+            enabled: true
+          },
+          enabled: true,
+          filename: 'test',
+          level: 'trace',
+          path: 'log/',
+          stream: memstream
+        },
+        null,
+        'test'
+      )
+
+      let testHttp = generateMockRequestAndResponse()
+      let chunks = 0
+
+      testHttp.req.url = testHttp.req.url + '?username=ed&password=octopus'
+
+      memstream.on('data', function (chunk) {
+        let output = JSON.parse(chunk.toString())
+        chunks++
+
+        if (output.name === 'dadi-test') {
+          assert(
+            output.msg.indexOf('octopus') > 0,
+            'path does not contain all parameters'
+          )
+        } else if (output.name === 'access') {
+          assert(
+            output.msg.indexOf('octopus') > 0,
+            'path does not contain all parameters'
+          )
+        }
+
+        if (chunks >= 2) {
+          memstream.removeAllListeners('data')
+          return done() // only finish after accesslog and info
+        }
+      })
+
+      logger.requestLogger(testHttp.req, testHttp.res, testHttp.next) // fire
+    })
+  })
 })