Andal.LND/edx-platform
4
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Revert "Show Enterprise display name if we should hide sensitive username."

Browse Source 
This reverts commit d58bd06928.

This commit caused performance issues due to increased API load from checking the
enterprise association on each page load
This commit is contained in:
George Babey
2018-04-13 18:42:23 -04:00
parent f7dd174873
commit a155aad01c
12 changed files with 86 additions and 129 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
common/djangoapps/util/views.py
View File

@@ -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(request.user)
enterprise_learner_data = enterprise_api.get_enterprise_learner_data(site=request.site, user=request.user)
if enterprise_learner_data:
context["tags"]["learner_type"] = "enterprise_learner"

6
lms/djangoapps/course_wiki/tests/tests.py
View File

@@ -205,14 +205,10 @@ class WikiRedirectTestCase(EnterpriseTestConsentRequired, LoginEnrollmentTestCas
self.assertEqual(resp.status_code, 200)
@patch.dict("django.conf.settings.FEATURES", {'ALLOW_WIKI_ROOT_ACCESS': True})
@patch('openedx.features.enterprise_support.api.enterprise_customer_for_request')
def test_consent_required(self, mock_enterprise_customer_for_request):
def test_consent_required(self):
"""
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

6
lms/djangoapps/courseware/tests/test_course_info.py
View File

@@ -68,16 +68,12 @@ 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?
@mock.patch('openedx.features.enterprise_support.api.enterprise_customer_for_request')
def test_redirection_missing_enterprise_consent(self, mock_enterprise_customer_for_request):
def test_redirection_missing_enterprise_consent(self):
"""
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)

6
lms/djangoapps/courseware/tests/test_views.py
View File

@@ -2596,14 +2596,10 @@ class EnterpriseConsentTestCase(EnterpriseTestConsentRequired, ModuleStoreTestCa
CourseOverview.load_from_module_store(self.course.id)
CourseEnrollmentFactory(user=self.user, course_id=self.course.id)
@patch('openedx.features.enterprise_support.api.enterprise_customer_for_request')
def test_consent_required(self, mock_enterprise_customer_for_request):
def test_consent_required(self):
"""
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)),

6
lms/djangoapps/discussion/tests/test_views.py
View File

@@ -1659,14 +1659,10 @@ class EnterpriseConsentTestCase(EnterpriseTestConsentRequired, ForumsEnableMixin
self.addCleanup(translation.deactivate)
@patch('openedx.features.enterprise_support.api.enterprise_customer_for_request')
def test_consent_required(self, mock_enterprise_customer_for_request, mock_request):
def test_consent_required(self, 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)

2
lms/djangoapps/support/views/contact_us.py
View File

@@ -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(request.user)
enterprise_learner_data = enterprise_api.get_enterprise_learner_data(site=request.site, user=request.user)
if enterprise_learner_data:
tags.append('enterprise_learner')

9
lms/templates/courseware/progress.html
View File

@@ -11,14 +11,7 @@ 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>
<%block name="headextra">
@@ -61,7 +54,7 @@ username = get_enterprise_learner_generic_name(request) or student.username
</div>
% endif
<h2 class="hd hd-2 progress-certificates-title">
${_("Course Progress for Student '{username}' ({email})").format(username=username, email=student.email)}
${_("Course Progress for Student '{username}' ({email})").format(username=student.username, email=student.email)}
</h2>
<div class="wrapper-msg wrapper-auto-cert">

19
lms/templates/header/user_dropdown.html
View File

@@ -2,29 +2,32 @@
<%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
%>
<%
## 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
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)
%>
<div class="nav-item hidden-mobile">
<a href="${reverse('dashboard')}" class="menu-title">
<img class="user-image-frame" src="${profile_image_url}" alt="">
<span class="sr-only">${_("Dashboard for:")}</span>
<span class="username">${displayname}</span>
<span class="username">${username}</span>
</a>
</div>
<div class="nav-item hidden-mobile nav-item-dropdown" tabindex="-1">

16
lms/templates/user_dropdown.html
View File

@@ -2,21 +2,19 @@
<%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:
<div class="nav-item nav-item-hidden-collapsed container">
<div class="nav align-items-center">

121
openedx/features/enterprise_support/api.py
View File

@@ -161,72 +161,73 @@ class EnterpriseApiClient(object):
LOGGER.exception(message)
raise EnterpriseApiException(message)
def fetch_enterprise_learner_data(self, user):
def fetch_enterprise_learner_data(self, site, user): # pylint: disable=unused-argument
"""
Fetch information related to enterprise from the Enterprise Service.
Example:
fetch_enterprise_learner_data(user)
fetch_enterprise_learner_data(site, user)
Argument:
site: (Site) site instance
user: (User) django auth user
Returns:
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"
},
"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": [
{
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",
"entitlement_id": 69
}
],
"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": [
{
"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
}
]
},
"user_id": 5,
"user": {
"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
}
]
}
],
"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
}
]
}
],
"next": null,
"start": 0,
"previous": null
}
}
Raises:
@@ -365,7 +366,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.user)
learner_data = get_enterprise_learner_data(request.site, request.user)
if learner_data:
enterprise_customer_uuid = learner_data[0]['enterprise_customer']['uuid']
@@ -408,7 +409,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(user)
enterprise_learner_details = get_enterprise_learner_data(request.site, user)
if not enterprise_learner_details:
consent_needed = False
else:
@@ -498,14 +499,14 @@ def get_enterprise_consent_url(request, course_id, user=None, return_to=None, en
return full_url
def get_enterprise_learner_data(user):
def get_enterprise_learner_data(site, user):
"""
Client API operation adapter/wrapper
"""
if not enterprise_enabled():
return None
enterprise_learner_data = EnterpriseApiClient(user=user).fetch_enterprise_learner_data(user)
enterprise_learner_data = EnterpriseApiClient(user=user).fetch_enterprise_learner_data(site=site, user=user)
if enterprise_learner_data:
return enterprise_learner_data['results']
@@ -517,7 +518,7 @@ def get_enterprise_customer_for_learner(site, user):
if not enterprise_enabled():
return {}
enterprise_learner_data = get_enterprise_learner_data(user)
enterprise_learner_data = get_enterprise_learner_data(site, user)
if enterprise_learner_data:
return enterprise_learner_data[0]['enterprise_customer']

5
openedx/features/enterprise_support/tests/mixins/enterprise.py
View File

@@ -220,7 +220,6 @@ 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')
@@ -233,7 +232,6 @@ class EnterpriseTestConsentRequired(SimpleTestCase):
mock_enterprise_enabled,
mock_reverse,
mock_enterprise_customer_uuid_for_request,
mock_get_enterprise_learner_generic_name,
status_code=200,
):
"""
@@ -246,9 +244,6 @@ 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'

17
openedx/features/enterprise_support/utils.py
View File

@@ -220,20 +220,3 @@ 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 ''
)