From 240cbb63bf9965c63d7a3cc9032f91410f414d46 Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Thu, 16 Jul 2020 08:16:58 +0200 Subject: [PATCH] Fixed #31790 -- Fixed setting SameSite and Secure cookies flags in HttpResponse.delete_cookie(). Cookies with the "SameSite" flag set to None and without the "secure" flag will be soon rejected by latest browser versions. This affects sessions and messages cookies. --- django/contrib/messages/storage/cookie.py | 6 +++++- django/contrib/sessions/middleware.py | 1 + django/http/response.py | 15 ++++++++++----- docs/ref/request-response.txt | 6 +++++- docs/releases/2.2.15.txt | 17 +++++++++++++++++ docs/releases/3.0.9.txt | 7 ++++++- docs/releases/index.txt | 1 + tests/messages_tests/test_cookie.py | 5 +++++ tests/responses/test_cookie.py | 12 ++++++++++++ tests/sessions_tests/tests.py | 6 ++++-- 10 files changed, 66 insertions(+), 10 deletions(-) create mode 100644 docs/releases/2.2.15.txt diff --git a/django/contrib/messages/storage/cookie.py b/django/contrib/messages/storage/cookie.py index 78256ca3b2..b51e292aa0 100644 --- a/django/contrib/messages/storage/cookie.py +++ b/django/contrib/messages/storage/cookie.py @@ -92,7 +92,11 @@ class CookieStorage(BaseStorage): samesite=settings.SESSION_COOKIE_SAMESITE, ) else: - response.delete_cookie(self.cookie_name, domain=settings.SESSION_COOKIE_DOMAIN) + response.delete_cookie( + self.cookie_name, + domain=settings.SESSION_COOKIE_DOMAIN, + samesite=settings.SESSION_COOKIE_SAMESITE, + ) def _store(self, messages, response, remove_oldest=True, *args, **kwargs): """ diff --git a/django/contrib/sessions/middleware.py b/django/contrib/sessions/middleware.py index 63013eef7a..95ad30ce7f 100644 --- a/django/contrib/sessions/middleware.py +++ b/django/contrib/sessions/middleware.py @@ -42,6 +42,7 @@ class SessionMiddleware(MiddlewareMixin): settings.SESSION_COOKIE_NAME, path=settings.SESSION_COOKIE_PATH, domain=settings.SESSION_COOKIE_DOMAIN, + samesite=settings.SESSION_COOKIE_SAMESITE, ) patch_vary_headers(response, ('Cookie',)) else: diff --git a/django/http/response.py b/django/http/response.py index e00bcacefb..c0ed93c44e 100644 --- a/django/http/response.py +++ b/django/http/response.py @@ -210,13 +210,18 @@ class HttpResponseBase: value = signing.get_cookie_signer(salt=key + salt).sign(value) return self.set_cookie(key, value, **kwargs) - def delete_cookie(self, key, path='/', domain=None): - # Most browsers ignore the Set-Cookie header if the cookie name starts - # with __Host- or __Secure- and the cookie doesn't use the secure flag. - secure = key.startswith(('__Secure-', '__Host-')) + def delete_cookie(self, key, path='/', domain=None, samesite=None): + # Browsers can ignore the Set-Cookie header if the cookie doesn't use + # the secure flag and: + # - the cookie name starts with "__Host-" or "__Secure-", or + # - the samesite is "none". + secure = ( + key.startswith(('__Secure-', '__Host-')) or + (samesite and samesite.lower() == 'none') + ) self.set_cookie( key, max_age=0, path=path, domain=domain, secure=secure, - expires='Thu, 01 Jan 1970 00:00:00 GMT', + expires='Thu, 01 Jan 1970 00:00:00 GMT', samesite=samesite, ) # Common methods used by subclasses diff --git a/docs/ref/request-response.txt b/docs/ref/request-response.txt index 705da1842a..80e3e1e6ed 100644 --- a/docs/ref/request-response.txt +++ b/docs/ref/request-response.txt @@ -890,7 +890,7 @@ Methods Using ``samesite='None'`` (string) was allowed. -.. method:: HttpResponse.delete_cookie(key, path='/', domain=None) +.. method:: HttpResponse.delete_cookie(key, path='/', domain=None, samesite=None) Deletes the cookie with the given key. Fails silently if the key doesn't exist. @@ -899,6 +899,10 @@ Methods values you used in ``set_cookie()`` -- otherwise the cookie may not be deleted. + .. versionchanged:: 2.2.15 + + The ``samesite`` argument was added. + .. method:: HttpResponse.close() This method is called at the end of the request directly by the WSGI diff --git a/docs/releases/2.2.15.txt b/docs/releases/2.2.15.txt new file mode 100644 index 0000000000..9bd2246b44 --- /dev/null +++ b/docs/releases/2.2.15.txt @@ -0,0 +1,17 @@ +=========================== +Django 2.2.15 release notes +=========================== + +*Expected August 3, 2020* + +Django 2.2.15 fixes a bug in 2.2.14. + +Bugfixes +======== + +* Allowed setting the ``SameSite`` cookie flag in + :meth:`.HttpResponse.delete_cookie` (:ticket:`31790`). + +* Fixed setting the ``Secure`` cookie flag in + :meth:`.HttpResponse.delete_cookie` for cookies that use ``samesite='none'`` + (:ticket:`31790`). diff --git a/docs/releases/3.0.9.txt b/docs/releases/3.0.9.txt index adc0f30b10..08e16a681f 100644 --- a/docs/releases/3.0.9.txt +++ b/docs/releases/3.0.9.txt @@ -9,4 +9,9 @@ Django 3.0.9 fixes several bugs in 3.0.8. Bugfixes ======== -* ... +* Allowed setting the ``SameSite`` cookie flag in + :meth:`.HttpResponse.delete_cookie` (:ticket:`31790`). + +* Fixed setting the ``Secure`` cookie flag in + :meth:`.HttpResponse.delete_cookie` for cookies that use ``samesite='none'`` + (:ticket:`31790`). diff --git a/docs/releases/index.txt b/docs/releases/index.txt index e7f1517902..d37faf03c7 100644 --- a/docs/releases/index.txt +++ b/docs/releases/index.txt @@ -55,6 +55,7 @@ versions of the documentation contain the release notes for any later releases. .. toctree:: :maxdepth: 1 + 2.2.15 2.2.14 2.2.13 2.2.12 diff --git a/tests/messages_tests/test_cookie.py b/tests/messages_tests/test_cookie.py index 5675cd15eb..f1428fdf32 100644 --- a/tests/messages_tests/test_cookie.py +++ b/tests/messages_tests/test_cookie.py @@ -1,5 +1,6 @@ import json +from django.conf import settings from django.contrib.messages import constants from django.contrib.messages.storage.base import Message from django.contrib.messages.storage.cookie import ( @@ -85,6 +86,10 @@ class CookieTests(BaseTests, SimpleTestCase): self.assertEqual(response.cookies['messages'].value, '') self.assertEqual(response.cookies['messages']['domain'], '.example.com') self.assertEqual(response.cookies['messages']['expires'], 'Thu, 01 Jan 1970 00:00:00 GMT') + self.assertEqual( + response.cookies['messages']['samesite'], + settings.SESSION_COOKIE_SAMESITE, + ) def test_get_bad_cookie(self): request = self.get_request() diff --git a/tests/responses/test_cookie.py b/tests/responses/test_cookie.py index a52443eefe..c7c35219b2 100644 --- a/tests/responses/test_cookie.py +++ b/tests/responses/test_cookie.py @@ -105,6 +105,7 @@ class DeleteCookieTests(SimpleTestCase): self.assertEqual(cookie['path'], '/') self.assertEqual(cookie['secure'], '') self.assertEqual(cookie['domain'], '') + self.assertEqual(cookie['samesite'], '') def test_delete_cookie_secure_prefix(self): """ @@ -118,3 +119,14 @@ class DeleteCookieTests(SimpleTestCase): cookie_name = '__%s-c' % prefix response.delete_cookie(cookie_name) self.assertIs(response.cookies[cookie_name]['secure'], True) + + def test_delete_cookie_secure_samesite_none(self): + # delete_cookie() sets the secure flag if samesite='none'. + response = HttpResponse() + response.delete_cookie('c', samesite='none') + self.assertIs(response.cookies['c']['secure'], True) + + def test_delete_cookie_samesite(self): + response = HttpResponse() + response.delete_cookie('c', samesite='lax') + self.assertEqual(response.cookies['c']['samesite'], 'lax') diff --git a/tests/sessions_tests/tests.py b/tests/sessions_tests/tests.py index 6c6d7dd3f2..248dae82aa 100644 --- a/tests/sessions_tests/tests.py +++ b/tests/sessions_tests/tests.py @@ -758,8 +758,9 @@ class SessionMiddlewareTests(TestCase): # Set-Cookie: sessionid=; expires=Thu, 01 Jan 1970 00:00:00 GMT; Max-Age=0; Path=/ self.assertEqual( 'Set-Cookie: {}=""; expires=Thu, 01 Jan 1970 00:00:00 GMT; ' - 'Max-Age=0; Path=/'.format( + 'Max-Age=0; Path=/; SameSite={}'.format( settings.SESSION_COOKIE_NAME, + settings.SESSION_COOKIE_SAMESITE, ), str(response.cookies[settings.SESSION_COOKIE_NAME]) ) @@ -789,8 +790,9 @@ class SessionMiddlewareTests(TestCase): # Path=/example/ self.assertEqual( 'Set-Cookie: {}=""; Domain=.example.local; expires=Thu, ' - '01 Jan 1970 00:00:00 GMT; Max-Age=0; Path=/example/'.format( + '01 Jan 1970 00:00:00 GMT; Max-Age=0; Path=/example/; SameSite={}'.format( settings.SESSION_COOKIE_NAME, + settings.SESSION_COOKIE_SAMESITE, ), str(response.cookies[settings.SESSION_COOKIE_NAME]) )