From e1fcac93c5925deb38f692e7c43472a5c53ea146 Mon Sep 17 00:00:00 2001 From: Jason Bau Date: Wed, 16 Jul 2014 12:07:25 -0400 Subject: [PATCH] fix shib reg from course about page --- cms/envs/common.py | 4 ++ common/djangoapps/external_auth/views.py | 4 +- lms/djangoapps/courseware/tests/test_about.py | 43 +++++++++++++++++++ lms/djangoapps/courseware/views.py | 4 ++ lms/djangoapps/instructor/enrollment.py | 5 +-- lms/envs/common.py | 4 ++ lms/templates/courseware/course_about.html | 11 +++-- 7 files changed, 66 insertions(+), 9 deletions(-) diff --git a/cms/envs/common.py b/cms/envs/common.py index a9be299409..f605d4296a 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -638,3 +638,7 @@ for app_name in OPTIONAL_APPS: ### ADVANCED_SECURITY_CONFIG # Empty by default ADVANCED_SECURITY_CONFIG = {} + +### External auth usage -- prefixes for ENROLLMENT_DOMAIN +SHIBBOLETH_DOMAIN_PREFIX = 'shib:' +OPENID_DOMAIN_PREFIX = 'openid:' diff --git a/common/djangoapps/external_auth/views.py b/common/djangoapps/external_auth/views.py index a9e62147c7..b958f43e9c 100644 --- a/common/djangoapps/external_auth/views.py +++ b/common/djangoapps/external_auth/views.py @@ -53,8 +53,8 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey log = logging.getLogger("edx.external_auth") AUDIT_LOG = logging.getLogger("audit") -SHIBBOLETH_DOMAIN_PREFIX = 'shib:' -OPENID_DOMAIN_PREFIX = 'openid:' +SHIBBOLETH_DOMAIN_PREFIX = settings.SHIBBOLETH_DOMAIN_PREFIX +OPENID_DOMAIN_PREFIX = settings.OPENID_DOMAIN_PREFIX # ----------------------------------------------------------------------------- # OpenID Common diff --git a/lms/djangoapps/courseware/tests/test_about.py b/lms/djangoapps/courseware/tests/test_about.py index 824890e348..44bb1e2b87 100644 --- a/lms/djangoapps/courseware/tests/test_about.py +++ b/lms/djangoapps/courseware/tests/test_about.py @@ -2,10 +2,12 @@ Test the about xblock """ import mock +from mock import patch import pytz import datetime from django.test.utils import override_settings from django.core.urlresolvers import reverse +from django.conf import settings from .helpers import LoginEnrollmentTestCase from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase @@ -16,6 +18,7 @@ from student.tests.factories import UserFactory, CourseEnrollmentAllowedFactory # HTML for registration button REG_STR = "
" +SHIB_ERROR_STR = "The currently logged-in user account does not have permission to enroll in this course." @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) @@ -167,6 +170,46 @@ class AboutWithInvitationOnly(ModuleStoreTestCase): self.assertIn(REG_STR, resp.content) +@patch.dict(settings.FEATURES, {'RESTRICT_ENROLL_BY_REG_METHOD': True}) +@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) +class AboutTestCaseShibCourse(LoginEnrollmentTestCase, ModuleStoreTestCase): + """ + Test cases covering about page behavior for courses that use shib enrollment domain ("shib courses") + """ + def setUp(self): + self.course = CourseFactory.create(enrollment_domain="shib:https://idp.stanford.edu/") + + self.about = ItemFactory.create( + category="about", parent_location=self.course.location, + data="OOGIE BLOOGIE", display_name="overview" + ) + + def test_logged_in_shib_course(self): + """ + For shib courses, logged in users will see the register button, but get rejected once they click there + """ + self.setup_user() + url = reverse('about_course', args=[self.course.id.to_deprecated_string()]) + resp = self.client.get(url) + self.assertEqual(resp.status_code, 200) + self.assertIn("OOGIE BLOOGIE", resp.content) + self.assertIn("Register for 999", resp.content) + self.assertIn(SHIB_ERROR_STR, resp.content) + self.assertIn(REG_STR, resp.content) + + def test_anonymous_user_shib_course(self): + """ + For shib courses, anonymous users will also see the register button + """ + url = reverse('about_course', args=[self.course.id.to_deprecated_string()]) + resp = self.client.get(url) + self.assertEqual(resp.status_code, 200) + self.assertIn("OOGIE BLOOGIE", resp.content) + self.assertIn("Register for 999", resp.content) + self.assertIn(SHIB_ERROR_STR, resp.content) + self.assertIn(REG_STR, resp.content) + + @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) class AboutWithClosedEnrollment(ModuleStoreTestCase): """ diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index af5015c1d2..3c501f63ab 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -48,6 +48,7 @@ from opaque_keys import InvalidKeyError from microsite_configuration import microsite from opaque_keys.edx.locations import SlashSeparatedCourseKey +from instructor.enrollment import uses_shib log = logging.getLogger("edx.courseware") @@ -655,6 +656,8 @@ def course_about(request, course_id): # - Student cannot enroll in course active_reg_button = not(registered or is_course_full or not can_enroll) + is_shib_course = uses_shib(course) + return render_to_response('courseware/course_about.html', { 'course': course, 'staff_access': staff_access, @@ -669,6 +672,7 @@ def course_about(request, course_id): 'can_enroll': can_enroll, 'invitation_only': invitation_only, 'active_reg_button': active_reg_button, + 'is_shib_course': is_shib_course, }) diff --git a/lms/djangoapps/instructor/enrollment.py b/lms/djangoapps/instructor/enrollment.py index dc29c7ca0c..7e6ac0f2f1 100644 --- a/lms/djangoapps/instructor/enrollment.py +++ b/lms/djangoapps/instructor/enrollment.py @@ -19,9 +19,6 @@ from student.models import anonymous_id_for_user from microsite_configuration import microsite -# For determining if a shibboleth course -SHIBBOLETH_DOMAIN_PREFIX = 'shib:' - class EmailEnrollmentState(object): """ Store the complete enrollment state of an email in a class """ @@ -361,4 +358,4 @@ def uses_shib(course): Returns a boolean indicating if Shibboleth authentication is set for this course. """ - return course.enrollment_domain and course.enrollment_domain.startswith(SHIBBOLETH_DOMAIN_PREFIX) + return course.enrollment_domain and course.enrollment_domain.startswith(settings.SHIBBOLETH_DOMAIN_PREFIX) diff --git a/lms/envs/common.py b/lms/envs/common.py index d1d764341d..e50fe6fb77 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1649,3 +1649,7 @@ THIRD_PARTY_AUTH = {} ### ADVANCED_SECURITY_CONFIG # Empty by default ADVANCED_SECURITY_CONFIG = {} + +### External auth usage -- prefixes for ENROLLMENT_DOMAIN +SHIBBOLETH_DOMAIN_PREFIX = 'shib:' +OPENID_DOMAIN_PREFIX = 'openid:' diff --git a/lms/templates/courseware/course_about.html b/lms/templates/courseware/course_about.html index 896342d793..31a60ba1db 100644 --- a/lms/templates/courseware/course_about.html +++ b/lms/templates/courseware/course_about.html @@ -14,7 +14,6 @@ <%! from microsite_configuration import microsite %> <%inherit file="../main.html" /> - <%block name="headextra"> <% @@ -173,7 +172,10 @@ % elif invitation_only and not can_enroll: ${_("Enrollment in this course is by invitation only")} - % elif not can_enroll: + ## Shib courses need the enrollment button to be displayed even when can_enroll is False, + ## because AnonymousUsers cause can_enroll for shib courses to be False, but we need them to be able to click + ## so that they can register and become a real user that can enroll. + % elif not is_shib_course and not can_enroll: ${_("Enrollment is Closed")} %else: @@ -339,7 +341,10 @@ -%if active_reg_button: +## Need to put this hidden form on the page so that the registration button works. +## Since it's no harm to display a hidden form, we display it with the most permissive conditional +## which is when the student is not registered. +%if active_reg_button or is_shib_course: