Fixed #32902 -- Fixed CsrfViewMiddleware.process_response()'s cookie reset logic.
Thanks Florian Apolloner and Shai Berger for reviews.
This commit is contained in:
parent
311401d9a2
commit
a2e1f1e295
|
@ -437,15 +437,25 @@ class CsrfViewMiddleware(MiddlewareMixin):
|
||||||
return self._accept(request)
|
return self._accept(request)
|
||||||
|
|
||||||
def process_response(self, request, response):
|
def process_response(self, request, response):
|
||||||
if not getattr(request, 'csrf_cookie_needs_reset', False):
|
# Send the CSRF cookie whenever the cookie is being used (even if the
|
||||||
if getattr(response, 'csrf_cookie_set', False):
|
# client already has it) in order to renew the expiry timer, but only
|
||||||
return response
|
# if it hasn't already been sent during this request-response cycle.
|
||||||
|
# Also, send the cookie no matter what if a reset was requested.
|
||||||
if not request.META.get("CSRF_COOKIE_USED", False):
|
if (
|
||||||
return response
|
getattr(request, 'csrf_cookie_needs_reset', False) or (
|
||||||
|
request.META.get('CSRF_COOKIE_USED') and
|
||||||
# Set the CSRF cookie even if it's already set, so we renew
|
not getattr(response, 'csrf_cookie_set', False)
|
||||||
# the expiry timer.
|
)
|
||||||
|
):
|
||||||
self._set_token(request, response)
|
self._set_token(request, response)
|
||||||
|
# Update state to prevent _set_token() from being unnecessarily
|
||||||
|
# called again in process_response() by other instances of
|
||||||
|
# CsrfViewMiddleware. This can happen e.g. when both a decorator
|
||||||
|
# and middleware are used. However, the csrf_cookie_needs_reset
|
||||||
|
# attribute is still respected in subsequent calls e.g. in case
|
||||||
|
# rotate_token() is called in process_response() later by custom
|
||||||
|
# middleware but before those subsequent calls.
|
||||||
response.csrf_cookie_set = True
|
response.csrf_cookie_set = True
|
||||||
|
request.csrf_cookie_needs_reset = False
|
||||||
|
|
||||||
return response
|
return response
|
||||||
|
|
|
@ -14,8 +14,9 @@ from django.test import SimpleTestCase, override_settings
|
||||||
from django.views.decorators.csrf import csrf_exempt, requires_csrf_token
|
from django.views.decorators.csrf import csrf_exempt, requires_csrf_token
|
||||||
|
|
||||||
from .views import (
|
from .views import (
|
||||||
ensure_csrf_cookie_view, non_token_view_using_request_processor,
|
ensure_csrf_cookie_view, ensured_and_protected_view,
|
||||||
post_form_view, sandwiched_rotate_token_view, token_view,
|
non_token_view_using_request_processor, post_form_view, protected_view,
|
||||||
|
sandwiched_rotate_token_view, token_view,
|
||||||
)
|
)
|
||||||
|
|
||||||
# This is a test (unmasked) CSRF cookie / secret.
|
# This is a test (unmasked) CSRF cookie / secret.
|
||||||
|
@ -1065,6 +1066,32 @@ class CsrfViewMiddlewareTests(CsrfViewMiddlewareTestMixin, SimpleTestCase):
|
||||||
resp = mw.process_view(req, token_view, (), {})
|
resp = mw.process_view(req, token_view, (), {})
|
||||||
self.assertIsNone(resp)
|
self.assertIsNone(resp)
|
||||||
|
|
||||||
|
def test_cookie_reset_only_once(self):
|
||||||
|
"""
|
||||||
|
A CSRF cookie that needs to be reset is reset only once when the view
|
||||||
|
is decorated with both ensure_csrf_cookie and csrf_protect.
|
||||||
|
"""
|
||||||
|
# Pass an unmasked cookie to trigger a cookie reset.
|
||||||
|
req = self._get_POST_request_with_token(cookie=TEST_SECRET)
|
||||||
|
resp = ensured_and_protected_view(req)
|
||||||
|
self.assertContains(resp, 'OK')
|
||||||
|
csrf_cookie = self._read_csrf_cookie(req, resp)
|
||||||
|
actual_secret = _unmask_cipher_token(csrf_cookie)
|
||||||
|
self.assertEqual(actual_secret, TEST_SECRET)
|
||||||
|
# set_cookie() was called only once and with the expected secret.
|
||||||
|
self.assertCookiesSet(req, resp, [TEST_SECRET])
|
||||||
|
|
||||||
|
def test_invalid_cookie_replaced_on_GET(self):
|
||||||
|
"""
|
||||||
|
A CSRF cookie with the wrong format is replaced during a GET request.
|
||||||
|
"""
|
||||||
|
req = self._get_request(cookie='badvalue')
|
||||||
|
resp = protected_view(req)
|
||||||
|
self.assertContains(resp, 'OK')
|
||||||
|
csrf_cookie = self._read_csrf_cookie(req, resp)
|
||||||
|
self.assertTrue(csrf_cookie, msg='No CSRF cookie was sent.')
|
||||||
|
self.assertEqual(len(csrf_cookie), CSRF_TOKEN_LENGTH)
|
||||||
|
|
||||||
def test_bare_secret_accepted_and_replaced(self):
|
def test_bare_secret_accepted_and_replaced(self):
|
||||||
"""
|
"""
|
||||||
The csrf token is reset from a bare secret.
|
The csrf token is reset from a bare secret.
|
||||||
|
|
|
@ -33,6 +33,17 @@ class _CsrfCookieRotator(MiddlewareMixin):
|
||||||
csrf_rotating_token = decorator_from_middleware(_CsrfCookieRotator)
|
csrf_rotating_token = decorator_from_middleware(_CsrfCookieRotator)
|
||||||
|
|
||||||
|
|
||||||
|
@csrf_protect
|
||||||
|
def protected_view(request):
|
||||||
|
return HttpResponse('OK')
|
||||||
|
|
||||||
|
|
||||||
|
@csrf_protect
|
||||||
|
@ensure_csrf_cookie
|
||||||
|
def ensured_and_protected_view(request):
|
||||||
|
return TestingHttpResponse('OK')
|
||||||
|
|
||||||
|
|
||||||
@csrf_protect
|
@csrf_protect
|
||||||
@csrf_rotating_token
|
@csrf_rotating_token
|
||||||
@ensure_csrf_cookie
|
@ensure_csrf_cookie
|
||||||
|
|
Loading…
Reference in New Issue