From 23631b839a2941885a45c866b2e0a76e9f27e805 Mon Sep 17 00:00:00 2001 From: Jason Bau Date: Tue, 4 Jun 2013 10:34:15 -0700 Subject: [PATCH 1/6] AWS settings hook for addl installed apps + lms/wsgi_apache.py wsgi_apache.py passes an apache env variable for SERVICE_VARIANT to the os environment, where it's normally set when we use gunicorn --- lms/envs/aws.py | 4 ++++ lms/wsgi_apache.py | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+) create mode 100644 lms/wsgi_apache.py diff --git a/lms/envs/aws.py b/lms/envs/aws.py index c8c49c2b1e..a237788163 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -138,6 +138,10 @@ MKTG_URL_LINK_MAP.update(ENV_TOKENS.get('MKTG_URL_LINK_MAP', {})) #Timezone overrides TIME_ZONE = ENV_TOKENS.get('TIME_ZONE', TIME_ZONE) +#Additional installed apps +for app in ENV_TOKENS.get('ADDL_INSTALLED_APPS', []): + INSTALLED_APPS += (app,) + for feature, value in ENV_TOKENS.get('MITX_FEATURES', {}).items(): MITX_FEATURES[feature] = value diff --git a/lms/wsgi_apache.py b/lms/wsgi_apache.py new file mode 100644 index 0000000000..e2d8a23dc0 --- /dev/null +++ b/lms/wsgi_apache.py @@ -0,0 +1,18 @@ +import os + +os.environ.setdefault("DJANGO_SETTINGS_MODULE", "lms.envs.aws") +# This application object is used by the development server +# as well as any WSGI server configured to use this file. +from django.core.wsgi import WSGIHandler +_application = WSGIHandler() + +def application(environ, start_response): + #copy SERVICE_VARIANT from apache environ to os environ + os.environ.setdefault("SERVICE_VARIANT", environ.get("SERVICE_VARIANT", "lms")) + return _application(environ, start_response) + +from django.conf import settings +from xmodule.modulestore.django import modulestore + +for store_name in settings.MODULESTORE: + modulestore(store_name) From 824fb9a3111888296662579834d49b564cc34c93 Mon Sep 17 00:00:00 2001 From: Jason Bau Date: Mon, 3 Jun 2013 17:48:44 -0700 Subject: [PATCH 2/6] The bulk of Shibboleth authentication for Stanford Highlights: * The url '/shib-login/' interfaces with apache/mod_shib via request.META to handle shibboleth login and registrations * Courses can designate 'enrollment_domains' to limit enrollment to users with a linked ExternalAuthMap verified by a particular identity provider * Tests * Logging Changes to be committed: new file: common/djangoapps/external_auth/migrations/0001_initial.py new file: common/djangoapps/external_auth/migrations/__init__.py new file: common/djangoapps/external_auth/tests/test_shib.py modified: common/djangoapps/external_auth/views.py modified: common/djangoapps/student/views.py modified: common/lib/xmodule/xmodule/course_module.py modified: lms/djangoapps/courseware/access.py modified: lms/djangoapps/courseware/tests/test_access.py modified: lms/envs/common.py modified: lms/envs/dev.py modified: lms/envs/test.py modified: lms/templates/courseware/course_about.html modified: lms/templates/dashboard.html modified: lms/templates/extauth_failure.html modified: lms/templates/navigation.html modified: lms/templates/register.html modified: lms/templates/signup_modal.html modified: lms/urls.py renamed: lms/wsgi_apache.py -> lms/wsgi_apache_lms.py --- .../external_auth/migrations/0001_initial.py | 90 +++++ .../external_auth/migrations/__init__.py | 0 .../external_auth/tests/test_shib.py | 368 ++++++++++++++++++ common/djangoapps/external_auth/views.py | 152 +++++++- common/djangoapps/student/views.py | 30 +- common/lib/xmodule/xmodule/course_module.py | 2 + lms/djangoapps/courseware/access.py | 23 +- .../courseware/tests/test_access.py | 6 +- lms/envs/common.py | 4 + lms/envs/dev.py | 3 + lms/envs/test.py | 4 + lms/templates/courseware/course_about.html | 27 +- lms/templates/dashboard.html | 10 +- lms/templates/extauth_failure.html | 4 +- lms/templates/navigation.html | 18 +- lms/templates/register.html | 27 +- lms/templates/signup_modal.html | 18 +- lms/urls.py | 15 + lms/{wsgi_apache.py => wsgi_apache_lms.py} | 11 +- 19 files changed, 779 insertions(+), 33 deletions(-) create mode 100644 common/djangoapps/external_auth/migrations/0001_initial.py create mode 100644 common/djangoapps/external_auth/migrations/__init__.py create mode 100644 common/djangoapps/external_auth/tests/test_shib.py rename lms/{wsgi_apache.py => wsgi_apache_lms.py} (53%) diff --git a/common/djangoapps/external_auth/migrations/0001_initial.py b/common/djangoapps/external_auth/migrations/0001_initial.py new file mode 100644 index 0000000000..a5ebf023df --- /dev/null +++ b/common/djangoapps/external_auth/migrations/0001_initial.py @@ -0,0 +1,90 @@ +# -*- coding: utf-8 -*- +import datetime +from south.db import db +from south.v2 import SchemaMigration +from django.db import models + + +class Migration(SchemaMigration): + + def forwards(self, orm): + # Adding model 'ExternalAuthMap' + db.create_table('external_auth_externalauthmap', ( + ('id', self.gf('django.db.models.fields.AutoField')(primary_key=True)), + ('external_id', self.gf('django.db.models.fields.CharField')(max_length=255, db_index=True)), + ('external_domain', self.gf('django.db.models.fields.CharField')(max_length=255, db_index=True)), + ('external_credentials', self.gf('django.db.models.fields.TextField')(blank=True)), + ('external_email', self.gf('django.db.models.fields.CharField')(max_length=255, db_index=True)), + ('external_name', self.gf('django.db.models.fields.CharField')(db_index=True, max_length=255, blank=True)), + ('user', self.gf('django.db.models.fields.related.OneToOneField')(to=orm['auth.User'], unique=True, null=True)), + ('internal_password', self.gf('django.db.models.fields.CharField')(max_length=31, blank=True)), + ('dtcreated', self.gf('django.db.models.fields.DateTimeField')(auto_now_add=True, blank=True)), + ('dtsignup', self.gf('django.db.models.fields.DateTimeField')(null=True)), + )) + db.send_create_signal('external_auth', ['ExternalAuthMap']) + + # Adding unique constraint on 'ExternalAuthMap', fields ['external_id', 'external_domain'] + db.create_unique('external_auth_externalauthmap', ['external_id', 'external_domain']) + + + def backwards(self, orm): + # Removing unique constraint on 'ExternalAuthMap', fields ['external_id', 'external_domain'] + db.delete_unique('external_auth_externalauthmap', ['external_id', 'external_domain']) + + # Deleting model 'ExternalAuthMap' + db.delete_table('external_auth_externalauthmap') + + + models = { + 'auth.group': { + 'Meta': {'object_name': 'Group'}, + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}), + 'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}) + }, + 'auth.permission': { + 'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'}, + 'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '50'}) + }, + 'auth.user': { + 'Meta': {'object_name': 'User'}, + 'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}), + 'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}), + 'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}), + 'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'}) + }, + 'contenttypes.contenttype': { + 'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"}, + 'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '100'}) + }, + 'external_auth.externalauthmap': { + 'Meta': {'unique_together': "(('external_id', 'external_domain'),)", 'object_name': 'ExternalAuthMap'}, + 'dtcreated': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}), + 'dtsignup': ('django.db.models.fields.DateTimeField', [], {'null': 'True'}), + 'external_credentials': ('django.db.models.fields.TextField', [], {'blank': 'True'}), + 'external_domain': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'external_email': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'external_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'external_name': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '255', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'internal_password': ('django.db.models.fields.CharField', [], {'max_length': '31', 'blank': 'True'}), + 'user': ('django.db.models.fields.related.OneToOneField', [], {'to': "orm['auth.User']", 'unique': 'True', 'null': 'True'}) + } + } + + complete_apps = ['external_auth'] \ No newline at end of file diff --git a/common/djangoapps/external_auth/migrations/__init__.py b/common/djangoapps/external_auth/migrations/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/common/djangoapps/external_auth/tests/test_shib.py b/common/djangoapps/external_auth/tests/test_shib.py new file mode 100644 index 0000000000..f342aa4c74 --- /dev/null +++ b/common/djangoapps/external_auth/tests/test_shib.py @@ -0,0 +1,368 @@ +""" +Tests for Shibboleth Authentication +@jbau +""" +import unittest + +from django.conf import settings +from django.http import HttpResponseRedirect +from django.test.client import RequestFactory +from django.test.utils import override_settings +from django.core.urlresolvers import reverse +from django.contrib.auth.models import AnonymousUser, User +from django.contrib.sessions.backends.base import SessionBase + +from xmodule.modulestore.tests.factories import CourseFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.inheritance import own_metadata +from xmodule.modulestore.django import modulestore + +from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE + +from external_auth.models import ExternalAuthMap +from external_auth.views import shib_login, course_specific_login, course_specific_register + +from student.views import create_account, change_enrollment +from student.models import UserProfile, Registration, CourseEnrollment +from student.tests.factories import UserFactory + +#Shib is supposed to provide 'REMOTE_USER', 'givenName', 'sn', 'mail', 'Shib-Identity-Provider' +#attributes via request.META. We can count on 'Shib-Identity-Provider', and 'REMOTE_USER' being present +#b/c of how mod_shib works but should test the behavior with the rest of the attributes present/missing + +#For the sake of python convention we'll make all of these variable names ALL_CAPS +IDP = 'https://idp.stanford.edu/' +REMOTE_USER = 'test_user@stanford.edu' +MAILS = [None, '', 'test_user@stanford.edu'] +GIVENNAMES = [None, '', 'Jason', 'jason; John; bob'] # At Stanford, the givenNames can be a list delimited by ';' +SNS = [None, '', 'Bau', 'bau; smith'] # At Stanford, the sns can be a list delimited by ';' + + +def gen_all_identities(): + """A generator for all combinations of identity inputs""" + def _build_identity_dict(mail, given_name, surname): + """ Helper function to return a dict of test identity """ + meta_dict = {} + meta_dict.update({'Shib-Identity-Provider': IDP, + 'REMOTE_USER': REMOTE_USER}) + if mail is not None: + meta_dict.update({'mail': mail}) + if given_name is not None: + meta_dict.update({'givenName': given_name}) + if surname is not None: + meta_dict.update({'sn': surname}) + return meta_dict + + for mail in MAILS: + for given_name in GIVENNAMES: + for surname in SNS: + yield _build_identity_dict(mail, given_name, surname) + + +@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE) +class ShibSPTest(ModuleStoreTestCase): + """ + Tests for the Shibboleth SP, which communicates via request.META + (Apache environment variables set by mod_shib) + """ + factory = RequestFactory() + + def setUp(self): + self.store = modulestore() + + @unittest.skipUnless(settings.MITX_FEATURES.get('AUTH_USE_SHIB'), True) + def test_shib_login(self): + """ + Tests that a user with a shib ExternalAuthMap gets logged in while when + shib-login is called, while a user without such gets the registration form. + """ + + student = UserFactory.create() + extauth = ExternalAuthMap(external_id='testuser@stanford.edu', + external_email='', + external_domain='shib:https://idp.stanford.edu/', + external_credentials="", + user=student) + student.save() + extauth.save() + + idps = ['https://idp.stanford.edu/', 'https://someother.idp.com/'] + remote_users = ['testuser@stanford.edu', 'testuser2@someother_idp.com'] + + for idp in idps: + for remote_user in remote_users: + request = self.factory.get('/shib-login') + request.session = SessionBase() # empty session + request.META.update({'Shib-Identity-Provider': idp, + 'REMOTE_USER': remote_user}) + request.user = AnonymousUser() + response = shib_login(request) + if idp == "https://idp.stanford.edu" and remote_user == 'testuser@stanford.edu': + self.assertIsInstance(response, HttpResponseRedirect) + self.assertEqual(request.user, student) + self.assertEqual(response['Location'], '/') + else: + self.assertEqual(response.status_code, 200) + self.assertContains(response, "Register for") + + @unittest.skipUnless(settings.MITX_FEATURES.get('AUTH_USE_SHIB'), True) + def test_registration_form(self): + """ + Tests the registration form showing up with the proper parameters. + + Uses django test client for its session support + """ + for identity in gen_all_identities(): + self.client.logout() + request_kwargs = {'path': '/shib-login/', 'data': {}, 'follow': False} + request_kwargs.update(identity) + response = self.client.get(**request_kwargs) # identity k/v pairs will show up in request.META + + self.assertEquals(response.status_code, 200) + mail_input_HTML = '<input class="" id="email" type="email" name="email"' + if not identity.get('mail'): + self.assertContains(response, mail_input_HTML) + else: + self.assertNotContains(response, mail_input_HTML) + sn_empty = identity.get('sn', '') == '' + given_name_empty = identity.get('givenName', '') == '' + fullname_input_HTML = '<input id="name" type="text" name="name"' + if sn_empty and given_name_empty: + self.assertContains(response, fullname_input_HTML) + else: + self.assertNotContains(response, fullname_input_HTML) + + #clean up b/c we don't want existing ExternalAuthMap for the next run + self.client.session['ExternalAuthMap'].delete() + + @unittest.skipUnless(settings.MITX_FEATURES.get('AUTH_USE_SHIB'), True) + def test_registration_formSubmit(self): + """ + Tests user creation after the registration form that pops is submitted. If there is no shib + ExternalAuthMap in the session, then the created user should take the username and email from the + request. + + Uses django test client for its session support + """ + for identity in gen_all_identities(): + #First we pop the registration form + self.client.logout() + request1_kwargs = {'path': '/shib-login/', 'data': {}, 'follow': False} + request1_kwargs.update(identity) + response1 = self.client.get(**request1_kwargs) + #Then we have the user answer the registration form + postvars = {'email': 'post_email@stanford.edu', + 'username': 'post_username', + 'password': 'post_password', + 'name': 'post_name', + 'terms_of_service': 'true', + 'honor_code': 'true'} + #use RequestFactory instead of TestClient here because we want access to request.user + request2 = self.factory.post('/create_account', data=postvars) + request2.session = self.client.session + request2.user = AnonymousUser() + response2 = create_account(request2) + + user = request2.user + mail = identity.get('mail') + #check that the created user has the right email, either taken from shib or user input + if mail: + self.assertEqual(user.email, mail) + self.assertEqual(list(User.objects.filter(email=postvars['email'])), []) + self.assertIsNotNone(User.objects.get(email=mail)) # get enforces only 1 such user + else: + self.assertEqual(user.email, postvars['email']) + self.assertEqual(list(User.objects.filter(email=mail)), []) + self.assertIsNotNone(User.objects.get(email=postvars['email'])) # get enforces only 1 such user + + #check that the created user profile has the right name, either taken from shib or user input + profile = UserProfile.objects.get(user=user) + sn_empty = identity.get('sn', '') == '' + given_name_empty = identity.get('givenName', '') == '' + if sn_empty and given_name_empty: + self.assertEqual(profile.name, postvars['name']) + else: + self.assertEqual(profile.name, request2.session['ExternalAuthMap'].external_name) + + #clean up for next loop + request2.session['ExternalAuthMap'].delete() + UserProfile.objects.filter(user=user).delete() + Registration.objects.filter(user=user).delete() + user.delete() + + @unittest.skipUnless(settings.MITX_FEATURES.get('AUTH_USE_SHIB'), True) + def test_course_specificLoginAndReg(self): + """ + Tests that the correct course specific login and registration urls work for shib + """ + course = CourseFactory.create(org='MITx', number='999', display_name='Robot Super Course') + + # Test for cases where course is found + for domain in ["", "shib:https://idp.stanford.edu/"]: + #set domains + course.enrollment_domain = domain + metadata = own_metadata(course) + metadata['enrollment_domain'] = domain + self.store.update_metadata(course.location.url(), metadata) + + #setting location to test that GET params get passed through + login_request = self.factory.get('/course_specific_login/MITx/999/Robot_Super_Course' + + '?course_id=MITx/999/Robot_Super_Course' + + '&enrollment_action=enroll') + reg_request = self.factory.get('/course_specific_register/MITx/999/Robot_Super_Course' + + '?course_id=MITx/999/course/Robot_Super_Course' + + '&enrollment_action=enroll') + + login_response = course_specific_login(login_request, 'MITx/999/Robot_Super_Course') + reg_response = course_specific_register(login_request, 'MITx/999/Robot_Super_Course') + + if "shib" in domain: + self.assertIsInstance(login_response, HttpResponseRedirect) + self.assertEqual(login_response['Location'], + reverse('shib-login') + + '?course_id=MITx/999/Robot_Super_Course' + + '&enrollment_action=enroll') + self.assertIsInstance(login_response, HttpResponseRedirect) + self.assertEqual(reg_response['Location'], + reverse('shib-login') + + '?course_id=MITx/999/Robot_Super_Course' + + '&enrollment_action=enroll') + else: + self.assertIsInstance(login_response, HttpResponseRedirect) + self.assertEqual(login_response['Location'], + reverse('signin_user') + + '?course_id=MITx/999/Robot_Super_Course' + + '&enrollment_action=enroll') + self.assertIsInstance(login_response, HttpResponseRedirect) + self.assertEqual(reg_response['Location'], + reverse('register_user') + + '?course_id=MITx/999/Robot_Super_Course' + + '&enrollment_action=enroll') + + # Now test for non-existent course + #setting location to test that GET params get passed through + login_request = self.factory.get('/course_specific_login/DNE/DNE/DNE' + + '?course_id=DNE/DNE/DNE' + + '&enrollment_action=enroll') + reg_request = self.factory.get('/course_specific_register/DNE/DNE/DNE' + + '?course_id=DNE/DNE/DNE/Robot_Super_Course' + + '&enrollment_action=enroll') + + login_response = course_specific_login(login_request, 'DNE/DNE/DNE') + reg_response = course_specific_register(login_request, 'DNE/DNE/DNE') + + self.assertIsInstance(login_response, HttpResponseRedirect) + self.assertEqual(login_response['Location'], + reverse('signin_user') + + '?course_id=DNE/DNE/DNE' + + '&enrollment_action=enroll') + self.assertIsInstance(login_response, HttpResponseRedirect) + self.assertEqual(reg_response['Location'], + reverse('register_user') + + '?course_id=DNE/DNE/DNE' + + '&enrollment_action=enroll') + + @unittest.skipUnless(settings.MITX_FEATURES.get('AUTH_USE_SHIB'), True) + def test_enrollment_limit_by_domain(self): + """ + Tests that the enrollmentDomain setting is properly limiting enrollment to those who have + the proper external auth + """ + + #create 2 course, one with limited enrollment one without + course1 = CourseFactory.create(org='Stanford', number='123', display_name='Shib Only') + course1.enrollment_domain = 'shib:https://idp.stanford.edu/' + metadata = own_metadata(course1) + metadata['enrollment_domain'] = course1.enrollment_domain + self.store.update_metadata(course1.location.url(), metadata) + + course2 = CourseFactory.create(org='MITx', number='999', display_name='Robot Super Course') + course2.enrollment_domain = '' + metadata = own_metadata(course2) + metadata['enrollment_domain'] = course2.enrollment_domain + self.store.update_metadata(course2.location.url(), metadata) + + # create 3 kinds of students, external_auth matching course1, external_auth not matching, no external auth + student1 = UserFactory.create() + student1.save() + extauth = ExternalAuthMap(external_id='testuser@stanford.edu', + external_email='', + external_domain='shib:https://idp.stanford.edu/', + external_credentials="", + user=student1) + extauth.save() + + student2 = UserFactory.create() + student2.username = "teststudent2" + student2.email = "teststudent2@other.edu" + student2.save() + extauth = ExternalAuthMap(external_id='testuser1@other.edu', + external_email='', + external_domain='shib:https://other.edu/', + external_credentials="", + user=student2) + extauth.save() + + student3 = UserFactory.create() + student3.username = "teststudent3" + student3.email = "teststudent3@gmail.com" + student3.save() + + #Tests the two case for courses, limited and not + for course in [course1, course2]: + for student in [student1, student2, student3]: + request = self.factory.post('/change_enrollment') + request.POST.update({'enrollment_action': 'enroll', + 'course_id': course.id}) + request.user = student + response = change_enrollment(request) + #if course is not limited or student has correct shib extauth then enrollment should be allowed + if course is course2 or student is student1: + self.assertEqual(response.status_code, 200) + self.assertEqual(CourseEnrollment.objects.filter(user=student, course_id=course.id).count(), 1) + #clean up + CourseEnrollment.objects.filter(user=student, course_id=course.id).delete() + else: + self.assertEqual(response.status_code, 400) + self.assertEqual(CourseEnrollment.objects.filter(user=student, course_id=course.id).count(), 0) + + @unittest.skipUnless(settings.MITX_FEATURES.get('AUTH_USE_SHIB'), True) + def test_shib_login_enrollment(self): + """ + A functionality test that a student with an existing shib login can auto-enroll in a class with GET params + """ + if not settings.MITX_FEATURES.get('AUTH_USE_SHIB'): + return + + student = UserFactory.create() + extauth = ExternalAuthMap(external_id='testuser@stanford.edu', + external_email='', + external_domain='shib:https://idp.stanford.edu/', + external_credentials="", + internal_password="password", + user=student) + student.set_password("password") + student.save() + extauth.save() + + course = CourseFactory.create(org='Stanford', number='123', display_name='Shib Only') + course.enrollment_domain = 'shib:https://idp.stanford.edu/' + metadata = own_metadata(course) + metadata['enrollment_domain'] = course.enrollment_domain + self.store.update_metadata(course.location.url(), metadata) + + #use django test client for sessions and url processing + #no enrollment before trying + self.assertEqual(CourseEnrollment.objects.filter(user=student, course_id=course.id).count(), 0) + self.client.logout() + request_kwargs = {'path': '/shib-login/', + 'data': {'enrollment_action': 'enroll', 'course_id': course.id}, + 'follow': False, + 'REMOTE_USER': 'testuser@stanford.edu', + 'Shib-Identity-Provider': 'https://idp.stanford.edu/'} + response = self.client.get(**request_kwargs) + #successful login is a redirect to "/" + self.assertEqual(response.status_code, 302) + self.assertEqual(response['location'], 'http://testserver/') + #now there is enrollment + self.assertEqual(CourseEnrollment.objects.filter(user=student, course_id=course.id).count(), 1) diff --git a/common/djangoapps/external_auth/views.py b/common/djangoapps/external_auth/views.py index 23b46aa803..8288b27ec9 100644 --- a/common/djangoapps/external_auth/views.py +++ b/common/djangoapps/external_auth/views.py @@ -6,17 +6,24 @@ import re import string import fnmatch +from textwrap import dedent from external_auth.models import ExternalAuthMap from external_auth.djangostore import DjangoOpenIDStore from django.conf import settings from django.contrib.auth import REDIRECT_FIELD_NAME, authenticate, login from django.contrib.auth.models import User +from django.core.urlresolvers import reverse +from django.core.validators import validate_email +from django.core.exceptions import ValidationError + from student.models import UserProfile, TestCenterUser, TestCenterRegistration -from django.http import HttpResponse, HttpResponseRedirect +from django.http import HttpResponse, HttpResponseRedirect, HttpRequest from django.utils.http import urlquote from django.shortcuts import redirect +from django.utils.translation import ugettext as _ + from mitxmako.shortcuts import render_to_response, render_to_string try: from django.views.decorators.csrf import csrf_exempt @@ -40,6 +47,7 @@ from courseware.model_data import ModelDataCache from xmodule.modulestore.django import modulestore from xmodule.course_module import CourseDescriptor from xmodule.modulestore import Location +from xmodule.modulestore.exceptions import ItemNotFoundError log = logging.getLogger("mitx.external_auth") @@ -157,7 +165,15 @@ def external_login_or_signup(request, login(request, user) request.session.set_expiry(0) - student_views.try_change_enrollment(request) + + # Now to try enrollment + # Need to special case Shibboleth here because it logs in via a GET. + # testing request.method for extra paranoia + if 'shib:' in external_domain and request.method == 'GET': + enroll_request = make_shib_enrollment_request(request) + student_views.try_change_enrollment(enroll_request) + else: + student_views.try_change_enrollment(request) log.info("Login success - {0} ({1})".format(user.username, user.email)) if retfun is None: return redirect('/') @@ -188,14 +204,25 @@ def signup(request, eamap=None): context = {'has_extauth_info': True, 'show_signup_immediately': True, + 'extauth_id': eamap.external_id, 'extauth_email': eamap.external_email, 'extauth_username': username, 'extauth_name': eamap.external_name, } + # detect if full name is blank and ask for it from user + context['ask_for_fullname'] = eamap.external_name.strip() == '' + + # validate provided mail and if it's not valid ask the user + try: + validate_email(eamap.external_email) + context['ask_for_email'] = False + except ValidationError: + context['ask_for_email'] = True + log.debug('Doing signup for %s' % eamap.external_email) - return student_views.index(request, extra_context=context) + return student_views.register_user(request, extra_context=context) # ----------------------------------------------------------------------------- @@ -304,6 +331,125 @@ def ssl_login(request): retfun=retfun) +# ----------------------------------------------------------------------------- +# Shibboleth (Stanford and others. Uses *Apache* environment variables) +# ----------------------------------------------------------------------------- +def shib_login(request, retfun=None): + """ + Uses Apache's REMOTE_USER environment variable as the external id. + This in turn typically uses EduPersonPrincipalName + http://www.incommonfederation.org/attributesummary.html#eduPersonPrincipal + but the configuration is in the shibboleth software. + """ + shib_error_msg = _(dedent( + """ + Your university identity server did not return your ID information to us. + Please try logging in again. (You may need to restart your browser.) + """)) + + if not request.META.get('REMOTE_USER'): + return default_render_failure(request, shib_error_msg) + else: + #if we get here, the user has authenticated properly + attrs = ['REMOTE_USER', 'givenName', 'sn', 'mail', + 'Shib-Identity-Provider'] + shib = {} + + for attr in attrs: + shib[attr] = request.META.get(attr, '') + + #Clean up first name, last name, and email address + #TODO: Make this less hardcoded re: format, but split will work + #even if ";" is not present since we are accessing 1st element + shib['sn'] = shib['sn'].split(";")[0].strip().capitalize() + shib['givenName'] = shib['givenName'].split(";")[0].strip().capitalize() + + return external_login_or_signup(request, + external_id=shib['REMOTE_USER'], + external_domain="shib:" + shib['Shib-Identity-Provider'], + credentials=shib, + email=shib['mail'], + fullname="%s %s" % (shib['givenName'], shib['sn']), + retfun=retfun) + + +def make_shib_enrollment_request(request): + """ + Need this hack function because shibboleth logins don't happen over POST + but change_enrollment expects its request to be a POST, with + enrollment_action and course_id POST parameters. + """ + enroll_request = HttpRequest() + enroll_request.user = request.user + enroll_request.session = request.session + enroll_request.method = "POST" + + # copy() also makes GET and POST mutable + # See https://docs.djangoproject.com/en/dev/ref/request-response/#django.http.QueryDict.update + enroll_request.GET = request.GET.copy() + enroll_request.POST = request.POST.copy() + + # also have to copy these GET parameters over to POST + if "enrollment_action" not in enroll_request.POST and "enrollment_action" in enroll_request.GET: + enroll_request.POST.setdefault('enrollment_action', enroll_request.GET.get('enrollment_action')) + if "course_id" not in enroll_request.POST and "course_id" in enroll_request.GET: + enroll_request.POST.setdefault('course_id', enroll_request.GET.get('course_id')) + + return enroll_request + + +def course_specific_login(request, course_id): + """ + Dispatcher function for selecting the specific login method + required by the course + """ + query_string = request.META.get("QUERY_STRING", '') + + try: + course = course_from_id(course_id) + except ItemNotFoundError: + #couldn't find the course, will just return vanilla signin page + return redirect_with_querystring('signin_user', query_string) + + #now the dispatching conditionals. Only shib for now + if settings.MITX_FEATURES.get('AUTH_USE_SHIB') and 'shib:' in course.enrollment_domain: + return redirect_with_querystring('shib-login', query_string) + + #Default fallthrough to normal signin page + return redirect_with_querystring('signin_user', query_string) + + +def course_specific_register(request, course_id): + """ + Dispatcher function for selecting the specific registration method + required by the course + """ + query_string = request.META.get("QUERY_STRING", '') + + try: + course = course_from_id(course_id) + except ItemNotFoundError: + #couldn't find the course, will just return vanilla registration page + return redirect_with_querystring('register_user', query_string) + + #now the dispatching conditionals. Only shib for now + if settings.MITX_FEATURES.get('AUTH_USE_SHIB') and 'shib:' in course.enrollment_domain: + #shib-login takes care of both registration and login flows + return redirect_with_querystring('shib-login', query_string) + + #Default fallthrough to normal registration page + return redirect_with_querystring('register_user', query_string) + + +def redirect_with_querystring(view_name, query_string): + """ + Helper function to add query string to redirect views + """ + if query_string: + return redirect("%s?%s" % (reverse(view_name), query_string)) + return redirect(view_name) + + # ----------------------------------------------------------------------------- # OpenID Provider # ----------------------------------------------------------------------------- diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index f129f1b4b1..98587cd782 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -230,7 +230,7 @@ def signin_user(request): @ensure_csrf_cookie -def register_user(request): +def register_user(request, extra_context={}): """ This view will display the non-modal registration form """ @@ -241,6 +241,8 @@ def register_user(request): 'course_id': request.GET.get('course_id'), 'enrollment_action': request.GET.get('enrollment_action') } + context.update(extra_context) + return render_to_response('register.html', context) @@ -282,9 +284,19 @@ def dashboard(request): # Get the 3 most recent news top_news = _get_news(top=3) if not settings.MITX_FEATURES.get('ENABLE_MKTG_SITE', False) else None + + # get info w.r.t ExternalAuthMap + external_auth_map = None + if 'external_auth' in settings.INSTALLED_APPS: + from external_auth.models import ExternalAuthMap + try: + external_auth_map = ExternalAuthMap.objects.get(user=user) + except ExternalAuthMap.DoesNotExist: + pass context = {'courses': courses, 'message': message, + 'external_auth_map': external_auth_map, 'staff_access': staff_access, 'errored_courses': errored_courses, 'show_courseware_links_for': show_courseware_links_for, @@ -571,11 +583,19 @@ def create_account(request, post_override=None): # if doing signup for an external authorization, then get email, password, name from the eamap # don't use the ones from the form, since the user could have hacked those + # unless originally we didn't get a valid email or name from the external auth DoExternalAuth = 'ExternalAuthMap' in request.session if DoExternalAuth: eamap = request.session['ExternalAuthMap'] - email = eamap.external_email - name = eamap.external_name + try: + validate_email(eamap.external_email) + email = eamap.external_email + except ValidationError: + email = post_vars.get('email', '') + if eamap.external_name.strip() == '': + name = post_vars.get('name', '') + else: + name = eamap.external_name password = eamap.internal_password post_vars = dict(post_vars.items()) post_vars.update(dict(email=email, name=name, password=password)) @@ -665,8 +685,6 @@ def create_account(request, post_override=None): login(request, login_user) request.session.set_expiry(0) - try_change_enrollment(request) - if DoExternalAuth: eamap.user = login_user eamap.dtsignup = datetime.datetime.now(UTC) @@ -678,6 +696,8 @@ def create_account(request, post_override=None): login_user.is_active = True login_user.save() + try_change_enrollment(request) + statsd.increment("common.student.account_created") js = {'success': True} diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 945c3a3cfa..62ebe12a03 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -179,6 +179,8 @@ class CourseFields(object): checklists = List(scope=Scope.settings) info_sidebar_name = String(scope=Scope.settings, default='Course Handouts') show_timezone = Boolean(help="True if timezones should be shown on dates in the courseware", scope=Scope.settings, default=True) + enrollment_domain = String(help="External login method associated with user accounts allowed to register in course", + scope=Scope.settings) # An extra property is used rather than the wiki_slug/number because # there are courses that change the number for different runs. This allows diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index 07987a8edf..195d29061d 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -15,6 +15,7 @@ from xmodule.modulestore import Location from xmodule.x_module import XModule, XModuleDescriptor from student.models import CourseEnrollmentAllowed +from external_auth.models import ExternalAuthMap from courseware.masquerade import is_masquerading_as_student from django.utils.timezone import UTC @@ -130,15 +131,33 @@ def _has_access_course_desc(user, course, action): def can_enroll(): """ - If the course has an enrollment period, check whether we are in it. + First check if restriction of enrollment by login method is enabled, both + globally and by the course. + If it is, then the user must pass the criterion set by the course, e.g. that ExternalAuthMap + was set by 'shib:https://idp.stanford.edu/", in addition to requirements below. + Rest of requirements: + Enrollment can only happen in the course enrollment period, if one exists. + or + + (CourseEnrollmentAllowed always overrides) (staff can always enroll) """ + # if using registration method to restrict (say shibboleth) + if settings.MITX_FEATURES.get('RESTRICT_ENROLL_BY_REG_METHOD') and course.enrollment_domain: + if user is not None and user.is_authenticated() and \ + ExternalAuthMap.objects.filter(user=user, external_domain=course.enrollment_domain): + debug("Allow: external_auth of " + course.enrollment_domain) + reg_method_ok = True + else: + reg_method_ok = False + else: + reg_method_ok = True #if not using this access check, it's always OK. now = datetime.now(UTC()) start = course.enrollment_start end = course.enrollment_end - if (start is None or now > start) and (end is None or now < end): + if reg_method_ok and (start is None or now > start) and (end is None or now < end): # in enrollment period, so any user is allowed to enroll. debug("Allow: in enrollment period") return True diff --git a/lms/djangoapps/courseware/tests/test_access.py b/lms/djangoapps/courseware/tests/test_access.py index 34d064971f..0748c9a59c 100644 --- a/lms/djangoapps/courseware/tests/test_access.py +++ b/lms/djangoapps/courseware/tests/test_access.py @@ -81,7 +81,7 @@ class AccessTestCase(TestCase): u = Mock() yesterday = datetime.datetime.now(UTC()) - datetime.timedelta(days=1) tomorrow = datetime.datetime.now(UTC()) + datetime.timedelta(days=1) - c = Mock(enrollment_start=yesterday, enrollment_end=tomorrow) + c = Mock(enrollment_start=yesterday, enrollment_end=tomorrow, enrollment_domain='') # User can enroll if it is between the start and end dates self.assertTrue(access._has_access_course_desc(u, c, 'enroll')) @@ -91,7 +91,7 @@ class AccessTestCase(TestCase): u = Mock(email='test@edx.org', is_staff=False) u.is_authenticated.return_value = True - c = Mock(enrollment_start=tomorrow, enrollment_end=tomorrow, id='edX/test/2012_Fall') + c = Mock(enrollment_start=tomorrow, enrollment_end=tomorrow, id='edX/test/2012_Fall', enrollment_domain='') allowed = CourseEnrollmentAllowedFactory(email=u.email, course_id=c.id) @@ -101,7 +101,7 @@ class AccessTestCase(TestCase): u = Mock(email='test@edx.org', is_staff=True) u.is_authenticated.return_value = True - c = Mock(enrollment_start=tomorrow, enrollment_end=tomorrow, id='edX/test/Whenever') + c = Mock(enrollment_start=tomorrow, enrollment_end=tomorrow, id='edX/test/Whenever', enrollment_domain='') self.assertTrue(access._has_access_course_desc(u, c, 'enroll')) # TODO: diff --git a/lms/envs/common.py b/lms/envs/common.py index 076528e91e..be570a9796 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -91,6 +91,10 @@ MITX_FEATURES = { 'AUTH_USE_OPENID': False, 'AUTH_USE_MIT_CERTIFICATES': False, 'AUTH_USE_OPENID_PROVIDER': False, + 'AUTH_USE_SHIB': False, + + # Enables ability to restrict enrollment in specific courses by the user account login method + 'RESTRICT_ENROLL_BY_REG_METHOD': False, # analytics experiments 'ENABLE_INSTRUCTOR_ANALYTICS': False, diff --git a/lms/envs/dev.py b/lms/envs/dev.py index 17bce93991..b1519b77bc 100644 --- a/lms/envs/dev.py +++ b/lms/envs/dev.py @@ -232,6 +232,9 @@ FILE_UPLOAD_HANDLERS = ( 'django.core.files.uploadhandler.TemporaryFileUploadHandler', ) +MITX_FEATURES['AUTH_USE_SHIB'] = True +MITX_FEATURES['RESTRICT_ENROLL_BY_REG_METHOD'] = True + ########################### PIPELINE ################################# PIPELINE_SASS_ARGUMENTS = '--debug-info --require {proj_dir}/static/sass/bourbon/lib/bourbon.rb'.format(proj_dir=PROJECT_ROOT) diff --git a/lms/envs/test.py b/lms/envs/test.py index 3ccfa24014..3a6c641841 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -137,6 +137,10 @@ SECRET_KEY = '85920908f28904ed733fe576320db18cabd7b6cd' MITX_FEATURES['AUTH_USE_OPENID'] = True MITX_FEATURES['AUTH_USE_OPENID_PROVIDER'] = True +################################## SHIB ####################################### +MITX_FEATURES['AUTH_USE_SHIB'] = True +MITX_FEATURES['RESTRICT_ENROLL_BY_REG_METHOD'] = True + OPENID_CREATE_USERS = False OPENID_UPDATE_DETAILS_FROM_SREG = True OPENID_USE_AS_ADMIN_LOGIN = False diff --git a/lms/templates/courseware/course_about.html b/lms/templates/courseware/course_about.html index cc4b2ec317..15317de207 100644 --- a/lms/templates/courseware/course_about.html +++ b/lms/templates/courseware/course_about.html @@ -24,6 +24,26 @@ event.preventDefault(); }); + ## making the conditional around this entire JS block for sanity + %if settings.MITX_FEATURES.get('RESTRICT_ENROLL_BY_REG_METHOD') and course.enrollment_domain: + $('#class_enroll_form').on('ajax:complete', function(event, xhr) { + if(xhr.status == 200) { + location.href = "${reverse('dashboard')}"; + } else if (xhr.status == 403) { + location.href = "${reverse('course-specific-register', args=[course.id])}?course_id=${course.id}&enrollment_action=enroll"; + } else if (xhr.status == 400) { //This means the user did not have permission + $('#register_error').html('This course has restricted enrollment. Sorry, you do not have permission to enroll.<br />' + + 'You may need to log out and re-login with a university account, such as WebAuth' + ).css("display", "block"); + } else { + $('#register_error').html( + (xhr.responseText ? xhr.responseText : 'An error occurred. Please try again later.') + ).css("display", "block"); + } + }); + + %else: + $('#class_enroll_form').on('ajax:complete', function(event, xhr) { if(xhr.status == 200) { location.href = "${reverse('dashboard')}"; @@ -35,13 +55,16 @@ ).css("display", "block"); } }); + + %endif + + })(this) </script> <script src="${static.url('js/course_info.js')}"></script> </%block> - <%block name="title"><title>About ${course.number}
@@ -92,7 +115,7 @@ - +
diff --git a/lms/templates/extauth_failure.html b/lms/templates/extauth_failure.html index fa53ab1084..330c63e604 100644 --- a/lms/templates/extauth_failure.html +++ b/lms/templates/extauth_failure.html @@ -2,10 +2,10 @@ "http://www.w3.org/TR/html4/strict.dtd"> - OpenID failed + External Authentication failed -

