From d7ed021b8d73f54cd83738515bed8687a0d221cc Mon Sep 17 00:00:00 2001 From: "zia.fazal@arbisoft.com" Date: Wed, 4 Dec 2019 20:10:33 +0500 Subject: [PATCH] Added ability to logout from IDP Logout link should be displayed only for learner portal Added changed to display only for learner portal Added unit tests check third_party_auth is enabled Changes to extend SSO logout link feature to Oauth providers Fixed quality violations Removed unncessary assert Reviewer feedback changes --- common/djangoapps/third_party_auth/models.py | 7 +++ .../djangoapps/third_party_auth/pipeline.py | 16 ++++++ .../third_party_auth/tests/test_pipeline.py | 34 ++++++++++++ lms/envs/common.py | 2 +- lms/templates/logout.html | 55 ++++++++++++------- .../djangoapps/user_authn/views/logout.py | 21 +++++++ .../user_authn/views/tests/test_logout.py | 27 ++++++++- 7 files changed, 137 insertions(+), 25 deletions(-) diff --git a/common/djangoapps/third_party_auth/models.py b/common/djangoapps/third_party_auth/models.py index 58d8725f65..08fb3ec3b6 100644 --- a/common/djangoapps/third_party_auth/models.py +++ b/common/djangoapps/third_party_auth/models.py @@ -673,6 +673,13 @@ class SAMLProviderConfig(ProviderConfig): """ Get social auth uid from remote id by prepending idp_slug to the remote id """ return '{}:{}'.format(self.slug, remote_id) + def get_setting(self, name): + """ Get the value of a setting, or raise KeyError """ + if self.other_settings: + other_settings = json.loads(self.other_settings) + return other_settings[name] + raise KeyError + def get_config(self): """ Return a SAMLIdentityProvider instance for use by SAMLAuthBackend. diff --git a/common/djangoapps/third_party_auth/pipeline.py b/common/djangoapps/third_party_auth/pipeline.py index 908c0d06ba..b9fe7dc2a4 100644 --- a/common/djangoapps/third_party_auth/pipeline.py +++ b/common/djangoapps/third_party_auth/pipeline.py @@ -83,6 +83,7 @@ from social_core.pipeline import partial from social_core.pipeline.social_auth import associate_by_email from social_core.utils import module_member, slugify +import third_party_auth from edxmako.shortcuts import render_to_string from lms.djangoapps.verify_student.models import SSOVerification from lms.djangoapps.verify_student.utils import earliest_allowed_verification_date @@ -220,6 +221,21 @@ def get(request): return pipeline_data +def get_idp_logout_url_from_running_pipeline(request): + """ + Returns: IdP's logout url associated with running pipeline + """ + if third_party_auth.is_enabled(): + running_pipeline = get(request) + if running_pipeline: + tpa_provider = provider.Registry.get_from_pipeline(running_pipeline) + if tpa_provider: + try: + return tpa_provider.get_setting('logout_url') + except KeyError: + logger.info(u'[THIRD_PARTY_AUTH] idP [%s] logout_url setting not defined', tpa_provider.name) + + def get_real_social_auth_object(request): """ At times, the pipeline will have a "social" kwarg that contains a dictionary diff --git a/common/djangoapps/third_party_auth/tests/test_pipeline.py b/common/djangoapps/third_party_auth/tests/test_pipeline.py index 436c3455f1..cbd2113159 100644 --- a/common/djangoapps/third_party_auth/tests/test_pipeline.py +++ b/common/djangoapps/third_party_auth/tests/test_pipeline.py @@ -1,13 +1,18 @@ """Unit tests for third_party_auth/pipeline.py.""" +import json +import ddt +import mock import unittest from third_party_auth import pipeline from third_party_auth.tests import testutil +from third_party_auth.tests.testutil import simulate_running_pipeline @unittest.skipUnless(testutil.AUTH_FEATURE_ENABLED, testutil.AUTH_FEATURES_KEY + ' not enabled') +@ddt.ddt class ProviderUserStateTestCase(testutil.TestCase): """Tests ProviderUserState behavior.""" @@ -15,3 +20,32 @@ class ProviderUserStateTestCase(testutil.TestCase): google_provider = self.configure_google_provider(enabled=True) state = pipeline.ProviderUserState(google_provider, object(), None) self.assertEqual(google_provider.provider_id + '_unlink_form', state.get_unlink_form_name()) + + @ddt.data( + ('saml', 'tpa-saml'), + ('oauth', 'google-oauth2'), + ) + @ddt.unpack + def test_get_idp_logout_url_from_running_pipeline(self, idp_type, backend_name): + """ + Test idp logout url setting for running pipeline + """ + self.enable_saml() + idp_slug = "test" + idp_config = {"logout_url": "http://example.com/logout"} + getattr(self, 'configure_{idp_type}_provider'.format(idp_type=idp_type))( + enabled=True, + name="Test Provider", + slug=idp_slug, + backend_name=backend_name, + other_settings=json.dumps(idp_config) + ) + request = mock.MagicMock() + kwargs = { + "response": { + "idp_name": idp_slug + } + } + with simulate_running_pipeline("third_party_auth.pipeline", backend_name, **kwargs): + logout_url = pipeline.get_idp_logout_url_from_running_pipeline(request) + self.assertEqual(idp_config['logout_url'], logout_url) diff --git a/lms/envs/common.py b/lms/envs/common.py index 72b0100d93..e9a52627cd 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -3843,7 +3843,7 @@ BLOCKSTORE_API_URL = 'http://localhost:18250/api/v1/' XBLOCK_RUNTIME_V2_EPHEMERAL_DATA_CACHE = 'default' ########################## LEARNER PORTAL ############################## -LEARNER_PORTAL_URL_ROOT = 'https://learner-portal-localhost:18000' +LEARNER_PORTAL_URL_ROOT = 'http://localhost:8734' ######################### MICROSITE ############################### MICROSITE_ROOT_DIR = '/edx/app/edxapp/edx-microsite' diff --git a/lms/templates/logout.html b/lms/templates/logout.html index 617c463ad1..8dbeb92ba5 100644 --- a/lms/templates/logout.html +++ b/lms/templates/logout.html @@ -5,30 +5,45 @@ {% block title %}{% trans "Signed Out" as tmsg %}{{ tmsg | force_escape }} | {{ block.super }}{% endblock %} {% block body %} - {% if enterprise_target %} - {% comment %} - For enterprise SSO flow we intentionally drop learner's session. - We are showing this signin message instead of logout message - to avoid any confusion for learner in that case. - {% endcomment %} -

