Merge pull request #15587 from open-craft/jill/tpa-hint-setting

Adds settings.FEATURES['THIRD_PARTY_AUTH_HINT']

common/djangoapps/student/helpers.py

@@ -2,6 +2,7 @@
 import logging
 import mimetypes
 import urllib
+import urlparse
 from datetime import datetime
 
 from django.conf import settings
@@ -16,6 +17,7 @@ from pytz import UTC
 import third_party_auth
 from course_modes.models import CourseMode
 from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification, VerificationDeadline
+from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
 from openedx.core.djangoapps.theming.helpers import get_themes
 
 # Enumeration of per-course verification statuses
@@ -240,6 +242,8 @@ def get_next_url_for_login_page(request):
 
     Otherwise, we go to the ?next= query param or to the dashboard if nothing else is
     specified.
+
+    If THIRD_PARTY_AUTH_HINT is set, then `tpa_hint=<hint>` is added as a query parameter.
     """
     redirect_to = get_redirect_to(request)
     if not redirect_to:
@@ -247,6 +251,7 @@ def get_next_url_for_login_page(request):
             redirect_to = reverse('dashboard')
         except NoReverseMatch:
             redirect_to = reverse('home')
+
     if any(param in request.GET for param in POST_AUTH_PARAMS):
         # Before we redirect to next/dashboard, we need to handle auto-enrollment:
         params = [(param, request.GET[param]) for param in POST_AUTH_PARAMS if param in request.GET]
@@ -255,6 +260,23 @@ def get_next_url_for_login_page(request):
         # Note: if we are resuming a third party auth pipeline, then the next URL will already
         # be saved in the session as part of the pipeline state. That URL will take priority
         # over this one.
+
+    # Append a tpa_hint query parameter, if one is configured
+    tpa_hint = configuration_helpers.get_value(
+        "THIRD_PARTY_AUTH_HINT",
+        settings.FEATURES.get("THIRD_PARTY_AUTH_HINT", '')
+    )
+    if tpa_hint:
+        # Don't add tpa_hint if we're already in the TPA pipeline (prevent infinite loop),
+        # and don't overwrite any existing tpa_hint params (allow tpa_hint override).
+        running_pipeline = third_party_auth.pipeline.get(request)
+        (scheme, netloc, path, query, fragment) = list(urlparse.urlsplit(redirect_to))
+        if not running_pipeline and 'tpa_hint' not in query:
+            params = urlparse.parse_qs(query)
+            params['tpa_hint'] = [tpa_hint]
+            query = urllib.urlencode(params, doseq=True)
+            redirect_to = urlparse.urlunsplit((scheme, netloc, path, query, fragment))
+
     return redirect_to
 
 

common/djangoapps/student/tests/test_helpers.py

@@ -4,12 +4,16 @@ import logging
 
 import ddt
 from django.conf import settings
+from django.contrib.sessions.middleware import SessionMiddleware
 from django.core.urlresolvers import reverse
 from django.test import TestCase
 from django.test.client import RequestFactory
+from django.test.utils import override_settings
+from mock import patch
 from testfixtures import LogCapture
 
 from student.helpers import get_next_url_for_login_page
+from openedx.core.djangoapps.site_configuration.tests.test_util import with_site_configuration_context
 
 LOGGER_NAME = "student.helpers"
 
@@ -23,6 +27,13 @@ class TestLoginHelper(TestCase):
         super(TestLoginHelper, self).setUp()
         self.request = RequestFactory()
 
+    @staticmethod
+    def _add_session(request):
+        """Annotate the request object with a session"""
+        middleware = SessionMiddleware()
+        middleware.process_request(request)
+        request.session.save()
+
     @ddt.data(
         ("https://www.amazon.com", "text/html", None,
          "Unsafe redirect parameter detected after login page: u'https://www.amazon.com'"),
@@ -56,3 +67,38 @@ class TestLoginHelper(TestCase):
         req.META["HTTP_ACCEPT"] = "text/html"  # pylint: disable=no-member
         next_page = get_next_url_for_login_page(req)
         self.assertEqual(next_page, u'/dashboard')
+
+    @patch('student.helpers.third_party_auth.pipeline.get')
+    @ddt.data(
+        # Test requests outside the TPA pipeline - tpa_hint should be added.
+        (None, '/dashboard', '/dashboard', False),
+        ('', '/dashboard', '/dashboard', False),
+        ('', '/dashboard?tpa_hint=oa2-google-oauth2', '/dashboard?tpa_hint=oa2-google-oauth2', False),
+        ('saml-idp', '/dashboard', '/dashboard?tpa_hint=saml-idp', False),
+        # THIRD_PARTY_AUTH_HINT can be overridden via the query string
+        ('saml-idp', '/dashboard?tpa_hint=oa2-google-oauth2', '/dashboard?tpa_hint=oa2-google-oauth2', False),
+
+        # Test requests inside the TPA pipeline - tpa_hint should not be added, preventing infinite loop.
+        (None, '/dashboard', '/dashboard', True),
+        ('', '/dashboard', '/dashboard', True),
+        ('', '/dashboard?tpa_hint=oa2-google-oauth2', '/dashboard?tpa_hint=oa2-google-oauth2', True),
+        ('saml-idp', '/dashboard', '/dashboard', True),
+        # OK to leave tpa_hint overrides in place.
+        ('saml-idp', '/dashboard?tpa_hint=oa2-google-oauth2', '/dashboard?tpa_hint=oa2-google-oauth2', True),
+    )
+    @ddt.unpack
+    def test_third_party_auth_hint(self, tpa_hint, next_url, expected_url, running_pipeline, mock_running_pipeline):
+        mock_running_pipeline.return_value = running_pipeline
+
+        def validate_login():
+            req = self.request.get(reverse("login") + "?next={url}".format(url=next_url))
+            req.META["HTTP_ACCEPT"] = "text/html"  # pylint: disable=no-member
+            self._add_session(req)
+            next_page = get_next_url_for_login_page(req)
+            self.assertEqual(next_page, expected_url)
+
+        with override_settings(FEATURES=dict(settings.FEATURES, THIRD_PARTY_AUTH_HINT=tpa_hint)):
+            validate_login()
+
+        with with_site_configuration_context(configuration=dict(THIRD_PARTY_AUTH_HINT=tpa_hint)):
+            validate_login()

lms/djangoapps/student_account/test/test_views.py

@@ -458,15 +458,63 @@ class StudentAccountLoginAndRegistrationTest(ThirdPartyAuthTestMixin, UrlResetMi
         response = self.client.get(reverse('signin_user'), params, HTTP_ACCEPT="text/html")
         self.assertNotIn(response.content, tpa_hint)
 
-    def test_hinted_login_dialog_disabled(self):
+    @ddt.data(
+        ('signin_user', 'login'),
+        ('register_user', 'register'),
+    )
+    @ddt.unpack
+    def test_hinted_login_dialog_disabled(self, url_name, auth_entry):
         """Test that the dialog doesn't show up for hinted logins when disabled. """
         self.google_provider.skip_hinted_login_dialog = True
         self.google_provider.save()
         params = [("next", "/courses/something/?tpa_hint=oa2-google-oauth2")]
