From 10df49324d276e3441b1c07bd9fd44fa0338a678 Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Wed, 6 Jul 2022 19:06:03 +0200 Subject: [PATCH 2/5] misc: retry passive sso when session expire and logout if current session does not match IDP_OPENED_SESSION cookie value (#67090) --- tests/test_saml_auth.py | 40 +++++++++++++++++++- wcs/root.py | 84 +++++++++++++++++++++++++---------------- 2 files changed, 91 insertions(+), 33 deletions(-) diff --git a/tests/test_saml_auth.py b/tests/test_saml_auth.py index 4c65df2e8..514a852bd 100644 --- a/tests/test_saml_auth.py +++ b/tests/test_saml_auth.py @@ -22,6 +22,7 @@ from wcs.qommon.ident.idp import MethodAdminDirectory from wcs.qommon.misc import get_lasso_server from wcs.qommon.saml2 import Saml2Directory, SOAPException +from .test_fc_auth import get_session from .test_hobo_notify import PROFILE from .utilities import clean_temporary_pub, create_temporary_pub, get_app @@ -604,7 +605,6 @@ def test_opened_session_cookie(pub): cookie_store = http.cookies.SimpleCookie() cookie_store.load(resp.headers['Set-Cookie']) assert list(cookie_store.keys()) == [cookie_name] - assert 'Secure' in resp.headers['Set-Cookie'] assert 'HttpOnly' in resp.headers['Set-Cookie'] assert 'SameSite=None' in resp.headers['Set-Cookie'] assert 'path=/' in resp.headers['Set-Cookie'] @@ -615,6 +615,44 @@ def test_opened_session_cookie(pub): ) assert cookie_name in app.cookies + # if we try again, no passive authentication occurs + resp = app.get('/?parameter=value') + assert resp.status_int != 302 + + # if IDP_OPENED_SESSION is modified, then passive authentication is tried again + app.set_cookie('IDP_OPENED_SESSION', '2') + resp = app.get('/?parameter=value') + assert resp.status_int == 302 + + # simulate a saml login + user = pub.user_class() + user.store() + request = mock.Mock() + request.get_environ.return_value = '1.1.1.1' + with mock.patch('quixote.session.get_request', return_value=request), mock.patch( + 'wcs.qommon.saml2', return_value=mock.Mock(cookies={'IDP_OPENED_SESSION': '2'}) + ): + session = get_session_manager().session_class(id=None) + session.set_user(user.id) + session.opened_session_value = '2' + session.id = 'abcd' + session.store() + app.set_cookie(pub.config.session_cookie_name, session.id) + assert get_session(app).opened_session_value == '2' + + resp = app.get('/?parameter=value') + assert resp.status_int == 200 + assert get_session(app).opened_session_value == '2' + assert get_session(app).user == user.id + # PASSIVE_TRIED_COOKIE was removed, since we logged in. + assert cookie_name not in app.cookies + + # if OPENED_SESSION_COOKIE change then we are logged out + app.set_cookie('IDP_OPENED_SESSION', '3') + resp = app.get('/?parameter=value') + assert not get_session(app) + assert not get_session_manager().session_class.get(session.id, ignore_errors=True) + def test_no_opened_session_cookie(pub): app = get_app(pub) diff --git a/wcs/root.py b/wcs/root.py index 07fc281cd..5ae2c4b3f 100644 --- a/wcs/root.py +++ b/wcs/root.py @@ -287,6 +287,10 @@ class RootDirectory(Directory): self.forced_language = False self.feed_substitution_parts() + output = self.try_passive_sso() + if output: + return output + response = get_response() if not hasattr(response, 'filter'): response.filter = {} @@ -304,43 +308,59 @@ class RootDirectory(Directory): except errors.TraversalError: pass - output = root.RootDirectory()._q_traverse(path) - return self.automatic_sso(output) - - def automatic_sso(self, output): - request = get_request() - response = get_response() + return root.RootDirectory()._q_traverse(path) + def try_passive_sso(self): publisher = get_publisher() OPENED_SESSION_COOKIE = publisher.get_site_option('idp_session_cookie_name') PASSIVE_TRIED_COOKIE = '%s-passive-auth-tried' % publisher.config.session_cookie_name - if OPENED_SESSION_COOKIE not in request.cookies and PASSIVE_TRIED_COOKIE in request.cookies: + + if not OPENED_SESSION_COOKIE: + return + ident_methods = get_cfg('identification', {}).get('methods', []) + idps = get_cfg('idp', {}) + if len(idps) != 1: + return + if ident_methods and 'idp' not in ident_methods: + return + + request = get_request() + cookies = request.cookies + response = get_response() + + # expire PASSIVE_TRIED_COOKIE if already logged or if not equal to PASSIVE_TRIED_COOKIE + if PASSIVE_TRIED_COOKIE in cookies and ( + request.user or cookies.get(PASSIVE_TRIED_COOKIE) != cookies.get(OPENED_SESSION_COOKIE) + ): response.expire_cookie(PASSIVE_TRIED_COOKIE) - return output - elif OPENED_SESSION_COOKIE in request.cookies and PASSIVE_TRIED_COOKIE not in request.cookies: - ident_methods = get_cfg('identification', {}).get('methods', []) - idps = get_cfg('idp', {}) - if request.user: - return output - if len(idps) != 1: - return output - if ident_methods and 'idp' not in ident_methods: - return output - response.set_cookie( - PASSIVE_TRIED_COOKIE, - '1', - secure=1, - httponly=1, - path=publisher.config.session_cookie_path, - domain=publisher.config.session_cookie_domain, - ) - url = request.get_url() - query = request.get_query() - if query: - url += '?' + query - return root.tryauth(url) - else: - return output + + if request.user: + if request.session.opened_session_value and request.session.opened_session_value != cookies.get( + OPENED_SESSION_COOKIE + ): + # logout current user if saved value for OPENED_SESSION_COOKIE differs from the current one + get_session_manager().expire_session() + else: + # already logged, stop here. + return + if OPENED_SESSION_COOKIE not in cookies or cookies.get(OPENED_SESSION_COOKIE) == cookies.get( + PASSIVE_TRIED_COOKIE + ): + # no session on the idp or passive sso already tried, stop here. + return + response.set_cookie( + PASSIVE_TRIED_COOKIE, + cookies.get(OPENED_SESSION_COOKIE), + secure=request.scheme == 'https', + httponly=1, + path=publisher.config.session_cookie_path, + domain=publisher.config.session_cookie_domain, + ) + url = request.get_url() + query = request.get_query() + if query: + url += '?' + query + return root.tryauth(url) def _q_lookup(self, component): if ( -- 2.37.2