From 65882e5987dce1eed36bf8466df5b91652b7b87a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9rome=20Perrin?= Date: Mon, 19 Nov 2018 04:46:10 +0100 Subject: [PATCH] core: set SameSite=Lax on authentication cookie https://tools.ietf.org/html/draft-ietf-httpbis-rfc6265bis-02 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. --- .../portal_components/test.erp5.testFacebookLogin.py | 1 + .../portal_components/test.erp5.testGoogleLogin.py | 1 + .../portal_skins/erp5_auto_logout/setAuthCookie.py | 1 + product/ERP5Security/tests/testERP5Security.py | 5 +++++ 4 files changed, 8 insertions(+) diff --git a/bt5/erp5_oauth_facebook_login/TestTemplateItem/portal_components/test.erp5.testFacebookLogin.py b/bt5/erp5_oauth_facebook_login/TestTemplateItem/portal_components/test.erp5.testFacebookLogin.py index 41db914f123..7054b760e9b 100644 --- a/bt5/erp5_oauth_facebook_login/TestTemplateItem/portal_components/test.erp5.testFacebookLogin.py +++ b/bt5/erp5_oauth_facebook_login/TestTemplateItem/portal_components/test.erp5.testFacebookLogin.py @@ -121,6 +121,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): """ diff --git a/bt5/erp5_oauth_google_login/TestTemplateItem/portal_components/test.erp5.testGoogleLogin.py b/bt5/erp5_oauth_google_login/TestTemplateItem/portal_components/test.erp5.testGoogleLogin.py index b39b808f11c..16c8a3f69b9 100644 --- a/bt5/erp5_oauth_google_login/TestTemplateItem/portal_components/test.erp5.testGoogleLogin.py +++ b/bt5/erp5_oauth_google_login/TestTemplateItem/portal_components/test.erp5.testGoogleLogin.py @@ -158,6 +158,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): """ diff --git a/product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_auto_logout/setAuthCookie.py b/product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_auto_logout/setAuthCookie.py index 8e0d73867eb..39c7a16c81e 100644 --- a/product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_auto_logout/setAuthCookie.py +++ b/product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_auto_logout/setAuthCookie.py @@ -21,5 +21,6 @@ resp.setCookie( path='/', secure=getattr(portal, 'REQUEST', {}).get('SERVER_URL', '').startswith('https:'), http_only=True, + same_site='Lax', **kw ) diff --git a/product/ERP5Security/tests/testERP5Security.py b/product/ERP5Security/tests/testERP5Security.py index 55563e2e8a6..ba4b0978f0b 100644 --- a/product/ERP5Security/tests/testERP5Security.py +++ b/product/ERP5Security/tests/testERP5Security.py @@ -1301,6 +1301,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): -- 2.30.9