diff --git a/lms/djangoapps/course_wiki/tests/tests.py b/lms/djangoapps/course_wiki/tests/tests.py index 66fcacc493..1272f01f0a 100644 --- a/lms/djangoapps/course_wiki/tests/tests.py +++ b/lms/djangoapps/course_wiki/tests/tests.py @@ -217,4 +217,4 @@ class WikiRedirectTestCase(EnterpriseTestConsentRequired, LoginEnrollmentTestCas (reverse('course_wiki', kwargs={'course_id': course_id}), 302), ('/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) diff --git a/openedx/features/enterprise_support/api.py b/openedx/features/enterprise_support/api.py index 9a1e4d6aff..c2a2b99d07 100644 --- a/openedx/features/enterprise_support/api.py +++ b/openedx/features/enterprise_support/api.py @@ -212,7 +212,7 @@ class EnterpriseApiClient(object): }, "data_sharing_consent_records": [ { - "username": "myself", + "username": "staff", "enterprise_customer_uuid": "cf246b88-d5f6-4908-a522-fc307e0b0c59", "exists": true, "course_id": "course-v1:edX DemoX Demo_Course", @@ -324,18 +324,15 @@ def enterprise_enabled(): 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 - the request being made is tied to a particular EnterpriseCustomer. + Check all the context clues of the request to gather a particular EnterpriseCustomer's UUID. """ - if not enterprise_enabled(): return None - enterprise_customer = None + enterprise_customer_uuid = None sso_provider_id = request.GET.get('tpa_hint') - running_pipeline = get_partial_pipeline(request) if running_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): enterprise_customer_identity_provider__provider_id=sso_provider_id ).uuid except EnterpriseCustomer.DoesNotExist: - # If there is not an EnterpriseCustomer linked to this SSO provider, set - # the UUID variable to be null. - enterprise_customer_uuid = None + pass else: # Check if we got an Enterprise UUID passed directly as either a query # parameter, or as a value in the Enterprise cookie. @@ -371,6 +366,16 @@ def enterprise_customer_for_request(request): if learner_data: 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 we were able to obtain an EnterpriseCustomer UUID, go ahead # 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 if not enterprise_enabled(): return '' - if user is None: - user = request.user + user = user or request.user if not consent_needed_for_course(request, user, course_id, enrollment_exists=enrollment_exists): return None @@ -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,)) url_params = { + 'enterprise_customer_uuid': enterprise_customer_uuid_for_request(request), 'course_id': course_id, 'next': request.build_absolute_uri(return_path), 'failure_url': request.build_absolute_uri( diff --git a/openedx/features/enterprise_support/tests/mixins/enterprise.py b/openedx/features/enterprise_support/tests/mixins/enterprise.py index fca9840477..d628640d78 100644 --- a/openedx/features/enterprise_support/tests/mixins/enterprise.py +++ b/openedx/features/enterprise_support/tests/mixins/enterprise.py @@ -191,9 +191,12 @@ class EnterpriseServiceMockMixin(object): }, 'data_sharing_consent': [ { - 'user': 1, - 'state': 'enabled', - 'enabled': True + "username": "verified", + "enterprise_customer_uuid": enterprise_customer_uuid, + "exists": True, + "course_id": "course-v1:edX DemoX Demo_Course", + "consent_provided": True, + "consent_required": False } ] } @@ -216,41 +219,48 @@ class EnterpriseTestConsentRequired(SimpleTestCase): """ 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, 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): if args[0] == 'grant_data_sharing_permissions': return '/enterprise/grant_data_sharing_permissions' return reverse(*args, **kwargs) - with mock.patch('openedx.features.enterprise_support.api.reverse', side_effect=mock_consent_reverse): - with mock.patch('openedx.features.enterprise_support.api.enterprise_enabled', return_value=True): - with mock.patch( - 'openedx.features.enterprise_support.api.consent_needed_for_course' - ) as mock_consent_necessary: - # Ensure that when consent is necessary, the user is redirected to the consent page. - mock_consent_necessary.return_value = True - response = client.get(url) - while(response.status_code == 302 and 'grant_data_sharing_permissions' not in response.url): - response = client.get(response.url) - self.assertEqual(response.status_code, 302) - self.assertIn('grant_data_sharing_permissions', response.url) # pylint: disable=no-member + mock_reverse.side_effect = mock_consent_reverse + mock_enterprise_enabled.return_value = True + mock_enterprise_customer_uuid_for_request.return_value = 'fake-uuid' + # Ensure that when consent is necessary, the user is redirected to the consent page. + mock_consent_necessary.return_value = True + response = client.get(url) + while(response.status_code == 302 and 'grant_data_sharing_permissions' not in response.url): + response = client.get(response.url) + 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. - mock_consent_necessary.return_value = False - response = client.get(url) - self.assertEqual(response.status_code, status_code) + # Ensure that when consent is not necessary, the user continues through to the requested page. + mock_consent_necessary.return_value = False + response = client.get(url) + self.assertEqual(response.status_code, status_code) - # If we were expecting a redirect, ensure it's not to the data sharing permission page - if status_code == 302: - self.assertNotIn('grant_data_sharing_permissions', response.url) # pylint: disable=no-member - return response + # If we were expecting a redirect, ensure it's not to the data sharing permission page + if status_code == 302: + self.assertNotIn('grant_data_sharing_permissions', response.url) # pylint: disable=no-member + return response diff --git a/openedx/features/enterprise_support/tests/test_api.py b/openedx/features/enterprise_support/tests/test_api.py index f8c60969e9..d9e5a4408e 100644 --- a/openedx/features/enterprise_support/tests/test_api.py +++ b/openedx/features/enterprise_support/tests/test_api.py @@ -320,22 +320,30 @@ class TestEnterpriseApi(EnterpriseServiceMockMixin, CacheIsolationTestCase): mock_enterprise_enabled.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.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. """ + def fake_reverse(*args, **kwargs): if args[0] == 'grant_data_sharing_permissions': return '/enterprise/grant_data_sharing_permissions' return reverse(*args, **kwargs) + enterprise_customer_uuid_for_request_mock.return_value = 'cf246b88-d5f6-4908-a522-fc307e0b0c59' reverse_mock.side_effect = fake_reverse needed_for_course_mock.return_value = True - 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. ) @@ -345,8 +353,9 @@ class TestEnterpriseApi(EnterpriseServiceMockMixin, CacheIsolationTestCase): expected_url = ( '/enterprise/grant_data_sharing_permissions?course_id=course-v1%3AedX%2BDemoX%2BDemo_' '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' - 'es%2Fcourse-v1%3AedX%2BDemoX%2BDemo_Course%2Finfo' + 'rse-v1%253AedX%252BDemoX%252BDemo_Course&enterprise_customer_uuid=cf246b88-d5f6-4908' + '-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) self.assertEqual(actual_url, expected_url)