Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an headerBlacklist option #217

Merged
merged 3 commits into from
Aug 6, 2019
Merged

Add an headerBlacklist option #217

merged 3 commits into from
Aug 6, 2019

Conversation

maxday
Copy link
Contributor

@maxday maxday commented Jul 26, 2019

Hello !

The use case here is to prevent logging some sensitive content stored in headers (such as cookies or authorization bearer tokens)

I know this is already achievable using requestFilter option, but it seems to me that obfuscating headers is a common practice which should not require extra code in requestFilter.

What do you think ?

Content of this PR :

  • new option : headerBlacklist
  • default option value : []
  • tests
  • deletion of one duplicated test

Thanks for reviewing this PR, let me know what do you think, I tried to ship production code ready but any comments is appreciated.

:)

@maxday
Copy link
Contributor Author

maxday commented Aug 6, 2019

Hi, @golopot @bithavoc
Did you get a chance to review this PR ?
Thanks !

@bithavoc
Copy link
Owner

bithavoc commented Aug 6, 2019

@maxday Sorry I missed this. looks good, I see multiple commits though, want me to squash and merge or do you prefer to squash them yourself?

@maxday
Copy link
Contributor Author

maxday commented Aug 6, 2019

@bithavoc no worries, you can squash and merge from your side !
thanks

@bithavoc bithavoc merged commit dc119ac into bithavoc:master Aug 6, 2019
@bithavoc
Copy link
Owner

bithavoc commented Aug 6, 2019

Released express-winston@3.3.0

Thank you @maxday

@alokrajiv
Copy link
Contributor

Think the typescript def for this really useful LoggerOption was missed. @bithavoc I've added a PR #228. Would you be able to help release it, if it looks fine?

Thanks for the wonderful PR @maxday. ( and @bithavoc ofcourse- for the package itself!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants