fix: removal of temporary saml toggle

Author: ktyagiapphelix2u

common/djangoapps/third_party_auth/management/commands/tests/test_saml.py

@@ -69,16 +69,16 @@ def setUp(self):
         # not processed.
         self.saml_config = SAMLConfigurationFactory.create(
             enabled=False,
-            site__domain='testserver.fake',
-            site__name='testserver.fake'
+            site__domain='setup.testserver.fake',
+            site__name='setup.testserver.fake'
         )
         self.provider_config = SAMLProviderConfigFactory.create(
-            site__domain='testserver.fake',
-            site__name='testserver.fake',
-            slug='test-shib',
-            name='TestShib College',
-            entity_id='https://idp.testshib.org/idp/shibboleth',
-            metadata_source='https://www.testshib.org/metadata/testshib-providers.xml',
+            site__domain='setup.testserver.fake',
+            site__name='setup.testserver.fake',
+            slug='setup-test-shib',
+            name='Setup TestShib College',
+            entity_id='https://idp.testshib.org/idp/setup-shibboleth',
+            metadata_source='https://www.testshib.org/metadata/setup-testshib-providers.xml',
         )
 
     def _setup_test_configs_for_run_checks(self):
@@ -169,7 +169,7 @@ def test_fetch_saml_metadata(self):
         # Create enabled configurations
         self.__create_saml_configurations__()
 
-        expected = "\nDone.\n1 provider(s) found in database.\n0 skipped and 1 attempted.\n1 updated and 0 failed.\n"
+        expected = "\nDone.\n2 provider(s) found in database.\n1 skipped and 1 attempted.\n1 updated and 0 failed.\n"
         call_command("saml", pull=True, stdout=self.stdout)
         assert expected in self.stdout.getvalue()
 
@@ -182,7 +182,7 @@ def test_fetch_saml_metadata_failure(self):
         # Create enabled configurations
         self.__create_saml_configurations__()
 
-        expected = "\nDone.\n1 provider(s) found in database.\n0 skipped and 1 attempted.\n0 updated and 1 failed.\n"
+        expected = "\nDone.\n2 provider(s) found in database.\n1 skipped and 1 attempted.\n0 updated and 1 failed.\n"
 
         with self.assertRaisesRegex(CommandError, r"HTTPError: 404 Client Error"):
             call_command("saml", pull=True, stdout=self.stdout)
@@ -228,7 +228,7 @@ def test_fetch_multiple_providers_data(self):
             }
         )
 
-        expected = '\n3 provider(s) found in database.\n0 skipped and 3 attempted.\n2 updated and 1 failed.\n'
+        expected = '\n4 provider(s) found in database.\n1 skipped and 3 attempted.\n2 updated and 1 failed.\n'
         with self.assertRaisesRegex(CommandError, r"MetadataParseError: Can't find EntityDescriptor for entityID"):
             call_command("saml", pull=True, stdout=self.stdout)
         assert expected in self.stdout.getvalue()
@@ -250,8 +250,8 @@ def test_fetch_multiple_providers_data(self):
             }
         )
 
-        # Four configurations -- one will be skipped and three attempted, with similar results.
-        expected = '\nDone.\n4 provider(s) found in database.\n1 skipped and 3 attempted.\n0 updated and 1 failed.\n'
+        # Five configurations -- two will be skipped and three attempted, with similar results.
+        expected = '\nDone.\n5 provider(s) found in database.\n2 skipped and 3 attempted.\n0 updated and 1 failed.\n'
         with self.assertRaisesRegex(CommandError, r"MetadataParseError: Can't find EntityDescriptor for entityID"):
             call_command("saml", pull=True, stdout=self.stdout)
         assert expected in self.stdout.getvalue()
@@ -266,7 +266,7 @@ def test_saml_request_exceptions(self, mocked_get):
 
         mocked_get.side_effect = exceptions.SSLError
 
