diff --git a/common/djangoapps/util/views.py b/common/djangoapps/util/views.py index cdf5f860a3..e8c039e7b0 100644 --- a/common/djangoapps/util/views.py +++ b/common/djangoapps/util/views.py @@ -455,7 +455,7 @@ def submit_feedback(request): context = get_feedback_form_context(request) #Update the tag info with 'enterprise_learner' if the user belongs to an enterprise customer. - enterprise_learner_data = enterprise_api.get_enterprise_learner_data(site=request.site, user=request.user) + enterprise_learner_data = enterprise_api.get_enterprise_learner_data(request.user) if enterprise_learner_data: context["tags"]["learner_type"] = "enterprise_learner" diff --git a/lms/djangoapps/course_wiki/tests/tests.py b/lms/djangoapps/course_wiki/tests/tests.py index 8328880c83..a18f051a45 100644 --- a/lms/djangoapps/course_wiki/tests/tests.py +++ b/lms/djangoapps/course_wiki/tests/tests.py @@ -205,10 +205,14 @@ class WikiRedirectTestCase(EnterpriseTestConsentRequired, LoginEnrollmentTestCas self.assertEqual(resp.status_code, 200) @patch.dict("django.conf.settings.FEATURES", {'ALLOW_WIKI_ROOT_ACCESS': True}) - def test_consent_required(self): + @patch('openedx.features.enterprise_support.api.enterprise_customer_for_request') + def test_consent_required(self, mock_enterprise_customer_for_request): """ Test that enterprise data sharing consent is required when enabled for the various courseware views. """ + # ENT-924: Temporary solution to replace sensitive SSO usernames. + mock_enterprise_customer_for_request.return_value = None + # Public wikis can be accessed by non-enrolled users, and so direct access is not gated by the consent page course = CourseFactory.create() course.allow_public_wiki_access = False diff --git a/lms/djangoapps/courseware/tests/test_course_info.py b/lms/djangoapps/courseware/tests/test_course_info.py index 168276894b..04ec1bf688 100644 --- a/lms/djangoapps/courseware/tests/test_course_info.py +++ b/lms/djangoapps/courseware/tests/test_course_info.py @@ -68,12 +68,16 @@ class CourseInfoTestCase(EnterpriseTestConsentRequired, LoginEnrollmentTestCase, self.assertNotIn("You are not currently enrolled in this course", resp.content) # TODO: LEARNER-611: If this is only tested under Course Info, does this need to move? - def test_redirection_missing_enterprise_consent(self): + @mock.patch('openedx.features.enterprise_support.api.enterprise_customer_for_request') + def test_redirection_missing_enterprise_consent(self, mock_enterprise_customer_for_request): """ Verify that users viewing the course info who are enrolled, but have not provided data sharing consent, are first redirected to a consent page, and then, once they've provided consent, are able to view the course info. """ + # ENT-924: Temporary solution to replace sensitive SSO usernames. + mock_enterprise_customer_for_request.return_value = None + self.setup_user() self.enroll(self.course) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 91d22ce92c..d304b427f6 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -2596,10 +2596,14 @@ class EnterpriseConsentTestCase(EnterpriseTestConsentRequired, ModuleStoreTestCa CourseOverview.load_from_module_store(self.course.id) CourseEnrollmentFactory(user=self.user, course_id=self.course.id) - def test_consent_required(self): + @patch('openedx.features.enterprise_support.api.enterprise_customer_for_request') + def test_consent_required(self, mock_enterprise_customer_for_request): """ Test that enterprise data sharing consent is required when enabled for the various courseware views. """ + # ENT-924: Temporary solution to replace sensitive SSO usernames. + mock_enterprise_customer_for_request.return_value = None + course_id = unicode(self.course.id) for url in ( reverse("courseware", kwargs=dict(course_id=course_id)), diff --git a/lms/djangoapps/discussion/tests/test_views.py b/lms/djangoapps/discussion/tests/test_views.py index eaa16956d1..a3448bacb2 100644 --- a/lms/djangoapps/discussion/tests/test_views.py +++ b/lms/djangoapps/discussion/tests/test_views.py @@ -1659,10 +1659,14 @@ class EnterpriseConsentTestCase(EnterpriseTestConsentRequired, ForumsEnableMixin self.addCleanup(translation.deactivate) - def test_consent_required(self, mock_request): + @patch('openedx.features.enterprise_support.api.enterprise_customer_for_request') + def test_consent_required(self, mock_enterprise_customer_for_request, mock_request): """ Test that enterprise data sharing consent is required when enabled for the various discussion views. """ + # ENT-924: Temporary solution to replace sensitive SSO usernames. + mock_enterprise_customer_for_request.return_value = None + thread_id = 'dummy' course_id = unicode(self.course.id) mock_request.side_effect = make_mock_request_impl(course=self.course, text='dummy', thread_id=thread_id) diff --git a/lms/djangoapps/support/views/contact_us.py b/lms/djangoapps/support/views/contact_us.py index b8753e3448..197a8bac7a 100644 --- a/lms/djangoapps/support/views/contact_us.py +++ b/lms/djangoapps/support/views/contact_us.py @@ -33,7 +33,7 @@ class ContactUsView(View): if request.user.is_authenticated(): context['user_enrollments'] = CourseEnrollment.enrollments_for_user_with_overviews_preload(request.user) - enterprise_learner_data = enterprise_api.get_enterprise_learner_data(site=request.site, user=request.user) + enterprise_learner_data = enterprise_api.get_enterprise_learner_data(request.user) if enterprise_learner_data: tags.append('enterprise_learner') diff --git a/lms/templates/courseware/progress.html b/lms/templates/courseware/progress.html index 0945c6b5f5..f80738146b 100644 --- a/lms/templates/courseware/progress.html +++ b/lms/templates/courseware/progress.html @@ -11,7 +11,14 @@ from django.core.urlresolvers import reverse from django.conf import settings from django.utils.http import urlquote_plus from six import text_type + +from openedx.features.enterprise_support.utils import get_enterprise_learner_generic_name %> + +<% +username = get_enterprise_learner_generic_name(request) or student.username +%> + <%block name="bodyclass">view-in-course view-progress <%block name="headextra"> @@ -54,7 +61,7 @@ from six import text_type % endif

