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.
This commit is contained in:
Benjamin Dauvergne 2020-11-07 11:30:21 +01:00
parent 2eeb1c6067
commit 7514632fe6
3 changed files with 43 additions and 3 deletions

View File

@ -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')

View File

@ -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

View File

@ -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']