Merge pull request #2768 from carsongee/cg/ssl-next-url

Modified ssl certificate authentication to handle next redirection

cms/djangoapps/contentstore/views/public.py

@@ -9,7 +9,8 @@ from django.conf import settings
 
 from edxmako.shortcuts import render_to_response
 
-from external_auth.views import ssl_login_shortcut, ssl_get_cert_from_request
+from external_auth.views import (ssl_login_shortcut, ssl_get_cert_from_request,
+                                 redirect_with_get)
 from microsite_configuration import microsite
 
 __all__ = ['signup', 'login_page', 'howitworks']
@@ -26,7 +27,7 @@ def signup(request):
     if settings.FEATURES.get('AUTH_USE_CERTIFICATES_IMMEDIATE_SIGNUP'):
         # Redirect to course to login to process their certificate if SSL is enabled
         # and registration is disabled.
-        return redirect(reverse('login'))
+        return redirect_with_get('login', request.GET, False)
 
     return render_to_response('register.html', {'csrf': csrf_token})
 
@@ -43,7 +44,11 @@ def login_page(request):
         # SSL login doesn't require a login view, so redirect
         # to course now that the user is authenticated via
         # the decorator.
-        return redirect('/course')
+        next_url = request.GET.get('next')
+        if next_url:
+            return redirect(next_url)
+        else:
+            return redirect('/course')
     if settings.FEATURES.get('AUTH_USE_CAS'):
         # If CAS is enabled, redirect auth handling to there
         return redirect(reverse('cas-login'))

common/djangoapps/external_auth/tests/test_ssl.py

@@ -21,19 +21,29 @@ from mock import Mock
 from edxmako.middleware import MakoMiddleware
 from external_auth.models import ExternalAuthMap
 import external_auth.views
+from student.roles import CourseStaffRole
 from student.tests.factories import UserFactory
+from student.models import CourseEnrollment
+from xmodule.modulestore import Location
+from xmodule.modulestore.django import loc_mapper
 from xmodule.modulestore.exceptions import InsufficientSpecificationError
+from xmodule.modulestore.tests.django_utils import (ModuleStoreTestCase,
+                                                    mixed_store_config)
+from xmodule.modulestore.tests.factories import CourseFactory
 
 FEATURES_WITH_SSL_AUTH = settings.FEATURES.copy()
 FEATURES_WITH_SSL_AUTH['AUTH_USE_CERTIFICATES'] = True
 FEATURES_WITH_SSL_AUTH_IMMEDIATE_SIGNUP = FEATURES_WITH_SSL_AUTH.copy()
 FEATURES_WITH_SSL_AUTH_IMMEDIATE_SIGNUP['AUTH_USE_CERTIFICATES_IMMEDIATE_SIGNUP'] = True
+FEATURES_WITH_SSL_AUTH_AUTO_ACTIVATE = FEATURES_WITH_SSL_AUTH_IMMEDIATE_SIGNUP.copy()
+FEATURES_WITH_SSL_AUTH_AUTO_ACTIVATE['BYPASS_ACTIVATION_EMAIL_FOR_EXTAUTH'] = True
 FEATURES_WITHOUT_SSL_AUTH = settings.FEATURES.copy()
 FEATURES_WITHOUT_SSL_AUTH['AUTH_USE_CERTIFICATES'] = False
 
+TEST_DATA_MIXED_MODULESTORE = mixed_store_config(settings.COMMON_TEST_DATA_ROOT, {})
 
 @override_settings(FEATURES=FEATURES_WITH_SSL_AUTH)
-class SSLClientTest(TestCase):
+class SSLClientTest(ModuleStoreTestCase):
     """
     Tests SSL Authentication code sections of external_auth
     """
@@ -170,7 +180,8 @@ class SSLClientTest(TestCase):
         response = self.client.get(
             reverse('dashboard'), follow=True,
             SSL_CLIENT_S_DN=self.AUTH_DN.format(self.USER_NAME, self.USER_EMAIL))
-        self.assertIn(reverse('dashboard'), response['location'])
+        self.assertEquals(('http://testserver/dashboard', 302),
+                          response.redirect_chain[-1])
         self.assertIn(SESSION_KEY, self.client.session)
 
     @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
@@ -183,7 +194,8 @@ class SSLClientTest(TestCase):
         response = self.client.get(
             reverse('register_user'), follow=True,
             SSL_CLIENT_S_DN=self.AUTH_DN.format(self.USER_NAME, self.USER_EMAIL))
-        self.assertIn(reverse('dashboard'), response['location'])
+        self.assertEquals(('http://testserver/dashboard', 302),
+                          response.redirect_chain[-1])
         self.assertIn(SESSION_KEY, self.client.session)
 
     @unittest.skipUnless(settings.ROOT_URLCONF == 'cms.urls', 'Test only valid in cms')
@@ -225,7 +237,8 @@ class SSLClientTest(TestCase):
         response = self.client.get(
             reverse('signin_user'), follow=True,
             SSL_CLIENT_S_DN=self.AUTH_DN.format(self.USER_NAME, self.USER_EMAIL))
