From 358e73c0828fbd6381b7d9c30055d540e2693a0b Mon Sep 17 00:00:00 2001 From: Jesse Shapiro Date: Wed, 8 Feb 2017 12:31:38 -0500 Subject: [PATCH] Remove data sharing consent from logistration --- common/djangoapps/student/forms.py | 13 -- common/djangoapps/student/views.py | 8 -- common/djangoapps/util/enterprise_helpers.py | 78 ------------ .../util/tests/test_enterprise_helpers.py | 111 ------------------ .../djangoapps/user_api/tests/test_views.py | 37 ------ openedx/core/djangoapps/user_api/views.py | 4 - 6 files changed, 251 deletions(-) diff --git a/common/djangoapps/student/forms.py b/common/djangoapps/student/forms.py index 2b0acbe783..ff1511bd9a 100644 --- a/common/djangoapps/student/forms.py +++ b/common/djangoapps/student/forms.py @@ -188,19 +188,6 @@ class AccountCreationForm(forms.Form): "required": _("To enroll, you must follow the honor code.") } ) - elif field_name == 'data_sharing_consent': - if field_value == "required": - self.fields[field_name] = TrueField( - error_messages={ - "required": _( - "You must consent to data sharing to register." - ) - } - ) - elif field_value == 'optional': - self.fields[field_name] = forms.BooleanField( - required=False - ) else: required = field_value == "required" min_length = 1 if field_name in ("gender", "level_of_education") else 2 diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 9d8d8efc63..7552387267 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -46,7 +46,6 @@ from social.exceptions import AuthException, AuthAlreadyAssociated from edxmako.shortcuts import render_to_response, render_to_string -from util.enterprise_helpers import data_sharing_consent_requirement_at_login from course_modes.models import CourseMode from shoppingcart.api import order_history from student.models import ( @@ -1667,10 +1666,6 @@ def create_account_with_params(request, params): if should_link_with_social_auth or (third_party_auth.is_enabled() and pipeline.running(request)): params["password"] = pipeline.make_random_password() - # Add a form requirement for data sharing consent if the EnterpriseCustomer - # for the request requires it at login - extra_fields['data_sharing_consent'] = data_sharing_consent_requirement_at_login(request) - # 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 @@ -1767,9 +1762,6 @@ def create_account_with_params(request, params): if third_party_auth.is_enabled() and pipeline.running(request): running_pipeline = pipeline.get(request) third_party_provider = provider.Registry.get_from_pipeline(running_pipeline) - # Store received data sharing consent field values in the pipeline for use - # by any downstream pipeline elements which require them. - running_pipeline['kwargs']['data_sharing_consent'] = form.cleaned_data.get('data_sharing_consent', None) # Track the user's registration if hasattr(settings, 'LMS_SEGMENT_KEY') and settings.LMS_SEGMENT_KEY: diff --git a/common/djangoapps/util/enterprise_helpers.py b/common/djangoapps/util/enterprise_helpers.py index 9117b09f4e..cfc5688da6 100644 --- a/common/djangoapps/util/enterprise_helpers.py +++ b/common/djangoapps/util/enterprise_helpers.py @@ -29,84 +29,6 @@ def enterprise_enabled(): return 'enterprise' in settings.INSTALLED_APPS -def data_sharing_consent_requested(request): - """ - Determine if the EnterpriseCustomer for a given HTTP request - requests data sharing consent - """ - if not enterprise_enabled(): - return False - return active_provider_requests_data_sharing(request) - - -def data_sharing_consent_required_at_login(request): - """ - Determines if data sharing consent is required at - a given location - """ - if not enterprise_enabled(): - return False - return active_provider_enforces_data_sharing(request, EnterpriseCustomer.AT_LOGIN) - - -def data_sharing_consent_requirement_at_login(request): - """ - Returns either 'optional' or 'required' based on where we are. - """ - if not enterprise_enabled(): - return None - if data_sharing_consent_required_at_login(request): - return 'required' - if data_sharing_consent_requested(request): - return 'optional' - return None - - -def insert_enterprise_fields(request, form_desc): - """ - Enterprise methods which modify the logistration form are called from this method. - """ - if not enterprise_enabled(): - return - add_data_sharing_consent_field(request, form_desc) - - -def add_data_sharing_consent_field(request, form_desc): - """ - Adds a checkbox field to be selected if the user consents to share data with - the EnterpriseCustomer attached to the SSO provider with which they're authenticating. - """ - enterprise_customer = get_enterprise_customer_for_request(request) - required = data_sharing_consent_required_at_login(request) - - if not data_sharing_consent_requested(request): - return - - label = _( - "I agree to allow {platform_name} to share data about my enrollment, " - "completion and performance in all {platform_name} courses and programs " - "where my enrollment is sponsored by {ec_name}." - ).format( - platform_name=configuration_helpers.get_value("PLATFORM_NAME", settings.PLATFORM_NAME), - ec_name=enterprise_customer.name - ) - - error_msg = _( - "To link your account with {ec_name}, you are required to consent to data sharing." - ).format( - ec_name=enterprise_customer.name - ) - - form_desc.add_field( - "data_sharing_consent", - label=label, - field_type="checkbox", - default=False, - required=required, - error_messages={"required": error_msg}, - ) - - def insert_enterprise_pipeline_elements(pipeline): """ If the enterprise app is enabled, insert additional elements into the diff --git a/common/djangoapps/util/tests/test_enterprise_helpers.py b/common/djangoapps/util/tests/test_enterprise_helpers.py index db76bf2b66..d5a596bdd2 100644 --- a/common/djangoapps/util/tests/test_enterprise_helpers.py +++ b/common/djangoapps/util/tests/test_enterprise_helpers.py @@ -8,10 +8,6 @@ import mock from util.enterprise_helpers import ( enterprise_enabled, - data_sharing_consent_requested, - data_sharing_consent_required_at_login, - data_sharing_consent_requirement_at_login, - insert_enterprise_fields, insert_enterprise_pipeline_elements, set_enterprise_branding_filter_param, get_enterprise_branding_filter_param, @@ -32,10 +28,6 @@ class TestEnterpriseHelpers(unittest.TestCase): the utilities to return the expected default values. """ mock_enterprise_enabled.return_value = False - self.assertFalse(data_sharing_consent_requested(None)) - self.assertFalse(data_sharing_consent_required_at_login(None)) - self.assertEqual(data_sharing_consent_requirement_at_login(None), None) - self.assertEqual(insert_enterprise_fields(None, None), None) self.assertEqual(insert_enterprise_pipeline_elements(None), None) def test_enterprise_enabled(self): @@ -45,109 +37,6 @@ class TestEnterpriseHelpers(unittest.TestCase): """ self.assertTrue(enterprise_enabled()) - @mock.patch('enterprise.tpa_pipeline.get_enterprise_customer_for_request') - def test_data_sharing_consent_requested(self, mock_get_ec): - """ - Test that we correctly check whether data sharing consent is requested. - """ - request = mock.MagicMock(session={'partial_pipeline': 'thing'}) - mock_get_ec.return_value = mock.MagicMock(requests_data_sharing_consent=True) - self.assertTrue(data_sharing_consent_requested(request)) - mock_get_ec.return_value = mock.MagicMock(requests_data_sharing_consent=False) - self.assertFalse(data_sharing_consent_requested(request)) - mock_get_ec.return_value = None - self.assertFalse(data_sharing_consent_requested(request)) - request = mock.MagicMock(session={}) - self.assertFalse(data_sharing_consent_requested(request)) - - @mock.patch('enterprise.tpa_pipeline.get_enterprise_customer_for_request') - def test_data_sharing_consent_required(self, mock_get_ec): - """ - Test that we correctly check whether data sharing consent is required at login. - """ - check_method = mock.MagicMock(return_value=True) - request = mock.MagicMock(session={'partial_pipeline': 'thing'}) - mock_get_ec.return_value = mock.MagicMock(enforces_data_sharing_consent=check_method) - self.assertTrue(data_sharing_consent_required_at_login(request)) - check_method.return_value = False - mock_get_ec.return_value = mock.MagicMock(enforces_data_sharing_consent=check_method) - self.assertFalse(data_sharing_consent_required_at_login(request)) - mock_get_ec.return_value = None - self.assertFalse(data_sharing_consent_required_at_login(request)) - request = mock.MagicMock(session={}) - self.assertFalse(data_sharing_consent_required_at_login(request)) - - @mock.patch('enterprise.tpa_pipeline.get_enterprise_customer_for_request') - def test_data_sharing_consent_requirement(self, mock_get_ec): - """ - Test that we get the correct requirement string for the current consent statae. - """ - request = mock.MagicMock(session={'partial_pipeline': 'thing'}) - mock_ec = mock.MagicMock( - enforces_data_sharing_consent=mock.MagicMock(return_value=True), - requests_data_sharing_consent=True, - ) - mock_get_ec.return_value = mock_ec - self.assertEqual(data_sharing_consent_requirement_at_login(request), 'required') - mock_ec.enforces_data_sharing_consent.return_value = False - self.assertEqual(data_sharing_consent_requirement_at_login(request), 'optional') - mock_ec.requests_data_sharing_consent = False - self.assertEqual(data_sharing_consent_requirement_at_login(request), None) - - @mock.patch('util.enterprise_helpers.get_enterprise_customer_for_request') - @mock.patch('enterprise.tpa_pipeline.get_enterprise_customer_for_request') - @mock.patch('util.enterprise_helpers.configuration_helpers') - def test_insert_enterprise_fields(self, mock_config_helpers, mock_get_ec, mock_get_ec2): - """ - Test that the insertion of the enterprise fields is processed as expected. - """ - request = mock.MagicMock(session={'partial_pipeline': 'thing'}) - mock_ec = mock.MagicMock( - enforces_data_sharing_consent=mock.MagicMock(return_value=True), - requests_data_sharing_consent=True, - ) - # Name values in a MagicMock constructor don't fill a `name` attribute - mock_ec.name = 'MassiveCorp' - mock_get_ec.return_value = mock_ec - mock_get_ec2.return_value = mock_ec - mock_config_helpers.get_value.return_value = 'OpenEdX' - form_desc = mock.MagicMock() - form_desc.add_field.return_value = None - expected_label = ( - "I agree to allow OpenEdX to share data about my enrollment, " - "completion and performance in all OpenEdX courses and programs " - "where my enrollment is sponsored by MassiveCorp." - ) - expected_err_msg = ( - "To link your account with MassiveCorp, you are required to consent to data sharing." - ) - insert_enterprise_fields(request, form_desc) - mock_ec.enforces_data_sharing_consent.return_value = False - insert_enterprise_fields(request, form_desc) - calls = [ - mock.call( - 'data_sharing_consent', - label=expected_label, - field_type='checkbox', - default=False, - required=True, - error_messages={'required': expected_err_msg} - ), - mock.call( - 'data_sharing_consent', - label=expected_label, - field_type='checkbox', - default=False, - required=False, - error_messages={'required': expected_err_msg} - ) - ] - form_desc.add_field.assert_has_calls(calls) - form_desc.add_field.reset_mock() - mock_ec.requests_data_sharing_consent = False - insert_enterprise_fields(request, form_desc) - form_desc.add_field.assert_not_called() - def test_set_enterprise_branding_filter_param(self): """ Test that the enterprise customer branding parameters are setting correctly. diff --git a/openedx/core/djangoapps/user_api/tests/test_views.py b/openedx/core/djangoapps/user_api/tests/test_views.py index e9496e440c..5451f51774 100644 --- a/openedx/core/djangoapps/user_api/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/tests/test_views.py @@ -1026,43 +1026,6 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): } ) - @mock.patch('util.enterprise_helpers.active_provider_requests_data_sharing') - @mock.patch('util.enterprise_helpers.active_provider_enforces_data_sharing') - @mock.patch('util.enterprise_helpers.get_enterprise_customer_for_request') - @mock.patch('util.enterprise_helpers.configuration_helpers') - def test_register_form_consent_field(self, config_helper, get_ec, mock_enforce, mock_request): - """ - Test that if we have an EnterpriseCustomer active for the request, and that - EnterpriseCustomer is set to require data sharing consent, the correct - field is added to the form descriptor. - """ - fake_ec = mock.MagicMock( - enforces_data_sharing_consent=mock.MagicMock(return_value=True), - requests_data_sharing_consent=True, - ) - fake_ec.name = 'MegaCorp' - get_ec.return_value = fake_ec - config_helper.get_value.return_value = 'OpenEdX' - mock_request.return_value = True - mock_enforce.return_value = True - self._assert_reg_field( - dict(), - { - u"name": u"data_sharing_consent", - u"type": u"checkbox", - u"required": True, - u"label": ( - "I agree to allow OpenEdX to share data about my enrollment, " - "completion and performance in all OpenEdX courses and programs " - "where my enrollment is sponsored by MegaCorp." - ), - u"defaultValue": False, - u"errorMessages": { - u'required': u'To link your account with MegaCorp, you are required to consent to data sharing.', - } - } - ) - @mock.patch('openedx.core.djangoapps.user_api.views._') def test_register_form_level_of_education_translations(self, fake_gettext): fake_gettext.side_effect = lambda text: text + ' TRANSLATED' diff --git a/openedx/core/djangoapps/user_api/views.py b/openedx/core/djangoapps/user_api/views.py index ba5a412fad..2d62520990 100644 --- a/openedx/core/djangoapps/user_api/views.py +++ b/openedx/core/djangoapps/user_api/views.py @@ -31,7 +31,6 @@ from student.cookies import set_logged_in_cookies from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.lib.api.authentication import SessionAuthenticationAllowInactiveUser from util.json_request import JsonResponse -from util.enterprise_helpers import insert_enterprise_fields from .preferences.api import get_country_time_zones, update_email_opt_in from .helpers import FormDescription, shim_student_view, require_post_params from .models import UserPreference, UserProfile @@ -280,9 +279,6 @@ class RegistrationView(APIView): required=self._is_field_required(field_name) ) - # Add any Enterprise fields if the app is enabled - insert_enterprise_fields(request, form_desc) - return HttpResponse(form_desc.to_json(), content_type="application/json") @method_decorator(csrf_exempt)