From 7ab0cb07308d54eba30a69c6dfccd1250e24c720 Mon Sep 17 00:00:00 2001 From: stephensanchez Date: Thu, 4 Dec 2014 15:14:14 +0000 Subject: [PATCH] 3rd-party pipeline supports updating email optin Fix all the 3rd party auth scenarios. Update pipeline to preserve parameters. Updating tests --- common/djangoapps/student/helpers.py | 7 ++- common/djangoapps/student/views.py | 4 +- .../djangoapps/third_party_auth/pipeline.py | 61 ++++++++++++++++--- .../djangoapps/third_party_auth/settings.py | 2 +- .../tests/test_change_enrollment.py | 40 ++++++++++-- lms/djangoapps/student_account/views.py | 7 ++- 6 files changed, 102 insertions(+), 19 deletions(-) diff --git a/common/djangoapps/student/helpers.py b/common/djangoapps/student/helpers.py index d874595af9..1aea3a6a4c 100644 --- a/common/djangoapps/student/helpers.py +++ b/common/djangoapps/student/helpers.py @@ -14,7 +14,7 @@ from third_party_auth import ( # pylint: disable=unused-import from verify_student.models import SoftwareSecurePhotoVerification # pylint: disable=F0401 -def auth_pipeline_urls(auth_entry, redirect_url=None, course_id=None): +def auth_pipeline_urls(auth_entry, redirect_url=None, course_id=None, email_opt_in=None): """Retrieve URLs for each enabled third-party auth provider. These URLs are used on the "sign up" and "sign in" buttons @@ -40,6 +40,10 @@ def auth_pipeline_urls(auth_entry, redirect_url=None, course_id=None): Note that `redirect_url` takes precedence over the redirect to the track selection page. + email_opt_in (unicode): The user choice to opt in for organization wide emails. If set to 'true' + (case insensitive), user will be opted into organization-wide email. All other values will + be treated as False, and the user will be opted out of organization-wide email. + Returns: dict mapping provider names to URLs @@ -72,6 +76,7 @@ def auth_pipeline_urls(auth_entry, redirect_url=None, course_id=None): provider.NAME: pipeline.get_login_url( provider.NAME, auth_entry, enroll_course_id=course_id, + email_opt_in=email_opt_in, redirect_url=pipeline_redirect ) for provider in provider.Registry.enabled() diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 591853fa04..12995d3894 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -373,7 +373,7 @@ def signin_user(request): # party auth pipeline; distinct from the actual instance of the running # pipeline, if any. 'pipeline_running': 'true' if pipeline.running(request) else 'false', - 'pipeline_url': auth_pipeline_urls(pipeline.AUTH_ENTRY_LOGIN, course_id=course_id), + 'pipeline_url': auth_pipeline_urls(pipeline.AUTH_ENTRY_LOGIN, course_id=course_id, email_opt_in=email_opt_in), 'platform_name': microsite.get_value( 'platform_name', settings.PLATFORM_NAME @@ -405,7 +405,7 @@ def register_user(request, extra_context=None): 'enrollment_action': request.GET.get('enrollment_action'), 'name': '', 'running_pipeline': None, - 'pipeline_urls': auth_pipeline_urls(pipeline.AUTH_ENTRY_REGISTER, course_id=course_id), + 'pipeline_urls': auth_pipeline_urls(pipeline.AUTH_ENTRY_REGISTER, course_id=course_id, email_opt_in=email_opt_in), 'platform_name': microsite.get_value( 'platform_name', settings.PLATFORM_NAME diff --git a/common/djangoapps/third_party_auth/pipeline.py b/common/djangoapps/third_party_auth/pipeline.py index 4564f5533a..f8eb13511f 100644 --- a/common/djangoapps/third_party_auth/pipeline.py +++ b/common/djangoapps/third_party_auth/pipeline.py @@ -101,10 +101,12 @@ from . import provider # `AUTH_ENROLL_COURSE_ID_KEY` provides the course ID that a student # is trying to enroll in, used to generate analytics events # and auto-enroll students. +from user_api.api import profile AUTH_ENTRY_KEY = 'auth_entry' AUTH_REDIRECT_KEY = 'next' AUTH_ENROLL_COURSE_ID_KEY = 'enroll_course_id' +AUTH_EMAIL_OPT_IN_KEY = 'email_opt_in' AUTH_ENTRY_DASHBOARD = 'dashboard' AUTH_ENTRY_LOGIN = 'login' @@ -250,7 +252,7 @@ def _get_enabled_provider_by_name(provider_name): return enabled_provider -def _get_url(view_name, backend_name, auth_entry=None, redirect_url=None, enroll_course_id=None): +def _get_url(view_name, backend_name, auth_entry=None, redirect_url=None, enroll_course_id=None, email_opt_in=None): """Creates a URL to hook into social auth endpoints.""" kwargs = {'backend': backend_name} url = reverse(view_name, kwargs=kwargs) @@ -265,6 +267,9 @@ def _get_url(view_name, backend_name, auth_entry=None, redirect_url=None, enroll if enroll_course_id: query_params[AUTH_ENROLL_COURSE_ID_KEY] = enroll_course_id + if email_opt_in: + query_params[AUTH_EMAIL_OPT_IN_KEY] = email_opt_in + return u"{url}?{params}".format( url=url, params=urllib.urlencode(query_params) @@ -309,7 +314,7 @@ def get_disconnect_url(provider_name): return _get_url('social:disconnect', enabled_provider.BACKEND_CLASS.name) -def get_login_url(provider_name, auth_entry, redirect_url=None, enroll_course_id=None): +def get_login_url(provider_name, auth_entry, redirect_url=None, enroll_course_id=None, email_opt_in=None): """Gets the login URL for the endpoint that kicks off auth with a provider. Args: @@ -326,6 +331,11 @@ def get_login_url(provider_name, auth_entry, redirect_url=None, enroll_course_id enroll_course_id (string): If provided, auto-enroll the user in this course upon successful authentication. + email_opt_in (string): If set to 'true' (case insensitive), user will + be opted into organization-wide email. Any other string will + equate to False, and the user will be opted out of organization-wide + email. + Returns: String. URL that starts the auth pipeline for a provider. @@ -339,7 +349,8 @@ def get_login_url(provider_name, auth_entry, redirect_url=None, enroll_course_id enabled_provider.BACKEND_CLASS.name, auth_entry=auth_entry, redirect_url=redirect_url, - enroll_course_id=enroll_course_id + enroll_course_id=enroll_course_id, + email_opt_in=email_opt_in ) @@ -425,7 +436,6 @@ def running(request): def parse_query_params(strategy, response, *args, **kwargs): """Reads whitelisted query params, transforms them into pipeline args.""" auth_entry = strategy.session.get(AUTH_ENTRY_KEY) - if not (auth_entry and auth_entry in _AUTH_ENTRY_CHOICES): raise AuthEntryError(strategy.backend, 'auth_entry missing or invalid') @@ -499,7 +509,6 @@ def ensure_user_information( user_unset = user is None dispatch_to_login = is_login and (user_unset or user_inactive) reject_api_request = is_api and (user_unset or user_inactive) - if reject_api_request: # Content doesn't matter; we just want to exit the pipeline return HttpResponseBadRequest() @@ -512,20 +521,49 @@ def ensure_user_information( return if dispatch_to_login: - return redirect(AUTH_DISPATCH_URLS[AUTH_ENTRY_LOGIN], name='signin_user') + return redirect(_create_redirect_url(AUTH_DISPATCH_URLS[AUTH_ENTRY_LOGIN], strategy)) # TODO (ECOM-369): Consolidate this with `dispatch_to_login` # once the A/B test completes. # pylint: disable=fixme if dispatch_to_login_2: - return redirect(AUTH_DISPATCH_URLS[AUTH_ENTRY_LOGIN_2]) + return redirect(_create_redirect_url(AUTH_DISPATCH_URLS[AUTH_ENTRY_LOGIN_2], strategy)) if is_register and user_unset: - return redirect(AUTH_DISPATCH_URLS[AUTH_ENTRY_REGISTER], name='register_user') + return redirect(_create_redirect_url(AUTH_DISPATCH_URLS[AUTH_ENTRY_REGISTER], strategy)) # TODO (ECOM-369): Consolidate this with `is_register` # once the A/B test completes. # pylint: disable=fixme if is_register_2 and user_unset: - return redirect(AUTH_DISPATCH_URLS[AUTH_ENTRY_REGISTER_2]) + return redirect(_create_redirect_url(AUTH_DISPATCH_URLS[AUTH_ENTRY_REGISTER_2], strategy)) + + +def _create_redirect_url(url, strategy): + """ Given a URL and a Strategy, construct the appropriate redirect URL. + + Construct a redirect URL and append the URL parameters that should be preserved. + + Args: + url (string): The base URL to use for the redirect. + strategy (Strategy): Used to determine which URL parameters to append to the redirect. + + Returns: + A string representation of the URL, with parameters, for redirect. + """ + url_params = {} + enroll_course_id = strategy.session_get(AUTH_ENROLL_COURSE_ID_KEY) + if enroll_course_id: + url_params['course_id'] = enroll_course_id + url_params['enrollment_action'] = 'enroll' + email_opt_in = strategy.session_get(AUTH_EMAIL_OPT_IN_KEY) + if email_opt_in: + url_params[AUTH_EMAIL_OPT_IN_KEY] = email_opt_in + if url_params: + return u'{url}?{params}'.format( + url=url, + params=urllib.urlencode(url_params) + ) + else: + return url @partial.partial @@ -639,6 +677,11 @@ def change_enrollment(strategy, user=None, *args, **kwargs): if enroll_course_id: course_id = CourseKey.from_string(enroll_course_id) modes = CourseMode.modes_for_course_dict(course_id) + # If the email opt in parameter is found, set the preference. + email_opt_in = strategy.session_get(AUTH_EMAIL_OPT_IN_KEY) + if email_opt_in: + opt_in = email_opt_in.lower() == 'true' + profile.update_email_opt_in(user.username, course_id.org, opt_in) if CourseMode.can_auto_enroll(course_id, modes_dict=modes): try: CourseEnrollment.enroll(user, course_id, check_access=True) diff --git a/common/djangoapps/third_party_auth/settings.py b/common/djangoapps/third_party_auth/settings.py index a94cb10da3..7244f05bfe 100644 --- a/common/djangoapps/third_party_auth/settings.py +++ b/common/djangoapps/third_party_auth/settings.py @@ -46,7 +46,7 @@ If true, it: from . import provider -_FIELDS_STORED_IN_SESSION = ['auth_entry', 'next', 'enroll_course_id'] +_FIELDS_STORED_IN_SESSION = ['auth_entry', 'next', 'enroll_course_id', 'email_opt_in'] _MIDDLEWARE_CLASSES = ( 'third_party_auth.middleware.ExceptionMiddleware', ) diff --git a/common/djangoapps/third_party_auth/tests/test_change_enrollment.py b/common/djangoapps/third_party_auth/tests/test_change_enrollment.py index f85ae38d48..c067b93b4e 100644 --- a/common/djangoapps/third_party_auth/tests/test_change_enrollment.py +++ b/common/djangoapps/third_party_auth/tests/test_change_enrollment.py @@ -1,4 +1,6 @@ +# -*- coding: utf-8 -*- """Tests for the change enrollment step of the pipeline. """ +from collections import namedtuple import datetime import unittest @@ -17,6 +19,7 @@ from student.models import CourseEnrollment from xmodule.modulestore.tests.django_utils import ( ModuleStoreTestCase, mixed_store_config ) +from user_api.models import UserOrgTag MODULESTORE_CONFIG = mixed_store_config(settings.COMMON_TEST_DATA_ROOT, {}, include_xml=False) @@ -42,12 +45,12 @@ class PipelineEnrollmentTest(ModuleStoreTestCase): self.user = UserFactory.create() @ddt.data( - ([], "honor"), - (["honor", "verified", "audit"], "honor"), - (["professional"], None) + ([], "honor", u"False", u"False"), + (["honor", "verified", "audit"], "honor", u"True", u"True"), + (["professional"], None, u"Fålsœ", u"False") ) @ddt.unpack - def test_auto_enroll_step(self, course_modes, enrollment_mode): + def test_auto_enroll_step(self, course_modes, enrollment_mode, email_opt_in, email_opt_in_result): # Create the course modes for the test case for mode_slug in course_modes: CourseModeFactory.create( @@ -61,6 +64,7 @@ class PipelineEnrollmentTest(ModuleStoreTestCase): # when they started the auth process. strategy = self._fake_strategy() strategy.session_set('enroll_course_id', unicode(self.course.id)) + strategy.session_set('email_opt_in', email_opt_in) result = pipeline.change_enrollment(strategy, 1, user=self.user) # pylint: disable=assignment-from-no-return,redundant-keyword-arg self.assertEqual(result, {}) @@ -74,6 +78,11 @@ class PipelineEnrollmentTest(ModuleStoreTestCase): else: self.assertFalse(CourseEnrollment.is_enrolled(self.user, self.course.id)) + # Check that the Email Opt In option was set + tag = UserOrgTag.objects.get(user=self.user) + self.assertIsNotNone(tag) + self.assertEquals(tag.value, email_opt_in_result) + def test_add_white_label_to_cart(self): # Create a white label course (honor with a minimum price) CourseModeFactory.create( @@ -121,6 +130,29 @@ class PipelineEnrollmentTest(ModuleStoreTestCase): self.assertEqual(result, {}) self.assertFalse(CourseEnrollment.is_enrolled(self.user, self.course.id)) + def test_url_creation(self): + strategy = self._fake_strategy() + strategy.session_set('enroll_course_id', unicode(self.course.id)) + strategy.session_set('email_opt_in', u"False") + backend = namedtuple('backend', 'name') + backend.name = self.BACKEND_NAME + response = pipeline.ensure_user_information( + strategy=strategy, + pipeline_index=1, + details=None, + response=None, + uid=None, + is_register=True, + backend=backend + ) + self.assertIsNotNone(response) + self.assertEquals(response.status_code, 302) + + # Get the location + _, url = response._headers['location'] # pylint: disable=W0212 + self.assertIn("email_opt_in=False", url) + self.assertIn("course_id=".format(id=unicode(self.course.id)), url) + def _fake_strategy(self): """Simulate the strategy passed to the pipeline step. """ request = RequestFactory().get(pipeline.get_complete_url(self.BACKEND_NAME)) diff --git a/lms/djangoapps/student_account/views.py b/lms/djangoapps/student_account/views.py index 5239f8f27a..a67a446785 100644 --- a/lms/djangoapps/student_account/views.py +++ b/lms/djangoapps/student_account/views.py @@ -285,13 +285,16 @@ def _third_party_auth_context(request): } course_id = request.GET.get("course_id") + email_opt_in = request.GET.get('email_opt_in') login_urls = auth_pipeline_urls( third_party_auth.pipeline.AUTH_ENTRY_LOGIN_2, - course_id=course_id + course_id=course_id, + email_opt_in=email_opt_in ) register_urls = auth_pipeline_urls( third_party_auth.pipeline.AUTH_ENTRY_REGISTER_2, - course_id=course_id + course_id=course_id, + email_opt_in=email_opt_in ) if third_party_auth.is_enabled():