-        self.assertIn(reverse('dashboard'), response['location'])
+        self.assertEquals(('http://testserver/dashboard', 302),
+                          response.redirect_chain[-1])
         self.assertIn(SESSION_KEY, self.client.session)
 
 
@@ -316,3 +329,94 @@ class SSLClientTest(TestCase):
         self.assertEqual(1, len(ExternalAuthMap.objects.all()))
 
         self.assertTrue(self.mock.called)
+
+    @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
+    @override_settings(FEATURES=FEATURES_WITH_SSL_AUTH_AUTO_ACTIVATE,
+                       MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
+    def test_ssl_lms_redirection(self):
+        """
+        Auto signup auth user and ensure they return to the original
+        url they visited after being logged in.
+        """
+        course = CourseFactory.create(
+            org='MITx',
+            number='999',
+            display_name='Robot Super Course'
+        )
+
+        external_auth.views.ssl_login(self._create_ssl_request('/'))
+        user = User.objects.get(email=self.USER_EMAIL)
+        CourseEnrollment.enroll(user, course.id)
+        course_private_url = '/courses/MITx/999/Robot_Super_Course/courseware'
+
+        self.assertFalse(SESSION_KEY in self.client.session)
+
+        response = self.client.get(
+            course_private_url,
+            follow=True,
+            SSL_CLIENT_S_DN=self.AUTH_DN.format(self.USER_NAME, self.USER_EMAIL),
+            HTTP_ACCEPT='text/html'
+        )
+        self.assertEqual(('http://testserver{0}'.format(course_private_url), 302),
+                         response.redirect_chain[-1])
+        self.assertIn(SESSION_KEY, self.client.session)
+
+    @unittest.skipUnless(settings.ROOT_URLCONF == 'cms.urls', 'Test only valid in cms')
+    @override_settings(FEATURES=FEATURES_WITH_SSL_AUTH_AUTO_ACTIVATE)
+    def test_ssl_cms_redirection(self):
+        """
+        Auto signup auth user and ensure they return to the original
+        url they visited after being logged in.
+        """
+        course = CourseFactory.create(
+            org='MITx',
+            number='999',
+            display_name='Robot Super Course'
+        )
+
+        external_auth.views.ssl_login(self._create_ssl_request('/'))
+        user = User.objects.get(email=self.USER_EMAIL)
+        CourseEnrollment.enroll(user, course.id)
+
+        CourseStaffRole(course.location).add_users(user)
+        location = Location(['i4x', 'MITx', '999', 'course',
+                             Location.clean('Robot Super Course'), None])
+        new_location = loc_mapper().translate_location(
+            location.course_id, location, True, True
+        )
+        course_private_url = new_location.url_reverse('course/', '')
+        self.assertFalse(SESSION_KEY in self.client.session)
+
+        response = self.client.get(
+            course_private_url,
+            follow=True,
+            SSL_CLIENT_S_DN=self.AUTH_DN.format(self.USER_NAME, self.USER_EMAIL),
+            HTTP_ACCEPT='text/html'
+        )
+        self.assertEqual(('http://testserver{0}'.format(course_private_url), 302),
+                         response.redirect_chain[-1])
+        self.assertIn(SESSION_KEY, self.client.session)
+
+    @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
+    @override_settings(FEATURES=FEATURES_WITH_SSL_AUTH_AUTO_ACTIVATE)
+    def test_ssl_logout(self):
+        """
+        Because the branding view is cached for anonymous users and we
+        use that to login users, the browser wasn't actually making the
+        request to that view as the redirect was being cached. This caused
+        a redirect loop, and this test confirms that that won't happen.
+
+        Test is only in LMS because we don't use / in studio to login SSL users.
+        """
+        response = self.client.get(
+            reverse('dashboard'), follow=True,
+            SSL_CLIENT_S_DN=self.AUTH_DN.format(self.USER_NAME, self.USER_EMAIL))
+        self.assertEquals(('http://testserver/dashboard', 302),
+                          response.redirect_chain[-1])
+        self.assertIn(SESSION_KEY, self.client.session)
+        response = self.client.get(
+            reverse('logout'), follow=True,
+            SSL_CLIENT_S_DN=self.AUTH_DN.format(self.USER_NAME, self.USER_EMAIL)
+        )
+        # Make sure that even though we logged out, we have logged back in
+        self.assertIn(SESSION_KEY, self.client.session)

common/djangoapps/external_auth/views.py

@@ -440,7 +440,10 @@ def ssl_login(request):
 
     (_user, email, fullname) = _ssl_dn_extract_info(cert)
 
-    retfun = functools.partial(redirect, '/')
+    redirect_to = request.GET.get('next')
+    if not redirect_to:
+        redirect_to = '/'
+    retfun = functools.partial(redirect, redirect_to)
     return _external_login_or_signup(
         request,
         external_id=email,
@@ -580,14 +583,14 @@ def course_specific_login(request, course_id):
         course = course_from_id(course_id)
     except ItemNotFoundError:
         # couldn't find the course, will just return vanilla signin page
-        return _redirect_with_get_querydict('signin_user', request.GET)
+        return redirect_with_get('signin_user', request.GET)
 
     # now the dispatching conditionals.  Only shib for now
     if settings.FEATURES.get('AUTH_USE_SHIB') and course.enrollment_domain.startswith(SHIBBOLETH_DOMAIN_PREFIX):
-        return _redirect_with_get_querydict('shib-login', request.GET)
+        return redirect_with_get('shib-login', request.GET)
 
     # Default fallthrough to normal signin page
-    return _redirect_with_get_querydict('signin_user', request.GET)
+    return redirect_with_get('signin_user', request.GET)
 
 
 def course_specific_register(request, course_id):
@@ -599,24 +602,28 @@ def course_specific_register(request, course_id):
         course = course_from_id(course_id)
     except ItemNotFoundError:
         # couldn't find the course, will just return vanilla registration page
-        return _redirect_with_get_querydict('register_user', request.GET)
+        return redirect_with_get('register_user', request.GET)
 
     # now the dispatching conditionals.  Only shib for now
     if settings.FEATURES.get('AUTH_USE_SHIB') and course.enrollment_domain.startswith(SHIBBOLETH_DOMAIN_PREFIX):
         # shib-login takes care of both registration and login flows
-        return _redirect_with_get_querydict('shib-login', request.GET)
+        return redirect_with_get('shib-login', request.GET)
 
     # Default fallthrough to normal registration page
-    return _redirect_with_get_querydict('register_user', request.GET)
+    return redirect_with_get('register_user', request.GET)
 
 
-def _redirect_with_get_querydict(view_name, get_querydict):
+def redirect_with_get(view_name, get_querydict, do_reverse=True):
     """
         Helper function to carry over get parameters across redirects
         Using urlencode(safe='/') because the @login_required decorator generates 'next' queryparams with '/' unencoded
     """
+    if do_reverse:
+        url = reverse(view_name)
+    else:
+        url = view_name
     if get_querydict:
-        return redirect("%s?%s" % (reverse(view_name), get_querydict.urlencode(safe='/')))
+        return redirect("%s?%s" % (url, get_querydict.urlencode(safe='/')))
     return redirect(view_name)
 
 

common/djangoapps/student/views.py

@@ -26,6 +26,7 @@ from django.shortcuts import redirect
 from django_future.csrf import ensure_csrf_cookie
 from django.utils.http import cookie_date, base36_to_int
 from django.utils.translation import ugettext as _, get_language
+from django.views.decorators.cache import never_cache
 from django.views.decorators.http import require_POST, require_GET
 
 from django.template.response import TemplateResponse
@@ -327,7 +328,7 @@ def signin_user(request):
         # SSL login doesn't require a view, so redirect
         # branding and allow that to process the login if it
         # is enabled and the header is in the request.
-        return redirect(reverse('root'))
+        return external_auth.views.redirect_with_get('root', request.GET)
     if settings.FEATURES.get('AUTH_USE_CAS'):
         # If CAS is enabled, redirect auth handling to there
         return redirect(reverse('cas-login'))
@@ -360,7 +361,7 @@ def register_user(request, extra_context=None):
     if settings.FEATURES.get('AUTH_USE_CERTIFICATES_IMMEDIATE_SIGNUP'):
         # Redirect to branding to process their certificate if SSL is enabled
         # and registration is disabled.
-        return redirect(reverse('root'))
+        return external_auth.views.redirect_with_get('root', request.GET)
 
     context = {
         'course_id': request.GET.get('course_id'),
@@ -674,6 +675,7 @@ def _get_course_enrollment_domain(course_id):
         return None
 
 
+@never_cache
 @ensure_csrf_cookie
 def accounts_login(request):
     """
@@ -683,9 +685,9 @@ def accounts_login(request):
     if settings.FEATURES.get('AUTH_USE_CAS'):
         return redirect(reverse('cas-login'))
     if settings.FEATURES['AUTH_USE_CERTIFICATES']:
-        # SSL login doesn't require a view, so redirect
-        # to branding and allow that to process the login.
-        return redirect(reverse('root'))
+        # SSL login doesn't require a view, so login
+        # directly here
+        return external_auth.views.ssl_login(request)
     # 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')

lms/djangoapps/branding/views.py

@@ -25,6 +25,12 @@ def index(request):
 
     if settings.FEATURES.get('AUTH_USE_CERTIFICATES'):
         from external_auth.views import ssl_login
+        # Set next URL to dashboard if it isn't set to avoid
+        # caching a redirect to / that causes a redirect loop on logout
+        if not request.GET.get('next'):
+            req_new = request.GET.copy()
+            req_new['next'] = reverse('dashboard')
+            request.GET = req_new
         return ssl_login(request)
 
     enable_mktg_site = microsite.get_value(