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

Redacting sensitive headers when dumping request/response #226

Open
mortenlj opened this issue Oct 3, 2018 · 3 comments · May be fixed by #263
Open

Redacting sensitive headers when dumping request/response #226

mortenlj opened this issue Oct 3, 2018 · 3 comments · May be fixed by #263

Comments

@mortenlj
Copy link

mortenlj commented Oct 3, 2018

In our project we have implemented dumping of request and response when debug-mode is enabled. Since this might end up in logfiles or other places where sensitive data is unwanted, we implemented a blacklist for sensitive headers.

Could something similar be implemented in the dump_response and dump_all methods here? Then we could get rid of our own implementation and just use toolbelt. Maybe make it optional?

Feel free to look at/borrow from our implementation here: https://github.com/fiaas/k8s/blob/master/k8s/client.py

@sigmavirus24
Copy link
Collaborator

I'd be happy to merge a pull request that adds that functionality. I think it is a sensible addition. I'm wondering what the best interface for that would be, though.

I think the most versatile would be something where the toolbelt accepts a function. It calls the function with request_headers, request_body, response_headers, and response_body as parameters and (where the headers values are dictionaries, the body values are bytes) and receives those back modified (or not) and uses the new values for display.

That would allow users to do whatever level of sanitizing they want.

I'm wondering if it makes sense to include the URLs too since people might consider there to be sensitive information there.

One part of me wishes that it were easy to "clone" a Response object from requests so we could just pass that in, but it isn't.

@mortenlj
Copy link
Author

mortenlj commented Oct 3, 2018

I can see if I get the time to implement something. I do believe it would be useful with a "default" implementation that does something reasonably sane, but with the option of supplying your own sanitizer.

If I make a default sanitizer, should it be enabled by default, or should the default behavior be to not do any sanitizing at all? I guess the principle of least surprise dictates that there should be no sanitizing by default.

@mortenlj
Copy link
Author

mortenlj commented Oct 4, 2018

Hi again.

I have had a go at this now, and I don't think a common function to handle both request and response will work with the current code. If we had a simple way to clone a Response object then we could just manipulate that, but since that is not possible and the code for handling request and response is split the single function won't work.

If we start talking about multiple functions, then I think it makes sense to send in an object that implements a suitable interface. We can then have two implementations, one that does nothing and is default, and one that redacts the headers we consider risky. The user is free to make their own implementation and pass that in instead.

It would make the implementation quite a bit easier, although I appreciate that it feels a bit more enterprisy 🙂

I will start working in this direction, let me know if you don't like it. I hope to have something for you to review either today or tomorrow.

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