From c5442923fc67dde734c5fdbe6369d8b9f46cea3b Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Fri, 30 Sep 2022 00:46:05 +0200 Subject: [PATCH 2/2] views: improve handling of next_url for sp initiated logout (#69740) --- mellon/views.py | 14 +++++++++----- tests/test_sso_slo.py | 21 +++++++++++++++++++-- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/mellon/views.py b/mellon/views.py index dd6a692..c5f38f0 100644 --- a/mellon/views.py +++ b/mellon/views.py @@ -612,13 +612,13 @@ login = transaction.non_atomic_requests(csrf_exempt(LoginView.as_view())) class LogoutView(ProfileMixin, LogMixin, View): - def get(self, request, *args, **kwargs): + def get(self, request, *args, next_url=None, **kwargs): if 'SAMLRequest' in request.GET: return self.idp_logout(request, request.META['QUERY_STRING'], 'redirect') elif 'SAMLResponse' in request.GET: return self.sp_logout_response(request) else: - return self.sp_logout_request(request) + return self.sp_logout_request(request, next_url=next_url) def post(self, request, *args, **kwargs): return self.idp_logout(request, force_str(request.body), 'soap') @@ -721,10 +721,14 @@ class LogoutView(ProfileMixin, LogMixin, View): else: return HttpResponseRedirect(logout.msgUrl) - def sp_logout_request(self, request): + def sp_logout_request(self, request, next_url=None): '''Launch a logout request to the identity provider''' - next_url = request.GET.get(REDIRECT_FIELD_NAME) referer = request.headers.get('Referer') + if not next_url: + field_next_url = request.GET.get(REDIRECT_FIELD_NAME) + if field_next_url and utils.same_origin(request.build_absolute_uri(), field_next_url): + next_url = field_next_url + next_url = next_url or '/' if not referer or utils.same_origin(request.build_absolute_uri(), referer): if hasattr(request, 'user') and request.user.is_authenticated: logout = None @@ -754,7 +758,7 @@ class LogoutView(ProfileMixin, LogMixin, View): self.log.info('user logged out, SLO request sent to IdP') else: # anonymous user: if next_url is None redirect to referer - return HttpResponseRedirect(next_url or referer) + return HttpResponseRedirect(next_url) else: self.log.warning('logout refused referer %r is not of the same origin', referer) return HttpResponseRedirect(next_url) diff --git a/tests/test_sso_slo.py b/tests/test_sso_slo.py index b01c68e..614220a 100644 --- a/tests/test_sso_slo.py +++ b/tests/test_sso_slo.py @@ -254,20 +254,37 @@ def test_sso_slo(db, app, idp, caplog, sp_settings): # again, user is already logged out response = app.get(reverse('mellon_logout'), extra_environ={'HTTP_REFERER': '/some/path'}) - assert urlparse.urlparse(response['Location']).path == '/some/path' + assert urlparse.urlparse(response['Location']).path == '/' def test_sso_slo_next(db, app, idp, caplog, sp_settings): response = app.get(reverse('mellon_login')) url, body, relay_state = idp.process_authn_request_redirect(response['Location']) response = app.post(reverse('mellon_login'), params={'SAMLResponse': body, 'RelayState': relay_state}) - response = app.get(reverse('mellon_logout') + '?next=/some/path/') + response = app.get( + reverse('mellon_logout') + '?next=/some/path/', extra_environ={'HTTP_REFERER': '/other/path'} + ) assert urlparse.urlparse(response['Location']).path == '/singleLogout' url = idp.process_logout_request_redirect(response.location) response = app.get(url) assert response.location == '/some/path/' +def test_sso_slo_forced_next(db, app, idp, caplog, sp_settings, rf): + from mellon.views import logout + + response = app.get(reverse('mellon_login')) + url, body, relay_state = idp.process_authn_request_redirect(response['Location']) + response = app.post(reverse('mellon_login'), params={'SAMLResponse': body, 'RelayState': relay_state}) + + request = rf.get('/logout/?next=/some/path/') + request.session = app.session + request.user = mock.Mock() + request.user.is_authenticated = True + response = logout(request, next_url='/other/path/') + assert list(request.session.values()) == ['/other/path/'] + + def test_sso_idp_slo(db, app, idp, caplog, sp_settings): assert Session.objects.count() == 0 assert User.objects.count() == 0 -- 2.37.2