OpenID failed

+

External Authentication failed

${message}

diff --git a/lms/templates/navigation.html b/lms/templates/navigation.html index 190a58f691..a26e1ca367 100644 --- a/lms/templates/navigation.html +++ b/lms/templates/navigation.html @@ -95,16 +95,26 @@ site_status_msg = get_site_status_msg(course_id) % endif % if not settings.MITX_FEATURES['DISABLE_LOGIN_BUTTON']: -
  • - Register Now -
    • + % if course and settings.MITX_FEATURES.get('RESTRICT_ENROLL_BY_REG_METHOD') and course.enrollment_domain: +
  • + Register Now +
    • + % else: +
  • + Register Now +
    • + % endif % endif
    1. % if not settings.MITX_FEATURES['DISABLE_LOGIN_BUTTON']: - + % if course and settings.MITX_FEATURES.get('RESTRICT_ENROLL_BY_REG_METHOD') and course.enrollment_domain: + + % else: + + % endif % endif
    diff --git a/lms/templates/register.html b/lms/templates/register.html index 73a6df9319..2cad6955eb 100644 --- a/lms/templates/register.html +++ b/lms/templates/register.html @@ -136,16 +136,37 @@ % else:
    -

    Welcome ${extauth_email}

    +

    Welcome ${extauth_id}

    Enter a public username:

      + + % if ask_for_email: + +
    1. + + +
      2. + + % endif +
    2. Will be shown in any discussions or forums you participate in
      3. + + % if ask_for_fullname: + +
    3. + + + Needed for any certificates you may earn (cannot be changed later) +
      4. + + % endif +
    % endif @@ -246,6 +267,8 @@

    Registration Help

    + % if has_extauth_info is UNDEFINED: +

    Already registered?

    @@ -254,6 +277,8 @@

    + + % endif ## TODO: Use a %block tag or something to allow themes to ## override in a more generalizable fashion. diff --git a/lms/templates/signup_modal.html b/lms/templates/signup_modal.html index a68e36e902..9c1a868e2d 100644 --- a/lms/templates/signup_modal.html +++ b/lms/templates/signup_modal.html @@ -32,11 +32,23 @@ % else: -

    Welcome ${extauth_email}


    +

    Welcome ${extauth_id}


    Enter a public username:

    - + - + + + % if ask_for_email: + + + % endif + + + % if ask_for_fullname: + + + % endif + % endif diff --git a/lms/urls.py b/lms/urls.py index 1d34ebf3af..6a4819aedb 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -363,6 +363,21 @@ if settings.MITX_FEATURES.get('AUTH_USE_OPENID'): url(r'^openid/logo.gif$', 'django_openid_auth.views.logo', name='openid-logo'), ) +if settings.MITX_FEATURES.get('AUTH_USE_SHIB'): + urlpatterns += ( + url(r'^shib-login/$', 'external_auth.views.shib_login', name='shib-login'), + ) + +if settings.MITX_FEATURES.get('RESTRICT_ENROLL_BY_REG_METHOD'): + urlpatterns += ( + url(r'^course_specific_login/(?P[^/]+/[^/]+/[^/]+)/$', + 'external_auth.views.course_specific_login', name='course-specific-login'), + url(r'^course_specific_register/(?P[^/]+/[^/]+/[^/]+)/$', + 'external_auth.views.course_specific_register', name='course-specific-register'), + + ) + + if settings.MITX_FEATURES.get('AUTH_USE_OPENID_PROVIDER'): urlpatterns += ( url(r'^openid/provider/login/$', 'external_auth.views.provider_login', name='openid-provider-login'), diff --git a/lms/wsgi_apache.py b/lms/wsgi_apache_lms.py similarity index 53% rename from lms/wsgi_apache.py rename to lms/wsgi_apache_lms.py index e2d8a23dc0..0f9950ca41 100644 --- a/lms/wsgi_apache.py +++ b/lms/wsgi_apache_lms.py @@ -1,15 +1,12 @@ import os os.environ.setdefault("DJANGO_SETTINGS_MODULE", "lms.envs.aws") +os.environ.setdefault("SERVICE_VARIANT", "lms") + # This application object is used by the development server # as well as any WSGI server configured to use this file. -from django.core.wsgi import WSGIHandler -_application = WSGIHandler() - -def application(environ, start_response): - #copy SERVICE_VARIANT from apache environ to os environ - os.environ.setdefault("SERVICE_VARIANT", environ.get("SERVICE_VARIANT", "lms")) - return _application(environ, start_response) +from django.core.wsgi import get_wsgi_application +application = get_wsgi_application() from django.conf import settings from xmodule.modulestore.django import modulestore From a39a384ed22e9fe82e10293b146c1b14a3f7e787 Mon Sep 17 00:00:00 2001 From: Jason Bau Date: Fri, 14 Jun 2013 15:07:44 -0700 Subject: [PATCH 3/6] Handle the case where an existing user has email returned by shib By linking the users --- common/djangoapps/external_auth/views.py | 39 +++++++++++++++++++++--- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/common/djangoapps/external_auth/views.py b/common/djangoapps/external_auth/views.py index 8288b27ec9..d4a0b56293 100644 --- a/common/djangoapps/external_auth/views.py +++ b/common/djangoapps/external_auth/views.py @@ -147,11 +147,42 @@ def external_login_or_signup(request, internal_user = eamap.user if internal_user is None: - log.debug('No user for %s yet, doing signup' % eamap.external_email) - return signup(request, eamap) + if settings.MITX_FEATURES.get('AUTH_USE_SHIB'): + # if we are using shib, try to link accounts using email + try: + link_user = User.objects.get(email=eamap.external_email) + if not ExternalAuthMap.objects.filter(user=link_user).exists(): + # if there's no pre-existing linked eamap, we link the user + eamap.user = link_user + eamap.save() + internal_user = link_user + log.debug('Linking existing account for %s' % eamap.external_email) + # now pass through to log in + else: + # otherwise, set external_email to '' to ask for a new one at user signup + eamap.external_email = '' + eamap.save() + log.debug('User with external login found for %s, asking for new email during signup' % email) + return signup(request, eamap) + except User.DoesNotExist: + log.debug('No user for %s yet, doing signup' % eamap.external_email) + return signup(request, eamap) + else: + log.debug('No user for %s yet, doing signup' % eamap.external_email) + return signup(request, eamap) - uname = internal_user.username - user = authenticate(username=uname, password=eamap.internal_password) + # We trust shib's authentication, so no need to authenticate using the password again + if settings.MITX_FEATURES.get('AUTH_USE_SHIB'): + user = internal_user + # Assuming this 'AUTHENTICATION_BACKENDS' is set in settings, which I think is safe + if settings.AUTHENTICATION_BACKENDS: + auth_backend = settings.AUTHENTICATION_BACKENDS[0] + else: + auth_backend = 'django.contrib.auth.backends.ModelBackend' + user.backend = auth_backend + else: + uname = internal_user.username + user = authenticate(username=uname, password=eamap.internal_password) if user is None: log.warning("External Auth Login failed for %s / %s" % (uname, eamap.internal_password)) From ca649d3c33a8c660d6bf06eba75d17649b311589 Mon Sep 17 00:00:00 2001 From: Jason Bau Date: Fri, 14 Jun 2013 22:12:35 -0700 Subject: [PATCH 4/6] Turn off Agreement to Terms of Service for Stanford shib As stipulated by Stanford's office of general counsel --- common/djangoapps/external_auth/views.py | 5 +++++ common/djangoapps/student/views.py | 20 +++++++++++++++----- lms/templates/register.html | 5 +++++ 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/common/djangoapps/external_auth/views.py b/common/djangoapps/external_auth/views.py index d4a0b56293..097cdefe77 100644 --- a/common/djangoapps/external_auth/views.py +++ b/common/djangoapps/external_auth/views.py @@ -239,8 +239,13 @@ def signup(request, eamap=None): 'extauth_email': eamap.external_email, 'extauth_username': username, 'extauth_name': eamap.external_name, + 'ask_for_tos': True, } + # Can't have terms of service for Stanford users, according to Stanford's Office of General Counsel + if settings.MITX_FEATURES['AUTH_USE_SHIB'] and ('stanford' in eamap.external_domain): + context['ask_for_tos'] = False + # detect if full name is blank and ask for it from user context['ask_for_fullname'] = eamap.external_name.strip() == '' diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 98587cd782..8bc29fa671 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -613,17 +613,27 @@ def create_account(request, post_override=None): js['field'] = 'honor_code' return HttpResponse(json.dumps(js)) - if post_vars.get('terms_of_service', 'false') != u'true': - js['value'] = "You must accept the terms of service.".format(field=a) - js['field'] = 'terms_of_service' - return HttpResponse(json.dumps(js)) + # Can't have terms of service for Stanford users, according to Stanford's Office of General Counsel + if settings.MITX_FEATURES.get("AUTH_USE_SHIB") and DoExternalAuth and ("stanford" in eamap.external_domain): + pass + else: + if post_vars.get('terms_of_service', 'false') != u'true': + js['value'] = "You must accept the terms of service.".format(field=a) + js['field'] = 'terms_of_service' + return HttpResponse(json.dumps(js)) # Confirm appropriate fields are there. # TODO: Check e-mail format is correct. # TODO: Confirm e-mail is not from a generic domain (mailinator, etc.)? Not sure if # this is a good idea # TODO: Check password is sane - for a in ['username', 'email', 'name', 'password', 'terms_of_service', 'honor_code']: + + required_post_vars = ['username', 'email', 'name', 'password', 'terms_of_service', 'honor_code'] + if settings.MITX_FEATURES.get("AUTH_USE_SHIB") and DoExternalAuth and ("stanford" in eamap.external_domain): + # Can't have terms of service for Stanford users, according to Stanford's Office of General Counsel + required_post_vars = ['username', 'email', 'name', 'password', 'honor_code'] + + for a in required_post_vars: if len(post_vars[a]) < 2: error_str = {'username': 'Username must be minimum of two characters long.', 'email': 'A properly formatted e-mail is required.', diff --git a/lms/templates/register.html b/lms/templates/register.html index 2cad6955eb..1a42d402e5 100644 --- a/lms/templates/register.html +++ b/lms/templates/register.html @@ -231,11 +231,16 @@
    1. + + % if has_extauth_info is UNDEFINED or ask_for_tos : +
      + % endif +
      <% From 084160c1c9b76e0c09eb6221591503f9e1b1e3f2 Mon Sep 17 00:00:00 2001 From: Jason Bau Date: Wed, 19 Jun 2013 00:16:41 -0700 Subject: [PATCH 5/6] Finishing up tests/modifications per @ormsbee feedback --- .../external_auth/tests/test_shib.py | 196 +++++++++++------- common/djangoapps/external_auth/views.py | 49 +++-- common/djangoapps/student/views.py | 7 +- 3 files changed, 148 insertions(+), 104 deletions(-) diff --git a/common/djangoapps/external_auth/tests/test_shib.py b/common/djangoapps/external_auth/tests/test_shib.py index f342aa4c74..e5059e5635 100644 --- a/common/djangoapps/external_auth/tests/test_shib.py +++ b/common/djangoapps/external_auth/tests/test_shib.py @@ -1,3 +1,4 @@ +# coding=utf-8 """ Tests for Shibboleth Authentication @jbau @@ -6,11 +7,12 @@ import unittest from django.conf import settings from django.http import HttpResponseRedirect -from django.test.client import RequestFactory +from django.test.client import RequestFactory, Client as DjangoTestClient from django.test.utils import override_settings from django.core.urlresolvers import reverse from django.contrib.auth.models import AnonymousUser, User from django.contrib.sessions.backends.base import SessionBase +from django.utils.importlib import import_module from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase @@ -34,23 +36,27 @@ from student.tests.factories import UserFactory IDP = 'https://idp.stanford.edu/' REMOTE_USER = 'test_user@stanford.edu' MAILS = [None, '', 'test_user@stanford.edu'] -GIVENNAMES = [None, '', 'Jason', 'jason; John; bob'] # At Stanford, the givenNames can be a list delimited by ';' -SNS = [None, '', 'Bau', 'bau; smith'] # At Stanford, the sns can be a list delimited by ';' +GIVENNAMES = [None, '', 'Jason', 'jasön; John; bob'] # At Stanford, the givenNames can be a list delimited by ';' +SNS = [None, '', 'Bau', '包; smith'] # At Stanford, the sns can be a list delimited by ';' def gen_all_identities(): - """A generator for all combinations of identity inputs""" + """ + A generator for all combinations of test inputs. + Each generated item is a dict that represents what a shib IDP + could potentially pass to django via request.META, i.e. + setting (or not) request.META['givenName'], etc. + """ def _build_identity_dict(mail, given_name, surname): """ Helper function to return a dict of test identity """ - meta_dict = {} - meta_dict.update({'Shib-Identity-Provider': IDP, - 'REMOTE_USER': REMOTE_USER}) + meta_dict = {'Shib-Identity-Provider': IDP, + 'REMOTE_USER': REMOTE_USER} if mail is not None: - meta_dict.update({'mail': mail}) + meta_dict['mail'] = mail if given_name is not None: - meta_dict.update({'givenName': given_name}) + meta_dict['givenName'] = given_name if surname is not None: - meta_dict.update({'sn': surname}) + meta_dict['sn'] = surname return meta_dict for mail in MAILS: @@ -59,48 +65,84 @@ def gen_all_identities(): yield _build_identity_dict(mail, given_name, surname) -@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE) +@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE, SESSION_ENGINE='django.contrib.sessions.backends.cache') class ShibSPTest(ModuleStoreTestCase): """ Tests for the Shibboleth SP, which communicates via request.META (Apache environment variables set by mod_shib) """ - factory = RequestFactory() + request_factory = RequestFactory() def setUp(self): self.store = modulestore() + @unittest.skipUnless(settings.MITX_FEATURES.get('AUTH_USE_SHIB'), True) + def test_exception_shib_login(self): + """ + Tests that we get the error page when there is no REMOTE_USER + or Shib-Identity-Provider in request.META + """ + no_remote_user_request = self.request_factory.get('/shib-login') + no_remote_user_request.META.update({'Shib-Identity-Provider': IDP}) + no_remote_user_response = shib_login(no_remote_user_request) + self.assertEqual(no_remote_user_response.status_code, 403) + self.assertIn("identity server did not return your ID information", no_remote_user_response.content) + + no_idp_request = self.request_factory.get('/shib-login') + no_idp_request.META.update({'REMOTE_USER': REMOTE_USER}) + no_idp_response = shib_login(no_idp_request) + self.assertEqual(no_idp_response.status_code, 403) + self.assertIn("identity server did not return your ID information", no_idp_response.content) + + @unittest.skipUnless(settings.MITX_FEATURES.get('AUTH_USE_SHIB'), True) def test_shib_login(self): """ - Tests that a user with a shib ExternalAuthMap gets logged in while when - shib-login is called, while a user without such gets the registration form. + Tests that: + * shib credentials that match an existing ExternalAuthMap with a linked user logs the user in + * shib credentials that match an existing ExternalAuthMap without a linked user and also match the email + of an existing user without an existing ExternalAuthMap links the two and log the user in + * shib credentials that match an existing ExternalAuthMap without a linked user and also match the email + of an existing user that already has an ExternalAuthMap causes an error (403) + * shib credentials that do not match an existing ExternalAuthMap causes the registration form to appear """ - student = UserFactory.create() - extauth = ExternalAuthMap(external_id='testuser@stanford.edu', + user_w_map = UserFactory.create(email='withmap@stanford.edu') + extauth = ExternalAuthMap(external_id='withmap@stanford.edu', external_email='', external_domain='shib:https://idp.stanford.edu/', external_credentials="", - user=student) - student.save() + user=user_w_map) + user_wo_map = UserFactory.create(email='womap@stanford.edu') + user_w_map.save() + user_wo_map.save() extauth.save() idps = ['https://idp.stanford.edu/', 'https://someother.idp.com/'] - remote_users = ['testuser@stanford.edu', 'testuser2@someother_idp.com'] + remote_users = ['withmap@stanford.edu', 'womap@stanford.edu', 'testuser2@someother_idp.com'] for idp in idps: for remote_user in remote_users: - request = self.factory.get('/shib-login') - request.session = SessionBase() # empty session + request = self.request_factory.get('/shib-login') + request.session = import_module(settings.SESSION_ENGINE).SessionStore() # empty session request.META.update({'Shib-Identity-Provider': idp, - 'REMOTE_USER': remote_user}) + 'REMOTE_USER': remote_user, + 'mail': remote_user}) request.user = AnonymousUser() response = shib_login(request) - if idp == "https://idp.stanford.edu" and remote_user == 'testuser@stanford.edu': + if idp == "https://idp.stanford.edu/" and remote_user == 'withmap@stanford.edu': self.assertIsInstance(response, HttpResponseRedirect) - self.assertEqual(request.user, student) + self.assertEqual(request.user, user_w_map) self.assertEqual(response['Location'], '/') + elif idp == "https://idp.stanford.edu/" and remote_user == 'womap@stanford.edu': + self.assertIsNotNone(ExternalAuthMap.objects.get(user=user_wo_map)) + self.assertIsInstance(response, HttpResponseRedirect) + self.assertEqual(request.user, user_wo_map) + self.assertEqual(response['Location'], '/') + elif idp == "https://someother.idp.com/" and remote_user in \ + ['withmap@stanford.edu', 'womap@stanford.edu']: + self.assertEqual(response.status_code, 403) + self.assertIn("You have already created an account using an external login", response.content) else: self.assertEqual(response.status_code, 200) self.assertContains(response, "Register for") @@ -113,10 +155,9 @@ class ShibSPTest(ModuleStoreTestCase): Uses django test client for its session support """ for identity in gen_all_identities(): - self.client.logout() - request_kwargs = {'path': '/shib-login/', 'data': {}, 'follow': False} - request_kwargs.update(identity) - response = self.client.get(**request_kwargs) # identity k/v pairs will show up in request.META + client = DjangoTestClient() + # identity k/v pairs will show up in request.META + response = client.get(path='/shib-login/', data={}, follow=False, **identity) self.assertEquals(response.status_code, 200) mail_input_HTML = '<input class="" id="email" type="email" name="email"' @@ -124,8 +165,8 @@ class ShibSPTest(ModuleStoreTestCase): self.assertContains(response, mail_input_HTML) else: self.assertNotContains(response, mail_input_HTML) - sn_empty = identity.get('sn', '') == '' - given_name_empty = identity.get('givenName', '') == '' + sn_empty = not identity.get('sn') + given_name_empty = not identity.get('givenName') fullname_input_HTML = '<input id="name" type="text" name="name"' if sn_empty and given_name_empty: self.assertContains(response, fullname_input_HTML) @@ -133,7 +174,7 @@ class ShibSPTest(ModuleStoreTestCase): self.assertNotContains(response, fullname_input_HTML) #clean up b/c we don't want existing ExternalAuthMap for the next run - self.client.session['ExternalAuthMap'].delete() + client.session['ExternalAuthMap'].delete() @unittest.skipUnless(settings.MITX_FEATURES.get('AUTH_USE_SHIB'), True) def test_registration_formSubmit(self): @@ -146,10 +187,8 @@ class ShibSPTest(ModuleStoreTestCase): """ for identity in gen_all_identities(): #First we pop the registration form - self.client.logout() - request1_kwargs = {'path': '/shib-login/', 'data': {}, 'follow': False} - request1_kwargs.update(identity) - response1 = self.client.get(**request1_kwargs) + client = DjangoTestClient() + response1 = client.get(path='/shib-login/', data={}, follow=False, **identity) #Then we have the user answer the registration form postvars = {'email': 'post_email@stanford.edu', 'username': 'post_username', @@ -158,8 +197,8 @@ class ShibSPTest(ModuleStoreTestCase): 'terms_of_service': 'true', 'honor_code': 'true'} #use RequestFactory instead of TestClient here because we want access to request.user - request2 = self.factory.post('/create_account', data=postvars) - request2.session = self.client.session + request2 = self.request_factory.post('/create_account', data=postvars) + request2.session = client.session request2.user = AnonymousUser() response2 = create_account(request2) @@ -177,13 +216,12 @@ class ShibSPTest(ModuleStoreTestCase): #check that the created user profile has the right name, either taken from shib or user input profile = UserProfile.objects.get(user=user) - sn_empty = identity.get('sn', '') == '' - given_name_empty = identity.get('givenName', '') == '' + sn_empty = not identity.get('sn') + given_name_empty = not identity.get('givenName') if sn_empty and given_name_empty: self.assertEqual(profile.name, postvars['name']) else: self.assertEqual(profile.name, request2.session['ExternalAuthMap'].external_name) - #clean up for next loop request2.session['ExternalAuthMap'].delete() UserProfile.objects.filter(user=user).delete() @@ -206,12 +244,12 @@ class ShibSPTest(ModuleStoreTestCase): self.store.update_metadata(course.location.url(), metadata) #setting location to test that GET params get passed through - login_request = self.factory.get('/course_specific_login/MITx/999/Robot_Super_Course' + - '?course_id=MITx/999/Robot_Super_Course' + - '&enrollment_action=enroll') - reg_request = self.factory.get('/course_specific_register/MITx/999/Robot_Super_Course' + - '?course_id=MITx/999/course/Robot_Super_Course' + - '&enrollment_action=enroll') + login_request = self.request_factory.get('/course_specific_login/MITx/999/Robot_Super_Course' + + '?course_id=MITx/999/Robot_Super_Course' + + '&enrollment_action=enroll') + reg_request = self.request_factory.get('/course_specific_register/MITx/999/Robot_Super_Course' + + '?course_id=MITx/999/course/Robot_Super_Course' + + '&enrollment_action=enroll') login_response = course_specific_login(login_request, 'MITx/999/Robot_Super_Course') reg_response = course_specific_register(login_request, 'MITx/999/Robot_Super_Course') @@ -241,12 +279,12 @@ class ShibSPTest(ModuleStoreTestCase): # Now test for non-existent course #setting location to test that GET params get passed through - login_request = self.factory.get('/course_specific_login/DNE/DNE/DNE' + - '?course_id=DNE/DNE/DNE' + - '&enrollment_action=enroll') - reg_request = self.factory.get('/course_specific_register/DNE/DNE/DNE' + - '?course_id=DNE/DNE/DNE/Robot_Super_Course' + - '&enrollment_action=enroll') + login_request = self.request_factory.get('/course_specific_login/DNE/DNE/DNE' + + '?course_id=DNE/DNE/DNE' + + '&enrollment_action=enroll') + reg_request = self.request_factory.get('/course_specific_register/DNE/DNE/DNE' + + '?course_id=DNE/DNE/DNE/Robot_Super_Course' + + '&enrollment_action=enroll') login_response = course_specific_login(login_request, 'DNE/DNE/DNE') reg_response = course_specific_register(login_request, 'DNE/DNE/DNE') @@ -270,54 +308,54 @@ class ShibSPTest(ModuleStoreTestCase): """ #create 2 course, one with limited enrollment one without - course1 = CourseFactory.create(org='Stanford', number='123', display_name='Shib Only') - course1.enrollment_domain = 'shib:https://idp.stanford.edu/' - metadata = own_metadata(course1) - metadata['enrollment_domain'] = course1.enrollment_domain - self.store.update_metadata(course1.location.url(), metadata) + shib_course = CourseFactory.create(org='Stanford', number='123', display_name='Shib Only') + shib_course.enrollment_domain = 'shib:https://idp.stanford.edu/' + metadata = own_metadata(shib_course) + metadata['enrollment_domain'] = shib_course.enrollment_domain + self.store.update_metadata(shib_course.location.url(), metadata) - course2 = CourseFactory.create(org='MITx', number='999', display_name='Robot Super Course') - course2.enrollment_domain = '' - metadata = own_metadata(course2) - metadata['enrollment_domain'] = course2.enrollment_domain - self.store.update_metadata(course2.location.url(), metadata) + open_enroll_course = CourseFactory.create(org='MITx', number='999', display_name='Robot Super Course') + open_enroll_course.enrollment_domain = '' + metadata = own_metadata(open_enroll_course) + metadata['enrollment_domain'] = open_enroll_course.enrollment_domain + self.store.update_metadata(open_enroll_course.location.url(), metadata) - # create 3 kinds of students, external_auth matching course1, external_auth not matching, no external auth - student1 = UserFactory.create() - student1.save() + # create 3 kinds of students, external_auth matching shib_course, external_auth not matching, no external auth + shib_student = UserFactory.create() + shib_student.save() extauth = ExternalAuthMap(external_id='testuser@stanford.edu', external_email='', external_domain='shib:https://idp.stanford.edu/', external_credentials="", - user=student1) + user=shib_student) extauth.save() - student2 = UserFactory.create() - student2.username = "teststudent2" - student2.email = "teststudent2@other.edu" - student2.save() + other_ext_student = UserFactory.create() + other_ext_student.username = "teststudent2" + other_ext_student.email = "teststudent2@other.edu" + other_ext_student.save() extauth = ExternalAuthMap(external_id='testuser1@other.edu', external_email='', external_domain='shib:https://other.edu/', external_credentials="", - user=student2) + user=other_ext_student) extauth.save() - student3 = UserFactory.create() - student3.username = "teststudent3" - student3.email = "teststudent3@gmail.com" - student3.save() + int_student = UserFactory.create() + int_student.username = "teststudent3" + int_student.email = "teststudent3@gmail.com" + int_student.save() #Tests the two case for courses, limited and not - for course in [course1, course2]: - for student in [student1, student2, student3]: - request = self.factory.post('/change_enrollment') + for course in [shib_course, open_enroll_course]: + for student in [shib_student, other_ext_student, int_student]: + request = self.request_factory.post('/change_enrollment') request.POST.update({'enrollment_action': 'enroll', 'course_id': course.id}) request.user = student response = change_enrollment(request) #if course is not limited or student has correct shib extauth then enrollment should be allowed - if course is course2 or student is student1: + if course is open_enroll_course or student is shib_student: self.assertEqual(response.status_code, 200) self.assertEqual(CourseEnrollment.objects.filter(user=student, course_id=course.id).count(), 1) #clean up diff --git a/common/djangoapps/external_auth/views.py b/common/djangoapps/external_auth/views.py index 097cdefe77..1ae8edfc52 100644 --- a/common/djangoapps/external_auth/views.py +++ b/common/djangoapps/external_auth/views.py @@ -145,6 +145,7 @@ def external_login_or_signup(request, eamap.save() + log.info("External_Auth login_or_signup for %s : %s : %s : %s" % (external_domain, external_id, email, fullname)) internal_user = eamap.user if internal_user is None: if settings.MITX_FEATURES.get('AUTH_USE_SHIB'): @@ -156,19 +157,21 @@ def external_login_or_signup(request, eamap.user = link_user eamap.save() internal_user = link_user - log.debug('Linking existing account for %s' % eamap.external_email) + log.info('SHIB: Linking existing account for %s' % eamap.external_email) # now pass through to log in else: - # otherwise, set external_email to '' to ask for a new one at user signup - eamap.external_email = '' - eamap.save() - log.debug('User with external login found for %s, asking for new email during signup' % email) - return signup(request, eamap) + # otherwise, there must have been an error, b/c we've already linked a user with these external + # creds + failure_msg = _(dedent(""" + You have already created an account using an external login like WebAuth or Shibboleth. + Please contact %s for support """ + % getattr(settings, 'TECH_SUPPORT_EMAIL', 'techsupport@class.stanford.edu'))) + return default_render_failure(request, failure_msg) except User.DoesNotExist: - log.debug('No user for %s yet, doing signup' % eamap.external_email) + log.info('SHIB: No user for %s yet, doing signup' % eamap.external_email) return signup(request, eamap) else: - log.debug('No user for %s yet, doing signup' % eamap.external_email) + log.info('No user for %s yet, doing signup' % eamap.external_email) return signup(request, eamap) # We trust shib's authentication, so no need to authenticate using the password again @@ -180,6 +183,7 @@ def external_login_or_signup(request, else: auth_backend = 'django.contrib.auth.backends.ModelBackend' user.backend = auth_backend + log.info('SHIB: Logging in linked user %s' % user.email) else: uname = internal_user.username user = authenticate(username=uname, password=eamap.internal_password) @@ -193,14 +197,13 @@ def external_login_or_signup(request, # TODO: improve error page msg = 'Account not yet activated: please look for link in your email' return default_render_failure(request, msg) - login(request, user) request.session.set_expiry(0) # Now to try enrollment # Need to special case Shibboleth here because it logs in via a GET. # testing request.method for extra paranoia - if 'shib:' in external_domain and request.method == 'GET': + if settings.MITX_FEATURES.get('AUTH_USE_SHIB') and 'shib:' in external_domain and request.method == 'GET': enroll_request = make_shib_enrollment_request(request) student_views.try_change_enrollment(enroll_request) else: @@ -256,7 +259,7 @@ def signup(request, eamap=None): except ValidationError: context['ask_for_email'] = True - log.debug('Doing signup for %s' % eamap.external_email) + log.info('EXTAUTH: Doing signup for %s' % eamap.external_id) return student_views.register_user(request, extra_context=context) @@ -370,7 +373,7 @@ def ssl_login(request): # ----------------------------------------------------------------------------- # Shibboleth (Stanford and others. Uses *Apache* environment variables) # ----------------------------------------------------------------------------- -def shib_login(request, retfun=None): +def shib_login(request): """ Uses Apache's REMOTE_USER environment variable as the external id. This in turn typically uses EduPersonPrincipalName @@ -384,29 +387,31 @@ def shib_login(request, retfun=None): """)) if not request.META.get('REMOTE_USER'): + log.exception("SHIB: no REMOTE_USER found in request.META") + return default_render_failure(request, shib_error_msg) + elif not request.META.get('Shib-Identity-Provider'): + log.exception("SHIB: no Shib-Identity-Provider in request.META") return default_render_failure(request, shib_error_msg) else: #if we get here, the user has authenticated properly - attrs = ['REMOTE_USER', 'givenName', 'sn', 'mail', - 'Shib-Identity-Provider'] - shib = {} - - for attr in attrs: - shib[attr] = request.META.get(attr, '') + shib = {attr: request.META.get(attr, '') + for attr in ['REMOTE_USER', 'givenName', 'sn', 'mail', 'Shib-Identity-Provider']} #Clean up first name, last name, and email address #TODO: Make this less hardcoded re: format, but split will work #even if ";" is not present since we are accessing 1st element - shib['sn'] = shib['sn'].split(";")[0].strip().capitalize() - shib['givenName'] = shib['givenName'].split(";")[0].strip().capitalize() + shib['sn'] = shib['sn'].split(";")[0].strip().capitalize().decode('utf-8') + shib['givenName'] = shib['givenName'].split(";")[0].strip().capitalize().decode('utf-8') + + log.info("SHIB creds returned: %r" % shib) return external_login_or_signup(request, external_id=shib['REMOTE_USER'], external_domain="shib:" + shib['Shib-Identity-Provider'], credentials=shib, email=shib['mail'], - fullname="%s %s" % (shib['givenName'], shib['sn']), - retfun=retfun) + fullname=u'%s %s' % (shib['givenName'], shib['sn']), + ) def make_shib_enrollment_request(request): diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 8bc29fa671..0aac873c03 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -599,7 +599,7 @@ def create_account(request, post_override=None): password = eamap.internal_password post_vars = dict(post_vars.items()) post_vars.update(dict(email=email, name=name, password=password)) - log.debug('extauth test: post_vars = %s' % post_vars) + log.info('In create_account with external_auth: post_vars = %s' % post_vars) # Confirm we have a properly formed request for a in ['username', 'email', 'password', 'name']: @@ -699,10 +699,11 @@ def create_account(request, post_override=None): eamap.user = login_user eamap.dtsignup = datetime.datetime.now(UTC) eamap.save() - log.debug('Updated ExternalAuthMap for %s to be %s' % (post_vars['username'], eamap)) + log.info("User registered with external_auth %s" % post_vars['username']) + log.info('Updated ExternalAuthMap for %s to be %s' % (post_vars['username'], eamap)) if settings.MITX_FEATURES.get('BYPASS_ACTIVATION_EMAIL_FOR_EXTAUTH'): - log.debug('bypassing activation email') + log.info('bypassing activation email') login_user.is_active = True login_user.save() From aa4e27f77534ad35bece457e097f5226bb95d29a Mon Sep 17 00:00:00 2001 From: Jason Bau <jbau@stanford.edu> Date: Thu, 20 Jun 2013 18:12:06 -0700 Subject: [PATCH 6/6] Shib PR responses to @cpennington and @ormsbee comments * Changed unicode test cases to ascii encoding * Removed 'stanford' hardcoding in TOS logic in lieu of 'SHIB_DISABLE_TOS' MIT_FEATURES flag * made 'external_auth' always an installed_app in lms * log.exception changd to log.error where appropriate But: did not change skipping tests to changing settings, for reasons stated here: https://github.com/edx/edx-platform/pull/67#issuecomment-19790330 --- .../external_auth/tests/test_shib.py | 5 ++-- common/djangoapps/external_auth/views.py | 10 +++++--- common/djangoapps/student/views.py | 25 ++++++++++--------- lms/envs/common.py | 8 ++++++ lms/envs/test.py | 4 +-- 5 files changed, 30 insertions(+), 22 deletions(-) diff --git a/common/djangoapps/external_auth/tests/test_shib.py b/common/djangoapps/external_auth/tests/test_shib.py index e5059e5635..eb05b59afb 100644 --- a/common/djangoapps/external_auth/tests/test_shib.py +++ b/common/djangoapps/external_auth/tests/test_shib.py @@ -1,4 +1,3 @@ -# coding=utf-8 """ Tests for Shibboleth Authentication @jbau @@ -36,8 +35,8 @@ from student.tests.factories import UserFactory IDP = 'https://idp.stanford.edu/' REMOTE_USER = 'test_user@stanford.edu' MAILS = [None, '', 'test_user@stanford.edu'] -GIVENNAMES = [None, '', 'Jason', 'jasön; John; bob'] # At Stanford, the givenNames can be a list delimited by ';' -SNS = [None, '', 'Bau', '包; smith'] # At Stanford, the sns can be a list delimited by ';' +GIVENNAMES = [None, '', 'Jason', 'jas\xc3\xb6n; John; bob'] # At Stanford, the givenNames can be a list delimited by ';' +SNS = [None, '', 'Bau', '\xe5\x8c\x85; smith'] # At Stanford, the sns can be a list delimited by ';' def gen_all_identities(): diff --git a/common/djangoapps/external_auth/views.py b/common/djangoapps/external_auth/views.py index 1ae8edfc52..93ab70debb 100644 --- a/common/djangoapps/external_auth/views.py +++ b/common/djangoapps/external_auth/views.py @@ -245,8 +245,10 @@ def signup(request, eamap=None): 'ask_for_tos': True, } - # Can't have terms of service for Stanford users, according to Stanford's Office of General Counsel - if settings.MITX_FEATURES['AUTH_USE_SHIB'] and ('stanford' in eamap.external_domain): + # Some openEdX instances can't have terms of service for shib users, like + # according to Stanford's Office of General Counsel + if settings.MITX_FEATURES.get('AUTH_USE_SHIB') and settings.MITX_FEATURES.get('SHIB_DISABLE_TOS') and \ + ('shib' in eamap.external_domain): context['ask_for_tos'] = False # detect if full name is blank and ask for it from user @@ -387,10 +389,10 @@ def shib_login(request): """)) if not request.META.get('REMOTE_USER'): - log.exception("SHIB: no REMOTE_USER found in request.META") + log.error("SHIB: no REMOTE_USER found in request.META") return default_render_failure(request, shib_error_msg) elif not request.META.get('Shib-Identity-Provider'): - log.exception("SHIB: no Shib-Identity-Provider in request.META") + log.error("SHIB: no Shib-Identity-Provider in request.META") return default_render_failure(request, shib_error_msg) else: #if we get here, the user has authenticated properly diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 0aac873c03..1a49789a32 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -48,6 +48,8 @@ from courseware.access import has_access from courseware.views import get_module_for_descriptor, jump_to from courseware.model_data import ModelDataCache +from external_auth.models import ExternalAuthMap + from statsd import statsd from pytz import UTC @@ -287,12 +289,10 @@ def dashboard(request): # get info w.r.t ExternalAuthMap external_auth_map = None - if 'external_auth' in settings.INSTALLED_APPS: - from external_auth.models import ExternalAuthMap - try: - external_auth_map = ExternalAuthMap.objects.get(user=user) - except ExternalAuthMap.DoesNotExist: - pass + try: + external_auth_map = ExternalAuthMap.objects.get(user=user) + except ExternalAuthMap.DoesNotExist: + pass context = {'courses': courses, 'message': message, @@ -613,10 +613,12 @@ def create_account(request, post_override=None): js['field'] = 'honor_code' return HttpResponse(json.dumps(js)) - # Can't have terms of service for Stanford users, according to Stanford's Office of General Counsel - if settings.MITX_FEATURES.get("AUTH_USE_SHIB") and DoExternalAuth and ("stanford" in eamap.external_domain): - pass - else: + # Can't have terms of service for certain SHIB users, like at Stanford + tos_not_required = settings.MITX_FEATURES.get("AUTH_USE_SHIB") \ + and settings.MITX_FEATURES.get('SHIB_DISABLE_TOS') \ + and DoExternalAuth and ("shib" in eamap.external_domain) + + if not tos_not_required: if post_vars.get('terms_of_service', 'false') != u'true': js['value'] = "You must accept the terms of service.".format(field=a) js['field'] = 'terms_of_service' @@ -629,8 +631,7 @@ def create_account(request, post_override=None): # TODO: Check password is sane required_post_vars = ['username', 'email', 'name', 'password', 'terms_of_service', 'honor_code'] - if settings.MITX_FEATURES.get("AUTH_USE_SHIB") and DoExternalAuth and ("stanford" in eamap.external_domain): - # Can't have terms of service for Stanford users, according to Stanford's Office of General Counsel + if tos_not_required: required_post_vars = ['username', 'email', 'name', 'password', 'honor_code'] for a in required_post_vars: diff --git a/lms/envs/common.py b/lms/envs/common.py index be570a9796..8a554f5bb9 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -92,6 +92,10 @@ MITX_FEATURES = { 'AUTH_USE_MIT_CERTIFICATES': False, 'AUTH_USE_OPENID_PROVIDER': False, 'AUTH_USE_SHIB': False, + + # This flag disables the requirement of having to agree to the TOS for users registering + # with Shib. Feature was requested by Stanford's office of general counsel + 'SHIB_DISABLE_TOS': False, # Enables ability to restrict enrollment in specific courses by the user account login method 'RESTRICT_ENROLL_BY_REG_METHOD': False, @@ -704,6 +708,10 @@ INSTALLED_APPS = ( 'licenses', 'course_groups', + # External auth (OpenID, shib) + 'external_auth', + 'django_openid_auth', + #For the wiki 'wiki', # The new django-wiki from benjaoming 'django_notify', diff --git a/lms/envs/test.py b/lms/envs/test.py index 3a6c641841..e9b683487e 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -139,6 +139,7 @@ MITX_FEATURES['AUTH_USE_OPENID_PROVIDER'] = True ################################## SHIB ####################################### MITX_FEATURES['AUTH_USE_SHIB'] = True +MITX_FEATURES['SHIB_DISABLE_TOS'] = True MITX_FEATURES['RESTRICT_ENROLL_BY_REG_METHOD'] = True OPENID_CREATE_USERS = False @@ -146,9 +147,6 @@ OPENID_UPDATE_DETAILS_FROM_SREG = True OPENID_USE_AS_ADMIN_LOGIN = False OPENID_PROVIDER_TRUSTED_ROOTS = ['*'] -INSTALLED_APPS += ('external_auth',) -INSTALLED_APPS += ('django_openid_auth',) - ################################# CELERY ###################################### CELERY_ALWAYS_EAGER = True