diff --git a/common/djangoapps/external_auth/tests/test_ssl.py b/common/djangoapps/external_auth/tests/test_ssl.py index d72fb00b28..266938fdf0 100644 --- a/common/djangoapps/external_auth/tests/test_ssl.py +++ b/common/djangoapps/external_auth/tests/test_ssl.py @@ -3,6 +3,8 @@ Provides unit tests for SSL based authentication portions of the external_auth app. """ +import logging +import StringIO import unittest from django.conf import settings @@ -13,14 +15,19 @@ from django.test import TestCase from django.test.client import Client from django.test.client import RequestFactory from django.test.utils import override_settings +from mock import Mock +from edxmako.middleware import MakoMiddleware from external_auth.models import ExternalAuthMap import external_auth.views +from student.tests.factories import UserFactory FEATURES_WITH_SSL_AUTH = settings.FEATURES.copy() FEATURES_WITH_SSL_AUTH['AUTH_USE_MIT_CERTIFICATES'] = True FEATURES_WITH_SSL_AUTH_IMMEDIATE_SIGNUP = FEATURES_WITH_SSL_AUTH.copy() FEATURES_WITH_SSL_AUTH_IMMEDIATE_SIGNUP['AUTH_USE_MIT_CERTIFICATES_IMMEDIATE_SIGNUP'] = True +FEATURES_WITHOUT_SSL_AUTH = settings.FEATURES.copy() +FEATURES_WITHOUT_SSL_AUTH['AUTH_USE_MIT_CERTIFICATES'] = False @override_settings(FEATURES=FEATURES_WITH_SSL_AUTH) @@ -32,6 +39,7 @@ class SSLClientTest(TestCase): AUTH_DN = '/C=US/ST=Massachusetts/O=Massachusetts Institute of Technology/OU=Client CA v1/CN={0}/emailAddress={1}' USER_NAME = 'test_user_ssl' USER_EMAIL = 'test_user_ssl@EDX.ORG' + MOCK_URL = '/' def _create_ssl_request(self, url): """Creates a basic request for SSL use.""" @@ -41,6 +49,17 @@ class SSLClientTest(TestCase): middleware = SessionMiddleware() middleware.process_request(request) request.session.save() + MakoMiddleware().process_request(request) + return request + + def _create_normal_request(self, url): + """Creates sessioned request without SSL headers""" + request = self.factory.get(url) + request.user = AnonymousUser() + middleware = SessionMiddleware() + middleware.process_request(request) + request.session.save() + MakoMiddleware().process_request(request) return request def setUp(self): @@ -48,6 +67,7 @@ class SSLClientTest(TestCase): super(SSLClientTest, self).setUp() self.client = Client() self.factory = RequestFactory() + self.mock = Mock() @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') def test_ssl_login_with_signup_lms(self): @@ -184,3 +204,93 @@ class SSLClientTest(TestCase): SSL_CLIENT_S_DN=self.AUTH_DN.format(self.USER_NAME, self.USER_EMAIL)) self.assertIn(reverse('dashboard'), response['location']) self.assertIn('_auth_user_id', self.client.session) + + @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') + @override_settings(FEATURES=FEATURES_WITH_SSL_AUTH_IMMEDIATE_SIGNUP) + def test_ssl_bad_eamap(self): + """ + This tests the response when a user exists but their eamap + password doesn't match their internal password. + + This should start failing and can be removed when the + eamap.internal_password dependency is removed. + """ + external_auth.views.ssl_login(self._create_ssl_request('/')) + user = User.objects.get(email=self.USER_EMAIL) + user.set_password('not autogenerated') + user.save() + + # Validate user failed by checking log + output = StringIO.StringIO() + audit_log_handler = logging.StreamHandler(output) + audit_log = logging.getLogger("audit") + audit_log.addHandler(audit_log_handler) + + request = self._create_ssl_request('/') + external_auth.views.ssl_login(request) + self.assertIn('External Auth Login failed for', output.getvalue()) + + @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') + @override_settings(FEATURES=FEATURES_WITHOUT_SSL_AUTH) + def test_ssl_decorator_no_certs(self): + """Make sure no external auth happens without SSL enabled""" + + dec_mock = external_auth.views.ssl_login_shortcut(self.mock) + request = self._create_normal_request(self.MOCK_URL) + request.user = AnonymousUser() + # Call decorated mock function to make sure it passes + # the call through without hitting the external_auth functions and + # thereby creating an external auth map object. + dec_mock(request) + self.assertTrue(self.mock.called) + self.assertEqual(0, len(ExternalAuthMap.objects.all())) + + @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') + def test_ssl_login_decorator(self): + """Create mock function to test ssl login decorator""" + + dec_mock = external_auth.views.ssl_login_shortcut(self.mock) + + # Test that anonymous without cert doesn't create authmap + request = self._create_normal_request(self.MOCK_URL) + dec_mock(request) + self.assertTrue(self.mock.called) + self.assertEqual(0, len(ExternalAuthMap.objects.all())) + + # Test valid user + self.mock.reset_mock() + request = self._create_ssl_request(self.MOCK_URL) + dec_mock(request) + self.assertFalse(self.mock.called) + self.assertEqual(1, len(ExternalAuthMap.objects.all())) + + # Test logged in user gets called + self.mock.reset_mock() + request = self._create_ssl_request(self.MOCK_URL) + request.user = UserFactory() + dec_mock(request) + self.assertTrue(self.mock.called) + + @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') + @override_settings(FEATURES=FEATURES_WITH_SSL_AUTH_IMMEDIATE_SIGNUP) + def test_ssl_decorator_auto_signup(self): + """ + Test that with auto signup the decorator + will bypass registration and call retfun. + """ + + dec_mock = external_auth.views.ssl_login_shortcut(self.mock) + request = self._create_ssl_request(self.MOCK_URL) + dec_mock(request) + # Assert our user exists in both eamap and Users + try: + ExternalAuthMap.objects.get(external_id=self.USER_EMAIL) + except ExternalAuthMap.DoesNotExist, ex: + self.fail('User did not get properly added to external auth map, exception was {0}'.format(str(ex))) + try: + User.objects.get(email=self.USER_EMAIL) + except ExternalAuthMap.DoesNotExist, ex: + self.fail('User did not get properly added to internal users, exception was {0}'.format(str(ex))) + self.assertEqual(1, len(ExternalAuthMap.objects.all())) + + self.assertTrue(self.mock.called) diff --git a/common/djangoapps/external_auth/views.py b/common/djangoapps/external_auth/views.py index 42d0f2bf89..efbf4ccb63 100644 --- a/common/djangoapps/external_auth/views.py +++ b/common/djangoapps/external_auth/views.py @@ -177,10 +177,10 @@ def _external_login_or_signup(request, return default_render_failure(request, failure_msg) except User.DoesNotExist: log.info('SHIB: No user for %s yet, doing signup', eamap.external_email) - return _signup(request, eamap) + return _signup(request, eamap, retfun) else: log.info('No user for %s yet. doing signup', eamap.external_email) - return _signup(request, eamap) + return _signup(request, eamap, retfun) # We trust shib's authentication, so no need to authenticate using the password again uname = internal_user.username @@ -198,7 +198,7 @@ def _external_login_or_signup(request, if user is None: # we want to log the failure, but don't want to log the password attempted: AUDIT_LOG.warning('External Auth Login failed for "%s"', uname) - return _signup(request, eamap) + return _signup(request, eamap, retfun) if not user.is_active: AUDIT_LOG.warning('User "%s" is not active after external login', uname) @@ -237,7 +237,7 @@ def _flatten_to_ascii(txt): @ensure_csrf_cookie -def _signup(request, eamap): +def _signup(request, eamap, retfun=None): """ Present form to complete for signup via external authentication. Even though the user has external credentials, he/she still needs @@ -246,6 +246,9 @@ def _signup(request, eamap): eamap is an ExternalAuthMap object, specifying the external user for which to complete the signup. + + retfun is a function to execute for the return value, if immediate + signup is used. That allows @ssl_login_shortcut() to work. """ # save this for use by student.views.create_account request.session['ExternalAuthMap'] = eamap @@ -260,7 +263,11 @@ def _signup(request, eamap): terms_of_service=u'true') log.info('doing immediate signup for %s, params=%s', username, post_vars) student.views.create_account(request, post_vars) - return redirect('/') + # should check return content for successful completion before + if retfun is not None: + return retfun() + else: + return redirect('/') # default conjoin name, no spaces, flattened to ascii b/c django can't handle unicode usernames, sadly # but this only affects username, not fullname @@ -349,13 +356,27 @@ def ssl_login_shortcut(fn): based on existing ExternalAuth record and MIT ssl certificate. """ def wrapped(*args, **kwargs): + """ + This manages the function wrapping, by determining whether to inject + the _external signup or just continuing to the internal function + call. + """ + if not settings.FEATURES['AUTH_USE_MIT_CERTIFICATES']: return fn(*args, **kwargs) request = args[0] + + if request.user and request.user.is_authenticated(): # don't re-authenticate + return fn(*args, **kwargs) + cert = _ssl_get_cert_from_request(request) if not cert: # no certificate information - show normal login window return fn(*args, **kwargs) + def retfun(): + """Wrap function again for call by _external_login_or_signup""" + return fn(*args, **kwargs) + (_user, email, fullname) = _ssl_dn_extract_info(cert) return _external_login_or_signup( request, @@ -363,7 +384,8 @@ def ssl_login_shortcut(fn): external_domain="ssl:MIT", credentials=cert, email=email, - fullname=fullname + fullname=fullname, + retfun=retfun ) return wrapped @@ -397,7 +419,7 @@ def ssl_login(request): (_user, email, fullname) = _ssl_dn_extract_info(cert) - retfun = functools.partial(student.views.index, request) + retfun = functools.partial(redirect, '/') return _external_login_or_signup( request, external_id=email,