From d2981f684c91598f229e6c2fa2408a3a857da0da Mon Sep 17 00:00:00 2001 From: TigreModerata Date: Wed, 18 Jan 2023 21:42:26 +0100 Subject: [PATCH] feat: add sameSite parameter to unset_cookie (#2140) * Enhancement #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 --- docs/_newsfragments/2124.newandimproved.rst | 1 + docs/api/cookies.rst | 4 ++ falcon/response.py | 7 +- tests/test_cookies.py | 71 +++++++++++++++++++-- 4 files changed, 77 insertions(+), 6 deletions(-) create mode 100644 docs/_newsfragments/2124.newandimproved.rst diff --git a/docs/_newsfragments/2124.newandimproved.rst b/docs/_newsfragments/2124.newandimproved.rst new file mode 100644 index 000000000..dae847273 --- /dev/null +++ b/docs/_newsfragments/2124.newandimproved.rst @@ -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 \ No newline at end of file diff --git a/docs/api/cookies.rst b/docs/api/cookies.rst index 3b8d3ef13..c54be24ea 100644 --- a/docs/api/cookies.rst +++ b/docs/api/cookies.rst @@ -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. diff --git a/falcon/response.py b/falcon/response.py index a74d49255..37941991f 100644 --- a/falcon/response.py +++ b/falcon/response.py @@ -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 @@ -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. @@ -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 diff --git a/tests/test_cookies.py b/tests/test_cookies.py index 0312d5aca..72759f0e5 100644 --- a/tests/test_cookies.py +++ b/tests/test_cookies.py @@ -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') @@ -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 @@ -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) @@ -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):