{% trans "We are signing you in." as tmsg %}{{ tmsg | force_escape }}

- -

- {% filter force_escape %} - {% blocktrans %} - This may take a minute. If you are not redirected, go to the home page. - {% endblocktrans %} - {% endfilter %} -

- {% else %} + {% if show_tpa_logout_link %}

{% trans "You have signed out." as tmsg %}{{ tmsg | force_escape }}

- {% blocktrans trimmed asvar signout_msg1 %} - If you are not redirected within 5 seconds, {start_anchor}click here to go to the home page{end_anchor}. + {% blocktrans trimmed asvar sso_signout_msg %} + {start_anchor}Click here{end_anchor} to delete your single signed on (SSO) session. {% endblocktrans %} - {% interpolate_html signout_msg1 start_anchor=''|safe end_anchor=''|safe %} + {% interpolate_html sso_signout_msg start_anchor=''|safe end_anchor=''|safe %}

+ + {% else %} + {% if enterprise_target %} + {% comment %} + For enterprise SSO flow we intentionally drop learner's session. + We are showing this signin message instead of logout message + to avoid any confusion for learner in that case. + {% endcomment %} +

{% trans "We are signing you in." as tmsg %}{{ tmsg | force_escape }}

+ +

+ {% filter force_escape %} + {% blocktrans %} + This may take a minute. If you are not redirected, go to the home page. + {% endblocktrans %} + {% endfilter %} +

+ {% else %} +

{% trans "You have signed out." as tmsg %}{{ tmsg | force_escape }}

+ +

+ {% blocktrans trimmed asvar signout_msg1 %} + If you are not redirected within 5 seconds, {start_anchor}click here to go to the home page{end_anchor}. + {% endblocktrans %} + {% interpolate_html signout_msg1 start_anchor=''|safe end_anchor=''|safe %} +

