Merge pull request #3319 from Pylons/feature/more-samesite-work

Support samesite option in AuthtktAuthenticationPolicy and CookieCSRFStoragePolicy

5 files changed, 179 insertions, 34 deletions

diff --git a/CHANGES.rst b/CHANGES.rst
index 6ccd69a47..91dadfa79 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -37,6 +37,11 @@ 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 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.
   See https://github.com/Pylons/pyramid/pull/3302
diff --git a/pyramid/authentication.py b/pyramid/authentication.py
index 9d61e4d90..a9604e336 100644
--- a/pyramid/authentication.py
+++ b/pyramid/authentication.py
@@ -532,8 +532,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
@@ -541,8 +539,6 @@ class AuthTktAuthenticationPolicy(CallbackAuthenticationPolicy):
        and ``parent_domain``.
        Optional.
 
-       This option is available as of :app:`Pyramid` 1.5.
-
     ``hashalg``
 
        Default: ``sha512`` (the literal string).
@@ -555,8 +551,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``
@@ -566,8 +560,30 @@ 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.
+
+    .. 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`.
+
     """
 
     def __init__(self,
@@ -586,6 +602,7 @@ class AuthTktAuthenticationPolicy(CallbackAuthenticationPolicy):
                  hashalg='sha512',
                  parent_domain=False,
                  domain=None,
+                 samesite='Lax',
                  ):
         self.cookie = AuthTktCookieHelper(
             secret,
@@ -601,6 +618,7 @@ class AuthTktAuthenticationPolicy(CallbackAuthenticationPolicy):
             hashalg=hashalg,
             parent_domain=parent_domain,
             domain=domain,
+            samesite=samesite,
             )
         self.callback = callback
         self.debug = debug
@@ -793,10 +811,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()
 
@@ -806,7 +836,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 b023bda5f..da171d9af 100644
--- a/pyramid/csrf.py
+++ b/pyramid/csrf.py
@@ -106,11 +106,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,
@@ -119,7 +123,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 aeb4a467b..4efd76f2b 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=/'))
+        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].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
@@ -124,6 +124,19 @@ class TestCookieCSRFStoragePolicy(unittest.TestCase):
         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):