- ${_("Course Progress for Student '{username}' ({email})").format(username=student.username, email=student.email)} + ${_("Course Progress for Student '{username}' ({email})").format(username=username, email=student.email)}

diff --git a/lms/templates/header/user_dropdown.html b/lms/templates/header/user_dropdown.html index 9d1c04c791..73bb1e8820 100644 --- a/lms/templates/header/user_dropdown.html +++ b/lms/templates/header/user_dropdown.html @@ -2,32 +2,29 @@ <%page expression_filter="h"/> <%namespace name='static' file='static_content.html'/> -## This template should not use the target student's details when masquerading, see TNL-4895 -<% -self.real_user = getattr(user, 'real_user', user) -%> - <%! from django.core.urlresolvers import reverse from django.utils.translation import ugettext as _ from openedx.core.djangoapps.user_api.accounts.image_helpers import get_profile_image_urls_for_user from openedx.core.djangoapps.user_api.accounts.utils import retrieve_last_sitewide_block_completed - +from openedx.features.enterprise_support.utils import get_enterprise_learner_generic_name %> <% - profile_image_url = get_profile_image_urls_for_user(self.real_user)['medium'] - username = self.real_user.username - resume_block = retrieve_last_sitewide_block_completed(username) +## This template should not use the target student's details when masquerading, see TNL-4895 +self.real_user = getattr(user, 'real_user', user) +profile_image_url = get_profile_image_urls_for_user(self.real_user)['medium'] +username = self.real_user.username +resume_block = retrieve_last_sitewide_block_completed(username) +displayname = get_enterprise_learner_generic_name(request) or username %> -
${_("Dashboard for:")} - ${username} + ${displayname}
diff --git a/lms/templates/user_dropdown.html b/lms/templates/user_dropdown.html index 64049f3f21..1deddfac92 100644 --- a/lms/templates/user_dropdown.html +++ b/lms/templates/user_dropdown.html @@ -2,19 +2,21 @@ <%page expression_filter="h"/> <%namespace name='static' file='static_content.html'/> -## This template should not use the target student's details when masquerading, see TNL-4895 -<% -self.real_user = getattr(user, 'real_user', user) -username = self.real_user.username -profile_image_url = get_profile_image_urls_for_user(self.real_user)['medium'] -%> - <%! from django.core.urlresolvers import reverse from django.utils.translation import ugettext as _ from openedx.core.djangoapps.user_api.accounts.image_helpers import get_profile_image_urls_for_user +from openedx.features.enterprise_support.utils import get_enterprise_learner_generic_name %> + +<% +## This template should not use the target student's details when masquerading, see TNL-4895 +self.real_user = getattr(user, 'real_user', user) +username = get_enterprise_learner_generic_name(request) or self.real_user.username +profile_image_url = get_profile_image_urls_for_user(self.real_user)['medium'] +%> + % if uses_bootstrap:
diff --git a/openedx/features/enterprise_support/api.py b/openedx/features/enterprise_support/api.py index a995de64aa..f558c87ed1 100644 --- a/openedx/features/enterprise_support/api.py +++ b/openedx/features/enterprise_support/api.py @@ -161,73 +161,72 @@ class EnterpriseApiClient(object): LOGGER.exception(message) raise EnterpriseApiException(message) - def fetch_enterprise_learner_data(self, site, user): + def fetch_enterprise_learner_data(self, user): """ Fetch information related to enterprise from the Enterprise Service. Example: - fetch_enterprise_learner_data(site, user) + fetch_enterprise_learner_data(user) Argument: - site: (Site) site instance user: (User) django auth user Returns: - dict: { - "enterprise_api_response_for_learner": { - "count": 1, - "num_pages": 1, - "current_page": 1, - "results": [ - { - "enterprise_customer": { - "uuid": "cf246b88-d5f6-4908-a522-fc307e0b0c59", - "name": "TestShib", - "catalog": 2, - "active": true, - "site": { - "domain": "example.com", - "name": "example.com" - }, - "enable_data_sharing_consent": true, - "enforce_data_sharing_consent": "at_login", - "branding_configuration": { - "enterprise_customer": "cf246b88-d5f6-4908-a522-fc307e0b0c59", - "logo": "https://open.edx.org/sites/all/themes/edx_open/logo.png" - }, - "enterprise_customer_entitlements": [ - { - "enterprise_customer": "cf246b88-d5f6-4908-a522-fc307e0b0c59", - "entitlement_id": 69 - } - ] + dict: + { + "count": 1, + "num_pages": 1, + "current_page": 1, + "next": null, + "start": 0, + "previous": null + "results": [ + { + "enterprise_customer": { + "uuid": "cf246b88-d5f6-4908-a522-fc307e0b0c59", + "name": "TestShib", + "catalog": 2, + "active": true, + "site": { + "domain": "example.com", + "name": "example.com" }, - "user_id": 5, - "user": { - "username": "staff", - "first_name": "", - "last_name": "", - "email": "staff@example.com", - "is_staff": true, - "is_active": true, - "date_joined": "2016-09-01T19:18:26.026495Z" + "enable_data_sharing_consent": true, + "enforce_data_sharing_consent": "at_login", + "branding_configuration": { + "enterprise_customer": "cf246b88-d5f6-4908-a522-fc307e0b0c59", + "logo": "https://open.edx.org/sites/all/themes/edx_open/logo.png" }, - "data_sharing_consent_records": [ + "enterprise_customer_entitlements": [ { - "username": "staff", - "enterprise_customer_uuid": "cf246b88-d5f6-4908-a522-fc307e0b0c59", - "exists": true, - "course_id": "course-v1:edX DemoX Demo_Course", - "consent_provided": true, - "consent_required": false + "enterprise_customer": "cf246b88-d5f6-4908-a522-fc307e0b0c59", + "entitlement_id": 69 } - ] - } - ], - "next": null, - "start": 0, - "previous": null - } + ], + "replace_sensitive_sso_username": False, + }, + "user_id": 5, + "user": { + "username": "staff", + "first_name": "", + "last_name": "", + "email": "staff@example.com", + "is_staff": true, + "is_active": true, + "date_joined": "2016-09-01T19:18:26.026495Z" + }, + "data_sharing_consent_records": [ + { + "username": "staff", + "enterprise_customer_uuid": "cf246b88-d5f6-4908-a522-fc307e0b0c59", + "exists": true, + "course_id": "course-v1:edX DemoX Demo_Course", + "consent_provided": true, + "consent_required": false + } + ] + } + ], } Raises: @@ -366,7 +365,7 @@ def enterprise_customer_uuid_for_request(request): if not enterprise_customer_uuid and request.user.is_authenticated(): # If there's no way to get an Enterprise UUID for the request, check to see # if there's already an Enterprise attached to the requesting user on the backend. - learner_data = get_enterprise_learner_data(request.site, request.user) + learner_data = get_enterprise_learner_data(request.user) if learner_data: enterprise_customer_uuid = learner_data[0]['enterprise_customer']['uuid'] @@ -409,7 +408,7 @@ def consent_needed_for_course(request, user, course_id, enrollment_exists=False) if request.session.get(consent_key) is False: return False - enterprise_learner_details = get_enterprise_learner_data(request.site, user) + enterprise_learner_details = get_enterprise_learner_data(user) if not enterprise_learner_details: consent_needed = False else: @@ -499,14 +498,14 @@ def get_enterprise_consent_url(request, course_id, user=None, return_to=None, en return full_url -def get_enterprise_learner_data(site, user): +def get_enterprise_learner_data(user): """ Client API operation adapter/wrapper """ if not enterprise_enabled(): return None - enterprise_learner_data = EnterpriseApiClient(user=user).fetch_enterprise_learner_data(site=site, user=user) + enterprise_learner_data = EnterpriseApiClient(user=user).fetch_enterprise_learner_data(user) if enterprise_learner_data: return enterprise_learner_data['results'] @@ -518,7 +517,7 @@ def get_enterprise_customer_for_learner(site, user): if not enterprise_enabled(): return {} - enterprise_learner_data = get_enterprise_learner_data(site, user) + enterprise_learner_data = get_enterprise_learner_data(user) if enterprise_learner_data: return enterprise_learner_data[0]['enterprise_customer'] diff --git a/openedx/features/enterprise_support/tests/mixins/enterprise.py b/openedx/features/enterprise_support/tests/mixins/enterprise.py index d628640d78..2479b2b55b 100644 --- a/openedx/features/enterprise_support/tests/mixins/enterprise.py +++ b/openedx/features/enterprise_support/tests/mixins/enterprise.py @@ -220,6 +220,7 @@ class EnterpriseTestConsentRequired(SimpleTestCase): Mixin to help test the data_sharing_consent_required decorator. """ + @mock.patch('openedx.features.enterprise_support.utils.get_enterprise_learner_generic_name') @mock.patch('openedx.features.enterprise_support.api.enterprise_customer_uuid_for_request') @mock.patch('openedx.features.enterprise_support.api.reverse') @mock.patch('openedx.features.enterprise_support.api.enterprise_enabled') @@ -232,6 +233,7 @@ class EnterpriseTestConsentRequired(SimpleTestCase): mock_enterprise_enabled, mock_reverse, mock_enterprise_customer_uuid_for_request, + mock_get_enterprise_learner_generic_name, status_code=200, ): """ @@ -244,6 +246,9 @@ class EnterpriseTestConsentRequired(SimpleTestCase): return '/enterprise/grant_data_sharing_permissions' return reverse(*args, **kwargs) + # ENT-924: Temporary solution to replace sensitive SSO usernames. + mock_get_enterprise_learner_generic_name.return_value = '' + mock_reverse.side_effect = mock_consent_reverse mock_enterprise_enabled.return_value = True mock_enterprise_customer_uuid_for_request.return_value = 'fake-uuid' diff --git a/openedx/features/enterprise_support/utils.py b/openedx/features/enterprise_support/utils.py index 9ba7714114..68db78692a 100644 --- a/openedx/features/enterprise_support/utils.py +++ b/openedx/features/enterprise_support/utils.py @@ -220,3 +220,20 @@ def update_account_settings_context_for_enterprise(context, enterprise_customer) enterprise_context['sync_learner_profile_data'] = identity_provider.sync_learner_profile_data context.update(enterprise_context) + + +def get_enterprise_learner_generic_name(request): + """ + Get a generic name concatenating the Enterprise Customer name and 'Learner'. + + ENT-924: Temporary solution for hiding potentially sensitive SSO names. + When a more complete solution is put in place, delete this function and all of its uses. + """ + # Prevent a circular import. This function makes sense to be in this module though. And see function description. + from openedx.features.enterprise_support.api import enterprise_customer_for_request + enterprise_customer = enterprise_customer_for_request(request) + return ( + enterprise_customer['name'] + 'Learner' + if enterprise_customer and enterprise_customer['replace_sensitive_sso_username'] + else '' + ) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 3974b48cab..0a090a0b53 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -57,7 +57,7 @@ enum34==1.1.6 edx-completion==0.1.5 edx-django-oauth2-provider==1.2.5 edx-django-sites-extensions==2.3.1 -edx-enterprise==0.67.3 +edx-enterprise==0.67.4 edx-milestones==0.1.13 edx-oauth2-provider==1.2.2 edx-organizations==0.4.9