villadora / express-bunyan-logger

bunyan logger middleware for express
139 stars 74 forks source link

Added a 'deep exclusion' function #25

Closed jkerrwilson closed 8 years ago

jkerrwilson commented 9 years ago

Which allows specific properties of sub-objects to be uniquely chosen for exclusion.

We required this functionality to exclude potentially sensitive information from logs such as authorization tokens, or passwords send with authentication requests. The current excludes feature does not allow for partial logging of meta data, and we did not want to exclude the entire 'body' but just sensitive parts of it.

Note that the current excludes functionality is preserved.

To exclude object properties on a deeper level, an object can be passed as the excludes option:

        app.use(require('express-bunyan-logger')({
            logger: newLogger,
            excludes: {'body':['password']}
        }));

The function searches the object hierarchy provided and excludes only those properties specified in the base list. In the case above, any body properties called password will be removed. The "*" feature of the previous functionality is also preserved on sub-levels. The search is recursive, so more than one level can be specified.

jrthib commented 9 years ago

This would be really useful!

rimunroe commented 9 years ago

Has any progress been made on this? I can't think of a way I could safely use this package without being able to exclude passwords from the logged info.

Are there any workarounds that can be used in the meantime until this PR is fixed and merged?

jkerrwilson commented 9 years ago

Hey @rimunroe

I wrote this modification for a project we have at work, so I basically put in the required effort to make the feature work on our system. As far as I know, if you use this branch it still works; however, I am unable to spend the time to make CI tests pass, which I assume are required for a merge.

marcbachmann commented 9 years ago

I suggest you to use jsonpointer to exclude values. That simplifies everything. Just do a for loop and set the value to undefined. e.g.

app.use(require('express-bunyan-logger')({
  logger: newLogger,
  excludes: ['/body/password']
}));

excludes.forEach(function (pointer) { jsonpointer.set(meta, undefined) })
rtucker88 commented 8 years ago

@villadora is there any interest in merging this if the tests are passing? I would like to use this functionality and can likely fix the tests to pass to accommodate this change.

minispeck commented 8 years ago

@rtucker88 from the top of the readme as it currently reads:

This year as work content change, I have no spare time to maintaining the node modules, if anyone want to take or keep maintaining, just contact me via jky239@gmail.com with Title contains: "Wanted: npm package xxxx". Thx.

you should take ownership of the package, and i will happily use this new functionality :) we need it too.

marcbachmann commented 8 years ago

I can also merge it if it's working and there are some tests for it. Instead of adding that into this module, you can also implement that using a bunyan serializer: https://github.com/trentm/node-bunyan#standard-serializers

You just would have to add a function to handle a specific key. e.g.

var _ = require('lodash')

function exclude () {
  var keys = arguments
  return function (body) {
    return _.omit(body, keys)
  }
}

var logger = bunyan.createLogger({
    name: 'foo',
    serializers: {
      body: exclude('password')
    }
})

app.use(require('express-bunyan-logger')({logger: logger}))
minispeck commented 8 years ago

@marcbachmann thank you so much!!! I tried many ways to get my res serializer to exclude certain fields from the body. I couldn't find any info on how to do this or much detail about serializers in general.

would i still have a res serializer? would both be used in the case of a res with a body? I'm a little unclear on how these work as all the examples i've seen are the top level (req, res, err)

thanks!

marcbachmann commented 8 years ago

@marcbachmann thank you so much!!! I tried many ways to get my res serializer to exclude certain fields from the body. I couldn't find any info on how to do this or much detail about serializers in general.

You're welcome

would i still have a res serializer? would both be used in the case of a res with a body? I'm a little unclear on how these work as all the examples i've seen are the top level (req, res, err)

No, if you want to modify req.body and still keep the other serializers, you can do something like this:

var serializers = {
  err: bunyan.stdSerializers.err,
  req: function (req) {
    req = bunyan.stdSerializers.req(req)
    req.body = _.omit(req.body, 'password')
    return req
  },
  res: bunyan.stdSerializers.res 
}

var app = express()
app.use(require('express-bunyan-logger')({
    serializers: serializers
}));
minispeck commented 8 years ago

@marcbachmann : i tried both your suggested modified req serializer (console.log shows body is not a key of req when doing it that way) and the body serializer (console.log shows body is empty) on requests that i know contain body and indeed are logging body in trace logs. any ideas?

EDIT I ended up using shutterstock's fork which has a deep obfuscate functionality and is passing all tests - @marcbachmann you should pull their fork in

marcbachmann commented 8 years ago

the obfuscate option should cover this issue: https://github.com/villadora/express-bunyan-logger#optionsobfuscate