Fixed #26628 -- Changed CSRF logger to django.security.csrf #6707
Conversation
Hwesta
force-pushed
the
26628-logger-change
branch
3 times, most recently
from
d7e99bd
to
35db91d
Compare
| @@ -151,7 +151,8 @@ def _accept(self, request): | |||
| return None | |||
|
|
|||
| def _reject(self, request, reason): | |||
| logger.warning( | |||
| security_logger = logging.getLogger('django.security.csrf') | |||
There was a problem hiding this comment.
I don't think we need the django.request logger in this module anymore; why not get this logger at module level instead, rather than every time this method is called?
|
I think this also deserves a mention in the 1.11 release notes as a minor change. It's possible that someone may want to adjust their logging config as a result of this change. Besides that, flake8, and my other comment above, LGTM! |
|
(I'm erring on the side of caution by suggesting 1.11; I wouldn't have any objection to putting this into 1.10 if someone wants to argue for that. I think it's a small low-impact change.) |
| @@ -211,7 +211,7 @@ 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` | |||
| CSRF failures are logged as warnings to the :ref:`django-security-logger` ``csrf`` | |||
There was a problem hiding this comment.
Use:
:ref:`django.security.csrf <django-security-logger>`
and add a .. versionchanged:: 1.11 annotation below as described at https://docs.djangoproject.com/en/dev/internals/contributing/writing-documentation/#documenting-new-features
Hwesta
force-pushed
the
26628-logger-change
branch
from
35db91d
to
6892887
Compare
|
Updated for the feedback. |
Reworded docs to include non-SuspiciousOperation security loggers. Updated CSRF tests to check logger output.
Hwesta
force-pushed
the
26628-logger-change
branch
from
6892887
to
a422d20
Compare
|
merged in 55fec16, thanks! |
Reworded docs to include non-SuspiciousOperation security loggers. Updated CSRF tests to check logger output.
The docs looked like they are manually wrapped, and I wasn't sure if there was a way to make changes without noisy diffs.
Also, does this need to be added to patch notes, and if so which version?