Merge pull request #14900 from open-craft/haikuginger/sso-provider-session-expiry

[ENT-327] Allow per-SSO-provider session expiration limits

common/djangoapps/third_party_auth/migrations/0009_auto_20170415_1144.py

+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('third_party_auth', '0008_auto_20170413_1455'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='ltiproviderconfig',
+            name='max_session_length',
+            field=models.PositiveIntegerField(default=None, help_text='If this option is set, then users logging in using this SSO provider will have their session length limited to no longer than this value. If set to 0 (zero), the session will expire upon the user closing their browser. If left blank, the Django platform session default length will be used.', null=True, verbose_name=b'Max session length (seconds)', blank=True),
+        ),
+        migrations.AddField(
+            model_name='oauth2providerconfig',
+            name='max_session_length',
+            field=models.PositiveIntegerField(default=None, help_text='If this option is set, then users logging in using this SSO provider will have their session length limited to no longer than this value. If set to 0 (zero), the session will expire upon the user closing their browser. If left blank, the Django platform session default length will be used.', null=True, verbose_name=b'Max session length (seconds)', blank=True),
+        ),
+        migrations.AddField(
+            model_name='samlproviderconfig',
+            name='max_session_length',
+            field=models.PositiveIntegerField(default=None, help_text='If this option is set, then users logging in using this SSO provider will have their session length limited to no longer than this value. If set to 0 (zero), the session will expire upon the user closing their browser. If left blank, the Django platform session default length will be used.', null=True, verbose_name=b'Max session length (seconds)', blank=True),
+        ),
+    ]

common/djangoapps/third_party_auth/models.py

@@ -148,6 +148,18 @@ class ProviderConfig(ConfigurationModel):
             "URL query parameter mapping to this provider is included in the request."
         )
     )
+    max_session_length = models.PositiveIntegerField(
+        null=True,
+        blank=True,
+        default=None,
+        verbose_name='Max session length (seconds)',
+        help_text=_(
+            "If this option is set, then users logging in using this SSO provider will have "
+            "their session length limited to no longer than this value. If set to 0 (zero), "
+            "the session will expire upon the user closing their browser. If left blank, the "
+            "Django platform session default length will be used."
+        )
+    )
     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/saml.py

@@ -32,7 +32,7 @@ class SAMLAuthBackend(SAMLAuth):  # pylint: disable=abstract-method
         try:
             return self._config.get_setting(name)
         except KeyError:
-            return self.strategy.setting(name, default)
+            return self.strategy.setting(name, default, backend=self)
 
     def auth_url(self):
         """

common/djangoapps/third_party_auth/strategy.py

@@ -3,7 +3,8 @@ A custom Strategy for python-social-auth that allows us to fetch configuration f
 ConfigurationModels rather than django.settings
 """
 from .models import OAuth2ProviderConfig
-from .pipeline import AUTH_ENTRY_CUSTOM
+from .pipeline import AUTH_ENTRY_CUSTOM, get as get_pipeline_from_request
+from .provider import Registry
 from social.backends.oauth import OAuthAuth
 from social.strategies.django_strategy import DjangoStrategy
 
@@ -41,6 +42,15 @@ class ConfigurationModelStrategy(DjangoStrategy):
                 if error_url:
                     return error_url
 
+        # Special case: we want to get this particular setting directly from the provider database
+        # entry if possible; if we don't have the information, fall back to the default behavior.
+        if name == 'MAX_SESSION_LENGTH':
+            running_pipeline = get_pipeline_from_request(self.request) if self.request else None
+            if running_pipeline is not None:
+                provider_config = Registry.get_from_pipeline(running_pipeline)
+                if provider_config:
+                    return provider_config.max_session_length
+
         # At this point, we know 'name' is not set in a [OAuth2|LTI|SAML]ProviderConfig row.
         # It's probably a global Django setting like 'FIELDS_STORED_IN_SESSION':
         return super(ConfigurationModelStrategy, self).setting(name, default, backend)

common/djangoapps/third_party_auth/tests/specs/base.py

@@ -144,7 +144,7 @@ class IntegrationTestMixin(object):
         """
         raise NotImplementedError
 
-    def _test_return_login(self, user_is_activated=True):
+    def _test_return_login(self, user_is_activated=True, previous_session_timed_out=False):
         """ Test logging in to an account that is already linked. """
         # Make sure we're not logged in:
         dashboard_response = self.client.get(reverse('dashboard'))
@@ -156,12 +156,14 @@ class IntegrationTestMixin(object):
         # The user should be redirected to the provider:
         self.assertEqual(try_login_response.status_code, 302)
         login_response = self.do_provider_login(try_login_response['Location'])
-        # There will be one weird redirect required to set the login cookie:
-        self.assertEqual(login_response.status_code, 302)
-        self.assertEqual(login_response['Location'], self.url_prefix + self.complete_url)
-        # And then we should be redirected to the dashboard:
-        login_response = self.client.get(login_response['Location'])
-        self.assertEqual(login_response.status_code, 302)
+        # If the previous session was manually logged out, there will be one weird redirect
+        # required to set the login cookie (it sticks around if the main session times out):
+        if not previous_session_timed_out:
+            self.assertEqual(login_response.status_code, 302)
+            self.assertEqual(login_response['Location'], self.url_prefix + self.complete_url)
+            # And then we should be redirected to the dashboard:
+            login_response = self.client.get(login_response['Location'])
+            self.assertEqual(login_response.status_code, 302)
         if user_is_activated:
             url_expected = reverse('dashboard')
         else:

common/djangoapps/third_party_auth/tests/specs/test_testshib.py

 """
 Third_party_auth integration tests using a mock version of the TestShib provider
 """
+import datetime
 import ddt
 import unittest
 import httpretty
 import json
+import time
 from mock import patch
+from freezegun import freeze_time
 from social.apps.django_app.default.models import UserSocialAuth
 from unittest import skip
 
@@ -90,6 +93,7 @@ class SamlIntegrationTestUtilities(object):
         kwargs.setdefault('metadata_source', TESTSHIB_METADATA_URL)
         kwargs.setdefault('icon_class', 'fa-university')
         kwargs.setdefault('attr_email', 'urn:oid:1.3.6.1.4.1.5923.1.1.1.6')  # eduPersonPrincipalName
+        kwargs.setdefault('max_session_length', None)
         self.configure_saml_provider(**kwargs)
 
         if fetch_metadata:
@@ -207,6 +211,26 @@ class TestShibIntegrationTest(SamlIntegrationTestUtilities, IntegrationTestMixin
         self.assertEqual(num_failed, 0)
         self.assertEqual(len(failure_messages), 0)
 
+    def test_login_with_testshib_provider_short_session_length(self):
+        """
+        Test that when we have a TPA provider which as an explicit maximum
+        session length set, waiting for longer than that between requests
+        results in us being logged out.
+        """
+        # Configure the provider with a 10-second timeout
+        self._configure_testshib_provider(max_session_length=10)
+
+        now = datetime.datetime.utcnow()
+        with freeze_time(now):
+            # Test the login flow, adding the user in the process
+            super(TestShibIntegrationTest, self).test_login()
+
+        # Wait 30 seconds; longer than the manually-set 10-second timeout
+        later = now + datetime.timedelta(seconds=30)
+        with freeze_time(later):
+            # Test returning as a logged in user; this method verifies that we're logged out first.
+            self._test_return_login(previous_session_timed_out=True)
+
 
 @unittest.skipUnless(testutil.AUTH_FEATURE_ENABLED, 'third_party_auth not enabled')
 class SuccessFactorsIntegrationTest(SamlIntegrationTestUtilities, IntegrationTestMixin, testutil.SAMLTestCase):

common/djangoapps/third_party_auth/tests/test_admin.py

@@ -66,6 +66,9 @@ class Oauth2ProviderConfigAdminTest(testutil.TestCase):
         # Remove the icon_image from the POST data, to simulate unchanged icon_image
         post_data = models.model_to_dict(provider1)
         del post_data['icon_image']
+        # Remove max_session_length; it has a default null value which must be POSTed
+        # back as an absent value, rather than as a "null-like" included value.
+        del post_data['max_session_length']
 
         # Change the name, to verify POST
         post_data['name'] = 'Another name'

requirements/edx/base.txt

@@ -90,7 +90,10 @@ python-memcached==1.48
 django-memcached-hashring==0.1.2
 python-openid==2.2.5
 python-dateutil==2.1
-python-social-auth==0.2.21
+# We need to be able to set a maximum session length on our third-party auth providers;
+# our goal is to upstream these changes and return to the canonical version ASAP.
+# python-social-auth==0.2.21
+git+https://github.com/edx/python-social-auth@758985102cee98f440fae44ed99617b7cfef3473#egg=python-social-auth==0.2.21.edx.a
 pytz==2016.7
 pysrt==0.4.7
 PyYAML==3.10