Merge pull request #842 from edx/jbau/shib-revamp

Revamped + Enhanced Shibboleth support

common/djangoapps/external_auth/tests/test_helper.py

+"""
+Tests for utility functions in external_auth module
+"""
+from django.test import TestCase
+from external_auth.views import _safe_postlogin_redirect
+
+
+class ExternalAuthHelperFnTest(TestCase):
+    """
+    Unit tests for the external_auth.views helper function
+    """
+    def test__safe_postlogin_redirect(self):
+        """
+        Tests the _safe_postlogin_redirect function with different values of next
+        """
+        HOST = 'testserver'                               # pylint: disable=C0103
+        ONSITE1 = '/dashboard'                            # pylint: disable=C0103
+        ONSITE2 = '/courses/org/num/name/courseware'      # pylint: disable=C0103
+        ONSITE3 = 'http://{}/my/custom/url'.format(HOST)  # pylint: disable=C0103
+        OFFSITE1 = 'http://www.attacker.com'              # pylint: disable=C0103
+
+        for redirect_to in [ONSITE1, ONSITE2, ONSITE3]:
+            redir = _safe_postlogin_redirect(redirect_to, HOST)
+            self.assertEqual(redir.status_code, 302)
+            self.assertEqual(redir['location'], redirect_to)
+
+        redir2 = _safe_postlogin_redirect(OFFSITE1, HOST)
+        self.assertEqual(redir2.status_code, 302)
+        self.assertEqual("/", redir2['location'])

common/djangoapps/student/tests/test_login.py

@@ -2,14 +2,26 @@
 Tests for student activation and login
 '''
 import json
+import unittest
 from mock import patch
 
 from django.test import TestCase
 from django.test.client import Client
+from django.test.utils import override_settings
+from django.conf import settings
 from django.core.cache import cache
 from django.core.urlresolvers import reverse, NoReverseMatch
 from student.tests.factories import UserFactory, RegistrationFactory, UserProfileFactory
+from student.views import _parse_course_id_from_string, _get_course_enrollment_domain
 
+from xmodule.modulestore.tests.factories import CourseFactory
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, mixed_store_config
+from xmodule.modulestore.inheritance import own_metadata
+from xmodule.modulestore.django import editable_modulestore
+
+from external_auth.models import ExternalAuthMap
+
+TEST_DATA_MIXED_MODULESTORE = mixed_store_config(settings.COMMON_TEST_DATA_ROOT, {})
 
 class LoginTest(TestCase):
     '''
@@ -154,13 +166,109 @@ class LoginTest(TestCase):
 
     def _assert_audit_log(self, mock_audit_log, level, log_strings):
         """
-        Check that the audit log has received the expected call.
+        Check that the audit log has received the expected call as its last call.
         """
         method_calls = mock_audit_log.method_calls
-        self.assertEquals(len(method_calls), 1)
-        name, args, _kwargs = method_calls[0]
+        name, args, _kwargs = method_calls[-1]
         self.assertEquals(name, level)
         self.assertEquals(len(args), 1)
         format_string = args[0]
         for log_string in log_strings:
             self.assertIn(log_string, format_string)
+
+
+class UtilFnTest(TestCase):
+    """
+    Tests for utility functions in student.views
+    """
+    def test__parse_course_id_from_string(self):
+        """
+        Tests the _parse_course_id_from_string util function
+        """
+        COURSE_ID = u'org/num/run'                                # pylint: disable=C0103
+        COURSE_URL = u'/courses/{}/otherstuff'.format(COURSE_ID)  # pylint: disable=C0103
+        NON_COURSE_URL = u'/blahblah'                             # pylint: disable=C0103
+        self.assertEqual(_parse_course_id_from_string(COURSE_URL), COURSE_ID)
+        self.assertIsNone(_parse_course_id_from_string(NON_COURSE_URL))
+
+
+@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
+class ExternalAuthShibTest(ModuleStoreTestCase):
+    """
+    Tests how login_user() interacts with ExternalAuth, in particular Shib
+    """
+    def setUp(self):
+        self.store = editable_modulestore()
+        self.course = CourseFactory.create(org='Stanford', number='456', display_name='NO SHIB')
+        self.shib_course = CourseFactory.create(org='Stanford', number='123', display_name='Shib Only')
+        self.shib_course.enrollment_domain = 'shib:https://idp.stanford.edu/'
+        metadata = own_metadata(self.shib_course)
+        metadata['enrollment_domain'] = self.shib_course.enrollment_domain
+        self.store.update_metadata(self.shib_course.location.url(), metadata)
+        self.user_w_map = UserFactory.create(email='withmap@stanford.edu')
+        self.extauth = ExternalAuthMap(external_id='withmap@stanford.edu',
+                                       external_email='withmap@stanford.edu',
+                                       external_domain='shib:https://idp.stanford.edu/',
+                                       external_credentials="",
+                                       user=self.user_w_map)
+        self.user_w_map.save()
+        self.extauth.save()
+        self.user_wo_map = UserFactory.create(email='womap@gmail.com')
+        self.user_wo_map.save()
+
+    @unittest.skipUnless(settings.MITX_FEATURES.get('AUTH_USE_SHIB'), "AUTH_USE_SHIB not set")
+    def test_login_page_redirect(self):
+        """
+        Tests that when a shib user types their email address into the login page, they get redirected
+        to the shib login.
+        """
+        response = self.client.post(reverse('login'), {'email': self.user_w_map.email, 'password': ''})
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.content, json.dumps({'success': False, 'redirect': reverse('shib-login')}))
+
+    @unittest.skipUnless(settings.MITX_FEATURES.get('AUTH_USE_SHIB'), "AUTH_USE_SHIB not set")
+    def test__get_course_enrollment_domain(self):
+        """
+        Tests the _get_course_enrollment_domain utility function
+        """
+        self.assertIsNone(_get_course_enrollment_domain("I/DONT/EXIST"))
+        self.assertIsNone(_get_course_enrollment_domain(self.course.id))
+        self.assertEqual(self.shib_course.enrollment_domain, _get_course_enrollment_domain(self.shib_course.id))
+
+    @unittest.skipUnless(settings.MITX_FEATURES.get('AUTH_USE_SHIB'), "AUTH_USE_SHIB not set")
+    def test_login_required_dashboard(self):
+        """
+        Tests redirects to when @login_required to dashboard, which should always be the normal login,
+        since there is no course context
+        """
+        response = self.client.get(reverse('dashboard'))
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(response['Location'], 'http://testserver/accounts/login?next=/dashboard')
+
+    @unittest.skipUnless(settings.MITX_FEATURES.get('AUTH_USE_SHIB'), "AUTH_USE_SHIB not set")
+    def test_externalauth_login_required_course_context(self):
+        """
+        Tests the redirects when visiting course-specific URL with @login_required.
+        Should vary by course depending on its enrollment_domain
+        """
+        TARGET_URL = reverse('courseware', args=[self.course.id])            # pylint: disable=C0103
+        noshib_response = self.client.get(TARGET_URL, follow=True)
+        self.assertEqual(noshib_response.redirect_chain[-1],
+                         ('http://testserver/accounts/login?next={url}'.format(url=TARGET_URL), 302))
+        self.assertContains(noshib_response, ("<title>Log into your {platform_name} Account</title>"
+                                              .format(platform_name=settings.PLATFORM_NAME)))
+        self.assertEqual(noshib_response.status_code, 200)
+
+        TARGET_URL_SHIB = reverse('courseware', args=[self.shib_course.id])  # pylint: disable=C0103
+        shib_response = self.client.get(**{'path': TARGET_URL_SHIB,
+                                           'follow': True,
+                                           'REMOTE_USER': self.extauth.external_id,
+                                           'Shib-Identity-Provider': 'https://idp.stanford.edu/'})
+        # Test that the shib-login redirect page with ?next= and the desired page are part of the redirect chain
+        # The 'courseware' page actually causes a redirect itself, so it's not the end of the chain and we
+        # won't test its contents
+        self.assertEqual(shib_response.redirect_chain[-3],
+                         ('http://testserver/shib-login/?next={url}'.format(url=TARGET_URL_SHIB), 302))
+        self.assertEqual(shib_response.redirect_chain[-2],
+                         ('http://testserver{url}'.format(url=TARGET_URL_SHIB), 302))
+        self.assertEqual(shib_response.status_code, 200)

common/djangoapps/student/views.py

@@ -23,7 +23,8 @@ from django.core.urlresolvers import reverse
 from django.core.validators import validate_email, validate_slug, ValidationError
 from django.core.exceptions import ObjectDoesNotExist
 from django.db import IntegrityError, transaction
