From 87771a54d31562409d0d042d7f7e81dde5c28fc6 Mon Sep 17 00:00:00 2001 From: Chris McDonough Date: Wed, 1 Aug 2018 11:51:19 -0400 Subject: implement samesite option for AuthTktAuthenticationPolicy and CookieCSRFStoragePolicy --- CHANGES.rst | 4 ++ pyramid/authentication.py | 33 +++++++-- pyramid/csrf.py | 9 ++- pyramid/tests/test_authentication.py | 130 +++++++++++++++++++++++++++++------ pyramid/tests/test_csrf.py | 16 ++++- 5 files changed, 164 insertions(+), 28 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index e09c3723c..93f25bd69 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -37,6 +37,10 @@ Features ``pyramid.session.UnencryptedCookieSessionFactoryConfig``. See https://github.com/Pylons/pyramid/pull/3300 +- Modify ``pyramid.authentication.AuthTktAuthenticationPolicy`` and + ``pyramid.csrf.CookieCSRFStoragePolicy`` to support the SameSite option on + cookies. See https://github.com/Pylons/pyramid/pull/3319 + - Added new ``pyramid.httpexceptions.HTTPPermanentRedirect`` exception/response object for a HTTP 308 redirect. See https://github.com/Pylons/pyramid/pull/3302 diff --git a/pyramid/authentication.py b/pyramid/authentication.py index 445d6fcd2..f8fdbbf5c 100644 --- a/pyramid/authentication.py +++ b/pyramid/authentication.py @@ -565,8 +565,16 @@ class AuthTktAuthenticationPolicy(CallbackAuthenticationPolicy): steps. The output from debugging is useful for reporting to maillist or IRC channels when asking for support. + ``samesite`` + + Default: ``'Lax'``. The 'samesite' option of the session cookie. Set + the value to ``None`` to turn off the samesite option. + + This option is available as of :app:`Pyramid` 1.10. + Objects of this class implement the interface described by :class:`pyramid.interfaces.IAuthenticationPolicy`. + """ def __init__(self, @@ -585,6 +593,7 @@ class AuthTktAuthenticationPolicy(CallbackAuthenticationPolicy): hashalg='sha512', parent_domain=False, domain=None, + samesite='Lax', ): self.cookie = AuthTktCookieHelper( secret, @@ -600,6 +609,7 @@ class AuthTktAuthenticationPolicy(CallbackAuthenticationPolicy): hashalg=hashalg, parent_domain=parent_domain, domain=domain, + samesite=samesite, ) self.callback = callback self.debug = debug @@ -792,10 +802,22 @@ class AuthTktCookieHelper(object): binary_type: ('b64str', lambda x: b64encode(x)), } - def __init__(self, secret, cookie_name='auth_tkt', secure=False, - include_ip=False, timeout=None, reissue_time=None, - max_age=None, http_only=False, path="/", wild_domain=True, - hashalg='md5', parent_domain=False, domain=None): + def __init__(self, + secret, + cookie_name='auth_tkt', + secure=False, + include_ip=False, + timeout=None, + reissue_time=None, + max_age=None, + http_only=False, + path="/", + wild_domain=True, + hashalg='md5', + parent_domain=False, + domain=None, + samesite='Lax', + ): serializer = _SimpleSerializer() @@ -805,7 +827,8 @@ class AuthTktCookieHelper(object): max_age=max_age, httponly=http_only, path=path, - serializer=serializer + serializer=serializer, + samesite=samesite, ) self.secret = secret diff --git a/pyramid/csrf.py b/pyramid/csrf.py index 7c836e5ad..d762e2999 100644 --- a/pyramid/csrf.py +++ b/pyramid/csrf.py @@ -107,11 +107,15 @@ class CookieCSRFStoragePolicy(object): .. versionadded:: 1.9 + .. versionchanged: 1.10 + + Added the ``samesite`` option and made the default ``'Lax'``. + """ _token_factory = staticmethod(lambda: text_(uuid.uuid4().hex)) def __init__(self, cookie_name='csrf_token', secure=False, httponly=False, - domain=None, max_age=None, path='/'): + domain=None, max_age=None, path='/', samesite='Lax'): serializer = _SimpleSerializer() self.cookie_profile = CookieProfile( cookie_name=cookie_name, @@ -120,7 +124,8 @@ class CookieCSRFStoragePolicy(object): httponly=httponly, path=path, domains=[domain], - serializer=serializer + serializer=serializer, + samesite=samesite, ) self.cookie_name = cookie_name diff --git a/pyramid/tests/test_authentication.py b/pyramid/tests/test_authentication.py index b9a4c6be4..51ee674f1 100644 --- a/pyramid/tests/test_authentication.py +++ b/pyramid/tests/test_authentication.py @@ -462,7 +462,7 @@ class TestAuthTktAuthenticationPolicy(unittest.TestCase): inst = self._getTargetClass()( 'secret', callback=None, cookie_name=None, secure=False, include_ip=False, timeout=None, reissue_time=None, - hashalg='sha512', + hashalg='sha512', samesite=None, ) self.assertEqual(inst.callback, None) @@ -898,15 +898,57 @@ class TestAuthTktCookieHelper(unittest.TestCase): self.assertEqual(len(result), 3) self.assertEqual(result[0][0], 'Set-Cookie') - self.assertTrue(result[0][1].endswith('; Path=/')) + self.assertTrue(result[0][1].endswith('; Path=/; SameSite=Lax')) self.assertTrue(result[0][1].startswith('auth_tkt=')) self.assertEqual(result[1][0], 'Set-Cookie') - self.assertTrue(result[1][1].endswith('; Domain=localhost; Path=/')) + self.assertTrue(result[1][1].endswith( + '; Domain=localhost; Path=/; SameSite=Lax')) + self.assertTrue(result[1][1].startswith('auth_tkt=')) + + self.assertEqual(result[2][0], 'Set-Cookie') + self.assertTrue(result[2][1].endswith( + '; Domain=.localhost; Path=/; SameSite=Lax')) + self.assertTrue(result[2][1].startswith('auth_tkt=')) + + def test_remember_nondefault_samesite(self): + helper = self._makeOne('secret', samesite='Strict') + request = self._makeRequest() + result = helper.remember(request, 'userid') + self.assertEqual(len(result), 3) + + self.assertEqual(result[0][0], 'Set-Cookie') + self.assertTrue(result[0][1].endswith('; Path=/; SameSite=Strict')) + self.assertTrue(result[0][1].startswith('auth_tkt=')) + + self.assertEqual(result[1][0], 'Set-Cookie') + self.assertTrue(result[1][1].endswith( + '; Domain=localhost; Path=/; SameSite=Strict')) + self.assertTrue(result[1][1].startswith('auth_tkt=')) + + self.assertEqual(result[2][0], 'Set-Cookie') + self.assertTrue(result[2][1].endswith( + '; Domain=.localhost; Path=/; SameSite=Strict')) + self.assertTrue(result[2][1].startswith('auth_tkt=')) + + def test_remember_None_samesite(self): + helper = self._makeOne('secret', samesite=None) + request = self._makeRequest() + result = helper.remember(request, 'userid') + self.assertEqual(len(result), 3) + + self.assertEqual(result[0][0], 'Set-Cookie') + self.assertTrue(result[0][1].endswith('; Path=/')) # no samesite + self.assertTrue(result[0][1].startswith('auth_tkt=')) + + self.assertEqual(result[1][0], 'Set-Cookie') + self.assertTrue(result[1][1].endswith( + '; Domain=localhost; Path=/')) self.assertTrue(result[1][1].startswith('auth_tkt=')) self.assertEqual(result[2][0], 'Set-Cookie') - self.assertTrue(result[2][1].endswith('; Domain=.localhost; Path=/')) + self.assertTrue(result[2][1].endswith( + '; Domain=.localhost; Path=/')) self.assertTrue(result[2][1].startswith('auth_tkt=')) def test_remember_include_ip(self): @@ -916,15 +958,17 @@ class TestAuthTktCookieHelper(unittest.TestCase): self.assertEqual(len(result), 3) self.assertEqual(result[0][0], 'Set-Cookie') - self.assertTrue(result[0][1].endswith('; Path=/')) + self.assertTrue(result[0][1].endswith('; Path=/; SameSite=Lax')) self.assertTrue(result[0][1].startswith('auth_tkt=')) self.assertEqual(result[1][0], 'Set-Cookie') - self.assertTrue(result[1][1].endswith('; Domain=localhost; Path=/')) + self.assertTrue(result[1][1].endswith( + '; Domain=localhost; Path=/; SameSite=Lax')) self.assertTrue(result[1][1].startswith('auth_tkt=')) self.assertEqual(result[2][0], 'Set-Cookie') - self.assertTrue(result[2][1].endswith('; Domain=.localhost; Path=/')) + self.assertTrue(result[2][1].endswith( + '; Domain=.localhost; Path=/; SameSite=Lax')) self.assertTrue(result[2][1].startswith('auth_tkt=')) def test_remember_path(self): @@ -935,17 +979,18 @@ class TestAuthTktCookieHelper(unittest.TestCase): self.assertEqual(len(result), 3) self.assertEqual(result[0][0], 'Set-Cookie') - self.assertTrue(result[0][1].endswith('; Path=/cgi-bin/app.cgi/')) + self.assertTrue(result[0][1].endswith( + '; Path=/cgi-bin/app.cgi/; SameSite=Lax')) self.assertTrue(result[0][1].startswith('auth_tkt=')) self.assertEqual(result[1][0], 'Set-Cookie') self.assertTrue(result[1][1].endswith( - '; Domain=localhost; Path=/cgi-bin/app.cgi/')) + '; Domain=localhost; Path=/cgi-bin/app.cgi/; SameSite=Lax')) self.assertTrue(result[1][1].startswith('auth_tkt=')) self.assertEqual(result[2][0], 'Set-Cookie') self.assertTrue(result[2][1].endswith( - '; Domain=.localhost; Path=/cgi-bin/app.cgi/')) + '; Domain=.localhost; Path=/cgi-bin/app.cgi/; SameSite=Lax')) self.assertTrue(result[2][1].startswith('auth_tkt=')) def test_remember_http_only(self): @@ -955,7 +1000,7 @@ class TestAuthTktCookieHelper(unittest.TestCase): self.assertEqual(len(result), 3) self.assertEqual(result[0][0], 'Set-Cookie') - self.assertTrue(result[0][1].endswith('; HttpOnly')) + self.assertTrue(result[0][1].endswith('; HttpOnly; SameSite=Lax')) self.assertTrue(result[0][1].startswith('auth_tkt=')) self.assertEqual(result[1][0], 'Set-Cookie') @@ -991,11 +1036,12 @@ class TestAuthTktCookieHelper(unittest.TestCase): self.assertEqual(len(result), 2) self.assertEqual(result[0][0], 'Set-Cookie') - self.assertTrue(result[0][1].endswith('; Path=/')) + self.assertTrue(result[0][1].endswith('; Path=/; SameSite=Lax')) self.assertTrue(result[0][1].startswith('auth_tkt=')) self.assertEqual(result[1][0], 'Set-Cookie') - self.assertTrue(result[1][1].endswith('; Domain=localhost; Path=/')) + self.assertTrue(result[1][1].endswith( + '; Domain=localhost; Path=/; SameSite=Lax')) self.assertTrue(result[1][1].startswith('auth_tkt=')) def test_remember_parent_domain(self): @@ -1006,7 +1052,8 @@ class TestAuthTktCookieHelper(unittest.TestCase): self.assertEqual(len(result), 1) self.assertEqual(result[0][0], 'Set-Cookie') - self.assertTrue(result[0][1].endswith('; Domain=.example.com; Path=/')) + self.assertTrue(result[0][1].endswith( + '; Domain=.example.com; Path=/; SameSite=Lax')) self.assertTrue(result[0][1].startswith('auth_tkt=')) def test_remember_parent_domain_supercedes_wild_domain(self): @@ -1015,7 +1062,8 @@ class TestAuthTktCookieHelper(unittest.TestCase): request.domain = 'www.example.com' result = helper.remember(request, 'other') self.assertEqual(len(result), 1) - self.assertTrue(result[0][1].endswith('; Domain=.example.com; Path=/')) + self.assertTrue(result[0][1].endswith( + '; Domain=.example.com; Path=/; SameSite=Lax')) def test_remember_explicit_domain(self): helper = self._makeOne('secret', domain='pyramid.bazinga') @@ -1026,7 +1074,7 @@ class TestAuthTktCookieHelper(unittest.TestCase): self.assertEqual(result[0][0], 'Set-Cookie') self.assertTrue(result[0][1].endswith( - '; Domain=pyramid.bazinga; Path=/')) + '; Domain=pyramid.bazinga; Path=/; SameSite=Lax')) self.assertTrue(result[0][1].startswith('auth_tkt=')) def test_remember_domain_supercedes_parent_and_wild_domain(self): @@ -1037,7 +1085,7 @@ class TestAuthTktCookieHelper(unittest.TestCase): result = helper.remember(request, 'other') self.assertEqual(len(result), 1) self.assertTrue(result[0][1].endswith( - '; Domain=pyramid.bazinga; Path=/')) + '; Domain=pyramid.bazinga; Path=/; SameSite=Lax')) def test_remember_binary_userid(self): import base64 @@ -1138,6 +1186,48 @@ class TestAuthTktCookieHelper(unittest.TestCase): self.assertEqual(result[2][0], 'Set-Cookie') self.assertTrue("/tokens=foo|bar/" in result[2][1]) + def test_remember_samesite_nondefault(self): + helper = self._makeOne('secret', samesite='Strict') + request = self._makeRequest() + result = helper.remember(request, 'userid') + self.assertEqual(len(result), 3) +` + self.assertEqual(result[0][0], 'Set-Cookie') + cookieval = result[0][1] + self.assertTrue('SameSite=Strict' in + [x.strip() for x in cookieval.split(';')], cookieval) + + self.assertEqual(result[1][0], 'Set-Cookie') + cookieval = result[1][1] + self.assertTrue('SameSite=Strict' in + [x.strip() for x in cookieval.split(';')], cookieval) + + self.assertEqual(result[2][0], 'Set-Cookie') + cookieval = result[2][1] + self.assertTrue('SameSite=Strict' in + [x.strip() for x in cookieval.split(';')], cookieval) + + def test_remember_samesite_default(self): + helper = self._makeOne('secret') + request = self._makeRequest() + result = helper.remember(request, 'userid') + self.assertEqual(len(result), 3) + + self.assertEqual(result[0][0], 'Set-Cookie') + cookieval = result[0][1] + self.assertTrue('SameSite=Lax' in + [x.strip() for x in cookieval.split(';')], cookieval) + + self.assertEqual(result[1][0], 'Set-Cookie') + cookieval = result[1][1] + self.assertTrue('SameSite=Lax' in + [x.strip() for x in cookieval.split(';')], cookieval) + + self.assertEqual(result[2][0], 'Set-Cookie') + cookieval = result[2][1] + self.assertTrue('SameSite=Lax' in + [x.strip() for x in cookieval.split(';')], cookieval) + def test_remember_unicode_but_ascii_token(self): helper = self._makeOne('secret') request = self._makeRequest() @@ -1171,21 +1261,21 @@ class TestAuthTktCookieHelper(unittest.TestCase): self.assertEqual( value, 'auth_tkt=; Max-Age=0; Path=/; ' - 'expires=Wed, 31-Dec-97 23:59:59 GMT' + 'expires=Wed, 31-Dec-97 23:59:59 GMT; SameSite=Lax' ) name, value = headers[1] self.assertEqual(name, 'Set-Cookie') self.assertEqual( value, 'auth_tkt=; Domain=localhost; Max-Age=0; Path=/; ' - 'expires=Wed, 31-Dec-97 23:59:59 GMT' + 'expires=Wed, 31-Dec-97 23:59:59 GMT; SameSite=Lax' ) name, value = headers[2] self.assertEqual(name, 'Set-Cookie') self.assertEqual( value, 'auth_tkt=; Domain=.localhost; Max-Age=0; Path=/; ' - 'expires=Wed, 31-Dec-97 23:59:59 GMT' + 'expires=Wed, 31-Dec-97 23:59:59 GMT; SameSite=Lax' ) class TestAuthTicket(unittest.TestCase): diff --git a/pyramid/tests/test_csrf.py b/pyramid/tests/test_csrf.py index f01780ad8..234d4434c 100644 --- a/pyramid/tests/test_csrf.py +++ b/pyramid/tests/test_csrf.py @@ -122,6 +122,19 @@ class TestCookieCSRFStoragePolicy(unittest.TestCase): policy = self._makeOne() token = policy.get_csrf_token(request) request.response_callback(request, response) + self.assertEqual( + response.headerlist, + [('Set-Cookie', 'csrf_token={}; Path=/; SameSite=Lax'.format( + token))] + ) + + def test_get_cookie_csrf_nondefault_samesite(self): + response = MockResponse() + request = DummyRequest() + + policy = self._makeOne(samesite=None) + token = policy.get_csrf_token(request) + request.response_callback(request, response) self.assertEqual( response.headerlist, [('Set-Cookie', 'csrf_token={}; Path=/'.format(token))] @@ -151,7 +164,8 @@ class TestCookieCSRFStoragePolicy(unittest.TestCase): request.response_callback(request, response) self.assertEqual( response.headerlist, - [('Set-Cookie', 'csrf_token={}; Path=/'.format(token))] + [('Set-Cookie', 'csrf_token={}; Path=/; SameSite=Lax'.format(token) + )] ) def test_get_csrf_token_returns_the_new_token(self): -- cgit v1.2.3 From ea81209f257e757f3fd838a56606014a5ce61eb3 Mon Sep 17 00:00:00 2001 From: Chris McDonough Date: Wed, 1 Aug 2018 12:03:38 -0400 Subject: lol --- pyramid/tests/test_authentication.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyramid/tests/test_authentication.py b/pyramid/tests/test_authentication.py index 51ee674f1..7938d5a18 100644 --- a/pyramid/tests/test_authentication.py +++ b/pyramid/tests/test_authentication.py @@ -1191,7 +1191,7 @@ class TestAuthTktCookieHelper(unittest.TestCase): request = self._makeRequest() result = helper.remember(request, 'userid') self.assertEqual(len(result), 3) -` + self.assertEqual(result[0][0], 'Set-Cookie') cookieval = result[0][1] self.assertTrue('SameSite=Strict' in -- cgit v1.2.3 From 0ad05afc020d2790048d4ca85d936b4ea79eae13 Mon Sep 17 00:00:00 2001 From: Chris McDonough Date: Thu, 9 Aug 2018 18:04:42 -0400 Subject: address review comments by rayedo --- CHANGES.rst | 3 ++- pyramid/authentication.py | 20 ++++++++++++++------ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 93f25bd69..cd4647e82 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -39,7 +39,8 @@ Features - Modify ``pyramid.authentication.AuthTktAuthenticationPolicy`` and ``pyramid.csrf.CookieCSRFStoragePolicy`` to support the SameSite option on - cookies. See https://github.com/Pylons/pyramid/pull/3319 + cookies and set the default to ``'Lax'``. + See https://github.com/Pylons/pyramid/pull/3319 - Added new ``pyramid.httpexceptions.HTTPPermanentRedirect`` exception/response object for a HTTP 308 redirect. diff --git a/pyramid/authentication.py b/pyramid/authentication.py index f8fdbbf5c..412440a02 100644 --- a/pyramid/authentication.py +++ b/pyramid/authentication.py @@ -531,8 +531,6 @@ class AuthTktAuthenticationPolicy(CallbackAuthenticationPolicy): option. Optional. - This option is available as of :app:`Pyramid` 1.5. - ``domain`` Default: ``None``. If provided the auth_tkt cookie will only be @@ -540,8 +538,6 @@ class AuthTktAuthenticationPolicy(CallbackAuthenticationPolicy): and ``parent_domain``. Optional. - This option is available as of :app:`Pyramid` 1.5. - ``hashalg`` Default: ``sha512`` (the literal string). @@ -554,8 +550,6 @@ class AuthTktAuthenticationPolicy(CallbackAuthenticationPolicy): ``hashalg`` will imply that all existing users with a valid cookie will be required to re-login. - This option is available as of :app:`Pyramid` 1.4. - Optional. ``debug`` @@ -572,6 +566,20 @@ class AuthTktAuthenticationPolicy(CallbackAuthenticationPolicy): This option is available as of :app:`Pyramid` 1.10. + .. versionchanged:: 1.4 + + Added the ``hashalg`` option, defaulting to ``sha512``. + + .. versionchanged:: 1.5 + + Added the ``domain`` option. + + Added the ``parent_domain`` option. + + .. versionchanged:: 1.10 + + Added the ``samesite`` option and made the default ``'Lax'``. + Objects of this class implement the interface described by :class:`pyramid.interfaces.IAuthenticationPolicy`. -- cgit v1.2.3