search

markabrahams / node-net-snmp

JavaScript implementation of the Simple Network Management Protocol (SNMP)
207 stars 97 forks source link

Auth information in the notification callback #201

Closed tim-brand closed 2 years ago

tim-brand commented 2 years ago

In our solution we're using the community value to route the message to it's destination. It might be helpful for others too, so I added this as an option to the receiver options. The options is disabled by default, so that the information does not gets included somewhere by accident.

markabrahams commented 2 years ago

Hi @tim-brand - thanks for the PR!

Two things I'm contemplating before merging this: (1) Since "community" and "user" are PDU items, and are already modelled as fields under message.pdu, it seems that having these appear at that same place i.e. "pdu.user" or "pdu.community" is more consistent with the handling of these elsewhere. (2) I don't think logging auth and priv keys is necessary or desirable here, so I'm thinking it might be best to remove these from the user sub-object (logging all other user fields is fine).

How do those sound to you?

tim-brand commented 2 years ago

Hi @markabrahams, thanks for the good notes. I totally agree and will perform the changes.

tim-brand commented 2 years ago

@markabrahams Could you check this again? Thanks!

markabrahams commented 2 years ago

Hi @tim-brand - thanks for that!

I've merged the PR with a couple of changes: (1) Changed the value of the "user" key from the whole "user" object to just the name i.e. "user.name" (2) Changed the name of the option from "showAuthInCallback" to "includeAuthentication"

This is published in version 3.6.2 of the npm.

tim-brand commented 2 years ago

Sorry, I totally forgot to only use the name of the user object. It was my intention to do so. Thanks for the change and npm publish!