-        expected = "\nDone.\n1 provider(s) found in database.\n0 skipped and 1 attempted.\n0 updated and 1 failed.\n"
+        expected = "\nDone.\n2 provider(s) found in database.\n1 skipped and 1 attempted.\n0 updated and 1 failed.\n"
 
         with self.assertRaisesRegex(CommandError, "SSLError:"):
             call_command("saml", pull=True, stdout=self.stdout)
@@ -323,7 +323,7 @@ def test_xml_parse_exceptions(self, mocked_get):
         # create enabled configuration
         self.__create_saml_configurations__()
 
-        expected = "\nDone.\n1 provider(s) found in database.\n0 skipped and 1 attempted.\n0 updated and 1 failed.\n"
+        expected = "\nDone.\n2 provider(s) found in database.\n1 skipped and 1 attempted.\n0 updated and 1 failed.\n"
 
         with self.assertRaisesRegex(CommandError, "XMLSyntaxError:"):
             call_command("saml", pull=True, stdout=self.stdout)

common/djangoapps/third_party_auth/signals/handlers.py

@@ -7,7 +7,6 @@
 from edx_django_utils.monitoring import set_custom_attribute
 
 from common.djangoapps.third_party_auth.models import SAMLConfiguration, SAMLProviderConfig
-from common.djangoapps.third_party_auth.toggles import ENABLE_SAML_CONFIG_SIGNAL_HANDLERS
 
 
 @receiver(post_save, sender=SAMLConfiguration)
