From 74fd9ffd6bb8d07275455edcffa552d87bc639f1 Mon Sep 17 00:00:00 2001
From: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date: Sat, 7 Nov 2020 11:30:21 +0100
Subject: [PATCH] auth_oidc: enforce SameSite=Lax on the state cookie (#48347)

SameSite=Lax is needed for the cookie to be sent by the browser during
redirection chain from the provider. We could just depend on the fact
that cookie without SameSite are Lax by default, but it's better to be
explicit.
---
 src/authentic2/compat/cookies.py  | 22 ++++++++++++++++++++++
 src/authentic2_auth_oidc/views.py | 21 ++++++++++++++++++---
 tests/test_auth_oidc.py           |  3 +++
 3 files changed, 43 insertions(+), 3 deletions(-)
 create mode 100644 src/authentic2/compat/cookies.py

diff --git a/src/authentic2/compat/cookies.py b/src/authentic2/compat/cookies.py
new file mode 100644
index 00000000..f426e928
--- /dev/null
+++ b/src/authentic2/compat/cookies.py
@@ -0,0 +1,22 @@
+# authentic2 - versatile identity manager
+# Copyright (C) 2010-2019 Entr'ouvert
+#
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU Affero General Public License as published
+# by the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+import django
+
+if django.VERSION < (2, 1):
+    # Copied from Django >=2.1 / django.http.cookies
+    from http import cookies
+    cookies.Morsel._reserved.setdefault('samesite', 'SameSite')
diff --git a/src/authentic2_auth_oidc/views.py b/src/authentic2_auth_oidc/views.py
index dd8064bb..7d4a9291 100644
--- a/src/authentic2_auth_oidc/views.py
+++ b/src/authentic2_auth_oidc/views.py
@@ -21,6 +21,7 @@ import uuid
 
 import requests
 
+import django
 from django.conf import settings
 from django.core import signing
 from django.urls import reverse
@@ -31,6 +32,7 @@ from django.conf import settings
 from django.views.generic.base import View
 from django.http import HttpResponseBadRequest
 
+import authentic2.compat.cookies  # F401
 from authentic2.decorators import setting_enabled
 from authentic2.utils import redirect, login, good_next_url, authenticate
 
@@ -90,9 +92,22 @@ def oidc_login(request, pk, next_url=None, *args, **kwargs):
     logger.debug('auth_oidc: sent request %s to authorization endpoint "%s"',
                  params, provider.authorization_endpoint)
     response = redirect(request, provider.authorization_endpoint, params=params, resolve=False)
-    response.set_cookie(
-        'oidc-state', value=state_id, path=reverse('oidc-login-callback'),
-        httponly=True, secure=request.is_secure())
+
+    # As the oidc-state is used during a redirect from a third-party, we need
+    # it to user SameSite=Lax. See
+    # https://developer.mozilla.org/fr/docs/Web/HTTP/Headers/Set-Cookie/SameSite
+    # for more explanations.
+    if django.VERSION < (2, 1):
+        response.set_cookie(
+            'oidc-state', value=state_id, path=reverse('oidc-login-callback'),
+            httponly=True, secure=request.is_secure())
+        # work around lack of samesite parameter to set_cookie() in Django 1.11
+        # it also needs monkeypatch from authentic2.compat.cookies.
+        response.cookies['oidc-state']['samesite'] = 'Lax'
+    else:
+        response.set_cookie(
+            'oidc-state', value=state_id, path=reverse('oidc-login-callback'),
+            httponly=True, secure=request.is_secure(), samesite='lax')
     return response
 
 
diff --git a/tests/test_auth_oidc.py b/tests/test_auth_oidc.py
index 8e2e2a83..77f08821 100644
--- a/tests/test_auth_oidc.py
+++ b/tests/test_auth_oidc.py
@@ -818,6 +818,9 @@ def test_lost_state(app, caplog, code, oidc_provider, oidc_provider_jwkset, hook
     response = app.get('/login/?next=/whatever/')
     assert oidc_provider.name in response.text
     response = response.click(oidc_provider.name)
+    # As the oidc-state is used during a redirect from a third-party, we need
+    # it to be lax.
+    assert re.search('Set-Cookie.* oidc-state=.*SameSite=lax', str(response))
     qs = urlparse.parse_qs(urlparse.urlparse(response.location).query)
     state = qs['state']
 
-- 
2.29.2