Commit c4bd6cd2 by Romain Courteaud

core: set SameSite=Lax on authentication cookie

https://tools.ietf.org/html/draft-ietf-httpbis-rfc6265bis-02

SameSite=None breaks the compatibility with some browser versions.
https://www.chromium.org/updates/same-site/incompatible-clients

We choose Lax and not Strict so that we can open links to ERP5 from
external applications and so that OAuth Logins work. Implementing the
"two cookies, one for read one for write" approach suggested in
https://tools.ietf.org/html/draft-ietf-httpbis-rfc6265bis-02#section-8.8.2
would be too big change at this point.

Allow instances to surcharge the SameSite value for some specific domains if needed,
by surcharging the ERP5Site_getAuthCookieSameSite script.
parent 317c3bd1
......@@ -168,6 +168,7 @@ class TestFacebookLogin(ERP5TypeTestCase):
ac_cookie, = [v for (k, v) in response.listHeaders() if k.lower() == 'set-cookie' and '__ac_facebook_hash=' in v]
self.assertIn('; Secure', ac_cookie)
self.assertIn('; HTTPOnly', ac_cookie)
self.assertIn('; SameSite=Lax', ac_cookie)
def test_create_user_in_ERP5Site_createFacebookUserToOAuth(self):
"""
......
......@@ -210,6 +210,7 @@ class TestGoogleLogin(ERP5TypeTestCase):
ac_cookie, = [v for (k, v) in response.listHeaders() if k.lower() == 'set-cookie' and '__ac_google_hash=' in v]
self.assertIn('; Secure', ac_cookie)
self.assertIn('; HTTPOnly', ac_cookie)
self.assertIn('; SameSite=Lax', ac_cookie)
def test_create_user_in_ERP5Site_createGoogleUserToOAuth(self):
"""
......
<?xml version="1.0"?>
<ZopeData>
<record id="1" aka="AAAAAAAAAAE=">
<pickle>
<global name="PythonScript" module="Products.PythonScripts.PythonScript"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>Script_magic</string> </key>
<value> <int>3</int> </value>
</item>
<item>
<key> <string>_bind_names</string> </key>
<value>
<object>
<klass>
<global name="NameAssignments" module="Shared.DC.Scripts.Bindings"/>
</klass>
<tuple/>
<state>
<dictionary>
<item>
<key> <string>_asgns</string> </key>
<value>
<dictionary>
<item>
<key> <string>name_container</string> </key>
<value> <string>container</string> </value>
</item>
<item>
<key> <string>name_context</string> </key>
<value> <string>context</string> </value>
</item>
<item>
<key> <string>name_m_self</string> </key>
<value> <string>script</string> </value>
</item>
<item>
<key> <string>name_subpath</string> </key>
<value> <string>traverse_subpath</string> </value>
</item>
</dictionary>
</value>
</item>
</dictionary>
</state>
</object>
</value>
</item>
<item>
<key> <string>_params</string> </key>
<value> <string>scheme, hostname, port, path, user_agent</string> </value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>ERP5Site_getAuthCookieSameSite</string> </value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData>
from urlparse import urlparse
portal = context.getPortalObject()
kw = {}
expire_interval = portal.portal_preferences.getPreferredMaxUserInactivityDuration()
......@@ -15,12 +17,24 @@ portal.portal_sessions[
)
)
]['ac_renew'] = ac_renew
REQUEST = portal.REQUEST
parse_dict = urlparse(REQUEST.other.get('ACTUAL_URL'))
same_site = portal.ERP5Site_getAuthCookieSameSite(scheme=parse_dict.scheme,
hostname=parse_dict.hostname,
port=parse_dict.port,
path=parse_dict.path,
user_agent=REQUEST.environ.get('HTTP_USER_AGENT'))
if same_site not in ('None', 'Lax', 'Strict'):
# Do not use the SameSite attribute
same_site = None
resp.setCookie(
name=cookie_name,
value=cookie_value,
path='/',
secure=getattr(portal, 'REQUEST', {}).get('SERVER_URL', '').startswith('https:'),
secure=REQUEST.get('SERVER_URL', '').startswith('https:'),
http_only=True,
same_site='None',
same_site=same_site,
**kw
)
......@@ -1306,6 +1306,11 @@ class TestAuthenticationCookie(UserManagementTestCase):
# HTTPOnly flag so that javascript cannot access cookie
self.assertIn('; HTTPOnly', ac_cookie)
# SameSite=Lax flag so that cookie is not sent on cross origin requests.
# We set Lax (and not strict) so that opening a link to ERP5 from an
# external site does not prompt for login.
self.assertIn('; SameSite=Lax', ac_cookie)
class TestReindexObjectSecurity(UserManagementTestCase):
def afterSetUp(self):
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment