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.

Sign up for GitHub

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
Merged

Prevent sensative information from being logged #65

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
47a8471
Rearrange logging to prevent logging secrets
wwsean08 Apr 28, 2019
84254d4
use chunked_to_max and cleanup a long line
wwsean08 Apr 28, 2019
458e369
Fix failing unit tests
wwsean08 Apr 28, 2019
8671dc5
simplify the logic and add a unit test to validate that the sensitive…
wwsean08 Apr 30, 2019
735631d
Make changes based on feedback
wwsean08 May 1, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions request_logging/middleware.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
# Django < 1.10
from django.core.urlresolvers import resolve, Resolver404
from django.utils.termcolors import colorize
from django.views.debug import SafeExceptionReporterFilter

DEFAULT_LOG_LEVEL = logging.DEBUG
DEFAULT_HTTP_4XX_LOG_LEVEL = logging.ERROR
Expand Down Expand Up @@ -95,6 +96,7 @@ def __init__(self, get_response=None):
def __call__(self, request):
self.process_request( request )
response = self.get_response( request )
self.process_body(request)
self.process_response( request, response )
return response

Expand Down Expand Up @@ -153,24 +155,28 @@ def _log_request(self, request):
logging_context = self._get_logging_context(request, None)
self.logger.log(logging.INFO, method_path, logging_context)
self._log_request_headers(request, logging_context)
self._log_request_body(request, logging_context)

def _log_request_headers(self, request, logging_context):
headers = {k: v for k, v in request.META.items() if k.startswith('HTTP_')}

if headers:
self.logger.log(self.log_level, headers, logging_context)

def _log_request_body(self, request, logging_context):
def process_body(self, request):
if request.body:
content_type = request.META.get('CONTENT_TYPE', '')
is_multipart = content_type.startswith('multipart/form-data')
if is_multipart:
self.boundary = '--' + content_type[30:] # First 30 characters are "multipart/form-data; boundary="
if is_multipart:
self._log_multipart(self._chunked_to_max(request.body), logging_context)
else:
self.logger.log(self.log_level, self._chunked_to_max(request.body), logging_context)
if not self._should_log_route(request):
wwsean08 marked this conversation as resolved.
Show resolved Hide resolved
logging_context = self._get_logging_context(request, None)
content_type = request.META.get('CONTENT_TYPE', '')
is_multipart = content_type.startswith('multipart/form-data')
if is_multipart:
self.boundary = '--' + content_type[30:] # First 30 characters are "multipart/form-data; boundary="
self._log_multipart(self._chunked_to_max(request.body), logging_context)
else:
if request.POST:
safe_body = SafeExceptionReporterFilter().get_post_parameters(request).dict()
self.logger.log(self.log_level, self._chunked_to_max(str(safe_body)), logging_context)
else:
self.logger.log(self.log_level, self._chunked_to_max(request.body), logging_context)

def process_response(self, request, response):
resp_log = "{} {} - {}".format(request.method, request.get_full_path(), response.status_code)
Expand Down
8 changes: 6 additions & 2 deletions tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ def test_no_exception_risen(self, mock_log):
body = u"some body"
request = self.factory.post("/a-missing-route-somewhere", data={"file": body})
self.middleware.process_request(request)
self.middleware.process_body(request)
self._assert_logged(mock_log, body)


Expand All @@ -79,13 +80,15 @@ def test_request_body_logged(self, mock_log):
body = u"some body"
request = self.factory.post("/somewhere", data={"file": body})
self.middleware.process_request(request)
self.middleware.process_body(request)
self._assert_logged(mock_log, body)

def test_request_binary_logged(self, mock_log):
body = u"some body"
datafile = io.StringIO(body)
request = self.factory.post("/somewhere", data={"file": datafile})
self.middleware.process_request(request)
self.middleware.process_body(request)
self._assert_logged(mock_log, "(binary data)")

def test_request_jpeg_logged(self, mock_log):
Expand All @@ -96,6 +99,7 @@ def test_request_jpeg_logged(self, mock_log):
datafile = io.BytesIO(body)
request = self.factory.post("/somewhere", data={"file": datafile})
self.middleware.process_request(request)
self.middleware.process_body(request)
self._assert_logged(mock_log, "(multipart/form)")

def test_request_headers_logged(self, mock_log):
Expand Down Expand Up @@ -315,7 +319,7 @@ def test_default_max_body_length(self, mock_log):

body = DEFAULT_MAX_BODY_LENGTH * "0" + "1"
request = factory.post("/somewhere", data={"file": body})
middleware.process_request(request)
middleware.process_body(request)

request_body_str = request.body if isinstance(request.body, str) else request.body.decode()
self._assert_logged(mock_log, re.sub(r'\r?\n', '', request_body_str[:DEFAULT_MAX_BODY_LENGTH]))
Expand All @@ -328,7 +332,7 @@ def test_customized_max_body_length(self, mock_log):

body = 150 * "0" + "1"
request = factory.post("/somewhere", data={"file": body})
middleware.process_request(request)
middleware.process_body(request)

request_body_str = request.body if isinstance(request.body, str) else request.body.decode()
self._assert_logged(mock_log, re.sub(r'\r?\n', '', request_body_str[:150]))
Expand Down