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

Prevent sensative information from being logged #65

Merged
merged 5 commits into from
May 1, 2019

Conversation

wwsean08
Copy link
Contributor

@wwsean08 wwsean08 commented Apr 28, 2019

This change uses the SafeExceptionReportFilter when logging the body as outlined in issue #64 and also fixes #64. I have done some basic testing with a simple django application which uses the sensitive_post_parameters decorator and seen that the fields are filtered out.

One caveat is that this does change the behavior of logging the body, the decorator that specifies the variables to filter doesn't run until the view is called so logging the body needs to come after self.get_response( request ) otherwise it'll still have sensitive information in plaintest. Here's an example of what I get using this now on an application using allauth

{'csrfmiddlewaretoken': 'xAXmfqD1PYMttgqseguGHh1FQxWPkhxVPu07BHvXA9MICiNtSJJbU1pXrkfzVTFr', 'login': '[email protected]', 'password': '********************', 'next': '/'}

Previously the password would have been revealed. Also because SafeExceptionReportFilter is based on request.POST instead of request.body I have left a fallback in case that doesn't exist, I don't think that'll ever get hit though.

This change logs the body of the request after the view has run in order to allow django to mark sensative post parameters and cause them to be replaced with astericts in the logs.  See https://docs.djangoproject.com/en/2.2/howto/error-reporting/#django.views.decorators.debug.sensitive_post_parameters for more info
@tclancy
Copy link
Contributor

tclancy commented May 22, 2019

@wwsean08 or @kennethjiang can you submit a PR either backing this out or fixing #67 ?

@kennethjiang
Copy link
Contributor

Sorry for missing your question earlier @tclancy . Not sure about @wwsean08 but I can't reproduce the problem mentioned in #67

@tclancy
Copy link
Contributor

tclancy commented May 22, 2019

There's a test case to do it in the ticket.

@wwsean08
Copy link
Contributor Author

wwsean08 commented May 22, 2019

I'll look into #67 tonight and see if i can come up with a fix for the test case as I never ran into that during my testing.

@wwsean08 wwsean08 mentioned this pull request May 23, 2019
tclancy pushed a commit that referenced this pull request May 23, 2019
* Store off the request body to prevent django.http.request.RawPostDataException

* Revert broken changes to prevent logging sensitive data
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.

sensitive_post_parameters not honored
3 participants