@@ -20,10 +19,6 @@ def update_saml_provider_configs_on_configuration_change(sender, instance, creat
     configuration version, ensuring all providers remain aligned with the most
     current settings.
     """
-    # .. custom_attribute_name: saml_config_signal.enabled
-    # .. custom_attribute_description: Tracks whether the SAML config signal handler is enabled.
-    set_custom_attribute('saml_config_signal.enabled', ENABLE_SAML_CONFIG_SIGNAL_HANDLERS.is_enabled())
-
     # .. custom_attribute_name: saml_config_signal.new_config_id
     # .. custom_attribute_description: Records the ID of the new SAML configuration instance.
     set_custom_attribute('saml_config_signal.new_config_id', instance.id)
@@ -32,26 +27,25 @@ def update_saml_provider_configs_on_configuration_change(sender, instance, creat
     # .. custom_attribute_description: Records the slug of the SAML configuration instance.
     set_custom_attribute('saml_config_signal.slug', instance.slug)
 
-    if ENABLE_SAML_CONFIG_SIGNAL_HANDLERS.is_enabled():
-        try:
-            # Find all existing SAMLProviderConfig instances (current_set) that should be
-            # pointing to this slug but are pointing to an older version
-            existing_providers = SAMLProviderConfig.objects.current_set().filter(
-                saml_configuration__site_id=instance.site_id,
-                saml_configuration__slug=instance.slug
-            ).exclude(saml_configuration_id=instance.id).exclude(saml_configuration_id__isnull=True)
-
-            updated_count = 0
-            for provider_config in existing_providers:
-                provider_config.saml_configuration = instance
-                provider_config.save()
-                updated_count += 1
-
-            # .. custom_attribute_name: saml_config_signal.updated_count
-            # .. custom_attribute_description: The number of SAMLProviderConfig records updated to point to the new configuration.
-            set_custom_attribute('saml_config_signal.updated_count', updated_count)
-
-        except Exception as e:  # pylint: disable=broad-except
-            # .. custom_attribute_name: saml_config_signal.error_message
-            # .. custom_attribute_description: Records any error message that occurs during SAML provider config updates.
-            set_custom_attribute('saml_config_signal.error_message', str(e))
+    try:
+        # Find all existing SAMLProviderConfig instances (current_set) that should be
+        # pointing to this slug but are pointing to an older version
+        existing_providers = SAMLProviderConfig.objects.current_set().filter(
+            saml_configuration__site_id=instance.site_id,
+            saml_configuration__slug=instance.slug
+        ).exclude(saml_configuration_id=instance.id).exclude(saml_configuration_id__isnull=True)
+
+        updated_count = 0
+        for provider_config in existing_providers:
+            provider_config.saml_configuration = instance
+            provider_config.save()
+            updated_count += 1
+
+        # .. custom_attribute_name: saml_config_signal.updated_count
+        # .. custom_attribute_description: The number of SAMLProviderConfig records updated to point to the new configuration.
+        set_custom_attribute('saml_config_signal.updated_count', updated_count)
+
+    except Exception as e:  # pylint: disable=broad-except
+        # .. custom_attribute_name: saml_config_signal.error_message
+        # .. custom_attribute_description: Records any error message that occurs during SAML provider config updates.
+        set_custom_attribute('saml_config_signal.error_message', str(e))

common/djangoapps/third_party_auth/signals/tests/test_handlers.py

@@ -5,7 +5,7 @@
 import ddt
 from unittest import mock
 from unittest.mock import call
-from django.test import TestCase, override_settings
+from django.test import TestCase
 from django.contrib.sites.models import Site
 from common.djangoapps.third_party_auth.tests.factories import SAMLConfigurationFactory, SAMLProviderConfigFactory
 from common.djangoapps.third_party_auth.models import SAMLProviderConfig
@@ -34,64 +34,42 @@ def setUp(self):
         )
 
     @mock.patch('common.djangoapps.third_party_auth.signals.handlers.set_custom_attribute')
-    def test_saml_config_signal_handlers_disabled(self, mock_set_custom_attribute):
+    def test_saml_config_signal_handlers_with_error(self, mock_set_custom_attribute):
         """
-        Test behavior when SAML config signal handlers are disabled.
+        Test error handling when signal handlers encounter an exception.
 
-        Verifies that basic attributes are set but no provider updates are attempted.
+        Verifies that error information is properly captured when provider updates fail.
         """
-        with override_settings(ENABLE_SAML_CONFIG_SIGNAL_HANDLERS=False):
+        error_message = "Test error"
+        # Simulate an exception in the provider config update logic
+        with mock.patch(
+            'common.djangoapps.third_party_auth.models.SAMLProviderConfig.objects.current_set',
+            side_effect=Exception(error_message)
+        ):
             self.saml_config.entity_id = 'https://updated.example.com'
             self.saml_config.save()
 
-            expected_calls = [
-                call('saml_config_signal.enabled', False),
-                call('saml_config_signal.new_config_id', self.saml_config.id),
-                call('saml_config_signal.slug', 'test-config'),
-            ]
-
-            mock_set_custom_attribute.assert_has_calls(expected_calls, any_order=False)
-            assert mock_set_custom_attribute.call_count == 3
+        expected_calls = [
+            call('saml_config_signal.new_config_id', self.saml_config.id),
+            call('saml_config_signal.slug', 'test-config'),
+        ]
 
-    @mock.patch('common.djangoapps.third_party_auth.signals.handlers.set_custom_attribute')
-    def test_saml_config_signal_handlers_with_error(self, mock_set_custom_attribute):
-        """
-        Test error handling when signal handlers encounter an exception.
+        mock_set_custom_attribute.assert_has_calls(expected_calls, any_order=False)
+        assert mock_set_custom_attribute.call_count == 3
 
-        Verifies that error information is properly captured when provider updates fail.
-        """
-        error_message = "Test error"
-        with override_settings(ENABLE_SAML_CONFIG_SIGNAL_HANDLERS=True):
-            # Simulate an exception in the provider config update logic
-            with mock.patch(
-                'common.djangoapps.third_party_auth.models.SAMLProviderConfig.objects.current_set',
-                side_effect=Exception(error_message)
-            ):
-                self.saml_config.entity_id = 'https://updated.example.com'
-                self.saml_config.save()
-
-            expected_calls = [
-                call('saml_config_signal.enabled', True),
-                call('saml_config_signal.new_config_id', self.saml_config.id),
-                call('saml_config_signal.slug', 'test-config'),
-            ]
-
-            mock_set_custom_attribute.assert_has_calls(expected_calls, any_order=False)
-            assert mock_set_custom_attribute.call_count == 4
-
-            # Verify error message was logged
-            mock_set_custom_attribute.assert_any_call(
-                'saml_config_signal.error_message',
-                mock.ANY
-            )
-            error_calls = [
-                call for call in mock_set_custom_attribute.mock_calls
-                if call[1][0] == 'saml_config_signal.error_message'
-            ]
-            assert error_message in error_calls[0][1][1], (
-                f"Expected '{error_message}' in error message, "
-                f"got: {error_calls[0][1][1]}"
-            )
+        # Verify error message was logged
+        mock_set_custom_attribute.assert_any_call(
+            'saml_config_signal.error_message',
+            mock.ANY
+        )
+        error_calls = [
+            call for call in mock_set_custom_attribute.mock_calls
+            if call[1][0] == 'saml_config_signal.error_message'
+        ]
+        assert error_message in error_calls[0][1][1], (
+            f"Expected '{error_message}' in error message, "
+            f"got: {error_calls[0][1][1]}"
+        )
 
     def _get_current_provider(self, slug):
         """
@@ -125,7 +103,6 @@ def _get_site(self, site_id):
     )
     @ddt.unpack
     @mock.patch('common.djangoapps.third_party_auth.signals.handlers.set_custom_attribute')
-    @override_settings(ENABLE_SAML_CONFIG_SIGNAL_HANDLERS=True)
     def test_saml_provider_config_updates(self, provider_site_id, provider_slug,
                                           signal_saml_site_id, signal_saml_slug, is_provider_updated,
                                           mock_set_custom_attribute):
@@ -154,7 +131,6 @@ def test_saml_provider_config_updates(self, provider_site_id, provider_slug,
         current_provider = self._get_current_provider(provider_slug)
 
         expected_calls = [
-            call('saml_config_signal.enabled', True),
             call('saml_config_signal.new_config_id', new_saml_config.id),
             call('saml_config_signal.slug', signal_saml_slug),
         ]
@@ -177,7 +153,6 @@ def test_saml_provider_config_updates(self, provider_site_id, provider_slug,
         (2, 'slug', 1, 'default'),
     )
     @ddt.unpack
-    @override_settings(ENABLE_SAML_CONFIG_SIGNAL_HANDLERS=True)
     def test_saml_provider_with_null_config_not_updated(self, provider_site_id, provider_slug,
                                                         signal_saml_site_id, signal_saml_slug):
         """

common/djangoapps/third_party_auth/toggles.py

@@ -2,7 +2,7 @@
 Togglable settings for Third Party Auth
 """
 
-from edx_toggles.toggles import WaffleFlag, SettingToggle
+from edx_toggles.toggles import WaffleFlag
 
 THIRD_PARTY_AUTH_NAMESPACE = 'thirdpartyauth'
 
@@ -18,26 +18,6 @@
 APPLE_USER_MIGRATION_FLAG = WaffleFlag(f'{THIRD_PARTY_AUTH_NAMESPACE}.apple_user_migration', __name__)
 
 
-# .. toggle_name: ENABLE_SAML_CONFIG_SIGNAL_HANDLERS
-# .. toggle_implementation: SettingToggle
-# .. toggle_default: False
-# .. toggle_description: Controls whether SAML configuration signal handlers are active.
-#    When enabled (True), signal handlers will automatically update SAMLProviderConfig
-#    references when the associated SAMLConfiguration is updated.
-#    When disabled (False), SAMLProviderConfigs point to outdated SAMLConfiguration.
-# .. toggle_use_cases: temporary
-# .. toggle_creation_date: 2025-07-03
-# .. toggle_target_removal_date: 2026-01-01
-# .. toggle_warning: Disabling this toggle may result in SAMLProviderConfig instances
-#    pointing to outdated SAMLConfiguration records. Use the management command
-#    'saml --fix-references' to fix outdated references.
-ENABLE_SAML_CONFIG_SIGNAL_HANDLERS = SettingToggle(
-    "ENABLE_SAML_CONFIG_SIGNAL_HANDLERS",
-    default=False,
-    module_name=__name__
-)
-
-
 def is_apple_user_migration_enabled():
     """
     Returns a boolean if Apple users migration is in process.