From c53e93c27c759f32311a9b06604c40b62d9412b8 Mon Sep 17 00:00:00 2001 From: Troy Sankey Date: Tue, 3 Mar 2026 20:36:28 -0800 Subject: [PATCH] feat: remove enterprise imports from third_party_auth - Remove enterprise pipeline functions and insert_enterprise_pipeline_elements; enterprise pipeline steps are now injected by the enterprise plugin. - Add SAMLAccountDisconnected signal in SAMLAuth.disconnect() to replace the unlink_enterprise_user_from_idp import (moved to the enterprise plugin). - Added an ADR to help explain the migration. ENT-11566 Co-Authored-By: Claude Opus 4.6 --- common/djangoapps/third_party_auth/apps.py | 10 -- common/djangoapps/third_party_auth/models.py | 4 + .../djangoapps/third_party_auth/pipeline.py | 81 ++----------- common/djangoapps/third_party_auth/saml.py | 21 +++- .../third_party_auth/signals/__init__.py | 6 +- .../third_party_auth/signals/signals.py | 12 ++ .../tests/specs/test_testshib.py | 71 +++-------- .../third_party_auth/tests/test_utils.py | 25 ---- common/djangoapps/third_party_auth/utils.py | 9 -- ...-saml-admin-views-in-enterprise-plugin.rst | 20 ++-- ...rprise-decoupled-from-third-party-auth.rst | 110 ++++++++++++++++++ lms/envs/common.py | 6 +- openedx/features/enterprise_support/api.py | 49 -------- .../enterprise_support/tests/test_api.py | 60 ---------- requirements/constraints.txt | 2 +- requirements/edx/base.txt | 6 +- requirements/edx/development.txt | 3 +- requirements/edx/doc.txt | 6 +- requirements/edx/testing.txt | 6 +- 19 files changed, 200 insertions(+), 307 deletions(-) create mode 100644 common/djangoapps/third_party_auth/signals/signals.py create mode 100644 docs/decisions/0026-enterprise-decoupled-from-third-party-auth.rst diff --git a/common/djangoapps/third_party_auth/apps.py b/common/djangoapps/third_party_auth/apps.py index 5868162043c5..b715611bceb3 100644 --- a/common/djangoapps/third_party_auth/apps.py +++ b/common/djangoapps/third_party_auth/apps.py @@ -1,7 +1,6 @@ # pylint: disable=missing-module-docstring from django.apps import AppConfig -from django.conf import settings class ThirdPartyAuthConfig(AppConfig): # pylint: disable=missing-class-docstring @@ -11,12 +10,3 @@ class ThirdPartyAuthConfig(AppConfig): # pylint: disable=missing-class-docstrin def ready(self): # Import signal handlers to register them from .signals import handlers # noqa: F401 pylint: disable=unused-import - - # Note: Third-party auth settings are now defined statically in lms/envs/common.py - # However, the enterprise pipeline step must be inserted dynamically because - # it requires checking if enterprise is enabled, which can't be done at - # settings load time. - # Only insert enterprise elements if SOCIAL_AUTH_PIPELINE exists (LMS only, not CMS). - if hasattr(settings, 'SOCIAL_AUTH_PIPELINE'): - from openedx.features.enterprise_support.api import insert_enterprise_pipeline_elements - insert_enterprise_pipeline_elements(settings.SOCIAL_AUTH_PIPELINE) diff --git a/common/djangoapps/third_party_auth/models.py b/common/djangoapps/third_party_auth/models.py index f9ebc0a30af9..e68a16b1768b 100644 --- a/common/djangoapps/third_party_auth/models.py +++ b/common/djangoapps/third_party_auth/models.py @@ -237,6 +237,10 @@ class ProviderConfig(ConfigurationModel): help_text="Use the presence of a profile from a trusted third party as proof of identity verification.", ) + # Enterprise-only field: excludes this provider from the EnterpriseCustomer Django admin IDP + # dropdown. Added in ENT-1366 after social auth providers (Facebook, Google, etc.) were linked + # as enterprise IDPs, incorrectly associating all their users with an enterprise. Should ideally + # be migrated into the enterprise plugin. disable_for_enterprise_sso = models.BooleanField( default=False, verbose_name='Disabled for Enterprise TPA', diff --git a/common/djangoapps/third_party_auth/pipeline.py b/common/djangoapps/third_party_auth/pipeline.py index 0a774b6dcc77..757075a61221 100644 --- a/common/djangoapps/third_party_auth/pipeline.py +++ b/common/djangoapps/third_party_auth/pipeline.py @@ -86,10 +86,7 @@ def B(*args, **kwargs): from common.djangoapps.edxmako.shortcuts import render_to_string from common.djangoapps.third_party_auth.utils import ( get_associated_user_by_email_response, - get_user_from_email, - is_enterprise_customer_user, is_oauth_provider, - is_saml_provider, user_exists, ) from common.djangoapps.track import segment @@ -780,80 +777,18 @@ def associate_by_email_if_oauth(auth_entry, backend, details, user, strategy, *a return association_response -@partial.partial def associate_by_email_if_saml(auth_entry, backend, details, user, strategy, *args, **kwargs): """ - This pipeline step associates the current social auth with the user with the - same email address in the database. It defers to the social library's associate_by_email - implementation, which verifies that only a single database user is associated with the email. + Deprecated — enterprise SAML email association moved to + enterprise.tpa_pipeline.enterprise_associate_by_email. - This association is done ONLY if the user entered the pipeline belongs to SAML provider. + Retained as a no-op for backwards compatibility with custom pipeline configs. """ - from openedx.features.enterprise_support.api import enterprise_is_enabled - - def get_user(): - """ - This is the helper method to get the user from system by matching email. - """ - user_details = {'email': details.get('email')} if details else None - return get_user_from_email(user_details or {}) - - @enterprise_is_enabled() - def associate_by_email_if_enterprise_user(): - """ - If the learner arriving via SAML is already linked to the enterprise customer linked to the same IdP, - they should not be prompted for their edX password. - """ - try: - enterprise_customer_user = is_enterprise_customer_user(current_provider.provider_id, current_user) - logger.info( - '[Multiple_SSO_SAML_Accounts_Association_to_User] Enterprise user verification:' # noqa: UP032 - 'User Email: {email}, User ID: {user_id}, Provider ID: {provider_id},' - ' is_enterprise_customer_user: {enterprise_customer_user}'.format( - email=current_user.email, - user_id=current_user.id, - provider_id=current_provider.provider_id, - enterprise_customer_user=enterprise_customer_user, - ) - ) - - if enterprise_customer_user: - # this is python social auth pipeline default method to automatically associate social accounts - # if the email already matches a user account. - association_response, user_is_active = get_associated_user_by_email_response( - backend, details, user, *args, **kwargs) - - if not user_is_active: - logger.info( - '[Multiple_SSO_SAML_Accounts_Association_to_User] User association account is not' # noqa: UP032 # pylint: disable=line-too-long - ' active: User Email: {email}, User ID: {user_id}, Provider ID: {provider_id},' - ' is_enterprise_customer_user: {enterprise_customer_user}'.format( - email=current_user.email, - user_id=current_user.id, - provider_id=current_provider.provider_id, - enterprise_customer_user=enterprise_customer_user - ) - ) - return None - - return association_response - - except Exception as ex: # pylint: disable=broad-except - logger.exception('[Multiple_SSO_SAML_Accounts_Association_to_User] Error in' - ' saml multiple accounts association: User ID: %s, User Email: %s:,' - 'Provider ID: %s, Exception: %s', current_user.id, current_user.email, - current_provider.provider_id, ex) - - saml_provider, current_provider = is_saml_provider(strategy.request.backend.name, kwargs) - - if saml_provider: - # get the user by matching email if the pipeline user is not available. - current_user = user if user else get_user() - - # Verify that the user linked to enterprise customer of current identity provider and an active user - associate_response = associate_by_email_if_enterprise_user() if current_user else None - if associate_response: - return associate_response + logger.warning( + "associate_by_email_if_saml is deprecated and is now a no-op. " + "Enterprise SAML email association has moved to " + "enterprise.tpa_pipeline.enterprise_associate_by_email." + ) def user_details_force_sync(auth_entry, strategy, details, user=None, *args, **kwargs): # pylint: disable=keyword-arg-before-vararg diff --git a/common/djangoapps/third_party_auth/saml.py b/common/djangoapps/third_party_auth/saml.py index 19411dd13f58..572360e8e84e 100644 --- a/common/djangoapps/third_party_auth/saml.py +++ b/common/djangoapps/third_party_auth/saml.py @@ -141,11 +141,22 @@ def auth_url(self): def disconnect(self, *args, **kwargs): """ - Override of SAMLAuth.disconnect to unlink the learner from enterprise customer if associated. - """ - from openedx.features.enterprise_support.api import unlink_enterprise_user_from_idp - user = kwargs.get('user', None) - unlink_enterprise_user_from_idp(self.strategy.request, user, self.name) + Override of SAMLAuth.disconnect to emit a signal when a user disconnects their SAML account. + """ + from common.djangoapps.third_party_auth.signals import SAMLAccountDisconnected + # Emit the signal before super().disconnect() so that handlers (e.g. enterprise + # user unlinking) run while the social auth record still exists. + user = kwargs['user'] # Upstream social_core always passes a non-None user. + log.info( + '[THIRD_PARTY_AUTH] Emitting SAMLAccountDisconnected signal for user_id=%s, backend=%s', + user.id, self.name, + ) + SAMLAccountDisconnected.send( + sender=self.__class__, + request=self.strategy.request, + user=user, + saml_backend=self, + ) return super().disconnect(*args, **kwargs) def _check_entitlements(self, idp, attributes): diff --git a/common/djangoapps/third_party_auth/signals/__init__.py b/common/djangoapps/third_party_auth/signals/__init__.py index cf255a847f6c..50b81f139d9a 100644 --- a/common/djangoapps/third_party_auth/signals/__init__.py +++ b/common/djangoapps/third_party_auth/signals/__init__.py @@ -1 +1,5 @@ -# Signal handlers for third_party_auth app +""" +Signals and signal handlers for third_party_auth app +""" + +from common.djangoapps.third_party_auth.signals.signals import SAMLAccountDisconnected # noqa: F401 diff --git a/common/djangoapps/third_party_auth/signals/signals.py b/common/djangoapps/third_party_auth/signals/signals.py new file mode 100644 index 000000000000..6925c9d896ab --- /dev/null +++ b/common/djangoapps/third_party_auth/signals/signals.py @@ -0,0 +1,12 @@ +""" +Signals defined by the third_party_auth app. +""" +from django.dispatch import Signal + +# Signal fired when a user disconnects a SAML account. +# +# Keyword arguments sent with this signal: +# request (HttpRequest): The HTTP request during which the disconnect occurred. +# user (User): The Django User disconnecting the social auth account. +# saml_backend (social_core.backends.saml.SAMLAuth): The SAMLAuth backend instance (has a ``name`` attribute). +SAMLAccountDisconnected = Signal() diff --git a/common/djangoapps/third_party_auth/tests/specs/test_testshib.py b/common/djangoapps/third_party_auth/tests/specs/test_testshib.py index 27976de43908..eddc4dd9c514 100644 --- a/common/djangoapps/third_party_auth/tests/specs/test_testshib.py +++ b/common/djangoapps/third_party_auth/tests/specs/test_testshib.py @@ -12,9 +12,7 @@ import ddt import httpretty -from django.conf import settings from django.contrib import auth -from enterprise.models import EnterpriseCustomerIdentityProvider, EnterpriseCustomerUser from freezegun import freeze_time from social_core import actions from social_django import views as social_views @@ -25,11 +23,11 @@ from common.djangoapps.third_party_auth.exceptions import IncorrectConfigurationException from common.djangoapps.third_party_auth.saml import SapSuccessFactorsIdentityProvider from common.djangoapps.third_party_auth.saml import log as saml_log +from common.djangoapps.third_party_auth.signals import SAMLAccountDisconnected from common.djangoapps.third_party_auth.tasks import fetch_saml_metadata from common.djangoapps.third_party_auth.tests import testutil, utils from common.test.utils import assert_dict_contains_subset from openedx.core.djangoapps.user_authn.views.login import login_user -from openedx.features.enterprise_support.tests.factories import EnterpriseCustomerFactory from .base import IntegrationTestMixin @@ -220,13 +218,7 @@ class TestShibIntegrationTest(SamlIntegrationTestUtilities, IntegrationTestMixin "session_index": "1", } - @patch("openedx.features.enterprise_support.api.enterprise_customer_for_request") - @patch("openedx.features.enterprise_support.utils.third_party_auth.provider.Registry.get") - def test_full_pipeline_succeeds_for_unlinking_testshib_account( - self, - mock_auth_provider, - mock_enterprise_customer_for_request, - ): + def test_full_pipeline_succeeds_for_unlinking_testshib_account(self): # First, create, the request and strategy that store pipeline state, # configure the backend, and mock out wire traffic. @@ -245,30 +237,6 @@ def test_full_pipeline_succeeds_for_unlinking_testshib_account( # We're already logged in, so simulate that the cookie is set correctly self.set_logged_in_cookies(request) - # linking a learner with enterprise customer. - enterprise_customer = EnterpriseCustomerFactory() - assert EnterpriseCustomerUser.objects.count() == 0, "Precondition check: no link records should exist" - EnterpriseCustomerUser.objects.link_user(enterprise_customer, user.email) - assert ( - EnterpriseCustomerUser.objects.filter(enterprise_customer=enterprise_customer, user_id=user.id).count() == 1 - ) - EnterpriseCustomerIdentityProvider.objects.get_or_create( - enterprise_customer=enterprise_customer, provider_id=self.provider.provider_id - ) - - enterprise_customer_data = { - "uuid": enterprise_customer.uuid, - "name": enterprise_customer.name, - "identity_provider": "saml-default", - "identity_providers": [ - { - "provider_id": "saml-default", - } - ], - } - mock_auth_provider.return_value.backend_name = "tpa-saml" - mock_enterprise_customer_for_request.return_value = enterprise_customer_data - # Instrument the pipeline to get to the dashboard with the full expected state. self.client.get(pipeline.get_login_url(self.provider.provider_id, pipeline.AUTH_ENTRY_LOGIN)) @@ -285,31 +253,30 @@ def test_full_pipeline_succeeds_for_unlinking_testshib_account( # First we expect that we're in the linked state, with a backend entry. self.assert_social_auth_exists_for_user(request.user, strategy) - FEATURES_WITH_ENTERPRISE_ENABLED = settings.FEATURES.copy() - FEATURES_WITH_ENTERPRISE_ENABLED["ENABLE_ENTERPRISE_INTEGRATION"] = True - with patch.dict("django.conf.settings.FEATURES", FEATURES_WITH_ENTERPRISE_ENABLED): - # Fire off the disconnect pipeline without the user information. - actions.do_disconnect( - request.backend, None, None, redirect_field_name=auth.REDIRECT_FIELD_NAME, request=request - ) - assert ( - EnterpriseCustomerUser.objects.filter(enterprise_customer=enterprise_customer, user_id=user.id).count() - != 0 - ) + # Track signal emissions to verify the disconnect signal is sent. + signal_calls = [] + def signal_receiver(sender, **kwargs): + signal_calls.append(kwargs) + + SAMLAccountDisconnected.connect(signal_receiver) + try: # Fire off the disconnect pipeline to unlink. self.assert_redirect_after_pipeline_completes( actions.do_disconnect( request.backend, user, None, redirect_field_name=auth.REDIRECT_FIELD_NAME, request=request ) ) - # Now we expect to be in the unlinked state, with no backend entry. - self.assert_third_party_accounts_state(request, linked=False) - self.assert_social_auth_does_not_exist_for_user(user, strategy) - assert ( - EnterpriseCustomerUser.objects.filter(enterprise_customer=enterprise_customer, user_id=user.id).count() - == 0 - ) + finally: + SAMLAccountDisconnected.disconnect(signal_receiver) + + # Now we expect to be in the unlinked state, with no backend entry. + self.assert_third_party_accounts_state(request, linked=False) + self.assert_social_auth_does_not_exist_for_user(user, strategy) + # Verify that the SAMLAccountDisconnected signal was emitted. + # The actual enterprise user unlinking is handled by edx-enterprise's + # signal handler, not by openedx-platform. + assert len(signal_calls) == 1, f"Expected 1 signal emission, got {len(signal_calls)}" def get_response_data(self): """Gets dict (string -> object) of merged data about the user.""" diff --git a/common/djangoapps/third_party_auth/tests/test_utils.py b/common/djangoapps/third_party_auth/tests/test_utils.py index f4aca74826b4..9571f4834f31 100644 --- a/common/djangoapps/third_party_auth/tests/test_utils.py +++ b/common/djangoapps/third_party_auth/tests/test_utils.py @@ -18,17 +18,12 @@ create_or_update_bulk_saml_provider_data, get_associated_user_by_email_response, get_user_from_email, - is_enterprise_customer_user, is_oauth_provider, parse_metadata_xml, user_exists, validate_saml_metadata_url, ) from openedx.core.djangolib.testing.utils import skip_unless_lms -from openedx.features.enterprise_support.tests.factories import ( - EnterpriseCustomerIdentityProviderFactory, - EnterpriseCustomerUserFactory, -) @ddt.ddt @@ -59,26 +54,6 @@ def test_get_user(self): assert get_user_from_email({'email': 'test_user@example.com'}) assert not get_user_from_email({'email': 'invalid@example.com'}) - def test_is_enterprise_customer_user(self): - """ - Verify that if user is an enterprise learner. - """ - # Create users from factory - - user = UserFactory(username='test_user', email='test_user@example.com') - other_user = UserFactory(username='other_user', email='other_user@example.com') - customer_idp = EnterpriseCustomerIdentityProviderFactory.create( - provider_id='the-provider', - ) - customer = customer_idp.enterprise_customer - EnterpriseCustomerUserFactory.create( - enterprise_customer=customer, - user_id=user.id, - ) - - assert is_enterprise_customer_user('the-provider', user) - assert not is_enterprise_customer_user('the-provider', other_user) - @ddt.data( ('saml-farkle', False), ('oa2-fergus', True), diff --git a/common/djangoapps/third_party_auth/utils.py b/common/djangoapps/third_party_auth/utils.py index 7a520f47ccdd..27544eea0efa 100644 --- a/common/djangoapps/third_party_auth/utils.py +++ b/common/djangoapps/third_party_auth/utils.py @@ -11,7 +11,6 @@ from django.conf import settings from django.contrib.auth.models import User # pylint: disable=imported-auth-user from django.utils.timezone import now -from enterprise.models import EnterpriseCustomerIdentityProvider, EnterpriseCustomerUser from lxml import etree from onelogin.saml2.utils import OneLogin_Saml2_Utils from social_core.pipeline.social_auth import associate_by_email @@ -228,14 +227,6 @@ def is_saml_provider(backend, kwargs): current_provider.slug in [saml_provider.slug for saml_provider in saml_providers_list]), current_provider -def is_enterprise_customer_user(provider_id, user): - """ Verify that the user linked to enterprise customer of current identity provider""" - enterprise_idp = EnterpriseCustomerIdentityProvider.objects.get(provider_id=provider_id) - - return EnterpriseCustomerUser.objects.filter(enterprise_customer=enterprise_idp.enterprise_customer, - user_id=user.id).exists() - - def is_oauth_provider(backend_name, **kwargs): """ Verify that the third party provider uses oauth diff --git a/docs/decisions/0025-saml-admin-views-in-enterprise-plugin.rst b/docs/decisions/0025-saml-admin-views-in-enterprise-plugin.rst index f3cfae4f5e3f..98e0e98da0a4 100644 --- a/docs/decisions/0025-saml-admin-views-in-enterprise-plugin.rst +++ b/docs/decisions/0025-saml-admin-views-in-enterprise-plugin.rst @@ -40,12 +40,11 @@ edx-enterprise repository: * ``enterprise/api/v1/views/saml_provider_config.py`` * ``enterprise/api/v1/views/saml_provider_data.py`` -They will be registered in ``enterprise/api/v1/urls.py`` under the same -``auth/saml/v0/`` prefix, so the API contract is preserved for existing clients. +They will be registered under the same ``auth/saml/v0/`` prefix, so the API +contract is preserved for existing clients. The underlying ``SAMLProviderConfig``, ``SAMLProviderData``, and -``SAMLConfiguration`` models remain in -``common/djangoapps/third_party_auth/models.py`` because they are +``SAMLConfiguration`` models remain in openedx-platform because they are general-purpose SAML models used by the platform's authentication layer regardless of whether the enterprise plugin is installed. @@ -58,10 +57,9 @@ Consequences * **Dependency direction**: edx-enterprise now imports ``SAMLProviderConfig``, ``SAMLProviderData``, and several utility functions from - ``common.djangoapps.third_party_auth``. This is the correct dependency - direction (plugin depends on platform, not the reverse). - -* **Deployments without edx-enterprise**: The SAML admin API endpoints will not - be available in deployments that do not install edx-enterprise. This is - acceptable because the endpoints ONLY serve frontend-app-admin-portal - which doesn't work without edx-enterprise installed anyway. + ``common.djangoapps.third_party_auth``. This makes the plugin more sensitive to + platform code changes in these models/functions. But this isn't worse than + before, which was just the same problem in the opposite direction. Ideally + there'd be a cleaner and more stable set of hooks into TPA that enterprise + can leverage, but that's a major enhancement which is not in scope for this + phase of the migration work. diff --git a/docs/decisions/0026-enterprise-decoupled-from-third-party-auth.rst b/docs/decisions/0026-enterprise-decoupled-from-third-party-auth.rst new file mode 100644 index 000000000000..98e89dbf6e18 --- /dev/null +++ b/docs/decisions/0026-enterprise-decoupled-from-third-party-auth.rst @@ -0,0 +1,110 @@ +Enterprise Decoupled from Third-Party Auth +########################################## + +**Status**: Accepted +**Date**: 2026-04-16 + +----- + +Context +******* + +The ``third_party_auth`` Django app in openedx-platform has several coupling +points with enterprise-specific logic: + +* Enterprise-specific TPA pipeline steps are injected into the social auth + pipeline, including: + + - ``associate_by_email_if_saml``: resolves which existing LMS account should + own the incoming SSO identity, matching by email only when the existing + user is an ``EnterpriseCustomerUser`` for the current provider's + enterprise. Runs during the user-association phase, before the + ``create_user`` step. + + - Note: this step name was misleading because the logic was always + enterprise-specific but the name lacked "enterprise". It was a no-op when + the enterprise integration was disabled. + + - ``handle_enterprise_logistration``: establishes and manages the + enterprise relationship for the now-authenticated user, creating or + updating the ``EnterpriseCustomerUser`` record among other things. Runs + during the post-authentication phase (after user creation and social-auth + linking). + + - Note: this step was already migrated to enterprise, but it was still + imported/injected into the pipeline via ``third_party_auth`` app code. + +* ``is_enterprise_customer_user`` utility function which queries enterprise models. + +* SAML account disconnection triggers enterprise-specific cleanup to remove the + identity provider link. + +* The ``disable_for_enterprise_sso`` boolean field on ``OAuth2ProviderConfig`` + which exists solely to exclude social auth providers (e.g. Facebook, Google) + from the ``EnterpriseCustomer`` Django admin IDP dropdown. + +All these unconditional enterprise imports contribute to the reason +``edx-enterprise`` is a mandatory pip dependency for any openedx deployment. +As part of the broader effort to make ``edx-enterprise`` optional, all coupling +points must be removed from the ``third_party_auth`` Django app. + +Decision +******** + +The enterprise-specific logic is removed from ``third_party_auth`` and migrated +to the ``enterprise`` plugin: + +**Pipeline injection migrated** + +Enterprise pipeline steps are no longer injected at TPA app startup by +importing enterprise code. Instead, the ``enterprise`` plugin registers the +enterprise-specific steps itself (via its own ``plugin_settings()``). + +**associate_by_email pipeline step migrated** + +The platform-side ``common.djangoapps.third_party_auth.pipeline.associate_by_email_if_saml`` +pipeline step becomes a no-op for backwards compatibility with custom +pipeline configs. The actual step is migrated to the ``enterprise`` +plugin under a more accurate name (``enterprise.tpa_pipeline.enterprise_associate_by_email``). + +**SAML disconnect via Django signal** + +A new ``SAMLAccountDisconnected`` Django signal replaces the direct call +to enterprise unlink logic during SAML account disconnection. The +``enterprise`` plugin now handles the signal to perform the enterprise-specific +cleanup. + +Consequences +************ + +* **New signal contract**: The ``SAMLAccountDisconnected`` signal is a new + public contract between the platform and plugins. Its keyword arguments must + be treated as stable once released. + + - It's generally understood by the community that signal signatures are + treated as stable, and changes must be made with caution. + +* **Pipeline injection is sensitive to platform changes**: Enterprise pipeline + steps are injected relative to existing platform-defined steps in + ``SOCIAL_AUTH_PIPELINE``. If the platform renames, reorders, or removes steps + that the plugin keys off of, the injection will break. Platform maintainers + should continue to treat the pipeline step names as a stable interface. + + - To mitigate silent failures, the plugin raises a fatal exception blocking + django startup if step injection fails. + +* **``disable_for_enterprise_sso`` field left in platform**: The + ``disable_for_enterprise_sso`` field on ``OAuth2ProviderConfig`` remains in + the Third-Party Auth app for now. Leaving it does not block the immediate goal + of making ``edx-enterprise`` an optional pip dependency. It should ideally be + migrated into the enterprise plugin in the future, but its presence has + minimal impact on installations that do not use enterprise functionality — + it simply appears as an unused boolean on the provider config. + +References +********** + +* ADR 0025 (``0025-saml-admin-views-in-enterprise-plugin.rst``) — Related migration of SAML admin viewsets to edx-enterprise. +* OpenEdX forum thread announcing the pluginification of enterprise: https://discuss.openedx.org/t/rfc-pluginifying-edx-enterprise-looking-for-community-interest-collaborators/18316 +* ENT-11418 — 2U JIRA initiative for the overall pluginification of enterprise. +* ENT-11566 — 2U JIRA ticket for this specific migration. diff --git a/lms/envs/common.py b/lms/envs/common.py index 3ec166552166..55d432d155ca 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -339,9 +339,8 @@ # Social auth pipeline for third-party authentication. # Operators can override SOCIAL_AUTH_PIPELINE directly in their settings # to customize the pipeline. -# Note: The enterprise step (handle_enterprise_logistration) is inserted dynamically -# during app initialization by third_party_auth's AppConfig.ready() if enterprise -# is enabled. It cannot be included statically because it requires runtime checks. +# Note: Enterprise pipeline steps (enterprise_associate_by_email, handle_enterprise_logistration) +# are inserted dynamically via enterprise/settings/common.py plugin_settings(). SOCIAL_AUTH_PIPELINE = [ 'common.djangoapps.third_party_auth.pipeline.parse_query_params', 'social_core.pipeline.social_auth.social_details', @@ -349,7 +348,6 @@ 'social_core.pipeline.social_auth.auth_allowed', 'social_core.pipeline.social_auth.social_user', 'common.djangoapps.third_party_auth.pipeline.associate_by_email_if_login_api', - 'common.djangoapps.third_party_auth.pipeline.associate_by_email_if_saml', 'common.djangoapps.third_party_auth.pipeline.associate_by_email_if_oauth', 'common.djangoapps.third_party_auth.pipeline.get_username', 'common.djangoapps.third_party_auth.pipeline.set_pipeline_timeout', diff --git a/openedx/features/enterprise_support/api.py b/openedx/features/enterprise_support/api.py index 469f8d103324..ea497164b103 100644 --- a/openedx/features/enterprise_support/api.py +++ b/openedx/features/enterprise_support/api.py @@ -39,9 +39,7 @@ from enterprise.models import ( EnterpriseCourseEnrollment, EnterpriseCustomer, - EnterpriseCustomerIdentityProvider, EnterpriseCustomerUser, - PendingEnterpriseCustomerUser, ) except ImportError: # pragma: no cover pass @@ -1026,50 +1024,3 @@ def get_dashboard_consent_notification(request, user, course_enrollments): } ) return '' - - -@enterprise_is_enabled() -def insert_enterprise_pipeline_elements(pipeline): - """ - If the enterprise app is enabled, insert additional elements into the - pipeline related to enterprise. - """ - additional_elements = ( - 'enterprise.tpa_pipeline.handle_enterprise_logistration', - ) - - insert_point = pipeline.index('social_core.pipeline.social_auth.load_extra_data') - for index, element in enumerate(additional_elements): - pipeline.insert(insert_point + index, element) - - -@enterprise_is_enabled() -def unlink_enterprise_user_from_idp(request, user, idp_backend_name): - """ - Un-links learner from their enterprise identity provider - Args: - request (wsgi request): request object - user (User): user who initiated disconnect request - idp_backend_name (str): Name of identity provider's backend - - Returns: None - - """ - enterprise_customer = enterprise_customer_for_request(request) - if user and enterprise_customer: - enabled_providers = Registry.get_enabled_by_backend_name(idp_backend_name) - provider_ids = [enabled_provider.provider_id for enabled_provider in enabled_providers] - enterprise_customer_idps = EnterpriseCustomerIdentityProvider.objects.filter( - enterprise_customer__uuid=enterprise_customer['uuid'], - provider_id__in=provider_ids - ) - - if enterprise_customer_idps: - try: - # Unlink user email from each Enterprise Customer. - for enterprise_customer_idp in enterprise_customer_idps: - EnterpriseCustomerUser.objects.unlink_user( - enterprise_customer=enterprise_customer_idp.enterprise_customer, user_email=user.email - ) - except (EnterpriseCustomerUser.DoesNotExist, PendingEnterpriseCustomerUser.DoesNotExist): - pass diff --git a/openedx/features/enterprise_support/tests/test_api.py b/openedx/features/enterprise_support/tests/test_api.py index aa90f5c5f79b..444782eddd01 100644 --- a/openedx/features/enterprise_support/tests/test_api.py +++ b/openedx/features/enterprise_support/tests/test_api.py @@ -14,7 +14,6 @@ from django.test.utils import override_settings from django.urls import reverse from edx_django_utils.cache import TieredCache, get_cache_key -from enterprise.models import EnterpriseCustomerUser # pylint: disable=wrong-import-order from requests.exceptions import HTTPError from six.moves.urllib.parse import parse_qs from testfixtures import LogCapture @@ -50,8 +49,6 @@ get_enterprise_learner_data_from_api, get_enterprise_learner_data_from_db, get_enterprise_learner_portal_enabled_message, - 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 ( @@ -1075,21 +1072,6 @@ def test_utils_with_enterprise_disabled(self): the utilities to return the expected default values. """ assert not enterprise_enabled() - assert insert_enterprise_pipeline_elements(None) is None - - def test_utils_with_enterprise_enabled(self): - """ - Test that enabling enterprise integration (which is currently on by default) causes the - the utilities to return the expected values. - """ - assert enterprise_enabled() - pipeline = ['abc', 'social_core.pipeline.social_auth.load_extra_data', 'def'] - insert_enterprise_pipeline_elements(pipeline) - assert pipeline == \ - [ - 'abc', 'enterprise.tpa_pipeline.handle_enterprise_logistration', - 'social_core.pipeline.social_auth.load_extra_data', 'def' - ] @mock.patch('openedx.features.enterprise_support.api.get_enterprise_learner_data_from_db') def test_enterprise_customer_from_session_or_db_cache_miss_no_customer(self, mock_learner_data_from_db): @@ -1363,45 +1345,3 @@ def test_get_consent_notification_data(self, mock_override_model): 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( # pylint: disable=unused-variable # noqa: F841 - 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() diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 775df3b74554..88be0f00d1d2 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -44,7 +44,7 @@ django-stubs<6 # The team that owns this package will manually bump this package rather than having it pulled in automatically. # This is to allow them to better control its deployment and to do it in a process that works better # for them. -edx-enterprise==8.0.2 +edx-enterprise==8.0.8 # Date: 2023-07-26 # Our legacy Sass code is incompatible with anything except this ancient libsass version. diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 23755b9bae59..6edfe6a965cf 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -483,7 +483,7 @@ edx-drf-extensions==10.6.0 # enterprise-integrated-channels # openedx-authz # openedx-core -edx-enterprise==8.0.2 +edx-enterprise==8.0.8 # via # -c requirements/constraints.txt # -r requirements/edx/kernel.in @@ -1024,7 +1024,9 @@ python3-openid==3.2.0 ; python_version >= "3" # -r requirements/edx/kernel.in # social-auth-core python3-saml==1.16.0 - # via -r requirements/edx/kernel.in + # via + # -r requirements/edx/kernel.in + # edx-enterprise pytz==2026.1.post1 # via # -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index a1a15309e124..ad9e5362e9f1 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -756,7 +756,7 @@ edx-drf-extensions==10.6.0 # enterprise-integrated-channels # openedx-authz # openedx-core -edx-enterprise==8.0.2 +edx-enterprise==8.0.8 # via # -c requirements/constraints.txt # -r requirements/edx/doc.txt @@ -1788,6 +1788,7 @@ python3-saml==1.16.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt + # edx-enterprise pytz==2026.1.post1 # via # -r requirements/edx/doc.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 017783484811..436da015e558 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -573,7 +573,7 @@ edx-drf-extensions==10.6.0 # enterprise-integrated-channels # openedx-authz # openedx-core -edx-enterprise==8.0.2 +edx-enterprise==8.0.8 # via # -c requirements/constraints.txt # -r requirements/edx/base.txt @@ -1254,7 +1254,9 @@ python3-openid==3.2.0 ; python_version >= "3" # -r requirements/edx/base.txt # social-auth-core python3-saml==1.16.0 - # via -r requirements/edx/base.txt + # via + # -r requirements/edx/base.txt + # edx-enterprise pytz==2026.1.post1 # via # -r requirements/edx/base.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index c1c08c34693b..220c356afda9 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -589,7 +589,7 @@ edx-drf-extensions==10.6.0 # enterprise-integrated-channels # openedx-authz # openedx-core -edx-enterprise==8.0.2 +edx-enterprise==8.0.8 # via # -c requirements/constraints.txt # -r requirements/edx/base.txt @@ -1366,7 +1366,9 @@ python3-openid==3.2.0 ; python_version >= "3" # -r requirements/edx/base.txt # social-auth-core python3-saml==1.16.0 - # via -r requirements/edx/base.txt + # via + # -r requirements/edx/base.txt + # edx-enterprise pytz==2026.1.post1 # via # -r requirements/edx/base.txt