From 9b82185323a14d52a650290d865ddbfe9d2a6b34 Mon Sep 17 00:00:00 2001
From: Zaman Afzal <zamanafzal@gmail.com>
Date: Fri, 22 Nov 2019 20:46:36 +0500
Subject: [PATCH] Revert "ENT-2454 Modify third party auth pipeline to update
user enterprise (#22314)" (#22384)
This reverts commit 439e4b416987886cb1f7269401aa5f04dadd0bbc.
---
.../djangoapps/third_party_auth/pipeline.py | 11 +-
.../djangoapps/third_party_auth/settings.py | 1 -
common/djangoapps/third_party_auth/utils.py | 15 --
openedx/features/enterprise_support/api.py | 41 -----
.../features/enterprise_support/pipeline.py | 43 -----
.../tests/mixins/enterprise.py | 34 ----
.../enterprise_support/tests/test_api.py | 49 +-----
.../enterprise_support/tests/test_pipeline.py | 149 ------------------
8 files changed, 11 insertions(+), 332 deletions(-)
delete mode 100644 openedx/features/enterprise_support/pipeline.py
delete mode 100644 openedx/features/enterprise_support/tests/test_pipeline.py
diff --git a/common/djangoapps/third_party_auth/pipeline.py b/common/djangoapps/third_party_auth/pipeline.py
index b5579527e29..b4a683e9b5f 100644
--- a/common/djangoapps/third_party_auth/pipeline.py
+++ b/common/djangoapps/third_party_auth/pipeline.py
@@ -89,7 +89,7 @@ from lms.djangoapps.verify_student.models import SSOVerification
from lms.djangoapps.verify_student.utils import earliest_allowed_verification_date
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
from openedx.core.djangoapps.user_authn import cookies as user_authn_cookies
-from third_party_auth.utils import user_exists, is_provider_saml
+from third_party_auth.utils import user_exists
from track import segment
from util.json_request import JsonResponse
@@ -553,9 +553,16 @@ def ensure_user_information(strategy, auth_entry, backend=None, user=None, socia
return (current_provider and
(current_provider.skip_email_verification or current_provider.send_to_registration_first))
+ def is_provider_saml():
+ """ Verify that the third party provider uses SAML """
+ current_provider = provider.Registry.get_from_pipeline({'backend': current_partial.backend, 'kwargs': kwargs})
+ saml_providers_list = list(provider.Registry.get_enabled_by_backend_name('tpa-saml'))
+ return (current_provider and
+ current_provider.slug in [saml_provider.slug for saml_provider in saml_providers_list])
+
if not user:
# Use only email for user existence check in case of saml provider
- if is_provider_saml(current_partial.backend, kwargs):
+ if is_provider_saml():
user_details = {'email': details.get('email')} if details else None
else:
user_details = details
diff --git a/common/djangoapps/third_party_auth/settings.py b/common/djangoapps/third_party_auth/settings.py
index 12ba63ccb5d..aee84672786 100644
--- a/common/djangoapps/third_party_auth/settings.py
+++ b/common/djangoapps/third_party_auth/settings.py
@@ -64,7 +64,6 @@ def apply_settings(django_settings):
'third_party_auth.pipeline.user_details_force_sync',
'third_party_auth.pipeline.set_id_verification_status',
'third_party_auth.pipeline.set_logged_in_cookies',
- 'openedx.features.enterprise_support.pipeline.set_learner_active_enterprise',
'third_party_auth.pipeline.login_analytics',
]
diff --git a/common/djangoapps/third_party_auth/utils.py b/common/djangoapps/third_party_auth/utils.py
index a1fc46b656e..6b4e2e23b7a 100644
--- a/common/djangoapps/third_party_auth/utils.py
+++ b/common/djangoapps/third_party_auth/utils.py
@@ -6,8 +6,6 @@ from __future__ import absolute_import
from django.contrib.auth.models import User
-from . import provider
-
def user_exists(details):
"""
@@ -31,16 +29,3 @@ def user_exists(details):
return User.objects.filter(**user_queryset_filter).exists()
return False
-
-
-def is_provider_saml(backend_name, kwargs):
- """ Verify that the third party provider uses SAML """
- current_provider = provider.Registry.get_from_pipeline({'backend': backend_name, 'kwargs': kwargs})
- saml_providers_list = list(provider.Registry.get_enabled_by_backend_name('tpa-saml'))
- return (current_provider and
- current_provider.slug in [saml_provider.slug for saml_provider in saml_providers_list])
-
-
-def saml_idp_name(backend_name, idp_name):
- backend_type = backend_name.split('-')[1]
- return backend_type + '-' + idp_name
diff --git a/openedx/features/enterprise_support/api.py b/openedx/features/enterprise_support/api.py
index feab0f95750..fe226277ae9 100644
--- a/openedx/features/enterprise_support/api.py
+++ b/openedx/features/enterprise_support/api.py
@@ -266,32 +266,6 @@ class EnterpriseApiClient(object):
return response
- def post_active_enterprise_customer(self, username, enterprise_uuid, active_status):
- """
- Update learner's active enterprise
- """
- enterprise_status_changed = False
- data = {
- 'username': username,
- 'enterprise_customer': enterprise_uuid,
- 'active': active_status,
- }
- endpoint = getattr(self.client, 'enterprise-learner')
- try:
- endpoint.post(data=data)
- enterprise_status_changed = True
- except (HttpClientError, HttpServerError):
- message = (
- u'[Enterprise Support] An error occurred while posting EnterpriseCustomerUser active status. '
- u'Enterprise: {enterprise_uuid}, Status: {status}, User: {username}'
- ).format(
- enterprise_uuid=enterprise_uuid,
- status=active_status,
- username=username,
- )
- LOGGER.exception(message)
- return enterprise_status_changed
-
class EnterpriseApiServiceClient(EnterpriseServiceClientMixin, EnterpriseApiClient):
"""
@@ -718,18 +692,3 @@ def unlink_enterprise_user_from_idp(request, user, idp_backend_name):
)
except (EnterpriseCustomerUser.DoesNotExist, PendingEnterpriseCustomerUser.DoesNotExist):
pass
-
-
-@enterprise_is_enabled()
-def get_enterprise_customer_from_session(request):
- """Check if enterprise_customer is in the session."""
- enterprise_customer = None
- if 'enterprise_customer' in request.session:
- enterprise_customer = request.session.get('enterprise_customer')
- return enterprise_customer
-
-
-@enterprise_is_enabled()
-def activate_learner_enterprise(request, enterprise_customer):
- """Update enterprise_customer in the session."""
- request.session['enterprise_customer'] = enterprise_customer
diff --git a/openedx/features/enterprise_support/pipeline.py b/openedx/features/enterprise_support/pipeline.py
deleted file mode 100644
index ff2c8279676..00000000000
--- a/openedx/features/enterprise_support/pipeline.py
+++ /dev/null
@@ -1,43 +0,0 @@
-"""
-Pipeline for the SAML Enterprise feature.
-
-The Enterprise feature must be turned on for this pipeline to have any effect.
-"""
-
-from __future__ import absolute_import
-from openedx.features.enterprise_support.api import (
- EnterpriseApiClient,
- enterprise_is_enabled,
- get_enterprise_learner_data,
- get_enterprise_customer_from_session,
- activate_learner_enterprise
-)
-from openedx.core.djangoapps.user_api.accounts.utils import is_multiple_user_enterprises_feature_enabled
-from third_party_auth.utils import is_provider_saml, saml_idp_name
-
-
-@enterprise_is_enabled()
-def set_learner_active_enterprise(user=None, backend=None, strategy=None, **kwargs):
- """
- Make 'active' a user's enterprise,
- if the currently 'active' enterprise in EnterpriseCustomerUser does not match the SAML idp-enterprise
- """
- if is_multiple_user_enterprises_feature_enabled() and is_provider_saml(backend.name, kwargs):
- request = strategy.request
- idp_name = saml_idp_name(backend.name, kwargs['response']['idp_name'])
- enterprise_customer = get_enterprise_customer_from_session(request)
- if not enterprise_customer or idp_name != enterprise_customer['identity_provider']:
- learner_enterprises = get_enterprise_learner_data(user)
-
- if len(learner_enterprises) > 1:
- # Check and change the active enterprise_customer only if user is associated to multiple enterprises.
- idp_enterprise = [learner_enterprise['enterprise_customer'] for learner_enterprise
- in learner_enterprises if
- learner_enterprise['enterprise_customer']['identity_provider'] == idp_name]
- if idp_enterprise:
- uuid = idp_enterprise[0]['uuid']
- enterprise_status_changed = EnterpriseApiClient(user=user).post_active_enterprise_customer(
- user.username, uuid, True)
- if enterprise_status_changed:
- activate_learner_enterprise(request, idp_enterprise[0])
- return None
diff --git a/openedx/features/enterprise_support/tests/mixins/enterprise.py b/openedx/features/enterprise_support/tests/mixins/enterprise.py
index ecc2cbff6ac..d17d56e2541 100644
--- a/openedx/features/enterprise_support/tests/mixins/enterprise.py
+++ b/openedx/features/enterprise_support/tests/mixins/enterprise.py
@@ -105,40 +105,6 @@ class EnterpriseServiceMockMixin(object):
status=response_code or 200,
)
- def mock_post_learner_active_enterprise(
- self,
- username='test_user',
- enterprise_customer='uuid',
- active=True
- ):
- """
- Helper method to change the active enterprise_customer post API endpoint.
- """
- api_response = {
- username: username,
- enterprise_customer: enterprise_customer,
- active: active,
- }
- api_response_json = json.dumps(api_response)
- httpretty.register_uri(
- method=httpretty.POST,
- uri=self.get_enterprise_url('enterprise-learner'),
- body=api_response_json,
- content_type='application/json'
- )
-
- def mock_post_learner_active_enterprise_api_failure(self): # pylint: disable=invalid-name
- """
- Helper method to change the active enterprise_customer post API endpoint for a failure.
- """
- httpretty.register_uri(
- method=httpretty.POST,
- uri=self.get_enterprise_url('enterprise-learner'),
- body='{}',
- content_type='application/json',
- status=500
- )
-
def mock_consent_post(self, username, course_id, ec_uuid):
self.mock_consent_response(
username,
diff --git a/openedx/features/enterprise_support/tests/test_api.py b/openedx/features/enterprise_support/tests/test_api.py
index 9a5333ee3ce..a3636dc09a5 100644
--- a/openedx/features/enterprise_support/tests/test_api.py
+++ b/openedx/features/enterprise_support/tests/test_api.py
@@ -31,11 +31,9 @@ from openedx.features.enterprise_support.api import (
get_consent_required_courses,
get_dashboard_consent_notification,
get_enterprise_consent_url,
- insert_enterprise_pipeline_elements,
- get_enterprise_customer_from_session,
- activate_learner_enterprise,
+ insert_enterprise_pipeline_elements
)
-from openedx.features.enterprise_support.tests import FEATURES_WITH_ENTERPRISE_ENABLED, FAKE_ENTERPRISE_CUSTOMER
+from openedx.features.enterprise_support.tests import FEATURES_WITH_ENTERPRISE_ENABLED
from openedx.features.enterprise_support.tests.factories import EnterpriseCustomerUserFactory
from openedx.features.enterprise_support.tests.mixins.enterprise import EnterpriseServiceMockMixin
from openedx.features.enterprise_support.utils import clear_data_consent_share_cache, get_cache_key
@@ -517,46 +515,3 @@ class TestEnterpriseApi(EnterpriseServiceMockMixin, CacheIsolationTestCase):
'enterprise.tpa_pipeline.handle_enterprise_logistration',
'social_core.pipeline.social_auth.load_extra_data',
'def'])
-
- @httpretty.activate
- def test_post_active_enterprise_customer(self):
- user = UserFactory()
- self.mock_post_learner_active_enterprise()
- api_client = EnterpriseApiServiceClient()
- enterprise_status_changed = api_client.post_active_enterprise_customer(user.username, 'uuid-123', True)
- self.assertEqual(
- httpretty.last_request().parsed_body,
- {
- u'username': user.username,
- u'active': True,
- u'enterprise_customer': u'uuid-123',
- }
- )
- self.assertEqual(enterprise_status_changed, True)
-
- @httpretty.activate
- def test_post_active_enterprise_customer_api_fail(self):
- user = UserFactory()
- self.mock_post_learner_active_enterprise_api_failure()
- api_client = EnterpriseApiServiceClient()
- enterprise_status_changed = api_client.post_active_enterprise_customer(user.username, 'uuid-123', True)
- self.assertEqual(enterprise_status_changed, False)
-
- @ddt.data(
- (FAKE_ENTERPRISE_CUSTOMER, {'enterprise_customer': FAKE_ENTERPRISE_CUSTOMER}),
- (None, {}),
- )
- @ddt.unpack
- def test_enterprise_customer_from_session(self, fake_enterprise_customer, session_enterprise_customer):
- """
- Check enterprise_customer in session.
- """
- request = mock.MagicMock(session=session_enterprise_customer, site=SiteFactory(domain="example.com"))
- enterprise_customer = get_enterprise_customer_from_session(request)
- self.assertEqual(fake_enterprise_customer, enterprise_customer)
-
- def test_activate_learner_enterprise(self):
- request = mock.MagicMock(session={'enterprise_customer': {'uuid': '1cc'}},
- site=SiteFactory(domain="example.com"))
- activate_learner_enterprise(request, FAKE_ENTERPRISE_CUSTOMER)
- self.assertEqual(request.session.get('enterprise_customer'), FAKE_ENTERPRISE_CUSTOMER)
diff --git a/openedx/features/enterprise_support/tests/test_pipeline.py b/openedx/features/enterprise_support/tests/test_pipeline.py
deleted file mode 100644
index b6a89316bd0..00000000000
--- a/openedx/features/enterprise_support/tests/test_pipeline.py
+++ /dev/null
@@ -1,149 +0,0 @@
-"""
-Tests for SAML Enterprise Pipeline.
-"""
-
-from __future__ import absolute_import
-
-import mock
-from django.test import TestCase
-from django.test.utils import override_settings
-from student.tests.factories import UserFactory
-
-from openedx.core.djangolib.testing.utils import skip_unless_lms
-from openedx.features.enterprise_support import pipeline
-from openedx.features.enterprise_support.tests import FEATURES_WITH_ENTERPRISE_ENABLED
-
-
-@override_settings(FEATURES=FEATURES_WITH_ENTERPRISE_ENABLED)
-@skip_unless_lms
-class EnsureEnterpriseCustomerActiveStatusTestCase(TestCase):
- """Tests ensuring that we are only updating learner enterprise_customer when it is needed."""
-
- def setUp(self):
- super(EnsureEnterpriseCustomerActiveStatusTestCase, self).setUp()
- self.user = UserFactory.create()
- self.strategy = mock.MagicMock()
- self.request = mock.MagicMock(session={'enterprise_customer': {'identity_provider': 'saml-default'}})
- self.strategy.request = self.request
- self.backend = mock.MagicMock()
- self.backend.name = 'tpa-saml'
- self.saml_provider = mock.MagicMock(
- slug='unique_slug',
- send_to_registration_first=True,
- skip_email_verification=True
- )
-
- @mock.patch('openedx.features.enterprise_support.pipeline.is_multiple_user_enterprises_feature_enabled')
- @mock.patch('openedx.features.enterprise_support.pipeline.EnterpriseApiClient.post_active_enterprise_customer')
- @mock.patch('openedx.features.enterprise_support.pipeline.get_enterprise_learner_data')
- def test_enterprise_customer_in_session(self, mock_get_enterprise, mock_post_enterprise_customer,
- mocked_multiple_enterprises_feature):
- with mock.patch('third_party_auth.pipeline.provider.Registry.get_from_pipeline') as get_from_pipeline:
- get_from_pipeline.return_value = self.saml_provider
- with mock.patch(
- 'third_party_auth.pipeline.provider.Registry.get_enabled_by_backend_name'
- ) as enabled_saml_providers:
- mocked_multiple_enterprises_feature.return_value = True
- kwargs = {'response': {'idp_name': 'default'}}
- enabled_saml_providers.return_value = [self.saml_provider, ]
- pipeline.set_learner_active_enterprise(self.user, self.backend, self.strategy, **kwargs)
- self.assertFalse(mock_get_enterprise.called)
- self.assertFalse(mock_post_enterprise_customer.called)
-
- @mock.patch('openedx.features.enterprise_support.pipeline.is_multiple_user_enterprises_feature_enabled')
- @mock.patch('openedx.features.enterprise_support.pipeline.EnterpriseApiClient.post_active_enterprise_customer')
- @mock.patch('openedx.features.enterprise_support.pipeline.get_enterprise_learner_data')
- def test_update_enterprise_customer_status(self, mock_get_enterprise, mock_post_enterprise_customer,
- mocked_multiple_enterprises_feature):
- kwargs = {'response': {'idp_name': 'demo'}}
- enterprise_learner_data = [
- {'enterprise_customer': {'uuid': 'ab12', 'identity_provider': 'saml-default'}},
- {'enterprise_customer': {'uuid': 'cd34', 'identity_provider': 'saml-demo'}}
- ]
-
- with mock.patch('third_party_auth.pipeline.provider.Registry.get_from_pipeline') as get_from_pipeline:
- get_from_pipeline.return_value = self.saml_provider
- with mock.patch(
- 'third_party_auth.pipeline.provider.Registry.get_enabled_by_backend_name'
- ) as enabled_saml_providers:
- mocked_multiple_enterprises_feature.return_value = True
- enabled_saml_providers.return_value = [self.saml_provider, ]
- mock_get_enterprise.return_value = enterprise_learner_data
- mock_post_enterprise_customer.return_value = True
- pipeline.set_learner_active_enterprise(self.user, self.backend, self.strategy, **kwargs)
- mock_get_enterprise.assert_called_once()
- mock_post_enterprise_customer.assert_called_once()
- self.assertEqual(self.request.session.get('enterprise_customer'),
- enterprise_learner_data[1]['enterprise_customer'])
-
- @mock.patch('openedx.features.enterprise_support.pipeline.is_multiple_user_enterprises_feature_enabled')
- @mock.patch('openedx.features.enterprise_support.pipeline.EnterpriseApiClient.post_active_enterprise_customer')
- @mock.patch('openedx.features.enterprise_support.pipeline.get_enterprise_learner_data')
- def test_failed_update_enterprise_customer_status(self, mock_get_enterprise, mock_post_enterprise_customer,
- mocked_multiple_enterprises_feature):
- kwargs = {'response': {'idp_name': 'demo-test'}}
- enterprise_learner_data = [
- {'enterprise_customer': {'uuid': 'ab12', 'identity_provider': 'saml-default'}},
- {'enterprise_customer': {'uuid': 'cd34', 'identity_provider': 'saml-demo-test'}}
- ]
-
- with mock.patch('third_party_auth.pipeline.provider.Registry.get_from_pipeline') as get_from_pipeline:
- get_from_pipeline.return_value = self.saml_provider
- with mock.patch(
- 'third_party_auth.pipeline.provider.Registry.get_enabled_by_backend_name'
- ) as enabled_saml_providers:
- mocked_multiple_enterprises_feature.return_value = True
- enabled_saml_providers.return_value = [self.saml_provider, ]
- mock_get_enterprise.return_value = enterprise_learner_data
- mock_post_enterprise_customer.return_value = False
- pipeline.set_learner_active_enterprise(self.user, self.backend, self.strategy, **kwargs)
- mock_get_enterprise.assert_called_once()
- mock_post_enterprise_customer.assert_called_once()
- self.assertNotEqual(self.request.session.get('enterprise_customer'),
- enterprise_learner_data[1]['enterprise_customer'])
-
- @mock.patch('openedx.features.enterprise_support.pipeline.is_multiple_user_enterprises_feature_enabled')
- @mock.patch('openedx.features.enterprise_support.pipeline.EnterpriseApiClient.post_active_enterprise_customer')
- @mock.patch('openedx.features.enterprise_support.pipeline.get_enterprise_learner_data')
- def test_with_one_enterprise_customer(self, mock_get_enterprise, mock_post_enterprise_customer,
- mocked_multiple_enterprises_feature):
- kwargs = {'response': {'idp_name': 'demo-test'}}
- enterprise_learner_data = [
- {'enterprise_customer': {'uuid': 'cd34', 'identity_provider': 'saml-demo-test'}}
- ]
-
- with mock.patch('third_party_auth.pipeline.provider.Registry.get_from_pipeline') as get_from_pipeline:
- get_from_pipeline.return_value = self.saml_provider
- with mock.patch(
- 'third_party_auth.pipeline.provider.Registry.get_enabled_by_backend_name'
- ) as enabled_saml_providers:
- mocked_multiple_enterprises_feature.return_value = True
- enabled_saml_providers.return_value = [self.saml_provider, ]
- mock_get_enterprise.return_value = enterprise_learner_data
- mock_post_enterprise_customer.return_value = False
- pipeline.set_learner_active_enterprise(self.user, self.backend, self.strategy, **kwargs)
- mock_get_enterprise.assert_called_once()
- self.assertFalse(mock_post_enterprise_customer.called)
-
- @mock.patch('openedx.features.enterprise_support.pipeline.is_multiple_user_enterprises_feature_enabled')
- @mock.patch('openedx.features.enterprise_support.pipeline.EnterpriseApiClient.post_active_enterprise_customer')
- @mock.patch('openedx.features.enterprise_support.pipeline.get_enterprise_learner_data')
- def test_with_multiple_user_enterprises_featured_disabled(self, mock_get_enterprise, mock_post_enterprise_customer,
- mocked_multiple_enterprises_feature):
- kwargs = {'response': {'idp_name': 'demo-test'}}
- enterprise_learner_data = [
- {'enterprise_customer': {'uuid': 'cd34', 'identity_provider': 'saml-demo-test'}}
- ]
-
- with mock.patch('third_party_auth.pipeline.provider.Registry.get_from_pipeline') as get_from_pipeline:
- get_from_pipeline.return_value = self.saml_provider
- with mock.patch(
- 'third_party_auth.pipeline.provider.Registry.get_enabled_by_backend_name'
- ) as enabled_saml_providers:
- mocked_multiple_enterprises_feature.return_value = False
- enabled_saml_providers.return_value = [self.saml_provider, ]
- mock_get_enterprise.return_value = enterprise_learner_data
- mock_post_enterprise_customer.return_value = False
- pipeline.set_learner_active_enterprise(self.user, self.backend, self.strategy, **kwargs)
- self.assertFalse(mock_get_enterprise.called)
- self.assertFalse(mock_post_enterprise_customer.called)
--
GitLab