Fix infinite redirect loop on logout caused by django caching

cms/djangoapps/contentstore/views/public.py

@@ -11,7 +11,7 @@ from edxmako.shortcuts import render_to_response
 
 from external_auth.views import (ssl_login_shortcut, ssl_get_cert_from_request,
                                  redirect_with_get)
-from microsite_configuration.middleware import MicrositeConfiguration
+from microsite_configuration import microsite
 
 __all__ = ['signup', 'login_page', 'howitworks']
 
@@ -52,6 +52,7 @@ def login_page(request):
     if settings.FEATURES.get('AUTH_USE_CAS'):
         # If CAS is enabled, redirect auth handling to there
         return redirect(reverse('cas-login'))
+
     return render_to_response(
         'login.html',
         {

common/djangoapps/external_auth/tests/test_ssl.py

@@ -194,7 +194,8 @@ class SSLClientTest(ModuleStoreTestCase):
         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')
@@ -236,7 +237,8 @@ class SSLClientTest(ModuleStoreTestCase):
         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)
 
 
@@ -394,3 +396,27 @@ class SSLClientTest(ModuleStoreTestCase):
         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/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
@@ -328,7 +329,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 external_auth.views.redirect_with_get('root', request.GET)
+        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'))
@@ -675,6 +676,7 @@ def _get_course_enrollment_domain(course_id):
         return None
 
 
+@never_cache
 @ensure_csrf_cookie
 def accounts_login(request):
     """
@@ -684,9 +686,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 external_auth.views.redirect_with_get('root', request.GET)
+        # 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')
@@ -776,7 +778,7 @@ def login_user(request, error=""):  # pylint: disable-msg=too-many-statements,un
             # 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)
 
-    # see if account has been locked out due to excessive login failres
+    # see if account has been locked out due to excessive login failures
     user_found_by_email_lookup = user
     if user_found_by_email_lookup and LoginFailures.is_feature_enabled():
         if LoginFailures.is_user_locked_out(user_found_by_email_lookup):

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(