From 55fec16aafed30a9daa06d6ecdf8ca3ad361279e Mon Sep 17 00:00:00 2001 From: Holly Becker Date: Thu, 2 Jun 2016 17:24:48 -0700 Subject: [PATCH] Fixed #26628 -- Changed CSRF logger to django.security.csrf. --- django/middleware/csrf.py | 2 +- docs/ref/csrf.txt | 11 +++++++++-- docs/releases/1.11.txt | 3 +++ docs/topics/logging.txt | 15 +++++++++++---- tests/csrf_tests/tests.py | 39 +++++++++++++++++++++++++-------------- 5 files changed, 49 insertions(+), 21 deletions(-) diff --git a/django/middleware/csrf.py b/django/middleware/csrf.py index 7978620ece8..3f9a649c275 100644 --- a/django/middleware/csrf.py +++ b/django/middleware/csrf.py @@ -20,7 +20,7 @@ from django.utils.http import is_same_domain from django.utils.six.moves import zip from django.utils.six.moves.urllib.parse import urlparse -logger = logging.getLogger('django.request') +logger = logging.getLogger('django.security.csrf') REASON_NO_REFERER = "Referer checking failed - no Referer." REASON_BAD_REFERER = "Referer checking failed - %s does not match any trusted origins." diff --git a/docs/ref/csrf.txt b/docs/ref/csrf.txt index 16290cbfb5e..7baf83f2e5a 100644 --- a/docs/ref/csrf.txt +++ b/docs/ref/csrf.txt @@ -192,6 +192,8 @@ both is fine, and will incur minimal overhead. If you are using class-based views, you can refer to :ref:`Decorating class-based views`. +.. _csrf-rejected-requests: + Rejected requests ================= @@ -205,8 +207,13 @@ The error page, however, is not very friendly, so you may want to provide your own view for handling this condition. To do this, simply set the :setting:`CSRF_FAILURE_VIEW` setting. -CSRF failures are logged as warnings to the :ref:`django-request-logger` -logger. +CSRF failures are logged as warnings to the :ref:`django.security.csrf +` logger. + +.. versionchanged:: 1.11 + + In older versions, CSRF failures are logged to the ``django.request`` + logger. .. _how-csrf-works: diff --git a/docs/releases/1.11.txt b/docs/releases/1.11.txt index 85025753c36..ae3f3af4387 100644 --- a/docs/releases/1.11.txt +++ b/docs/releases/1.11.txt @@ -246,6 +246,9 @@ Miscellaneous * Support for SpatiaLite < 4.0 is dropped. +* CSRF failures are logged to the ``django.security.csrf ``` logger instead of + ``django.request``. + .. _deprecated-features-1.11: Features deprecated in 1.11 diff --git a/docs/topics/logging.txt b/docs/topics/logging.txt index 6388018ff53..bee525d6277 100644 --- a/docs/topics/logging.txt +++ b/docs/topics/logging.txt @@ -532,20 +532,23 @@ This logging does not include framework-level initialization (e.g. ``COMMIT``, and ``ROLLBACK``). Turn on query logging in your database if you wish to view all database queries. +.. _django-security-logger: + ``django.security.*`` ~~~~~~~~~~~~~~~~~~~~~~ The security loggers will receive messages on any occurrence of -:exc:`~django.core.exceptions.SuspiciousOperation`. There is a sub-logger for -each sub-type of SuspiciousOperation. The level of the log event depends on -where the exception is handled. Most occurrences are logged as a warning, while +:exc:`~django.core.exceptions.SuspiciousOperation` and other security-related +errors. There is a sub-logger for each subtype of security error, including all +``SuspiciousOperation``\s. The level of the log event depends on where the +exception is handled. Most occurrences are logged as a warning, while any ``SuspiciousOperation`` that reaches the WSGI handler will be logged as an error. For example, when an HTTP ``Host`` header is included in a request from a client that does not match :setting:`ALLOWED_HOSTS`, Django will return a 400 response, and an error message will be logged to the ``django.security.DisallowedHost`` logger. -These log events will reach the 'django' logger by default, which mails error +These log events will reach the ``django`` logger by default, which mails error events to admins when ``DEBUG=False``. Requests resulting in a 400 response due to a ``SuspiciousOperation`` will not be logged to the ``django.request`` logger, but only to the ``django.security`` logger. @@ -567,6 +570,10 @@ specific logger following this example: }, }, +Other ``django.security`` loggers not based on ``SuspiciousOperation`` are: + +* ``django.security.csrf``: For :ref:`CSRF failures `. + ``django.db.backends.schema`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/tests/csrf_tests/tests.py b/tests/csrf_tests/tests.py index 5ed7c9dc3dd..07eba4f9620 100644 --- a/tests/csrf_tests/tests.py +++ b/tests/csrf_tests/tests.py @@ -8,12 +8,13 @@ import warnings from django.conf import settings from django.http import HttpRequest, HttpResponse from django.middleware.csrf import ( - CSRF_TOKEN_LENGTH, CsrfViewMiddleware, - _compare_salted_tokens as equivalent_tokens, get_token, + CSRF_TOKEN_LENGTH, REASON_BAD_TOKEN, REASON_NO_CSRF_COOKIE, + CsrfViewMiddleware, _compare_salted_tokens as equivalent_tokens, get_token, ) from django.template import RequestContext, Template from django.template.context_processors import csrf from django.test import SimpleTestCase, override_settings +from django.test.utils import patch_logger from django.utils.encoding import force_bytes from django.utils.six import text_type from django.views.decorators.csrf import ( @@ -203,18 +204,22 @@ class CsrfViewMiddlewareTest(SimpleTestCase): Check that if no CSRF cookies is present, the middleware rejects the incoming request. This will stop login CSRF. """ - req = self._get_POST_no_csrf_cookie_request() - req2 = CsrfViewMiddleware().process_view(req, post_form_view, (), {}) - self.assertEqual(403, req2.status_code) + with patch_logger('django.security.csrf', 'warning') as logger_calls: + req = self._get_POST_no_csrf_cookie_request() + req2 = CsrfViewMiddleware().process_view(req, post_form_view, (), {}) + self.assertEqual(403, req2.status_code) + self.assertEqual(logger_calls[0], 'Forbidden (%s): ' % REASON_NO_CSRF_COOKIE) def test_process_request_csrf_cookie_no_token(self): """ Check that if a CSRF cookie is present but no token, the middleware rejects the incoming request. """ - req = self._get_POST_csrf_cookie_request() - req2 = CsrfViewMiddleware().process_view(req, post_form_view, (), {}) - self.assertEqual(403, req2.status_code) + with patch_logger('django.security.csrf', 'warning') as logger_calls: + req = self._get_POST_csrf_cookie_request() + req2 = CsrfViewMiddleware().process_view(req, post_form_view, (), {}) + self.assertEqual(403, req2.status_code) + self.assertEqual(logger_calls[0], 'Forbidden (%s): ' % REASON_BAD_TOKEN) def test_process_request_csrf_cookie_and_token(self): """ @@ -258,13 +263,17 @@ class CsrfViewMiddlewareTest(SimpleTestCase): """ req = TestingHttpRequest() req.method = 'PUT' - req2 = CsrfViewMiddleware().process_view(req, post_form_view, (), {}) - self.assertEqual(403, req2.status_code) + with patch_logger('django.security.csrf', 'warning') as logger_calls: + req2 = CsrfViewMiddleware().process_view(req, post_form_view, (), {}) + self.assertEqual(403, req2.status_code) + self.assertEqual(logger_calls[0], 'Forbidden (%s): ' % REASON_NO_CSRF_COOKIE) req = TestingHttpRequest() req.method = 'DELETE' - req2 = CsrfViewMiddleware().process_view(req, post_form_view, (), {}) - self.assertEqual(403, req2.status_code) + with patch_logger('django.security.csrf', 'warning') as logger_calls: + req2 = CsrfViewMiddleware().process_view(req, post_form_view, (), {}) + self.assertEqual(403, req2.status_code) + self.assertEqual(logger_calls[0], 'Forbidden (%s): ' % REASON_NO_CSRF_COOKIE) def test_put_and_delete_allowed(self): """ @@ -681,5 +690,7 @@ class CsrfViewMiddlewareTest(SimpleTestCase): self.assertIsNone(resp) req = CsrfPostRequest(token, raise_error=True) - resp = CsrfViewMiddleware().process_view(req, post_form_view, (), {}) - self.assertEqual(resp.status_code, 403) + with patch_logger('django.security.csrf', 'warning') as logger_calls: + resp = CsrfViewMiddleware().process_view(req, post_form_view, (), {}) + self.assertEqual(resp.status_code, 403) + self.assertEqual(logger_calls[0], 'Forbidden (%s): ' % REASON_BAD_TOKEN)