diff --git a/swh/web/auth/middlewares.py b/swh/web/auth/middlewares.py new file mode 100644 --- /dev/null +++ b/swh/web/auth/middlewares.py @@ -0,0 +1,43 @@ +# Copyright (C) 2020 The Software Heritage developers +# See the AUTHORS file at the top-level directory of this distribution +# License: GNU Affero General Public License version 3, or any later version +# See top-level LICENSE file for more information + +from django.contrib.auth import BACKEND_SESSION_KEY +from django.http.response import HttpResponseRedirect + +from swh.web.common.utils import reverse + + +class OIDCSessionRefreshMiddleware: + """ + Middleware for silently refreshing on OpenID Connect session from + the browser and get new access token. + """ + def __init__(self, get_response=None): + self.get_response = get_response + self.exempted_urls = [ + reverse(v) for v in ('logout', + 'oidc-login', + 'oidc-login-complete', + 'oidc-logout') + ] + + def __call__(self, request): + if (request.method != 'GET' + or request.user.is_authenticated + or BACKEND_SESSION_KEY not in request.session + or 'OIDC' not in request.session[BACKEND_SESSION_KEY] + or request.path in self.exempted_urls): + return self.get_response(request) + + # At that point, we know that a OIDC user was previously logged in. + # Access token has expired so we attempt a silent OIDC session refresh. + # If the latter failed because the session expired, user will be + # redirected to logout page and a link will be offered to login again. + # See implementation of "oidc-login-complete" view for more details. + next_path = request.get_full_path() + redirect_url = reverse('oidc-login', + query_params={'next_path': next_path, + 'prompt': 'none'}) + return HttpResponseRedirect(redirect_url) diff --git a/swh/web/auth/views.py b/swh/web/auth/views.py --- a/swh/web/auth/views.py +++ b/swh/web/auth/views.py @@ -11,7 +11,9 @@ from django.core.cache import cache from django.contrib.auth import authenticate, login, logout from django.http import HttpRequest -from django.http.response import HttpResponse, HttpResponseRedirect +from django.http.response import ( + HttpResponse, HttpResponseRedirect, HttpResponseServerError +) from swh.web.auth.models import OIDCUser from swh.web.auth.utils import gen_oidc_pkce_codes, get_oidc_client @@ -33,7 +35,8 @@ 'code_verifier': code_verifier, 'state': state, 'redirect_uri': redirect_uri, - 'next_path': request.GET.get('next_path'), + 'next_path': request.GET.get('next_path', ''), + 'prompt': request.GET.get('prompt', ''), } authorization_url_params = { @@ -41,6 +44,7 @@ 'code_challenge': code_challenge, 'code_challenge_method': 'S256', 'scope': 'openid', + 'prompt': request.GET.get('prompt', ''), } try: @@ -61,14 +65,26 @@ if 'login_data' not in request.session: raise Exception('Login process has not been initialized.') - if 'code' not in request.GET and 'state' not in request.GET: + login_data = request.session['login_data'] + next_path = login_data['next_path'] or request.build_absolute_uri('/') + + if 'error' in request.GET: + if login_data['prompt'] == 'none': + # Silent login failed because OIDC session expired. + # Redirect to logout page and inform user. + logout(request) + logout_url = reverse('logout', + query_params={'next_path': next_path, + 'remote_user': 1}) + return HttpResponseRedirect(logout_url) + return HttpResponseServerError(request.GET['error']) + + if 'code' not in request.GET or 'state' not in request.GET: raise BadInputExc('Missing query parameters for authentication.') # get CSRF token returned by OIDC server state = request.GET['state'] - login_data = request.session['login_data'] - if state != login_data['state']: raise BadInputExc('Wrong CSRF token, aborting login process.') @@ -82,10 +98,7 @@ login(request, user) - redirect_url = (login_data['next_path'] or - request.build_absolute_uri('/')) - - return HttpResponseRedirect(redirect_url) + return HttpResponseRedirect(next_path) except Exception as e: return handle_view_exception(request, e) diff --git a/swh/web/settings/common.py b/swh/web/settings/common.py --- a/swh/web/settings/common.py +++ b/swh/web/settings/common.py @@ -57,6 +57,7 @@ 'django.middleware.common.CommonMiddleware', 'django.middleware.csrf.CsrfViewMiddleware', 'django.contrib.auth.middleware.AuthenticationMiddleware', + 'swh.web.auth.middlewares.OIDCSessionRefreshMiddleware', 'django.contrib.messages.middleware.MessageMiddleware', 'django.middleware.clickjacking.XFrameOptionsMiddleware', 'swh.web.common.middlewares.ThrottlingHeadersMiddleware', diff --git a/swh/web/templates/logout.html b/swh/web/templates/logout.html --- a/swh/web/templates/logout.html +++ b/swh/web/templates/logout.html @@ -16,12 +16,27 @@ {% endblock %} {% block content %} +{% if 'next_path' in request.GET %} +
Your authenticated session expired so you have been automatically logged out.
+{% else %}You have been successfully logged out.
+{% endif %}{% if oidc_enabled and 'remote_user' in request.GET %} +{% if 'next_path' in request.GET %} + +{% else %} +{% endif %} {% else %} {% endif %} -Log in again.
+Log in again ? +{% if 'next_path' in request.GET %} ++Or go back to + +previous page. +
+{% endif %} {% endblock %} diff --git a/swh/web/tests/api/test_throttling.py b/swh/web/tests/api/test_throttling.py --- a/swh/web/tests/api/test_throttling.py +++ b/swh/web/tests/api/test_throttling.py @@ -19,6 +19,7 @@ scope2_limiter_rate, scope2_limiter_rate_post, scope3_limiter_rate, scope3_limiter_rate_post ) +from swh.web.urls import urlpatterns class MockViewScope1(APIView): @@ -61,7 +62,7 @@ return Response('bar_post') -urlpatterns = [ +urlpatterns += [ url(r'^scope1_class$', MockViewScope1.as_view()), url(r'^scope2_func$', mock_view_scope2), url(r'^scope3_class$', MockViewScope3.as_view()), diff --git a/swh/web/tests/auth/keycloak_mock.py b/swh/web/tests/auth/keycloak_mock.py --- a/swh/web/tests/auth/keycloak_mock.py +++ b/swh/web/tests/auth/keycloak_mock.py @@ -17,12 +17,13 @@ class KeycloackOpenIDConnectMock(KeycloakOpenIDConnect): - def __init__(self, auth_success=True): + def __init__(self, auth_success=True, exp=None): swhweb_config = get_config() super().__init__(swhweb_config['keycloak']['server_url'], swhweb_config['keycloak']['realm_name'], OIDC_SWH_WEB_CLIENT_ID) self.auth_success = auth_success + self.exp = exp self._keycloak.public_key = lambda: realm_public_key self._keycloak.well_know = lambda: { 'issuer': f'{self.server_url}realms/{self.realm_name}', @@ -66,14 +67,18 @@ decoded = super().decode_token(token, options) # tweak auth and exp time for tests expire_in = decoded['exp'] - decoded['auth_time'] - decoded['auth_time'] = int(timezone.now().timestamp()) - decoded['exp'] = decoded['auth_time'] + expire_in + if self.exp is not None: + decoded['exp'] = self.exp + decoded['auth_time'] = self.exp - expire_in + else: + decoded['auth_time'] = int(timezone.now().timestamp()) + decoded['exp'] = decoded['auth_time'] + expire_in decoded['groups'] = ['/staff'] return decoded -def mock_keycloak(mocker, auth_success=True): - kc_oidc_mock = KeycloackOpenIDConnectMock(auth_success) +def mock_keycloak(mocker, auth_success=True, exp=None): + kc_oidc_mock = KeycloackOpenIDConnectMock(auth_success, exp) mock_get_oidc_client = mocker.patch( 'swh.web.auth.views.get_oidc_client') mock_get_oidc_client.return_value = kc_oidc_mock diff --git a/swh/web/tests/auth/test_middlewares.py b/swh/web/tests/auth/test_middlewares.py new file mode 100644 --- /dev/null +++ b/swh/web/tests/auth/test_middlewares.py @@ -0,0 +1,46 @@ +# Copyright (C) 2020 The Software Heritage developers +# See the AUTHORS file at the top-level directory of this distribution +# License: GNU Affero General Public License version 3, or any later version +# See top-level LICENSE file for more information + +from datetime import datetime + +from django.test import modify_settings +import pytest + +from .keycloak_mock import mock_keycloak + +from swh.web.common.utils import reverse + + +@pytest.mark.django_db +@modify_settings(MIDDLEWARE={ + 'remove': ['swh.web.auth.middlewares.OIDCSessionRefreshMiddleware'] +}) +def test_oidc_session_refresh_middleware_disabled(client, mocker): + # authenticate but make session expires immediately + kc_oidc_mock = mock_keycloak(mocker, exp=int(datetime.now().timestamp())) + client.login(code='', code_verifier='', redirect_uri='') + kc_oidc_mock.authorization_code.assert_called() + + url = reverse('swh-web-homepage') + resp = client.get(url) + # no redirection for silent refresh + assert resp.status_code != 302 + + +@pytest.mark.django_db +def test_oidc_session_refresh_middleware_enabled(client, mocker): + # authenticate but make session expires immediately + kc_oidc_mock = mock_keycloak(mocker, exp=int(datetime.now().timestamp())) + client.login(code='', code_verifier='', redirect_uri='') + kc_oidc_mock.authorization_code.assert_called() + + url = reverse('swh-web-homepage') + resp = client.get(url) + + # should redirect for silent session refresh + assert resp.status_code == 302 + silent_refresh_url = reverse('oidc-login', query_params={'next_path': url, + 'prompt': 'none'}) + assert resp['location'] == silent_refresh_url diff --git a/swh/web/tests/auth/test_views.py b/swh/web/tests/auth/test_views.py --- a/swh/web/tests/auth/test_views.py +++ b/swh/web/tests/auth/test_views.py @@ -168,7 +168,8 @@ 'code_verifier': '', 'state': str(uuid.uuid4()), 'redirect_uri': '', - 'next': None, + 'next_path': '', + 'prompt': '', } session.save() @@ -196,7 +197,8 @@ 'code_verifier': '', 'state': str(uuid.uuid4()), 'redirect_uri': '', - 'next': None, + 'next_path': '', + 'prompt': '', } session.save() @@ -228,7 +230,8 @@ 'code_verifier': '', 'state': str(uuid.uuid4()), 'redirect_uri': '', - 'next': None, + 'next_path': '', + 'prompt': '', } session.save() @@ -273,3 +276,41 @@ # user should be logged out from Django anyway assert isinstance(request.user, AnonymousUser) + + +@pytest.mark.django_db +def test_oidc_silent_refresh_failure(client, mocker): + # mock Keycloak client + mock_keycloak(mocker) + + next_path = reverse('swh-web-homepage') + + # silent session refresh initialization + login_url = reverse('oidc-login', query_params={'next_path': next_path, + 'prompt': 'none'}) + response = client.get(login_url) + request = response.wsgi_request + + login_data = request.session['login_data'] + + # check prompt value has been registered in user session + assert 'prompt' in login_data + assert login_data['prompt'] == 'none' + + # simulate a failed silent session refresh + session_state = str(uuid.uuid4()) + + login_complete_url = reverse('oidc-login-complete', + query_params={'error': 'login_required', + 'state': login_data['state'], + 'session_state': session_state}) + + # login process finalization + response = client.get(login_complete_url) + request = response.wsgi_request + + # should redirect to logout page + assert response.status_code == 302 + logout_url = reverse('logout', query_params={'next_path': next_path, + 'remote_user': 1}) + assert response['location'] == logout_url