diff --git a/cms/djangoapps/contentstore/views/public.py b/cms/djangoapps/contentstore/views/public.py index da805a7bd7..3d37818a80 100644 --- a/cms/djangoapps/contentstore/views/public.py +++ b/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')) diff --git a/common/djangoapps/external_auth/tests/test_ssl.py b/common/djangoapps/external_auth/tests/test_ssl.py index cd9940bf7e..21a497d011 100644 --- a/common/djangoapps/external_auth/tests/test_ssl.py +++ b/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) diff --git a/common/djangoapps/external_auth/views.py b/common/djangoapps/external_auth/views.py index 0f3baf7b92..f7258f1c89 100644 --- a/common/djangoapps/external_auth/views.py +++ b/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) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 95daeec7e2..b199196679 100644 --- a/common/djangoapps/student/views.py +++ b/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') diff --git a/lms/djangoapps/branding/views.py b/lms/djangoapps/branding/views.py index 06939e04cc..b0549ce61b 100644 --- a/lms/djangoapps/branding/views.py +++ b/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(