Merge pull request #114 from zopefoundation/dataflake/same_site

Set the Cookie Auth Helper cookies with ``SameSite`` set to ``Strict``

.meta.toml

@@ -2,7 +2,7 @@
 # https://github.com/zopefoundation/meta/tree/master/config/zope-product
 [meta]
 template = "zope-product"
-commit-id = "92d6b6d157267896ae4098f4d4a5fb55d103319a"
+commit-id = "b5df3766ff8923477f3d24729b19504f0c401a2e"
 
 [python]
 with-pypy = false

CHANGES.rst

@@ -4,6 +4,16 @@ Change Log
 2.7.1 (unreleased)
 ------------------
 
+- Set the Cookie Auth Helper cookies with ``SameSite`` set to ``Lax`` by
+  default and allow admins to change the setting as well as the secure flag
+  from the Properties tab in the ZMI.
+  The ``SameSite`` cookie flag is not currently set, which causes modern
+  browsers to show warnings at the console. Such cookies may be rejected in the
+  future and break the Cookie Auth Helper. See
+  https://hacks.mozilla.org/2020/08/changes-to-samesite-cookie-behavior/
+  for information about the ``SameSite`` cookie attribute and why its handling
+  in browsers is changing.
+
 
 2.7.0 (2022-01-04)
 ------------------

src/Products/PluggableAuthService/plugins/CookieAuthHelper.py

@@ -96,12 +96,20 @@ class CookieAuthHelper(Folder, BasePlugin):
     zmi_icon = 'fas fa-cookie-bite'
     cookie_name = '__ginger_snap'
     login_path = 'login_form'
+    cookie_same_site = 'Lax'
+    cookie_same_site_choices = ('None', 'Lax', 'Strict')
+    cookie_secure = False
     security = ClassSecurityInfo()
 
     _properties = ({'id': 'title', 'label': 'Title',
                     'type': 'string', 'mode': 'w'},
                    {'id': 'cookie_name', 'label': 'Cookie Name',
                     'type': 'string', 'mode': 'w'},
+                   {'id': 'cookie_secure', 'type': 'boolean', 'mode': 'w',
+                    'label': 'Send cookie over HTTPS only'},
+                   {'id': 'cookie_same_site', 'type': 'selection',
+                    'label': 'Cookie SameSite restriction', 'mode': 'w',
+                    'select_variable': 'cookie_same_site_choices'},
                    {'id': 'login_path', 'label': 'Login Form',
                     'type': 'string', 'mode': 'w'})
 
@@ -176,7 +184,10 @@ def get_cookie_value(self, login, new_password):
     def updateCredentials(self, request, response, login, new_password):
         """ Respond to change of credentials (NOOP for basic auth). """
         cookie_val = self.get_cookie_value(login, new_password)
-        response.setCookie(self.cookie_name, quote(cookie_val), path='/')
+        cookie_secure = self.cookie_same_site == 'None' or self.cookie_secure
+        response.setCookie(self.cookie_name, quote(cookie_val),
+                           path='/', same_site=self.cookie_same_site,
+                           secure=cookie_secure)
 
     @security.private
     def resetCredentials(self, request, response):

src/Products/PluggableAuthService/plugins/tests/test_CookieAuthHelper.py

@@ -44,12 +44,14 @@ class FauxCookieResponse(FauxResponse):
 
     def __init__(self):
         self.cookies = {}
+        self.cookie_attributes = {}
         self.redirected = False
         self.status = '200'
         self.headers = {}
 
-    def setCookie(self, cookie_name, cookie_value, path):
+    def setCookie(self, cookie_name, cookie_value, path, **kw):
         self.cookies[(cookie_name, path)] = cookie_value
+        self.cookie_attributes[(cookie_name, path)] = kw
 
     def expireCookie(self, cookie_name, path):
         if (cookie_name, path) in self.cookies:
@@ -261,6 +263,41 @@ def test_extractCredentials_from_cookie_with_bad_binascii(self):
 
         self.assertEqual(helper.extractCredentials(request), {})
 
+    def test_updateCredentials(self):
+        helper = self._makeOne()
+        response = FauxCookieResponse()
+        request = FauxSettableRequest(RESPONSE=response)
+
+        username = codecs.encode(b'foo', 'hex_codec')
+        password = codecs.encode(b'b:ar', 'hex_codec')
+        cookie_str = b'%s:%s' % (username, password)
+        cookie_val = encodebytes(cookie_str)
+        cookie_val = cookie_val.rstrip()
+        if six.PY3:
+            cookie_val = cookie_val.decode('utf8')
+        request.set(helper.cookie_name, cookie_val)
+
+        # Defaults
+        helper.updateCredentials(request, response, 'new_user', 'new_pass')
+        cookie_attrs = response.cookie_attributes[(helper.cookie_name, '/')]
+        self.assertEqual(cookie_attrs['same_site'], 'Lax')
+        self.assertFalse(cookie_attrs['secure'])
+
+        # Setting the cookie same site value to "None" forces secure flag
+        helper.cookie_same_site = 'None'
+        helper.updateCredentials(request, response, 'new_user', 'new_pass')
+        cookie_attrs = response.cookie_attributes[(helper.cookie_name, '/')]
+        self.assertEqual(cookie_attrs['same_site'], 'None')
+        self.assertTrue(cookie_attrs['secure'])
+
+        # Setting the Secure flag manually
+        helper.cookie_same_site = 'Lax'
+        helper.cookie_secure = True
+        helper.updateCredentials(request, response, 'new_user', 'new_pass')
+        cookie_attrs = response.cookie_attributes[(helper.cookie_name, '/')]
+        self.assertEqual(cookie_attrs['same_site'], 'Lax')
+        self.assertTrue(cookie_attrs['secure'])
+
 
 class CookieAuthHelperIntegrationTests(pastc.PASTestCase):
 

tox.ini

@@ -25,7 +25,7 @@ commands_pre =
     py27,py35: {envbindir}/buildout -nc {toxinidir}/buildout4.cfg buildout:directory={envdir} buildout:develop={toxinidir} install test
     !py27-!py35: {envbindir}/buildout -nc {toxinidir}/buildout.cfg buildout:directory={envdir} buildout:develop={toxinidir} install test
 commands =
-    {envbindir}/test {posargs:-cv}
+    {envdir}/bin/test {posargs:-cv}
 
 [testenv:py27-zserver]
 commands_pre =
@@ -89,7 +89,7 @@ deps =
     coverage-python-version
 commands =
     mkdir -p {toxinidir}/parts/htmlcov
-    coverage run {envbindir}/test {posargs:-cv}
+    coverage run {envdir}/bin/test {posargs:-cv}
     coverage html
     coverage report -m --fail-under=89