Skip to content

Commit

Permalink
[2.2.x] Fixed #31790 -- Fixed setting SameSite cookies flag in HttpRe…
Browse files Browse the repository at this point in the history
…sponse.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.

Backport of 331324e from stable/3.0.x
  • Loading branch information
felixxm committed Jul 16, 2020
1 parent 6f09ee2 commit f1a6e6c
Show file tree
Hide file tree
Showing 9 changed files with 42 additions and 6 deletions.
6 changes: 5 additions & 1 deletion django/contrib/messages/storage/cookie.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,11 @@ def _update_cookie(self, encoded_data, response):
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):
"""
Expand Down
1 change: 1 addition & 0 deletions django/contrib/sessions/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ def process_response(self, request, response):
settings.SESSION_COOKIE_NAME,
path=settings.SESSION_COOKIE_PATH,
domain=settings.SESSION_COOKIE_DOMAIN,
samesite=settings.SESSION_COOKIE_SAMESITE,
)
else:
if accessed:
Expand Down
4 changes: 2 additions & 2 deletions django/http/response.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,13 +210,13 @@ def set_signed_cookie(self, key, value, salt='', **kwargs):
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):
def delete_cookie(self, key, path='/', domain=None, samesite=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-'))
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
Expand Down
6 changes: 5 additions & 1 deletion docs/ref/request-response.txt
Original file line number Diff line number Diff line change
Expand Up @@ -841,7 +841,7 @@ Methods
you will need to remember to pass it to the corresponding
:meth:`HttpRequest.get_signed_cookie` call.

.. 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.
Expand All @@ -850,6 +850,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
Expand Down
13 changes: 13 additions & 0 deletions docs/releases/2.2.15.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
===========================
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`).
1 change: 1 addition & 0 deletions docs/releases/index.txt
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,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
Expand Down
5 changes: 5 additions & 0 deletions tests/messages_tests/test_cookie.py
Original file line number Diff line number Diff line change
@@ -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 (
Expand Down Expand Up @@ -85,6 +86,10 @@ def test_cookie_setings(self):
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()
Expand Down
6 changes: 6 additions & 0 deletions tests/responses/test_cookie.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ def test_default(self):
self.assertEqual(cookie['path'], '/')
self.assertEqual(cookie['secure'], '')
self.assertEqual(cookie['domain'], '')
self.assertEqual(cookie['samesite'], '')

def test_delete_cookie_secure_prefix(self):
"""
Expand All @@ -115,3 +116,8 @@ def test_delete_cookie_secure_prefix(self):
cookie_name = '__%s-c' % prefix
response.delete_cookie(cookie_name)
self.assertEqual(response.cookies[cookie_name]['secure'], True)

def test_delete_cookie_samesite(self):
response = HttpResponse()
response.delete_cookie('c', samesite='lax')
self.assertEqual(response.cookies['c']['samesite'], 'lax')
6 changes: 4 additions & 2 deletions tests/sessions_tests/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -743,8 +743,9 @@ def test_session_delete_on_end(self):
# 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])
)
Expand Down Expand Up @@ -772,8 +773,9 @@ def test_session_delete_on_end_with_custom_domain_and_path(self):
# 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])
)
Expand Down

0 comments on commit f1a6e6c

Please sign in to comment.