diff --git a/openedx/features/enterprise_support/api.py b/openedx/features/enterprise_support/api.py index ceaff8ffb8..bb5931e927 100644 --- a/openedx/features/enterprise_support/api.py +++ b/openedx/features/enterprise_support/api.py @@ -37,7 +37,7 @@ try: ) from enterprise.api.v1.serializers import EnterpriseCustomerUserReadOnlySerializer from consent.models import DataSharingConsent, DataSharingConsentTextOverrides -except ImportError: +except ImportError: # pragma: no cover pass @@ -586,10 +586,16 @@ def consent_needed_for_course(request, user, course_id, enrollment_exists=False) ) if not consent_needed: + # TODO: https://openedx.atlassian.net/browse/ENT-3724 + # this whole code branch seems to do nothing other than log some misleading info: + # the consent requirement doesn't actually fail. If there's an enterprise or site mismatch, + # we'll still end up in the else branch of "if consent_needed:" below, where + # we'll log that consent is not needed, and ultimately, return False. + # Are we supposed to raise some exceptions in here? enterprises = [str(learner['enterprise_customer']['uuid']) for learner in enterprise_learner_details] if str(current_enterprise_uuid) not in enterprises: - LOGGER.info( + LOGGER.info( # pragma: no cover '[ENTERPRISE DSC] Consent requirement failed due to enterprise mismatch. ' 'USER: [%s], CurrentEnterprise: [%s], LearnerEnterprises: [%s]', user.username, @@ -599,7 +605,7 @@ def consent_needed_for_course(request, user, course_id, enrollment_exists=False) else: domains = [learner['enterprise_customer']['site']['domain'] for learner in enterprise_learner_details] if not Site.objects.filter(domain__in=domains).filter(id=request.site.id).exists(): - LOGGER.info( + LOGGER.info( # pragma: no cover '[ENTERPRISE DSC] Consent requirement failed due to site mismatch. ' 'USER: [%s], RequestSite: [%s], LearnerEnterpriseDomains: [%s]', user.username, diff --git a/openedx/features/enterprise_support/tests/test_api.py b/openedx/features/enterprise_support/tests/test_api.py index 453ff00613..77bb24c2c5 100644 --- a/openedx/features/enterprise_support/tests/test_api.py +++ b/openedx/features/enterprise_support/tests/test_api.py @@ -15,12 +15,14 @@ from django.test.utils import override_settings from django.urls import reverse from edx_django_utils.cache import get_cache_key from six.moves.urllib.parse import parse_qs +from slumber.exceptions import HttpClientError from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms from openedx.features.enterprise_support.api import ( _CACHE_MISS, ENTERPRISE_CUSTOMER_KEY_NAME, + EnterpriseApiException, add_enterprise_customer_to_session, ConsentApiClient, ConsentApiServiceClient, @@ -32,11 +34,15 @@ from openedx.features.enterprise_support.api import ( enterprise_customer_from_api, enterprise_customer_uuid_for_request, enterprise_enabled, + get_consent_notification_data, get_consent_required_courses, get_dashboard_consent_notification, get_enterprise_consent_url, + get_enterprise_learner_data_from_api, + get_enterprise_learner_data_from_db, get_enterprise_learner_portal_enabled_message, - insert_enterprise_pipeline_elements + insert_enterprise_pipeline_elements, + unlink_enterprise_user_from_idp, ) from openedx.features.enterprise_support.tests import FEATURES_WITH_ENTERPRISE_ENABLED from openedx.features.enterprise_support.tests.factories import ( @@ -47,6 +53,8 @@ from openedx.features.enterprise_support.tests.mixins.enterprise import Enterpri from openedx.features.enterprise_support.utils import clear_data_consent_share_cache from common.djangoapps.student.tests.factories import UserFactory +from enterprise.models import EnterpriseCustomerUser + class MockEnrollment(mock.MagicMock): """ @@ -104,6 +112,7 @@ class TestEnterpriseApi(EnterpriseServiceMockMixin, CacheIsolationTestCase): mocked_jwt_builder.assert_called_once_with(dummy_enterprise_user) # pylint: disable=protected-access self.assertEqual(enterprise_api_service_client.client._store['session'].auth.token, 'test-token') + return enterprise_api_service_client def _assert_get_enterprise_customer(self, api_client, enterprise_api_data_for_mock): """ @@ -159,6 +168,36 @@ class TestEnterpriseApi(EnterpriseServiceMockMixin, CacheIsolationTestCase): """ self._assert_api_client_with_user(EnterpriseApiClient, mock_jwt_builder) + @ddt.data(True, False) + @httpretty.activate + @mock.patch('openedx.features.enterprise_support.api.create_jwt_for_user') + def test_enterprise_api_client_with_user_post_enrollment(self, should_raise_http_error, mock_jwt_builder): + """ + Verify that enterprise API client uses the provided user to + authenticate and access enterprise API. + """ + api_client = self._assert_api_client_with_user(EnterpriseApiClient, mock_jwt_builder) + setattr(api_client.client, 'enterprise-course-enrollment', mock.Mock()) + mock_endpoint = getattr(api_client.client, 'enterprise-course-enrollment') + if should_raise_http_error: + mock_endpoint.post.side_effect = HttpClientError + + username = 'spongebob' + course_id = 'burger-flipping-101' + consent_granted = True + + if should_raise_http_error: + with self.assertRaises(EnterpriseApiException): + api_client.post_enterprise_course_enrollment(username, course_id, consent_granted) + else: + api_client.post_enterprise_course_enrollment(username, course_id, consent_granted) + + mock_endpoint.post.assert_called_once_with(data={ + 'username': username, + 'course_id': course_id, + 'consent_granted': consent_granted, + }) + @mock.patch('openedx.features.enterprise_support.api.enterprise_customer_uuid_for_request') @mock.patch('openedx.features.enterprise_support.api.EnterpriseApiClient') def test_enterprise_customer_from_api_cache_miss(self, mock_client_class, mock_uuid_from_request): @@ -185,7 +224,18 @@ class TestEnterpriseApi(EnterpriseServiceMockMixin, CacheIsolationTestCase): Verify that enterprise API consent service client uses the provided user to authenticate and access enterprise API. """ - self._assert_api_client_with_user(ConsentApiClient, mock_jwt_builder) + consent_client = self._assert_api_client_with_user(ConsentApiClient, mock_jwt_builder) + consent_client.consent_endpoint = mock.Mock() + + kwargs = { + 'foo': 'a', + 'bar': 'b', + } + consent_client.provide_consent(**kwargs) + consent_client.revoke_consent(**kwargs) + + consent_client.consent_endpoint.post.assert_called_once_with(kwargs) + consent_client.consent_endpoint.delete.assert_called_once_with(**kwargs) @httpretty.activate @mock.patch('openedx.features.enterprise_support.api.get_enterprise_learner_data_from_db') @@ -203,7 +253,7 @@ class TestEnterpriseApi(EnterpriseServiceMockMixin, CacheIsolationTestCase): mock_get_enterprise_learner_data.return_value = self.get_mock_enterprise_learner_results() self.mock_enterprise_learner_api() - # test not required consent for example non enterprise customer + # test that consent is not required for a non-enterprise customer self.mock_consent_not_required(user.username, course_id, ec_uuid) self.assertFalse(consent_needed_for_course(request, user, course_id)) @@ -221,6 +271,31 @@ class TestEnterpriseApi(EnterpriseServiceMockMixin, CacheIsolationTestCase): self.mock_consent_get(user.username, course_id, ec_uuid) self.assertFalse(consent_needed_for_course(request, user, course_id)) + # test when the enrollment already exists without a consent record existing. + clear_data_consent_share_cache(user.id, course_id) + self.mock_consent_missing(user.username, course_id, ec_uuid) + self.assertFalse(consent_needed_for_course(request, user, course_id, enrollment_exists=True)) + + @httpretty.activate + @mock.patch('openedx.features.enterprise_support.api.get_enterprise_learner_data_from_db') + def test_consent_needed_for_course_no_learner_data(self, mock_get_enterprise_learner_data): + user = UserFactory(username='janedoe') + request = mock.MagicMock( + user=user, + site=SiteFactory(domain="example.com"), + session={}, + COOKIES={}, + GET={}, + ) + ec_uuid = 'cf246b88-d5f6-4908-a522-fc307e0b0c59' + course_id = 'fake-course' + mock_get_enterprise_learner_data.return_value = None + self.mock_enterprise_learner_api() + + # test that consent is not required for a non-enterprise customer + self.mock_consent_not_required(user.username, course_id, ec_uuid) + self.assertFalse(consent_needed_for_course(request, user, course_id)) + @httpretty.activate @mock.patch('enterprise.models.EnterpriseCustomer.catalog_contains_course') def test_get_consent_required_courses(self, mock_catalog_contains_course): @@ -245,6 +320,76 @@ class TestEnterpriseApi(EnterpriseServiceMockMixin, CacheIsolationTestCase): consent_required = get_consent_required_courses(user, [course_id]) self.assertNotIn(course_id, consent_required) + def test_consent_not_required_for_non_enterprise_user(self): + user = UserFactory() + course_id = 'fake-course' + + consent_required_courses = get_consent_required_courses(user, [course_id]) + + assert set() == consent_required_courses + + @mock.patch('openedx.features.enterprise_support.api.create_jwt_for_user') + def test_fetch_enterprise_learner_data_unauthenticated(self, mock_jwt_builder): + api_client = self._assert_api_client_with_user(EnterpriseApiClient, mock_jwt_builder) + setattr(api_client.client, 'enterprise-learner', mock.Mock()) + mock_endpoint = getattr(api_client.client, 'enterprise-learner') + + user = mock.Mock(is_authenticated=False) + self.assertIsNone(api_client.fetch_enterprise_learner_data(user)) + + self.assertFalse(mock_endpoint.called) + + @mock.patch('openedx.features.enterprise_support.api.create_jwt_for_user') + def test_fetch_enterprise_learner_data(self, mock_jwt_builder): + api_client = self._assert_api_client_with_user(EnterpriseApiClient, mock_jwt_builder) + setattr(api_client.client, 'enterprise-learner', mock.Mock()) + mock_endpoint = getattr(api_client.client, 'enterprise-learner') + + user = mock.Mock(is_authenticated=True, username='spongebob') + response = api_client.fetch_enterprise_learner_data(user) + + assert mock_endpoint.return_value.get.return_value == response + mock_endpoint.return_value.get.assert_called_once_with(username=user.username) + + @mock.patch('openedx.features.enterprise_support.api.get_current_request') + @mock.patch('openedx.features.enterprise_support.api.create_jwt_for_user') + def test_fetch_enterprise_learner_data_http_error(self, mock_jwt_builder, mock_get_current_request): + api_client = self._assert_api_client_with_user(EnterpriseApiClient, mock_jwt_builder) + setattr(api_client.client, 'enterprise-learner', mock.Mock()) + mock_endpoint = getattr(api_client.client, 'enterprise-learner') + mock_endpoint.return_value.get.side_effect = HttpClientError + mock_get_current_request.return_value.META = { + 'PATH_INFO': 'whatever', + } + + user = mock.Mock(is_authenticated=True, username='spongebob') + + self.assertIsNone(api_client.fetch_enterprise_learner_data(user)) + + mock_endpoint.return_value.get.assert_called_once_with(username=user.username) + + @mock.patch('openedx.features.enterprise_support.api.EnterpriseApiClient') + def test_get_enterprise_learner_data_from_api(self, mock_api_client_class): + user = mock.Mock(is_authenticated=True) + mock_client = mock_api_client_class.return_value + mock_client.fetch_enterprise_learner_data.return_value = { + 'results': 'the-learner-data', + } + + learner_data = get_enterprise_learner_data_from_api(user) + + assert 'the-learner-data' == learner_data + mock_api_client_class.assert_called_once_with(user=user) + mock_client.fetch_enterprise_learner_data.assert_called_once_with(user) + + def test_get_enterprise_learner_data_from_db_no_data(self): + assert [] == get_enterprise_learner_data_from_db(self.user) + + def test_get_enterprise_learner_data_from_db(self): + enterprise_customer_user = EnterpriseCustomerUserFactory(user_id=self.user.id) + user_data = get_enterprise_learner_data_from_db(self.user)[0]['user'] + assert user_data['username'] == self.user.username + @httpretty.activate @mock.patch('openedx.features.enterprise_support.api.get_enterprise_learner_data_from_db') @mock.patch('openedx.features.enterprise_support.api.EnterpriseCustomer') @@ -466,12 +611,14 @@ class TestEnterpriseApi(EnterpriseServiceMockMixin, CacheIsolationTestCase): mock_get_consent_url.assert_called_once() + @ddt.data(True, False) @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, + is_return_to_null, needed_for_course_mock, reverse_mock, enterprise_customer_uuid_for_request_mock, @@ -490,17 +637,19 @@ class TestEnterpriseApi(EnterpriseServiceMockMixin, CacheIsolationTestCase): needed_for_course_mock.return_value = True request_mock = mock.MagicMock( user=self.user, + path='/request_path', build_absolute_uri=lambda x: 'http://localhost:8000' + x # Don't do it like this in prod. Ever. ) course_id = 'course-v1:edX+DemoX+Demo_Course' - return_to = 'info' + return_to = None if is_return_to_null else 'info' + expected_path = request_mock.path if is_return_to_null else '/courses/course-v1:edX+DemoX+Demo_Course/info' expected_url_args = { 'course_id': ['course-v1:edX+DemoX+Demo_Course'], 'failure_url': ['http://localhost:8000/dashboard?consent_failed=course-v1%3AedX%2BDemoX%2BDemo_Course'], 'enterprise_customer_uuid': ['cf246b88-d5f6-4908-a522-fc307e0b0c59'], - 'next': ['http://localhost:8000/courses/course-v1:edX+DemoX+Demo_Course/info'] + 'next': ['http://localhost:8000{}'.format(expected_path)] } actual_url = get_enterprise_consent_url(request_mock, course_id, return_to=return_to) @@ -671,9 +820,12 @@ class TestEnterpriseApi(EnterpriseServiceMockMixin, CacheIsolationTestCase): self.assertIsNone(actual_result) self.assertFalse(mock_learner_data_from_db.called) + @ddt.data(True, False) @mock.patch('openedx.features.enterprise_support.api.get_enterprise_learner_data_from_db') @override_settings(ENTERPRISE_LEARNER_PORTAL_BASE_URL='http://localhost') - def test_enterprise_learner_portal_message_cache_hit_customer_exists(self, mock_learner_data_from_db): + def test_enterprise_learner_portal_message_cache_hit_customer_exists( + self, enable_learner_portal, mock_learner_data_from_db + ): """ When customer data exists in the request session and it's a non-empty customer, then ``get_enterprise_learner_portal_enabled_message()`` should return @@ -682,7 +834,7 @@ class TestEnterpriseApi(EnterpriseServiceMockMixin, CacheIsolationTestCase): mock_enterprise_customer = { 'uuid': 'some-uuid', 'name': 'Best Corp', - 'enable_learner_portal': True, + 'enable_learner_portal': enable_learner_portal, 'slug': 'best-corp', } mock_request = mock.Mock(session={ @@ -690,9 +842,12 @@ class TestEnterpriseApi(EnterpriseServiceMockMixin, CacheIsolationTestCase): }) actual_result = get_enterprise_learner_portal_enabled_message(mock_request) - self.assertIn('custom dashboard for learning', actual_result) - self.assertIn('Best Corp', actual_result) - self.assertFalse(mock_learner_data_from_db.called) + if not enable_learner_portal: + self.assertIsNone(actual_result) + else: + self.assertIn('custom dashboard for learning', actual_result) + self.assertIn('Best Corp', actual_result) + self.assertFalse(mock_learner_data_from_db.called) @mock.patch('openedx.features.enterprise_support.api.get_partial_pipeline', return_value=None) def test_customer_uuid_for_request_sso_provider_id_customer_exists(self, mock_partial_pipeline): @@ -840,3 +995,73 @@ class TestEnterpriseApi(EnterpriseServiceMockMixin, CacheIsolationTestCase): add_enterprise_customer_to_session(mock_request, None) # verify that existing session value should not be updated for un-authenticate user self.assertEqual(mock_request.session[ENTERPRISE_CUSTOMER_KEY_NAME], enterprise_customer) + + def test_get_consent_notification_data_no_overrides(self): + enterprise_customer = { + 'name': 'abc', + 'uuid': 'cf246b88-d5f6-4908-a522-fc307e0b0c59' + } + + title_template, message_template = get_consent_notification_data(enterprise_customer) + + self.assertIsNone(title_template) + self.assertIsNone(message_template) + + @mock.patch('openedx.features.enterprise_support.api.DataSharingConsentTextOverrides') + def test_get_consent_notification_data(self, mock_override_model): + enterprise_customer = { + 'name': 'abc', + 'uuid': 'cf246b88-d5f6-4908-a522-fc307e0b0c59' + } + mock_override = mock.Mock( + declined_notification_title='the title', + declined_notification_message='the message', + ) + mock_override_model.objects.get.return_value = mock_override + + title_template, message_template = get_consent_notification_data(enterprise_customer) + + assert mock_override.declined_notification_title == title_template + assert mock_override.declined_notification_message == message_template + + @mock.patch('openedx.features.enterprise_support.api.Registry') + @mock.patch('openedx.features.enterprise_support.api.enterprise_customer_for_request') + def test_unlink_enterprise_user_from_idp(self, mock_customer_from_request, mock_registry): + customer_idp = EnterpriseCustomerIdentityProviderFactory.create( + provider_id='the-provider', + ) + customer = customer_idp.enterprise_customer + customer_user = EnterpriseCustomerUserFactory.create( + enterprise_customer=customer, + user_id=self.user.id, + ) + mock_customer_from_request.return_value = { + 'uuid': customer.uuid, + } + mock_registry.get_enabled_by_backend_name.return_value = [ + mock.Mock(provider_id='the-provider') + ] + request = mock.Mock() + + unlink_enterprise_user_from_idp(request, self.user, idp_backend_name='the-backend-name') + + assert 0 == EnterpriseCustomerUser.objects.filter(user_id=self.user.id).count() + + @mock.patch('openedx.features.enterprise_support.api.Registry') + @mock.patch('openedx.features.enterprise_support.api.enterprise_customer_for_request') + def test_unlink_enterprise_user_from_idp_no_customer_user(self, mock_customer_from_request, mock_registry): + customer_idp = EnterpriseCustomerIdentityProviderFactory.create( + provider_id='the-provider', + ) + customer = customer_idp.enterprise_customer + mock_customer_from_request.return_value = { + 'uuid': customer.uuid, + } + mock_registry.get_enabled_by_backend_name.return_value = [ + mock.Mock(provider_id='the-provider') + ] + request = mock.Mock() + + unlink_enterprise_user_from_idp(request, self.user, idp_backend_name='the-backend-name') + + assert 0 == EnterpriseCustomerUser.objects.filter(user_id=self.user.id).count()