-from django.http import HttpResponse, HttpResponseBadRequest, HttpResponseForbidden, HttpResponseNotAllowed, Http404
+from django.http import (HttpResponse, HttpResponseBadRequest, HttpResponseForbidden,
+                         HttpResponseNotAllowed, Http404)
 from django.shortcuts import redirect
 from django_future.csrf import ensure_csrf_cookie
 from django.utils.http import cookie_date, base36_to_int, urlencode
@@ -54,6 +55,7 @@ from courseware.courses import get_courses, sort_by_announcement
 from courseware.access import has_access
 
 from external_auth.models import ExternalAuthMap
+import external_auth.views
 
 from bulk_email.models import Optout
 
@@ -92,7 +94,7 @@ def index(request, extra_context={}, user=None):
     # The course selection work is done in courseware.courses.
     domain = settings.MITX_FEATURES.get('FORCE_UNIVERSITY_DOMAIN')  # normally False
     # do explicit check, because domain=None is valid
-    if domain == False:
+    if domain is False:
         domain = request.META.get('HTTP_HOST')
 
     courses = get_courses(None, domain=domain)
@@ -252,6 +254,8 @@ def register_user(request, extra_context=None):
     if extra_context is not None:
         context.update(extra_context)
 
+    if context.get("extauth_domain", '').startswith(external_auth.views.SHIBBOLETH_DOMAIN_PREFIX):
+        return render_to_response('register-shib.html', context)
     return render_to_response('register.html', context)
 
 
@@ -413,11 +417,49 @@ def change_enrollment(request):
     else:
         return HttpResponseBadRequest(_("Enrollment action is invalid"))
 
+
+def _parse_course_id_from_string(input_str):
+    """
+    Helper function to determine if input_str (typically the queryparam 'next') contains a course_id.
+    @param input_str:
+    @return: the course_id if found, None if not
+    """
+    m_obj = re.match(r'^/courses/(?P<course_id>[^/]+/[^/]+/[^/]+)', input_str)
+    if m_obj:
+        return m_obj.group('course_id')
+    return None
+
+
+def _get_course_enrollment_domain(course_id):
+    """
+    Helper function to get the enrollment domain set for a course with id course_id
+    @param course_id:
+    @return:
+    """
+    try:
+        course = course_from_id(course_id)
+        return course.enrollment_domain
+    except ItemNotFoundError:
+        return None
+
+
 @ensure_csrf_cookie
-def accounts_login(request, error=""):
+def accounts_login(request):
+    """
+    This view is mainly used as the redirect from the @login_required decorator.  I don't believe that
+    the login path linked from the homepage uses it.
+    """
     if settings.MITX_FEATURES.get('AUTH_USE_CAS'):
         return redirect(reverse('cas-login'))
-    return render_to_response('login.html', {'error': error})
+    # see if the "next" parameter has been set, whether it has a course context, and if so, whether
+    # there is a course-specific place to redirect
+    redirect_to = request.GET.get('next')
+    if redirect_to:
+        course_id = _parse_course_id_from_string(redirect_to)
+        if course_id and _get_course_enrollment_domain(course_id):
+            return external_auth.views.course_specific_login(request, course_id)
+    return render_to_response('login.html')
+
 
 # Need different levels of logging
 @ensure_csrf_cookie
@@ -435,6 +477,18 @@ def login_user(request, error=""):
         AUDIT_LOG.warning(u"Login failed - Unknown user email: {0}".format(email))
         user = None
 
+    # check if the user has a linked shibboleth account, if so, redirect the user to shib-login
+    # This behavior is pretty much like what gmail does for shibboleth.  Try entering some @stanford.edu
+    # address into the Gmail login.
+    if settings.MITX_FEATURES.get('AUTH_USE_SHIB') and user:
+        try:
+            eamap = ExternalAuthMap.objects.get(user=user)
+            if eamap.external_domain.startswith(external_auth.views.SHIBBOLETH_DOMAIN_PREFIX):
+                return HttpResponse(json.dumps({'success': False, 'redirect': reverse('shib-login')}))
+        except ExternalAuthMap.DoesNotExist:
+            # This is actually the common case, logging in user without external linked login
+            AUDIT_LOG.info("User %s w/o external auth attempting login", user)
+
     # if the user doesn't exist, we want to set the username to an invalid
     # username so that authentication is guaranteed to fail and we can take
     # advantage of the ratelimited backend
@@ -636,9 +690,10 @@ def create_account(request, post_override=None):
         return HttpResponse(json.dumps(js))
 
     # Can't have terms of service for certain SHIB users, like at Stanford
-    tos_not_required = settings.MITX_FEATURES.get("AUTH_USE_SHIB") \
-                       and settings.MITX_FEATURES.get('SHIB_DISABLE_TOS') \
-                       and DoExternalAuth and ("shib" in eamap.external_domain)
+    tos_not_required = (settings.MITX_FEATURES.get("AUTH_USE_SHIB") and
+                        settings.MITX_FEATURES.get('SHIB_DISABLE_TOS') and
+                        DoExternalAuth and
+                        eamap.external_domain.startswith(external_auth.views.SHIBBOLETH_DOMAIN_PREFIX))
 
     if not tos_not_required:
         if post_vars.get('terms_of_service', 'false') != u'true':

lms/envs/common.py

@@ -94,6 +94,8 @@ MITX_FEATURES = {
     'AUTH_USE_OPENID': False,
     'AUTH_USE_MIT_CERTIFICATES': False,
     'AUTH_USE_OPENID_PROVIDER': False,
+    # Even though external_auth is in common, shib assumes the LMS views / urls, so it should only be enabled
+    # in LMS
     'AUTH_USE_SHIB': False,
     'AUTH_USE_CAS': False,
 

lms/templates/login.html

@@ -55,6 +55,12 @@
           } else {
             location.href="${reverse('dashboard')}";
           }
+        } else if(json.hasOwnProperty('redirect')) {
+          var u=decodeURI(window.location.search);
+          if (!isExternal(json.redirect)) { // a paranoid check.  Our server is the one providing json.redirect
+              location.href=json.redirect+u;
+          } // else we just remain on this page, which is fine since this particular path implies a login failure
+            // that has been generated via packet tampering (json.redirect has been messed with).
         } else {
           toggleSubmitButton(true);
           $('.message.submission-error').addClass('is-shown').focus();

lms/templates/register-shib.html

+<%! from django.utils.translation import ugettext as _ %>
+
+<%inherit file="main.html" />
+
+<%namespace name='static' file='static_content.html'/>
+<%namespace file='main.html' import="login_query"/>
+
+<%! from django.core.urlresolvers import reverse %>
+<%! from django.utils import html %>
+<%! from django_countries.countries import COUNTRIES %>
+<%! from student.models import UserProfile %>
+<%! from datetime import date %>
+<%! import calendar %>
+
+<%block name="title"><title>${_("Preferences for {platform_name}").format(platform_name=settings.PLATFORM_NAME)}</title></%block>
+
+<%block name="js_extra">
+  <script type="text/javascript">
+    $(function() {
+
+      var view_name = 'view-register';
+
+      // adding js class for styling with accessibility in mind
+      $('body').addClass('js').addClass(view_name);
+
+      // new window/tab opening
+      $('a[rel="external"], a[class="new-vp"]')
+      .click( function() {
+      window.open( $(this).attr('href') );
+      return false;
+      });
+
+      // form field label styling on focus
+      $("form :input").focus(function() {
+        $("label[for='" + this.id + "']").parent().addClass("is-focused");
+      }).blur(function() {
+        $("label").parent().removeClass("is-focused");
+      });
+
+    });
+
+    (function() {
+      toggleSubmitButton(true);
+
+      $('#register-form').on('submit', function() {
+        toggleSubmitButton(false);
+      });
+
+      $('#register-form').on('ajax:error', function() {
+        toggleSubmitButton(true);
+      });
+
+      $('#register-form').on('ajax:success', function(event, json, xhr) {
+        if(json.success) {
+          location.href="${reverse('dashboard')}";
+       } else {
+         toggleSubmitButton(true);
+         $('.status.message.submission-error').addClass('is-shown').focus();
+         $('.status.message.submission-error .message-copy').html(json.value).stop().css("display", "block");
+         $(".field-error").removeClass('field-error');
+         $("[data-field='"+json.field+"']").addClass('field-error')
+       }
+      });
+    })(this);
+
+    function toggleSubmitButton(enable) {
+      var $submitButton = $('form .form-actions #submit');
+
+      if(enable) {
+        $submitButton.
+          removeClass('is-disabled').
+          removeProp('disabled').
+          html("${_('Update my {platform_name} Account').format(platform_name=settings.PLATFORM_NAME)}");
+      }
+      else {
+        $submitButton.
+          addClass('is-disabled').
+          prop('disabled', true).
+          html(gettext('Processing your account information &hellip;'));
+      }
+    }
+  </script>
+</%block>
+
+<section class="introduction">
+  <header>
+    <h1 class="sr">${_("Welcome {username}!  Please set your preferences below").format(username=extauth_id,
+                       platform_name=settings.PLATFORM_NAME)}</h1>
+  </header>
+</section>
+
+<%block name="login_button"></%block>
+<section class="register container">
+  <section role="main" class="content">
+    <form role="form" id="register-form" method="post" data-remote="true" action="/create_account" novalidate>
+
+      <!-- status messages -->
+      <div role="alert" class="status message">
+        <h3 class="message-title">${_("We're sorry, {platform_name} enrollment is not available in your region").format(platform_name=settings.PLATFORM_NAME)}</h3>
+        <p class="message-copy">Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Lorem ipsum dolor sit amet, consectetuer adipiscing elit.</p>
+      </div>
+
+      <div role="alert" class="status message submission-error" tabindex="-1">
+        <h3 class="message-title">${_("The following errors occured while processing your registration:")} </h3>
+        <ul class="message-copy"> </ul>
+      </div>
+
+      <p class="instructions">
+        ${_('Required fields are noted by <strong class="indicator">bold text and an asterisk (*)</strong>.')}
+      </p>
+
+      <fieldset class="group group-form group-form-requiredinformation">
+        <legend class="sr">${_('Required Information')}</legend>
+
+        <div class="message">
+          <p class="message-copy">${_("Enter a public username:")}</p>
+        </div>
+
+        <ol class="list-input">
+
+          <li class="field required text" id="field-username">
+            <label for="username">${_('Public Username')}</label>
+            <input id="username" type="text" name="username" value="${extauth_username}" placeholder="${_('example: JaneDoe')}" required aria-required="true" />
+            <span class="tip tip-input">${_('Will be shown in any discussions or forums you participate in')}</span>
+          </li>
+
+          % if ask_for_email:
+
+          <li class="field required text" id="field-email">
+            <label for="email">${_("E-mail")}</label>
+            <input class="" id="email" type="email" name="email" value="" placeholder="${_('example: username@domain.com')}" />
+          </li>
+
+          % endif
+
+
+          % if ask_for_fullname:
+
+          <li class="field required text" id="field-name">
+            <label for="name">${_('Full Name')}</label>
+            <input id="name" type="text" name="name" value="" placeholder="$_('example: Jane Doe')}" />
+          </li>
+
+          % endif
+
+        </ol>
+
+      </fieldset>
+
+      <fieldset class="group group-form group-form-accountacknowledgements">
+        <legend class="sr">${_("Account Acknowledgements")}</legend>
+
+        <ol class="list-input">
+          <li class="field-group">
+
+            % if ask_for_tos :
+
+            <div class="field required checkbox" id="field-tos">
+              <input id="tos-yes" type="checkbox" name="terms_of_service" value="true" required aria-required="true" />
+              <label for="tos-yes">${_('I agree to the {link_start}Terms of Service{link_end}').format(
+                link_start='<a href="{url}" class="new-vp">'.format(url=marketing_link('TOS')),
+                link_end='</a>')}</label>
+            </div>
+
+            % endif
+
+            <div class="field required checkbox" id="field-honorcode">
+              <input id="honorcode-yes" type="checkbox" name="honor_code" value="true" />
+              <%
+                ## TODO: provide a better way to override these links
+                if self.stanford_theme_enabled():
+                  honor_code_path = marketing_link('TOS') + "#honor"
+                else:
+                  honor_code_path = marketing_link('HONOR')
+              %>
+              <label for="honorcode-yes">${_('I agree to the {link_start}Honor Code{link_end}').format(
+                link_start='<a href="{url}" class="new-vp">'.format(url=honor_code_path),
+                link_end='</a>')}</label>
+            </div>
+          </li>
+        </ol>
+      </fieldset>
+
+% if course_id and enrollment_action:
+      <input type="hidden" name="enrollment_action" value="${enrollment_action | h}" />
+      <input type="hidden" name="course_id" value="${course_id | h}" />
+% endif
+
+      <div class="form-actions">
+        <button name="submit" type="submit" id="submit" class="action action-primary action-update">${_('Submit')} <span class="orn-plus">+</span> ${_('Update My Account')}</button>
+      </div>
+    </form>
+  </section>
+
+</section>