+ {% endif %} + + + {% endif %} - - {% endblock body %} diff --git a/openedx/core/djangoapps/user_authn/views/logout.py b/openedx/core/djangoapps/user_authn/views/logout.py index 466fb91ee7..093f1bab5f 100644 --- a/openedx/core/djangoapps/user_authn/views/logout.py +++ b/openedx/core/djangoapps/user_authn/views/logout.py @@ -15,6 +15,7 @@ from six.moves.urllib.parse import parse_qs, urlsplit, urlunsplit # pylint: dis from openedx.core.djangoapps.user_authn.cookies import delete_logged_in_cookies from openedx.core.djangoapps.user_authn.utils import is_safe_login_or_logout_redirect +from third_party_auth import pipeline as tpa_pipeline class LogoutView(TemplateView): @@ -29,6 +30,7 @@ class LogoutView(TemplateView): # Keep track of the page to which the user should ultimately be redirected. default_target = '/' + tpa_logout_url = '' def post(self, request, *args, **kwargs): """ @@ -68,6 +70,9 @@ class LogoutView(TemplateView): # We do not log here, because we have a handler registered to perform logging on successful logouts. request.is_from_logout = True + # Get third party auth provider's logout url + self.tpa_logout_url = tpa_pipeline.get_idp_logout_url_from_running_pipeline(request) + # Get the list of authorized clients before we clear the session. self.oauth_client_ids = request.session.get(edx_oauth2_provider.constants.AUTHORIZED_CLIENTS_SESSION_KEY, []) @@ -105,6 +110,20 @@ class LogoutView(TemplateView): unquoted_url = parse.unquote_plus(parse.quote(url)) return bool(re.match(r'^/enterprise/[a-z0-9\-]+/course', unquoted_url)) + def _show_tpa_logout_link(self, target, referrer): + """ + Return Boolean value indicating if TPA logout link needs to displayed or not. + We display TPA logout link when user has active SSO session and logout flow is + triggered via learner portal. + Args: + target: url of the page to land after logout + referrer: url of the page where logout request initiated + """ + if bool(target == self.default_target and self.tpa_logout_url) and settings.LEARNER_PORTAL_URL_ROOT in referrer: + return True + + return False + def get_context_data(self, **kwargs): context = super(LogoutView, self).get_context_data(**kwargs) @@ -134,6 +153,8 @@ class LogoutView(TemplateView): 'target': target, 'logout_uris': logout_uris, 'enterprise_target': self._is_enterprise_target(target), + 'tpa_logout_url': self.tpa_logout_url, + 'show_tpa_logout_link': self._show_tpa_logout_link(target, referrer), }) return context diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_logout.py b/openedx/core/djangoapps/user_authn/views/tests/test_logout.py index dcbc92afe8..1b09322dd3 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_logout.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_logout.py @@ -6,6 +6,7 @@ Tests for logout import unittest import ddt +import mock import six.moves.urllib.parse as parse # pylint: disable=import-error from django.conf import settings from django.test import TestCase @@ -13,7 +14,6 @@ from django.test.utils import override_settings from django.urls import reverse from edx_oauth2_provider.constants import AUTHORIZED_CLIENTS_SESSION_KEY from edx_oauth2_provider.tests.factories import ClientFactory, TrustedClientFactory -from mock import patch from student.tests.factories import UserFactory @@ -120,7 +120,7 @@ class LogoutTests(TestCase): } self.assertDictContainsSubset(expected, response.context_data) - @patch( + @mock.patch( 'django.conf.settings.IDA_LOGOUT_URI_LIST', ['http://fake.ida1/logout', 'http://fake.ida2/accounts/logout', ] ) @@ -147,7 +147,7 @@ class LogoutTests(TestCase): } self.assertDictContainsSubset(expected, response.context_data) - @patch( + @mock.patch( 'django.conf.settings.IDA_LOGOUT_URI_LIST', ['http://fake.ida1/logout', 'http://fake.ida2/accounts/logout', ] ) @@ -179,5 +179,26 @@ class LogoutTests(TestCase): expected = { 'logout_uris': [], 'target': '/', + 'show_tpa_logout_link': False, } self.assertDictContainsSubset(expected, response.context_data) + + def test_learner_portal_logout_having_idp_logout_url(self): + """ + Test when learner logout from learner portal having active SSO session + logout page should have link to logout url IdP. + """ + learner_portal_logout_url = '{}/logout'.format(settings.LEARNER_PORTAL_URL_ROOT) + idp_logout_url = 'http://mock-idp.com/logout' + client = self._create_oauth_client() + + with mock.patch( + 'openedx.core.djangoapps.user_authn.views.logout.tpa_pipeline.get_idp_logout_url_from_running_pipeline' + ) as mock_idp_logout_url: + mock_idp_logout_url.return_value = idp_logout_url + response = self._assert_session_logged_out(client, HTTP_REFERER=learner_portal_logout_url) + expected = { + 'tpa_logout_url': idp_logout_url, + 'show_tpa_logout_link': True, + } + self.assertDictContainsSubset(expected, response.context_data)