From aa4e27f77534ad35bece457e097f5226bb95d29a Mon Sep 17 00:00:00 2001 From: Jason Bau Date: Thu, 20 Jun 2013 18:12:06 -0700 Subject: [PATCH] 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