From 082f20db60848349250dec9f49d051db8b909647 Mon Sep 17 00:00:00 2001 From: Carson Gee Date: Fri, 21 Feb 2014 12:02:53 -0500 Subject: [PATCH 1/2] Remove SSL Certifcate auth reliance on internal password --- .../external_auth/tests/test_ssl.py | 21 +++++++++---------- common/djangoapps/external_auth/views.py | 6 ++++++ 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/common/djangoapps/external_auth/tests/test_ssl.py b/common/djangoapps/external_auth/tests/test_ssl.py index 33422aad76..8e1c7092e4 100644 --- a/common/djangoapps/external_auth/tests/test_ssl.py +++ b/common/djangoapps/external_auth/tests/test_ssl.py @@ -235,23 +235,22 @@ class SSLClientTest(TestCase): 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. + The internal password use for certificates has been removed + and this should not fail. """ + # Create account, break internal password, and activate account external_auth.views.ssl_login(self._create_ssl_request('/')) user = User.objects.get(email=self.USER_EMAIL) user.set_password('not autogenerated') + user.is_active = True 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()) + # Make sure we can still login + response = self.client.get( + reverse('signin_user'), follow=True, + SSL_CLIENT_S_DN=self.AUTH_DN.format(self.USER_NAME, self.USER_EMAIL)) + print(response) + self.assertIn('_auth_user_id', self.client.session) @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') @override_settings(FEATURES=FEATURES_WITHOUT_SSL_AUTH) diff --git a/common/djangoapps/external_auth/views.py b/common/djangoapps/external_auth/views.py index cfb655f4b2..9403f6b10b 100644 --- a/common/djangoapps/external_auth/views.py +++ b/common/djangoapps/external_auth/views.py @@ -151,6 +151,7 @@ def _external_login_or_signup(request, log.info(u"External_Auth login_or_signup for %s : %s : %s : %s", external_domain, external_id, email, fullname) uses_shibboleth = settings.FEATURES.get('AUTH_USE_SHIB') and external_domain.startswith(SHIBBOLETH_DOMAIN_PREFIX) + uses_certs = settings.FEATURES.get('AUTH_USE_CERTIFICATES') internal_user = eamap.user if internal_user is None: if uses_shibboleth: @@ -193,6 +194,11 @@ def _external_login_or_signup(request, auth_backend = 'django.contrib.auth.backends.ModelBackend' user.backend = auth_backend AUDIT_LOG.info('Linked user "%s" logged in via Shibboleth', user.email) + elif uses_certs: + # Certificates are trusted, so just link the user and log the action + user = internal_user + user.backend = 'django.contrib.auth.backens.ModelBackend' + AUDIT_LOG.info('Linked user "%s" logged in via SSL certificate', user.email) else: user = authenticate(username=uname, password=eamap.internal_password, request=request) if user is None: From 3303fb120b282d6feb79dfb4b0b66233f096259a Mon Sep 17 00:00:00 2001 From: Carson Gee Date: Fri, 21 Feb 2014 16:01:01 -0500 Subject: [PATCH 2/2] Review fixes --- common/djangoapps/external_auth/tests/test_ssl.py | 14 +++++++------- common/djangoapps/external_auth/views.py | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/common/djangoapps/external_auth/tests/test_ssl.py b/common/djangoapps/external_auth/tests/test_ssl.py index 8e1c7092e4..cd9940bf7e 100644 --- a/common/djangoapps/external_auth/tests/test_ssl.py +++ b/common/djangoapps/external_auth/tests/test_ssl.py @@ -8,6 +8,7 @@ import StringIO import unittest from django.conf import settings +from django.contrib.auth import SESSION_KEY from django.contrib.auth.models import AnonymousUser, User from django.contrib.sessions.middleware import SessionMiddleware from django.core.urlresolvers import reverse @@ -170,7 +171,7 @@ class SSLClientTest(TestCase): 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.assertIn('_auth_user_id', self.client.session) + 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_IMMEDIATE_SIGNUP) @@ -183,7 +184,7 @@ class SSLClientTest(TestCase): 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.assertIn('_auth_user_id', self.client.session) + 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_IMMEDIATE_SIGNUP) @@ -199,7 +200,7 @@ class SSLClientTest(TestCase): reverse('signup'), follow=True, SSL_CLIENT_S_DN=self.AUTH_DN.format(self.USER_NAME, self.USER_EMAIL)) # assert that we are logged in - self.assertIn('_auth_user_id', self.client.session) + self.assertIn(SESSION_KEY, self.client.session) # Now that we are logged in, make sure we don't see the registration page with self.assertRaisesRegexp(InsufficientSpecificationError, @@ -225,7 +226,7 @@ class SSLClientTest(TestCase): 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.assertIn('_auth_user_id', self.client.session) + self.assertIn(SESSION_KEY, self.client.session) @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') @@ -246,11 +247,10 @@ class SSLClientTest(TestCase): user.save() # Make sure we can still login - response = self.client.get( + self.client.get( reverse('signin_user'), follow=True, SSL_CLIENT_S_DN=self.AUTH_DN.format(self.USER_NAME, self.USER_EMAIL)) - print(response) - self.assertIn('_auth_user_id', self.client.session) + self.assertIn(SESSION_KEY, self.client.session) @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') @override_settings(FEATURES=FEATURES_WITHOUT_SSL_AUTH) diff --git a/common/djangoapps/external_auth/views.py b/common/djangoapps/external_auth/views.py index 9403f6b10b..76455b8f89 100644 --- a/common/djangoapps/external_auth/views.py +++ b/common/djangoapps/external_auth/views.py @@ -197,7 +197,7 @@ def _external_login_or_signup(request, elif uses_certs: # Certificates are trusted, so just link the user and log the action user = internal_user - user.backend = 'django.contrib.auth.backens.ModelBackend' + user.backend = 'django.contrib.auth.backends.ModelBackend' AUDIT_LOG.info('Linked user "%s" logged in via SSL certificate', user.email) else: user = authenticate(username=uname, password=eamap.internal_password, request=request)