From 3f6e690827464c5c542ddc1fbb325c6d6851fdd0 Mon Sep 17 00:00:00 2001 From: Jesse Shapiro Date: Mon, 17 Apr 2017 23:09:01 -0400 Subject: [PATCH] When visiting track selection page with a TPA hint, logout the existing session --- common/djangoapps/course_modes/views.py | 3 + .../djangoapps/third_party_auth/decorators.py | 60 ++++++++++++- .../migrations/0008_auto_20170413_1455.py | 29 ++++++ common/djangoapps/third_party_auth/models.py | 8 ++ .../third_party_auth/tests/test_decorators.py | 90 ++++++++++++++++++- 5 files changed, 187 insertions(+), 3 deletions(-) create mode 100644 common/djangoapps/third_party_auth/migrations/0008_auto_20170413_1455.py diff --git a/common/djangoapps/course_modes/views.py b/common/djangoapps/course_modes/views.py index 637103730d..ef351e1e05 100644 --- a/common/djangoapps/course_modes/views.py +++ b/common/djangoapps/course_modes/views.py @@ -28,6 +28,7 @@ from openedx.features.enterprise_support import api as enterprise_api from student.models import CourseEnrollment from util.db import outer_atomic from util import organizations_helpers as organization_api +from third_party_auth.decorators import tpa_hint_ends_existing_session class ChooseModeView(View): @@ -52,6 +53,7 @@ class ChooseModeView(View): """ return super(ChooseModeView, self).dispatch(*args, **kwargs) + @method_decorator(tpa_hint_ends_existing_session) @method_decorator(login_required) @method_decorator(transaction.atomic) def get(self, request, course_id, error=None): @@ -199,6 +201,7 @@ class ChooseModeView(View): return render_to_response("course_modes/choose.html", context) + @method_decorator(tpa_hint_ends_existing_session) @method_decorator(transaction.non_atomic_requests) @method_decorator(login_required) @method_decorator(outer_atomic(read_committed=True)) diff --git a/common/djangoapps/third_party_auth/decorators.py b/common/djangoapps/third_party_auth/decorators.py index 438d15d146..05385b11b2 100644 --- a/common/djangoapps/third_party_auth/decorators.py +++ b/common/djangoapps/third_party_auth/decorators.py @@ -2,11 +2,15 @@ Decorators that can be used to interact with third_party_auth. """ from functools import wraps -from urlparse import urlparse + +from six.moves.urllib.parse import urlencode, urlparse from django.conf import settings +from django.shortcuts import redirect from django.utils.decorators import available_attrs +from django.core.urlresolvers import reverse +from third_party_auth.provider import Registry from third_party_auth.models import LTIProviderConfig @@ -30,3 +34,57 @@ def xframe_allow_whitelisted(view_func): resp['X-Frame-Options'] = x_frame_option return resp return wraps(view_func, assigned=available_attrs(view_func))(wrapped_view) + + +def tpa_hint_ends_existing_session(func): + """ + Modifies a view function so that, if a tpa_hint URL parameter is received, the user is + already logged in, and the hinted SSO provider is so configured, the user is redirected + to a logout view and then back here. When they're directed back here, a URL parameter + called "session_cleared" will be attached to indicate that, even though a user is now + logged in, they should be passed through without intervention. + """ + + @wraps(func) + def inner(request, *args, **kwargs): + """ + Check for a TPA hint in combination with a logged in user, and log the user out + if the hinted provider specifies that they should be, and if they haven't already + been redirected to a logout by this decorator. + """ + sso_provider = None + provider_id = request.GET.get('tpa_hint') + decorator_already_processed = request.GET.get('session_cleared') == 'yes' + if provider_id and not decorator_already_processed: + # Check that there is a provider and that we haven't already processed this view. + if request.user and request.user.is_authenticated(): + try: + sso_provider = Registry.get(provider_id=provider_id) + except ValueError: + sso_provider = None + if sso_provider and sso_provider.drop_existing_session: + # Do the redirect only if the configured provider says we ought to. + return redirect( + '{}?{}'.format( + request.build_absolute_uri(reverse('logout')), + urlencode( + { + 'redirect_url': '{}?{}'.format( + request.path, + urlencode( + [ + ('tpa_hint', provider_id), + ('session_cleared', 'yes') + ] + ) + ) + } + ) + ) + ) + + else: + # Otherwise, pass everything through to the wrapped view. + return func(request, *args, **kwargs) + + return inner diff --git a/common/djangoapps/third_party_auth/migrations/0008_auto_20170413_1455.py b/common/djangoapps/third_party_auth/migrations/0008_auto_20170413_1455.py new file mode 100644 index 0000000000..ee742c1b82 --- /dev/null +++ b/common/djangoapps/third_party_auth/migrations/0008_auto_20170413_1455.py @@ -0,0 +1,29 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('third_party_auth', '0007_auto_20170406_0912'), + ] + + operations = [ + migrations.AddField( + model_name='ltiproviderconfig', + name='drop_existing_session', + field=models.BooleanField(default=False, help_text='Whether to drop an existing session when accessing a view decorated with third_party_auth.decorators.tpa_hint_ends_existing_session when a tpa_hint URL query parameter mapping to this provider is included in the request.'), + ), + migrations.AddField( + model_name='oauth2providerconfig', + name='drop_existing_session', + field=models.BooleanField(default=False, help_text='Whether to drop an existing session when accessing a view decorated with third_party_auth.decorators.tpa_hint_ends_existing_session when a tpa_hint URL query parameter mapping to this provider is included in the request.'), + ), + migrations.AddField( + model_name='samlproviderconfig', + name='drop_existing_session', + field=models.BooleanField(default=False, help_text='Whether to drop an existing session when accessing a view decorated with third_party_auth.decorators.tpa_hint_ends_existing_session when a tpa_hint URL query parameter mapping to this provider is included in the request.'), + ), + ] diff --git a/common/djangoapps/third_party_auth/models.py b/common/djangoapps/third_party_auth/models.py index d42c68df86..4ddc1bcda5 100644 --- a/common/djangoapps/third_party_auth/models.py +++ b/common/djangoapps/third_party_auth/models.py @@ -140,6 +140,14 @@ class ProviderConfig(ConfigurationModel): "authentication using the correct link is still possible." ), ) + drop_existing_session = models.BooleanField( + default=False, + help_text=_( + "Whether to drop an existing session when accessing a view decorated with " + "third_party_auth.decorators.tpa_hint_ends_existing_session when a tpa_hint " + "URL query parameter mapping to this provider is included in the request." + ) + ) prefix = None # used for provider_id. Set to a string value in subclass backend_name = None # Set to a field or fixed value in subclass accepts_logins = True # Whether to display a sign-in button when the provider is enabled diff --git a/common/djangoapps/third_party_auth/tests/test_decorators.py b/common/djangoapps/third_party_auth/tests/test_decorators.py index 9c6982238b..40fe3bc98c 100644 --- a/common/djangoapps/third_party_auth/tests/test_decorators.py +++ b/common/djangoapps/third_party_auth/tests/test_decorators.py @@ -2,13 +2,17 @@ Tests for third_party_auth decorators. """ import ddt +import json import unittest +from mock import MagicMock +from six.moves.urllib.parse import urlencode + from django.conf import settings -from django.http import HttpResponse +from django.http import HttpResponse, JsonResponse from django.test import RequestFactory -from third_party_auth.decorators import xframe_allow_whitelisted +from third_party_auth.decorators import tpa_hint_ends_existing_session, xframe_allow_whitelisted from third_party_auth.tests.testutil import TestCase @@ -18,6 +22,14 @@ def mock_view(_request): return HttpResponse() +@tpa_hint_ends_existing_session +def mock_hinted_view(request): + """ + Another test view for testing purposes. + """ + return JsonResponse({"tpa_hint": request.GET.get('tpa_hint')}) + + # remove this decorator once third_party_auth is enabled in CMS @unittest.skipIf( 'third_party_auth' not in settings.INSTALLED_APPS, @@ -57,3 +69,77 @@ class TestXFrameWhitelistDecorator(TestCase): request = self.construct_request(url) response = mock_view(request) self.assertEqual(response['X-Frame-Options'], 'DENY') + + +@unittest.skipIf( + 'third_party_auth' not in settings.INSTALLED_APPS, + 'third_party_auth is not currently installed in CMS' +) +@ddt.ddt +class TestTpaHintSessionEndingDecorator(TestCase): + """ + In cases where users may be sharing computers, we want to ensure + that a hinted link (has a tpa_hint query parameter) from an external site + will always force the user to login again with SSO, even if the user is already + logged in. (For SSO providers that enable this option.) + + This aims to prevent a situation where user 1 forgets to logout, + then user 2 logs in to the external site and takes a link to the + Open edX instance, but gets user 1's session. + """ + + url = '/protected_view' + + def setUp(self): + super(TestTpaHintSessionEndingDecorator, self).setUp() + self.enable_saml() + self.factory = RequestFactory() + + def get_user_mock(self, authenticated=False): + user = MagicMock() + user.is_authenticated.return_value = authenticated + return user + + def get_request_mock(self, authenticated=False, hinted=False, done=False): + user = self.get_user_mock(authenticated) + params = {} + if hinted: + params['tpa_hint'] = 'saml-realprovider' + if done: + params['session_cleared'] = 'yes' + url = '{}?{}'.format(self.url, urlencode(params)) + request = self.factory.get(url) + request.user = user + return request + + def create_provider(self, protected=False): + self.configure_saml_provider( + enabled=True, + name="Fancy real provider", + idp_slug="realprovider", + backend_name="tpa-saml", + drop_existing_session=protected, + ) + + @ddt.unpack + @ddt.data( + (True, True, False, False, False), + (True, True, True, False, True), + (True, True, True, True, False), + (True, True, False, True, False), + (False, True, True, False, False), + (True, False, True, False, False), + ) + def test_redirect_when_logged_in_with_hint(self, protected, authenticated, hinted, done, redirects): + self.create_provider(protected) + request = self.get_request_mock(authenticated, hinted, done) + response = mock_hinted_view(request) + if redirects: + self.assertRedirects( + response, + '/logout?redirect_url=%2Fprotected_view%3Ftpa_' + 'hint%3Dsaml-realprovider%26session_cleared%3Dyes', + fetch_redirect_response=False, + ) + else: + self.assertEqual(json.loads(response.content)['tpa_hint'], ('saml-realprovider' if hinted else None))