Commit c0d74f19 by Douglas Hall Committed by GitHub

Merge pull request #16136 from open-craft/uman/refactor-consent-view

[ENT-497] Update user-facing consent view to reflect new data model
parents db6570b7 7516453c
...@@ -217,4 +217,4 @@ class WikiRedirectTestCase(EnterpriseTestConsentRequired, LoginEnrollmentTestCas ...@@ -217,4 +217,4 @@ class WikiRedirectTestCase(EnterpriseTestConsentRequired, LoginEnrollmentTestCas
(reverse('course_wiki', kwargs={'course_id': course_id}), 302), (reverse('course_wiki', kwargs={'course_id': course_id}), 302),
('/courses/{}/wiki/'.format(course_id), 200), ('/courses/{}/wiki/'.format(course_id), 200),
): ):
self.verify_consent_required(self.client, url, status_code) self.verify_consent_required(self.client, url, status_code=status_code)
...@@ -212,7 +212,7 @@ class EnterpriseApiClient(object): ...@@ -212,7 +212,7 @@ class EnterpriseApiClient(object):
}, },
"data_sharing_consent_records": [ "data_sharing_consent_records": [
{ {
"username": "myself", "username": "staff",
"enterprise_customer_uuid": "cf246b88-d5f6-4908-a522-fc307e0b0c59", "enterprise_customer_uuid": "cf246b88-d5f6-4908-a522-fc307e0b0c59",
"exists": true, "exists": true,
"course_id": "course-v1:edX DemoX Demo_Course", "course_id": "course-v1:edX DemoX Demo_Course",
...@@ -324,18 +324,15 @@ def enterprise_enabled(): ...@@ -324,18 +324,15 @@ def enterprise_enabled():
return 'enterprise' in settings.INSTALLED_APPS and getattr(settings, 'ENABLE_ENTERPRISE_INTEGRATION', True) return 'enterprise' in settings.INSTALLED_APPS and getattr(settings, 'ENABLE_ENTERPRISE_INTEGRATION', True)
def enterprise_customer_for_request(request): def enterprise_customer_uuid_for_request(request):
""" """
Check all the context clues of the request to determine if Check all the context clues of the request to gather a particular EnterpriseCustomer's UUID.
the request being made is tied to a particular EnterpriseCustomer.
""" """
if not enterprise_enabled(): if not enterprise_enabled():
return None return None
enterprise_customer = None enterprise_customer_uuid = None
sso_provider_id = request.GET.get('tpa_hint') sso_provider_id = request.GET.get('tpa_hint')
running_pipeline = get_partial_pipeline(request) running_pipeline = get_partial_pipeline(request)
if running_pipeline: if running_pipeline:
# Determine if the user is in the middle of a third-party auth pipeline, # Determine if the user is in the middle of a third-party auth pipeline,
...@@ -354,9 +351,7 @@ def enterprise_customer_for_request(request): ...@@ -354,9 +351,7 @@ def enterprise_customer_for_request(request):
enterprise_customer_identity_provider__provider_id=sso_provider_id enterprise_customer_identity_provider__provider_id=sso_provider_id
).uuid ).uuid
except EnterpriseCustomer.DoesNotExist: except EnterpriseCustomer.DoesNotExist:
# If there is not an EnterpriseCustomer linked to this SSO provider, set pass
# the UUID variable to be null.
enterprise_customer_uuid = None
else: else:
# Check if we got an Enterprise UUID passed directly as either a query # Check if we got an Enterprise UUID passed directly as either a query
# parameter, or as a value in the Enterprise cookie. # parameter, or as a value in the Enterprise cookie.
...@@ -371,6 +366,16 @@ def enterprise_customer_for_request(request): ...@@ -371,6 +366,16 @@ def enterprise_customer_for_request(request):
if learner_data: if learner_data:
enterprise_customer_uuid = learner_data[0]['enterprise_customer']['uuid'] enterprise_customer_uuid = learner_data[0]['enterprise_customer']['uuid']
return enterprise_customer_uuid
def enterprise_customer_for_request(request):
"""
Check all the context clues of the request to determine if
the request being made is tied to a particular EnterpriseCustomer.
"""
enterprise_customer = None
enterprise_customer_uuid = enterprise_customer_uuid_for_request(request)
if enterprise_customer_uuid: if enterprise_customer_uuid:
# If we were able to obtain an EnterpriseCustomer UUID, go ahead # If we were able to obtain an EnterpriseCustomer UUID, go ahead
# and use it to attempt to retrieve EnterpriseCustomer details # and use it to attempt to retrieve EnterpriseCustomer details
...@@ -437,8 +442,7 @@ def get_enterprise_consent_url(request, course_id, user=None, return_to=None, en ...@@ -437,8 +442,7 @@ def get_enterprise_consent_url(request, course_id, user=None, return_to=None, en
if not enterprise_enabled(): if not enterprise_enabled():
return '' return ''
if user is None: user = user or request.user
user = request.user
if not consent_needed_for_course(request, user, course_id, enrollment_exists=enrollment_exists): if not consent_needed_for_course(request, user, course_id, enrollment_exists=enrollment_exists):
return None return None
...@@ -449,6 +453,7 @@ def get_enterprise_consent_url(request, course_id, user=None, return_to=None, en ...@@ -449,6 +453,7 @@ def get_enterprise_consent_url(request, course_id, user=None, return_to=None, en
return_path = reverse(return_to, args=(course_id,)) return_path = reverse(return_to, args=(course_id,))
url_params = { url_params = {
'enterprise_customer_uuid': enterprise_customer_uuid_for_request(request),
'course_id': course_id, 'course_id': course_id,
'next': request.build_absolute_uri(return_path), 'next': request.build_absolute_uri(return_path),
'failure_url': request.build_absolute_uri( 'failure_url': request.build_absolute_uri(
......
...@@ -191,9 +191,12 @@ class EnterpriseServiceMockMixin(object): ...@@ -191,9 +191,12 @@ class EnterpriseServiceMockMixin(object):
}, },
'data_sharing_consent': [ 'data_sharing_consent': [
{ {
'user': 1, "username": "verified",
'state': 'enabled', "enterprise_customer_uuid": enterprise_customer_uuid,
'enabled': True "exists": True,
"course_id": "course-v1:edX DemoX Demo_Course",
"consent_provided": True,
"consent_required": False
} }
] ]
} }
...@@ -216,41 +219,48 @@ class EnterpriseTestConsentRequired(SimpleTestCase): ...@@ -216,41 +219,48 @@ class EnterpriseTestConsentRequired(SimpleTestCase):
""" """
Mixin to help test the data_sharing_consent_required decorator. Mixin to help test the data_sharing_consent_required decorator.
""" """
def verify_consent_required(self, client, url, status_code=200):
@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')
@mock.patch('openedx.features.enterprise_support.api.consent_needed_for_course')
def verify_consent_required(
self,
client,
url,
mock_consent_necessary,
mock_enterprise_enabled,
mock_reverse,
mock_enterprise_customer_uuid_for_request,
status_code=200,
):
""" """
Verify that the given URL redirects to the consent page when consent is required, Verify that the given URL redirects to the consent page when consent is required,
and doesn't redirect to the consent page when consent is not required. and doesn't redirect to the consent page when consent is not required.
Arguments:
* self: ignored
* client: the TestClient instance to be used
* url: URL to test
* status_code: expected status code of URL when no data sharing consent is required.
""" """
def mock_consent_reverse(*args, **kwargs): def mock_consent_reverse(*args, **kwargs):
if args[0] == 'grant_data_sharing_permissions': if args[0] == 'grant_data_sharing_permissions':
return '/enterprise/grant_data_sharing_permissions' return '/enterprise/grant_data_sharing_permissions'
return reverse(*args, **kwargs) return reverse(*args, **kwargs)
with mock.patch('openedx.features.enterprise_support.api.reverse', side_effect=mock_consent_reverse): mock_reverse.side_effect = mock_consent_reverse
with mock.patch('openedx.features.enterprise_support.api.enterprise_enabled', return_value=True): mock_enterprise_enabled.return_value = True
with mock.patch( mock_enterprise_customer_uuid_for_request.return_value = 'fake-uuid'
'openedx.features.enterprise_support.api.consent_needed_for_course' # Ensure that when consent is necessary, the user is redirected to the consent page.
) as mock_consent_necessary: mock_consent_necessary.return_value = True
# Ensure that when consent is necessary, the user is redirected to the consent page. response = client.get(url)
mock_consent_necessary.return_value = True while(response.status_code == 302 and 'grant_data_sharing_permissions' not in response.url):
response = client.get(url) response = client.get(response.url)
while(response.status_code == 302 and 'grant_data_sharing_permissions' not in response.url): self.assertEqual(response.status_code, 302)
response = client.get(response.url) self.assertIn('grant_data_sharing_permissions', response.url) # pylint: disable=no-member
self.assertEqual(response.status_code, 302)
self.assertIn('grant_data_sharing_permissions', response.url) # pylint: disable=no-member
# Ensure that when consent is not necessary, the user continues through to the requested page. # Ensure that when consent is not necessary, the user continues through to the requested page.
mock_consent_necessary.return_value = False mock_consent_necessary.return_value = False
response = client.get(url) response = client.get(url)
self.assertEqual(response.status_code, status_code) self.assertEqual(response.status_code, status_code)
# If we were expecting a redirect, ensure it's not to the data sharing permission page # If we were expecting a redirect, ensure it's not to the data sharing permission page
if status_code == 302: if status_code == 302:
self.assertNotIn('grant_data_sharing_permissions', response.url) # pylint: disable=no-member self.assertNotIn('grant_data_sharing_permissions', response.url) # pylint: disable=no-member
return response return response
...@@ -320,22 +320,30 @@ class TestEnterpriseApi(EnterpriseServiceMockMixin, CacheIsolationTestCase): ...@@ -320,22 +320,30 @@ class TestEnterpriseApi(EnterpriseServiceMockMixin, CacheIsolationTestCase):
mock_enterprise_enabled.assert_called_once() mock_enterprise_enabled.assert_called_once()
mock_consent_necessary.assert_called_once() mock_consent_necessary.assert_called_once()
@httpretty.activate
@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.reverse')
@mock.patch('openedx.features.enterprise_support.api.consent_needed_for_course') @mock.patch('openedx.features.enterprise_support.api.consent_needed_for_course')
def test_get_enterprise_consent_url(self, needed_for_course_mock, reverse_mock): def test_get_enterprise_consent_url(
self,
needed_for_course_mock,
reverse_mock,
enterprise_customer_uuid_for_request_mock,
):
""" """
Verify that get_enterprise_consent_url correctly builds URLs. Verify that get_enterprise_consent_url correctly builds URLs.
""" """
def fake_reverse(*args, **kwargs): def fake_reverse(*args, **kwargs):
if args[0] == 'grant_data_sharing_permissions': if args[0] == 'grant_data_sharing_permissions':
return '/enterprise/grant_data_sharing_permissions' return '/enterprise/grant_data_sharing_permissions'
return reverse(*args, **kwargs) return reverse(*args, **kwargs)
enterprise_customer_uuid_for_request_mock.return_value = 'cf246b88-d5f6-4908-a522-fc307e0b0c59'
reverse_mock.side_effect = fake_reverse reverse_mock.side_effect = fake_reverse
needed_for_course_mock.return_value = True needed_for_course_mock.return_value = True
request_mock = mock.MagicMock( request_mock = mock.MagicMock(
user=None, user=self.user,
build_absolute_uri=lambda x: 'http://localhost:8000' + x # Don't do it like this in prod. Ever. build_absolute_uri=lambda x: 'http://localhost:8000' + x # Don't do it like this in prod. Ever.
) )
...@@ -345,8 +353,9 @@ class TestEnterpriseApi(EnterpriseServiceMockMixin, CacheIsolationTestCase): ...@@ -345,8 +353,9 @@ class TestEnterpriseApi(EnterpriseServiceMockMixin, CacheIsolationTestCase):
expected_url = ( expected_url = (
'/enterprise/grant_data_sharing_permissions?course_id=course-v1%3AedX%2BDemoX%2BDemo_' '/enterprise/grant_data_sharing_permissions?course_id=course-v1%3AedX%2BDemoX%2BDemo_'
'Course&failure_url=http%3A%2F%2Flocalhost%3A8000%2Fdashboard%3Fconsent_failed%3Dcou' 'Course&failure_url=http%3A%2F%2Flocalhost%3A8000%2Fdashboard%3Fconsent_failed%3Dcou'
'rse-v1%253AedX%252BDemoX%252BDemo_Course&next=http%3A%2F%2Flocalhost%3A8000%2Fcours' 'rse-v1%253AedX%252BDemoX%252BDemo_Course&enterprise_customer_uuid=cf246b88-d5f6-4908'
'es%2Fcourse-v1%3AedX%2BDemoX%2BDemo_Course%2Finfo' '-a522-fc307e0b0c59&next=http%3A%2F%2Flocalhost%3A8000%2Fcourses%2Fcourse-v1%3AedX%2B'
'DemoX%2BDemo_Course%2Finfo'
) )
actual_url = get_enterprise_consent_url(request_mock, course_id, return_to=return_to) actual_url = get_enterprise_consent_url(request_mock, course_id, return_to=return_to)
self.assertEqual(actual_url, expected_url) self.assertEqual(actual_url, expected_url)
......
...@@ -47,7 +47,7 @@ edx-lint==0.4.3 ...@@ -47,7 +47,7 @@ edx-lint==0.4.3
astroid==1.3.8 astroid==1.3.8
edx-django-oauth2-provider==1.2.5 edx-django-oauth2-provider==1.2.5
edx-django-sites-extensions==2.3.0 edx-django-sites-extensions==2.3.0
edx-enterprise==0.51.0 edx-enterprise==0.51.1
edx-oauth2-provider==1.2.2 edx-oauth2-provider==1.2.2
edx-opaque-keys==0.4.0 edx-opaque-keys==0.4.0
edx-organizations==0.4.6 edx-organizations==0.4.6
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment