Merge branch 'master' into pwnage101/ENT-11510

kiram15 · web-flow · commit 797bdc0f9ea8 · 2026-04-01T16:27:45.000Z
diff --git a/.gitignore b/.gitignore
@@ -97,6 +97,7 @@ venv/
 #enterprise/static/enterprise/bundles/
 
 # claude code personalization
+CLAUDE.md
 .claude/settings.local.json
 
 # Ralph
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -21,6 +21,10 @@ Unreleased
 ---------------------
 * feat: add AccountSettingsEnterpriseReadOnlyFieldsStep pipeline step (ENT-11510)
 
+[6.8.4] - 2026-03-31
+--------------------
+* fix: hard delete customer admin records from API
+
 [6.8.3] - 2026-03-27
 ---------------------
 * fix: Move settings reads out of AppConfig, into consumers
diff --git a/enterprise/api/utils.py b/enterprise/api/utils.py
@@ -14,7 +14,6 @@
 from enterprise.constants import (
     BRAZE_ADMIN_INVITE_CAMPAIGN_SETTING,
     BRAZE_LEARNER_INVITE_CAMPAIGN_SETTING,
-    ENTERPRISE_ADMIN_ROLE,
     ENTERPRISE_CATALOG_ADMIN_ROLE,
     ENTERPRISE_DASHBOARD_ADMIN_ROLE,
     ENTERPRISE_REPORTING_CONFIG_ADMIN_ROLE,
@@ -31,7 +30,6 @@
     EnterpriseFeatureUserRoleAssignment,
     EnterpriseGroup,
     PendingEnterpriseCustomerAdminUser,
-    SystemWideEnterpriseUserRoleAssignment,
 )
 from enterprise.tasks import send_enterprise_admin_invite_email
 
@@ -45,13 +43,12 @@ def get_existing_admin_emails(enterprise_customer: EnterpriseCustomer) -> Set[st
     Only includes admins who have:
     1. An EnterpriseCustomerAdmin record
     2. An active EnterpriseCustomerUser (active=True)
-    3. An active admin role assignment in SystemWideEnterpriseUserRoleAssignment
 
     Args:
         enterprise_customer: The enterprise customer instance.
 
     Returns:
-        Set of lowercased email addresses of active admins with valid role assignments.
+        Set of lowercased email addresses of active admins.
 
     Raises:
         DatabaseError: If database query fails.
@@ -62,20 +59,12 @@ def get_existing_admin_emails(enterprise_customer: EnterpriseCustomer) -> Set[st
         True
     """
     try:
-        # Get user IDs with active admin role assignments
-        users_with_admin_role = set(
-            SystemWideEnterpriseUserRoleAssignment.objects.filter(
-                enterprise_customer=enterprise_customer,
-                role__name=ENTERPRISE_ADMIN_ROLE,
-            ).values_list('user_id', flat=True)
-        )
-
-        # Return emails of admins who have active ECU AND active role assignment
+        # Return emails of admins who have active ECU
+        # Roles are not considered in customer admin lookup
         return set(
             EnterpriseCustomerAdmin.objects.filter(
                 enterprise_customer_user__enterprise_customer=enterprise_customer,
-                enterprise_customer_user__active=True,
-                enterprise_customer_user__user_id__in=users_with_admin_role,
+                enterprise_customer_user__active=True
             )
             .annotate(email_l=Lower(F("enterprise_customer_user__user_fk__email")))
             .values_list("email_l", flat=True)
diff --git a/enterprise/api/v1/views/enterprise_customer_admin.py b/enterprise/api/v1/views/enterprise_customer_admin.py
@@ -490,6 +490,14 @@ def _delete_active_admin(self, enterprise_customer_uuid, admin_record_id):
             enterprise_customer.uuid
         )
 
+        # Hard-delete the EnterpriseCustomerAdmin record
+        admin_record.delete()
+        logger.info(
+            "Hard deleted EnterpriseCustomerAdmin for EnterpriseCustomerUser id=%s in enterprise %s",
+            enterprise_customer_user.id,
+            enterprise_customer.uuid
+        )
+
         # Check if user has other roles for this enterprise
         # No locking needed - we're only checking existence, and ECU is already locked
         has_other_roles_in_enterprise = models.SystemWideEnterpriseUserRoleAssignment.objects.filter(
diff --git a/enterprise/filters/accounts.py b/enterprise/filters/accounts.py
@@ -32,6 +32,14 @@ def run_filter(self, readonly_fields, user):  # pylint: disable=arguments-differ
         """
         Add enterprise SSO-managed fields to the read-only fields set.
 
+        The original code migrated from openedx-platform can be distilled into 3 logical branches:
+
+        1. If NO identify provider (IdP) has sync enabled → no readonly fields added.
+        2. If one or more IdPs have sync enabled, AND user has social auth → append ALL readonly fields.
+        3. If one or more IdPs have sync enabled, AND user has NO social auth → append readonly fields MINUS 'name'.
+
+        Each return statement below is marked with the corresponding branch number.
+
         Arguments:
             readonly_fields (set): current set of read-only account field names.
             user (User): the Django User whose account settings are being updated.
@@ -46,35 +54,52 @@ def run_filter(self, readonly_fields, user):  # pylint: disable=arguments-differ
             .first()
         )
         if not enterprise_customer_user:
+            # Logical branch #1 (early exit)
             return {"readonly_fields": readonly_fields, "user": user}
 
         enterprise_customer = enterprise_customer_user.enterprise_customer
 
-        idp_qs = EnterpriseCustomerIdentityProvider.objects.filter(
-            enterprise_customer=enterprise_customer
+        idp_records = list(
+            EnterpriseCustomerIdentityProvider.objects
+            .filter(enterprise_customer=enterprise_customer)
         )
 
-        # look for default provider in case of multiple, or default to first
-        idp_record = idp_qs.filter(default_provider=True).first() or idp_qs.first()
-        if not idp_record:
-            return {"readonly_fields": readonly_fields, "user": user}
+        # Track whether any IdP for the customer is configured to sync learner profile data. If none are, then we can
+        # safely allow all fields to be editable since they won't get overwritten by the sync process
+        sync_learner_profile_data = False
 
-        identity_provider = third_party_auth.provider.Registry.get(
-            provider_id=idp_record.provider_id
-        )
+        # Accumulate a list of all identity providers for the customer. If the learner does NOT have any social auth
+        # account configured with these backends, then we can safely allow them to edit the 'name' field (full name)
+        provider_backend_names = []
+
+        for idp in idp_records:
+            identity_provider = third_party_auth.provider.Registry.get(
+                provider_id=idp.provider_id
+            )
+            if identity_provider and getattr(identity_provider, 'sync_learner_profile_data', False):
+                sync_learner_profile_data = True
+
+            backend_name = getattr(identity_provider, 'backend_name', None)
+            if backend_name:
+                provider_backend_names.append(backend_name)
 
-        if not identity_provider or not getattr(identity_provider, 'sync_learner_profile_data', False):
+        # If none of the IdPs for the customer are configured to sync, allow the fields to be editable
+        if not sync_learner_profile_data:
+            # Logical branch #1
             return {"readonly_fields": readonly_fields, "user": user}
 
+        # Determine if the learner has social auth configured.
+        has_social_auth = False
+        if provider_backend_names:
+            has_social_auth = UserSocialAuth.objects.filter(
+                provider__in=provider_backend_names, user=user
+            ).exists()
+
         enterprise_readonly = set(getattr(settings, 'ENTERPRISE_READONLY_ACCOUNT_FIELDS', []))
 
-        if 'name' in enterprise_readonly:
-            backend_name = getattr(identity_provider, 'backend_name', None)
-            has_social_auth = (
-                backend_name
-                and UserSocialAuth.objects.filter(provider=backend_name, user=user).exists()
-            )
-            if not has_social_auth:
-                enterprise_readonly = enterprise_readonly - {'name'}
+        # If the learner does NOT have social auth configured, then at least allow them to edit their name.
+        if not has_social_auth:
+            enterprise_readonly = enterprise_readonly - {'name'}
 
+        # Logical branch #2 and #3
         return {"readonly_fields": readonly_fields | enterprise_readonly, "user": user}
diff --git a/requirements/base.in b/requirements/base.in
@@ -42,6 +42,7 @@ requests
 rules
 slumber
 snowflake-connector-python
+social-auth-app-django
 stevedore
 testfixtures
 unicodecsv
diff --git a/requirements/dev.in b/requirements/dev.in
@@ -11,7 +11,6 @@ isort                     # to standardize order of imports
 pip-tools                 # Requirements file management
 pycodestyle               # PEP 8 compliance validation
 pydocstyle                # PEP 257 compliance validation
-social-auth-app-django    # adding explicit requirement to prevent defensive imports
 testfixtures              # Mock objects for unit tests and doc tests
 tox                       # virtualenv management for tests
 twine==1.11.0             # Utility for PyPI package uploads
diff --git a/requirements/dev.txt b/requirements/dev.txt
@@ -2,7 +2,7 @@
 # This file is autogenerated by pip-compile with Python 3.12
 # by the following command:
 #
-#    pip-compile --output-file=requirements/dev.txt requirements/dev.in
+#    make upgrade
 #
 accessible-pygments==0.0.5
     # via
diff --git a/requirements/doc.in b/requirements/doc.in
@@ -9,6 +9,5 @@ edx-braze-client
 pytest
 factory-boy
 readme_renderer           # Validates README.rst for usage on PyPI
-social-auth-app-django    # adding explicit requirement to prevent defensive imports
 Sphinx                    # Documentation builder
 sphinx-book-theme         # Common theme for all Open edX projects
diff --git a/requirements/doc.txt b/requirements/doc.txt
@@ -2,7 +2,7 @@
 # This file is autogenerated by pip-compile with Python 3.12
 # by the following command:
 #
-#    pip-compile --output-file=requirements/doc.txt requirements/doc.in
+#    make upgrade
 #
 accessible-pygments==0.0.5
     # via pydata-sphinx-theme
diff --git a/requirements/test.in b/requirements/test.in
@@ -12,5 +12,4 @@ mock                      # mocking library
 pytest-cov                # pytest extension for code coverage statistics
 pytest-django             # pytest extension for better Django support
 responses                 # utility library for mocking out the requests library
-social-auth-app-django    # adding explicit requirement to prevent defensive imports
 testfixtures              # Mock objects for unit tests and doc tests
diff --git a/requirements/test.txt b/requirements/test.txt
@@ -2,7 +2,7 @@
 # This file is autogenerated by pip-compile with Python 3.12
 # by the following command:
 #
-#    pip-compile --output-file=requirements/test.txt requirements/test.in
+#    make upgrade
 #
 amqp==5.3.1
     # via
diff --git a/tests/filters/test_accounts.py b/tests/filters/test_accounts.py
@@ -57,9 +57,7 @@ def test_adds_readonly_fields_when_sso_sync_enabled(
         ) = mock_ecu
         mock_idp_record = MagicMock()
         mock_idp_record.provider_id = 'saml-ubc'
-        idp_qs_mock = MagicMock()
-        idp_qs_mock.filter.return_value.first.return_value = mock_idp_record
-        mock_idp_objects.filter.return_value = idp_qs_mock
+        mock_idp_objects.filter.return_value = [mock_idp_record]
         mock_identity_provider = MagicMock()
         mock_identity_provider.sync_learner_profile_data = True
         mock_identity_provider.backend_name = 'tpa-saml'
@@ -95,9 +93,7 @@ def test_name_not_added_without_social_auth_record(
         ) = MagicMock()
         mock_idp_record = MagicMock()
         mock_idp_record.provider_id = 'saml-ubc'
-        idp_qs_mock = MagicMock()
-        idp_qs_mock.filter.return_value.first.return_value = mock_idp_record
-        mock_idp_objects.filter.return_value = idp_qs_mock
+        mock_idp_objects.filter.return_value = [mock_idp_record]
         mock_identity_provider = MagicMock()
         mock_identity_provider.sync_learner_profile_data = True
         mock_identity_provider.backend_name = 'tpa-saml'
@@ -131,10 +127,7 @@ def test_returns_unchanged_readonly_fields_when_no_idp_record(
             .select_related.return_value
             .first.return_value
         ) = MagicMock()
-        idp_qs_mock = MagicMock()
-        idp_qs_mock.filter.return_value.first.return_value = None
-        idp_qs_mock.first.return_value = None
-        mock_idp_objects.filter.return_value = idp_qs_mock
+        mock_idp_objects.filter.return_value = []
         step = self._make_step()
         fields = {'existing_field'}
         result = step.run_filter(readonly_fields=fields, user=user)
@@ -157,9 +150,7 @@ def test_returns_unchanged_readonly_fields_when_sync_not_enabled(
             .first.return_value
         ) = MagicMock()
         mock_idp_record = MagicMock(provider_id='saml-test')
-        idp_qs_mock = MagicMock()
-        idp_qs_mock.filter.return_value.first.return_value = mock_idp_record
-        mock_idp_objects.filter.return_value = idp_qs_mock
+        mock_idp_objects.filter.return_value = [mock_idp_record]
         mock_identity_provider = MagicMock()
         mock_identity_provider.sync_learner_profile_data = False
         mock_tpa = MagicMock()
@@ -169,3 +160,103 @@ def test_returns_unchanged_readonly_fields_when_sync_not_enabled(
         with patch('enterprise.filters.accounts.third_party_auth', mock_tpa):
             result = step.run_filter(readonly_fields=fields, user=user)
         self.assertEqual(result["readonly_fields"], fields)
+
+    @patch('enterprise.filters.accounts.UserSocialAuth')
+    @patch('enterprise.filters.accounts.EnterpriseCustomerIdentityProvider.objects')
+    @patch('enterprise.filters.accounts.EnterpriseCustomerUser.objects')
+    @override_settings(ENTERPRISE_READONLY_ACCOUNT_FIELDS=['name', 'email', 'country'])
+    def test_multiple_identity_providers_only_one_sync_enabled(
+        self, mock_ecu_objects, mock_idp_objects, mock_user_social_auth
+    ):
+        """
+        When multiple IdPs exist and only one has sync enabled, readonly fields are still added.
+        """
+        user = self._mock_user()
+        (
+            mock_ecu_objects.filter.return_value
+            .order_by.return_value
+            .select_related.return_value
+            .first.return_value
+        ) = MagicMock()
+
+        mock_idp_no_sync = MagicMock(provider_id='saml-no-sync')
+        mock_idp_with_sync = MagicMock(provider_id='saml-with-sync')
+        mock_idp_objects.filter.return_value = [mock_idp_no_sync, mock_idp_with_sync]
+
+        mock_provider_no_sync = MagicMock(sync_learner_profile_data=False, backend_name='tpa-saml-no-sync')
+        mock_provider_with_sync = MagicMock(sync_learner_profile_data=True, backend_name='tpa-saml-sync')
+
+        mock_tpa = MagicMock()
+        mock_tpa.provider.Registry.get.side_effect = lambda provider_id: (
+            mock_provider_no_sync if provider_id == 'saml-no-sync' else mock_provider_with_sync
+        )
+        mock_user_social_auth.objects.filter.return_value.exists.return_value = True
+
+        with patch('enterprise.filters.accounts.third_party_auth', mock_tpa):
+            step = self._make_step()
+            result = step.run_filter(readonly_fields=set(), user=user)
+
+        self.assertEqual(result["readonly_fields"], {"name", "email", "country"})
+
+    @patch('enterprise.filters.accounts.EnterpriseCustomerIdentityProvider.objects')
+    @patch('enterprise.filters.accounts.EnterpriseCustomerUser.objects')
+    def test_returns_unchanged_readonly_fields_when_registry_returns_none(
+        self, mock_ecu_objects, mock_idp_objects
+    ):
+        """
+        When Registry.get returns None for an IdP, sync_learner_profile_data stays False
+        and readonly_fields is returned unchanged.
+        """
+        user = self._mock_user()
+        (
+            mock_ecu_objects.filter.return_value
+            .order_by.return_value
+            .select_related.return_value
+            .first.return_value
+        ) = MagicMock()
+        mock_idp_record = MagicMock(provider_id='saml-unknown')
+        mock_idp_objects.filter.return_value = [mock_idp_record]
+
+        mock_tpa = MagicMock()
+        mock_tpa.provider.Registry.get.return_value = None
+
+        fields = {'existing_field'}
+        with patch('enterprise.filters.accounts.third_party_auth', mock_tpa):
+            step = self._make_step()
+            result = step.run_filter(readonly_fields=fields, user=user)
+
+        self.assertEqual(result["readonly_fields"], fields)
+
+    @patch('enterprise.filters.accounts.UserSocialAuth')
+    @patch('enterprise.filters.accounts.EnterpriseCustomerIdentityProvider.objects')
+    @patch('enterprise.filters.accounts.EnterpriseCustomerUser.objects')
+    @override_settings(ENTERPRISE_READONLY_ACCOUNT_FIELDS=['name', 'email'])
+    def test_name_excluded_when_sync_enabled_but_no_backend_name(
+        self, mock_ecu_objects, mock_idp_objects, mock_user_social_auth
+    ):
+        """
+        When sync is enabled but the provider has no backend_name, provider_backend_names
+        stays empty, UserSocialAuth is not queried, has_social_auth stays False,
+        and 'name' is excluded (branch #3 behavior).
+        """
+        user = self._mock_user()
+        (
+            mock_ecu_objects.filter.return_value
+            .order_by.return_value
+            .select_related.return_value
+            .first.return_value
+        ) = MagicMock()
+        mock_idp_record = MagicMock(provider_id='saml-ubc')
+        mock_idp_objects.filter.return_value = [mock_idp_record]
+        # sync is True but backend_name is falsy
+        mock_identity_provider = MagicMock(sync_learner_profile_data=True, backend_name=None)
+        mock_tpa = MagicMock()
+        mock_tpa.provider.Registry.get.return_value = mock_identity_provider
+
+        with patch('enterprise.filters.accounts.third_party_auth', mock_tpa):
+            step = self._make_step()
+            result = step.run_filter(readonly_fields=set(), user=user)
+
+        self.assertNotIn('name', result["readonly_fields"])
+        self.assertIn('email', result["readonly_fields"])
+        mock_user_social_auth.objects.filter.assert_not_called()
diff --git a/tests/test_enterprise/api/test_enterprise_customer_admin.py b/tests/test_enterprise/api/test_enterprise_customer_admin.py

Original file line number	Diff line number	Diff line change
`@@ -2,7 +2,7 @@`
`2`	`2`	`# This file is autogenerated by pip-compile with Python 3.12`
`3`	`3`	`# by the following command:`
`4`	`4`	`#`
`5`		`-# pip-compile --output-file=requirements/dev.txt requirements/dev.in`
	`5`	`+# make upgrade`
`6`	`6`	`#`
`7`	`7`	`accessible-pygments==0.0.5`
`8`	`8`	`# via`