From 05cb2a071bf969840af7a760bcac3ea79a55a1fe Mon Sep 17 00:00:00 2001 From: Renzo Lucioni Date: Fri, 24 Oct 2014 14:25:28 -0400 Subject: [PATCH 1/2] Prohibit automatic enrollment in prof ed courses when user authenticates via third-party auth --- .../student/tests/test_enrollment.py | 20 ++++++++++++++ .../djangoapps/third_party_auth/pipeline.py | 27 ++++++++++--------- 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/common/djangoapps/student/tests/test_enrollment.py b/common/djangoapps/student/tests/test_enrollment.py index 818c9bd9b6..9c994570a9 100644 --- a/common/djangoapps/student/tests/test_enrollment.py +++ b/common/djangoapps/student/tests/test_enrollment.py @@ -116,6 +116,26 @@ class EnrollmentTest(ModuleStoreTestCase): change_enrollment_third_party(is_register=True, strategy=strategy, user=self.user) self.assertTrue(CourseEnrollment.is_enrolled(self.user, self.course.id)) + def test_no_prof_ed_third_party_autoenroll(self): + """ + Test that a user authenticating via third party auth while attempting to enroll + in a professional education course is not automatically enrolled in the course. + """ + self.client.logout() + + # Create the course mode required for this test case + CourseModeFactory(course_id=self.course.id, mode_slug='professional') + + self.client.get(reverse('register_user'), {'course_id': self.course.id}) + self.client.login(username=self.USERNAME, password=self.PASSWORD) + self.dummy_request = RequestFactory().request() + self.dummy_request.session = self.client.session + strategy = DjangoStrategy(RequestFactory, request=self.dummy_request) + change_enrollment_third_party(is_register=True, strategy=strategy, user=self.user) + + # Verify that the user has not been enrolled in the course + self.assertFalse(CourseEnrollment.is_enrolled(self.user, self.course.id)) + def test_unenroll(self): # Enroll the student in the course CourseEnrollment.enroll(self.user, self.course.id, mode="honor") diff --git a/common/djangoapps/third_party_auth/pipeline.py b/common/djangoapps/third_party_auth/pipeline.py index c76e5199bb..d72b590bb7 100644 --- a/common/djangoapps/third_party_auth/pipeline.py +++ b/common/djangoapps/third_party_auth/pipeline.py @@ -69,8 +69,8 @@ from social.apps.django_app.default import models from social.exceptions import AuthException from social.pipeline import partial -from student.models import CourseEnrollment, CourseEnrollmentException -from opaque_keys.edx.locations import SlashSeparatedCourseKey +from student.models import CourseMode, CourseEnrollment, CourseEnrollmentException +from opaque_keys.edx.keys import CourseKey from logging import getLogger @@ -420,14 +420,15 @@ def change_enrollment(*args, **kwargs): a course, we automatically log them into that course. """ if kwargs['strategy'].session_get('registration_course_id'): - try: - CourseEnrollment.enroll( - kwargs['user'], - SlashSeparatedCourseKey.from_deprecated_string( - kwargs['strategy'].session_get('registration_course_id') - ) - ) - except CourseEnrollmentException: - pass - except Exception, e: - logger.exception(e) + course_id = CourseKey.from_string( + kwargs['strategy'].session_get('registration_course_id') + ) + available_modes = CourseMode.modes_for_course_dict(course_id) + + if 'honor' in available_modes: + try: + CourseEnrollment.enroll(kwargs['user'], course_id) + except CourseEnrollmentException: + pass + except Exception, e: + logger.exception(e) From 0cac6ea21bebaa691916ce5066746abf349385b1 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Mon, 27 Oct 2014 11:04:22 -0400 Subject: [PATCH 2/2] Fix pylint violations --- common/djangoapps/student/tests/test_enrollment.py | 14 +++++++------- common/djangoapps/third_party_auth/pipeline.py | 6 +++--- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/common/djangoapps/student/tests/test_enrollment.py b/common/djangoapps/student/tests/test_enrollment.py index 9c994570a9..9565fc5a23 100644 --- a/common/djangoapps/student/tests/test_enrollment.py +++ b/common/djangoapps/student/tests/test_enrollment.py @@ -59,7 +59,7 @@ class EnrollmentTest(ModuleStoreTestCase): # Professional ed # Expect that we're sent to the "choose your track" page # (which will, in turn, redirect us to a page where we can verify/pay) - # We should NOT be auto-enrolled, because that would be giving + # We should NOT be auto-enrolled, because that would be giving # away an expensive course for free :) (['professional'], 'course_modes_choose', None), ) @@ -110,9 +110,9 @@ class EnrollmentTest(ModuleStoreTestCase): self.client.logout() self.client.get(reverse('register_user'), {'course_id': self.course.id}) self.client.login(username=self.USERNAME, password=self.PASSWORD) - self.dummy_request = RequestFactory().request() - self.dummy_request.session = self.client.session - strategy = DjangoStrategy(RequestFactory, request=self.dummy_request) + dummy_request = RequestFactory().request() + dummy_request.session = self.client.session + strategy = DjangoStrategy(RequestFactory, request=dummy_request) change_enrollment_third_party(is_register=True, strategy=strategy, user=self.user) self.assertTrue(CourseEnrollment.is_enrolled(self.user, self.course.id)) @@ -128,9 +128,9 @@ class EnrollmentTest(ModuleStoreTestCase): self.client.get(reverse('register_user'), {'course_id': self.course.id}) self.client.login(username=self.USERNAME, password=self.PASSWORD) - self.dummy_request = RequestFactory().request() - self.dummy_request.session = self.client.session - strategy = DjangoStrategy(RequestFactory, request=self.dummy_request) + dummy_request = RequestFactory().request() + dummy_request.session = self.client.session + strategy = DjangoStrategy(RequestFactory, request=dummy_request) change_enrollment_third_party(is_register=True, strategy=strategy, user=self.user) # Verify that the user has not been enrolled in the course diff --git a/common/djangoapps/third_party_auth/pipeline.py b/common/djangoapps/third_party_auth/pipeline.py index d72b590bb7..a7ce75aae5 100644 --- a/common/djangoapps/third_party_auth/pipeline.py +++ b/common/djangoapps/third_party_auth/pipeline.py @@ -408,7 +408,7 @@ def login_analytics(*args, **kwargs): }, context={ 'Google Analytics': { - 'clientId': tracking_context.get('client_id') + 'clientId': tracking_context.get('client_id') } } ) @@ -430,5 +430,5 @@ def change_enrollment(*args, **kwargs): CourseEnrollment.enroll(kwargs['user'], course_id) except CourseEnrollmentException: pass - except Exception, e: - logger.exception(e) + except Exception as ex: + logger.exception(ex)