-        response = self.client.get(reverse('signin_user'), params, HTTP_ACCEPT="text/html")
+        response = self.client.get(reverse(url_name), params, HTTP_ACCEPT="text/html")
+        self.assertRedirects(
+            response,
+            'auth/login/google-oauth2/?auth_entry={}&next=%2Fcourses%2Fsomething%2F%3Ftpa_hint%3Doa2-google-oauth2'.format(auth_entry),
+            target_status_code=302
+        )
+
+    @override_settings(FEATURES=dict(settings.FEATURES, THIRD_PARTY_AUTH_HINT='oa2-google-oauth2'))
+    @ddt.data(
+        'signin_user',
+        'register_user',
+    )
+    def test_settings_tpa_hinted_login(self, url_name):
+        """
+        Ensure that settings.FEATURES['THIRD_PARTY_AUTH_HINT'] can set third_party_auth_hint.
+        """
+        params = [("next", "/courses/something/")]
+        response = self.client.get(reverse(url_name), params, HTTP_ACCEPT="text/html")
+        self.assertContains(response, '"third_party_auth_hint": "oa2-google-oauth2"')
+
+        # THIRD_PARTY_AUTH_HINT can be overridden via the query string
+        tpa_hint = self.hidden_enabled_provider.provider_id
+        params = [("next", "/courses/something/?tpa_hint={0}".format(tpa_hint))]
+        response = self.client.get(reverse(url_name), params, HTTP_ACCEPT="text/html")
+        self.assertContains(response, '"third_party_auth_hint": "{0}"'.format(tpa_hint))
+
+        # Even disabled providers in the query string will override THIRD_PARTY_AUTH_HINT
+        tpa_hint = self.hidden_disabled_provider.provider_id
+        params = [("next", "/courses/something/?tpa_hint={0}".format(tpa_hint))]
+        response = self.client.get(reverse(url_name), params, HTTP_ACCEPT="text/html")
+        self.assertNotIn(response.content, tpa_hint)
+
+    @override_settings(FEATURES=dict(settings.FEATURES, THIRD_PARTY_AUTH_HINT='oa2-google-oauth2'))
+    @ddt.data(
+        ('signin_user', 'login'),
+        ('register_user', 'register'),
+    )
+    @ddt.unpack
+    def test_settings_tpa_hinted_login_dialog_disabled(self, url_name, auth_entry):
+        """Test that the dialog doesn't show up for hinted logins when disabled via settings.THIRD_PARTY_AUTH_HINT. """
+        self.google_provider.skip_hinted_login_dialog = True
+        self.google_provider.save()
+        params = [("next", "/courses/something/")]
+        response = self.client.get(reverse(url_name), params, HTTP_ACCEPT="text/html")
         self.assertRedirects(
             response,
-            'auth/login/google-oauth2/?auth_entry=login&next=%2Fcourses%2Fsomething%2F%3Ftpa_hint%3Doa2-google-oauth2',
+            'auth/login/google-oauth2/?auth_entry={}&next=%2Fcourses%2Fsomething%2F%3Ftpa_hint%3Doa2-google-oauth2'.format(auth_entry),
             target_status_code=302
         )
 

lms/djangoapps/student_account/views.py

@@ -86,13 +86,17 @@ def login_and_registration_form(request, initial_mode="login"):
                 if tpa_hint_provider.skip_hinted_login_dialog:
                     # Forward the user directly to the provider's login URL when the provider is configured
                     # to skip the dialog.
+                    if initial_mode == "register":
+                        auth_entry = pipeline.AUTH_ENTRY_REGISTER
+                    else:
+                        auth_entry = pipeline.AUTH_ENTRY_LOGIN
                     return redirect(
-                        pipeline.get_login_url(provider_id, pipeline.AUTH_ENTRY_LOGIN, redirect_url=redirect_to)
+                        pipeline.get_login_url(provider_id, auth_entry, redirect_url=redirect_to)
                     )
                 third_party_auth_hint = provider_id
                 initial_mode = "hinted_login"
-        except (KeyError, ValueError, IndexError):
-            pass
+        except (KeyError, ValueError, IndexError) as ex:
+            log.error("Unknown tpa_hint provider: %s", ex)
 
     # If this is a themed site, revert to the old login/registration pages.
     # We need to do this for now to support existing themes.