Merge pull request #14896 from open-craft/haikuginger/block-existing-session-decorator

[ENT-326] Logout SSO users arriving at course modes view who already have session

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))

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

common/djangoapps/third_party_auth/migrations/0008_auto_20170413_1455.py

+# -*- 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.'),
+        ),
+    ]

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

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))