Skip to content

Commit

Permalink
feat: add sameSite parameter to unset_cookie (falconry#2140)
Browse files Browse the repository at this point in the history
* Enhancement falconry#2124: added the samesite parameter to unset_cookie.
Tests: added a test case to test_unset_cookies and to on_get in CookieUnset

* Added new option to documentation in cookies.srt

* Added tests, CookiesUnsetSameSite class and test_unset_cookies_samesite; added towncrier news fragment.

* Removed unused import logging from test_cookies

* Reverted changes to docs/changes4.0.0.rst

* Reverted changes to docs/changes4.0.0.rst

* Update 4.0.0.rst

Co-authored-by: Vytautas Liuolia <vytautas.liuolia@gmail.com>
  • Loading branch information
TigreModerata and vytas7 committed Jan 18, 2023
1 parent 16d4710 commit d2981f6
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 6 deletions.
1 change: 1 addition & 0 deletions docs/_newsfragments/2124.newandimproved.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Added kwarg samesite to :py:meth:`~falcon.Response.unset_cookie` to allow override of default ``Lax`` setting of `SameSite` on the unset cookie
4 changes: 4 additions & 0 deletions docs/api/cookies.rst
Original file line number Diff line number Diff line change
Expand Up @@ -164,3 +164,7 @@ default, although this may change in a future release.

.. _RFC 6265, Section 4.1.2.5:
https://tools.ietf.org/html/rfc6265#section-4.1.2.5

When unsetting a cookie, :py:meth:`~falcon.Response.unset_cookie`,
the default `SameSite` setting of the unset cookie is ``'Lax'``, but can be changed
by setting the 'samesite' kwarg.
7 changes: 5 additions & 2 deletions falcon/response.py
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ def set_cookie(

self._cookies[name]['samesite'] = same_site.capitalize()

def unset_cookie(self, name, domain=None, path=None):
def unset_cookie(self, name, samesite='Lax', domain=None, path=None):
"""Unset a cookie in the response.
Clears the contents of the cookie, and instructs the user
Expand All @@ -548,6 +548,9 @@ def unset_cookie(self, name, domain=None, path=None):
name (str): Cookie name
Keyword Args:
samesite (str): Allows to override the default 'Lax' same_site
setting for the unset cookie.
domain (str): Restricts the cookie to a specific domain and
any subdomains of that domain. By default, the user
agent will return the cookie only to the origin server.
Expand Down Expand Up @@ -591,7 +594,7 @@ def unset_cookie(self, name, domain=None, path=None):

# NOTE(CaselIT): Set SameSite to Lax to avoid setting invalid cookies.
# See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite#Fixing_common_warnings # noqa: E501
self._cookies[name]['samesite'] = 'Lax'
self._cookies[name]['samesite'] = samesite

if domain:
self._cookies[name]['domain'] = domain
Expand Down
71 changes: 67 additions & 4 deletions tests/test_cookies.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ def on_get(self, req, resp):
class CookieResourceSameSite:
def on_get(self, req, resp):
resp.set_cookie('foo', 'bar', same_site='Lax')
resp.set_cookie('barz', 'barz', same_site='')

def on_post(self, req, resp):
resp.set_cookie('bar', 'foo', same_site='STRICT')
Expand All @@ -75,6 +76,21 @@ def on_get(self, req, resp):
resp.unset_cookie('bar', path='/bar')
resp.unset_cookie('baz', domain='www.example.com')
resp.unset_cookie('foobar', path='/foo', domain='www.example.com')
resp.unset_cookie(
'barfoo', samesite='none', path='/foo', domain='www.example.com'
)


class CookieUnsetSameSite:
def on_get(self, req, resp):
# change lax to strict
resp.unset_cookie('foo', samesite='Strict')
# change strict to lax
resp.unset_cookie('bar')
# change none to ''
resp.unset_cookie('baz', samesite='')
# change '' to none
resp.unset_cookie('barz', samesite='None')


@pytest.fixture
Expand All @@ -84,6 +100,7 @@ def client(asgi):
app.add_route('/test-convert', CookieResourceMaxAgeFloatString())
app.add_route('/same-site', CookieResourceSameSite())
app.add_route('/unset-cookie', CookieUnset())
app.add_route('/unset-cookie-same-site', CookieUnsetSameSite())

return testing.TestClient(app)

Expand Down Expand Up @@ -169,20 +186,66 @@ def test_response_complex_case(client):

def test_unset_cookies(client):
result = client.simulate_get('/unset-cookie')
assert len(result.cookies) == 5

assert len(result.cookies) == 4

def test(cookie, path, domain):
def test(cookie, path, domain, samesite='Lax'):
assert cookie.value == '' # An unset cookie has an empty value
assert cookie.domain == domain
assert cookie.path == path
assert cookie.same_site == 'Lax'
assert cookie.same_site == samesite
assert cookie.expires < datetime.utcnow()

test(result.cookies['foo'], path=None, domain=None)
test(result.cookies['bar'], path='/bar', domain=None)
test(result.cookies['baz'], path=None, domain='www.example.com')
test(result.cookies['foobar'], path='/foo', domain='www.example.com')
test(
result.cookies['barfoo'], path='/foo', domain='www.example.com', samesite='none'
)


def test_unset_cookies_samesite(client):
# Test possible different samesite values in set_cookies
# foo, bar, lax
result_set_lax_empty = client.simulate_get('/same-site')
# bar, foo, strict
result_set_strict = client.simulate_post('/same-site')
# baz, foo, none
result_set_none = client.simulate_put('/same-site')

def test_set(cookie, value, samesite=None):
assert cookie.value == value
assert cookie.same_site == samesite

test_set(result_set_lax_empty.cookies['foo'], 'bar', samesite='Lax')
test_set(result_set_strict.cookies['bar'], 'foo', samesite='Strict')
test_set(result_set_none.cookies['baz'], 'foo', samesite='None')
# barz gets set with '', that is None value
test_set(result_set_lax_empty.cookies['barz'], 'barz')
test_set(result_set_lax_empty.cookies['barz'], 'barz', samesite=None)

# Unset the cookies with different samesite values
result_unset = client.simulate_get('/unset-cookie-same-site')
assert len(result_unset.cookies) == 4

def test_unset(cookie, samesite='Lax'):
assert cookie.value == '' # An unset cookie has an empty value
assert cookie.same_site == samesite
assert cookie.expires < datetime.utcnow()

test_unset(result_unset.cookies['foo'], samesite='Strict')
# default: bar is unset with no samesite param, so should go to Lax
test_unset(result_unset.cookies['bar'], samesite='Lax')
test_unset(result_unset.cookies['bar']) # default in test_unset

test_unset(
result_unset.cookies['baz'], samesite=None
) # baz gets unset to samesite = ''
test_unset(result_unset.cookies['barz'], samesite='None')
# test for false
assert result_unset.cookies['baz'].same_site != 'Strict'
assert result_unset.cookies['foo'].same_site != 'Lax'
assert not result_unset.cookies['baz'].same_site


def test_cookie_expires_naive(client):
Expand Down

0 comments on commit d2981f6

Please sign in to comment.