fix: cleanup of toggle and few custom attributes

CHANGELOG.rst

@@ -13,6 +13,14 @@ Unreleased
 ----------
 
 
+[4.6.2] - 2025-10-16
+--------------------
+
+Changed
+~~~~~~~
+
+* Removed temporary rollout toggles and simplified custom attribute tracking after successful OAuth session cleanup rollout.
+
 [4.6.1] - 2025-09-23
 --------------------
 

auth_backends/__init__.py

@@ -3,4 +3,4 @@
  These package is designed to be used primarily with Open edX Django projects, but should be compatible with non-edX
  projects as well.
 """
-__version__ = '4.6.1'  # pragma: no cover
+__version__ = '4.6.2'  # pragma: no cover

auth_backends/backends.py

@@ -7,23 +7,10 @@
 from django.contrib.auth import logout
 from django.dispatch import Signal
 from social_core.backends.oauth import BaseOAuth2
-from edx_toggles.toggles import SettingToggle
 from edx_django_utils.monitoring import set_custom_attribute
 
 logger = logging.getLogger(__name__)
 
-# .. toggle_name: ENABLE_OAUTH_SESSION_CLEANUP
-# .. toggle_implementation: SettingToggle
-# .. toggle_default: False
-# .. toggle_description: Controls whether to perform session cleanup during OAuth start.
-#    When enabled (True), existing user sessions are cleared before OAuth authentication
-#    to prevent user association conflicts. When disabled (False), session cleanup is skipped.
-#    This toggle allows for gradual rollout and quick rollback if issues arise.
-# .. toggle_use_cases: temporary
-# .. toggle_creation_date: 2025-09-25
-# .. toggle_target_removal_date: 2025-11-25
-ENABLE_OAUTH_SESSION_CLEANUP = SettingToggle("ENABLE_OAUTH_SESSION_CLEANUP", default=False)
-
 PROFILE_CLAIMS_TO_DETAILS_KEY_MAP = {
     'preferred_username': 'username',
     'email': 'email',
@@ -88,20 +75,10 @@ def logout_url(self):
             return self.end_session_url()
 
     def start(self):
-        """Initialize OAuth authentication with optional session cleanup."""
-
-        # .. custom_attribute_name: session_cleanup.toggle_enabled
-        # .. custom_attribute_description: Tracks whether the ENABLE_OAUTH_SESSION_CLEANUP
-        #    toggle is enabled during OAuth start.
-        set_custom_attribute('session_cleanup.toggle_enabled', ENABLE_OAUTH_SESSION_CLEANUP.is_enabled())
+        """Initialize OAuth authentication with session cleanup."""
 
         request = self.strategy.request if hasattr(self.strategy, 'request') else None
 
-        # .. custom_attribute_name: session_cleanup.has_request
-        # .. custom_attribute_description: Tracks whether a request object is available
-        #    during OAuth start. True if request exists, False if missing.
-        set_custom_attribute('session_cleanup.has_request', request is not None)
-
         user_authenticated = (
             request is not None and
             hasattr(request, 'user') and
@@ -113,28 +90,16 @@ def start(self):
         #    before session cleanup. True if user was logged in, False otherwise.
         set_custom_attribute('session_cleanup.logout_required', user_authenticated)
 
-        if user_authenticated and ENABLE_OAUTH_SESSION_CLEANUP.is_enabled():
+        if user_authenticated:
             existing_username = getattr(request.user, 'username', 'unknown')
 
-            # .. custom_attribute_name: session_cleanup.logged_out_username
-            # .. custom_attribute_description: Records the username that was logged out
-            #    during session cleanup for tracking and debugging purposes.
-            set_custom_attribute('session_cleanup.logged_out_username', existing_username)
-
             logger.info(
                 "OAuth start: Performing session cleanup for user '%s'",
                 existing_username
             )
 
             logout(request)
 
-            # .. custom_attribute_name: session_cleanup.logout_performed
-            # .. custom_attribute_description: Indicates that session cleanup was
-            #    actually performed during OAuth start.
-            set_custom_attribute('session_cleanup.logout_performed', True)
-        else:
-            set_custom_attribute('session_cleanup.logout_performed', False)
-
         return super().start()
 
     def authorization_url(self):

auth_backends/pipeline.py

@@ -4,24 +4,11 @@
 """
 import logging
 from django.contrib.auth import get_user_model
-from edx_toggles.toggles import SettingToggle
 from edx_django_utils.monitoring import set_custom_attribute
 
 logger = logging.getLogger(__name__)
 User = get_user_model()
 
-# .. toggle_name: SKIP_UPDATE_EMAIL_ON_USERNAME_MISMATCH
-# .. toggle_implementation: SettingToggle
-# .. toggle_default: False
-# .. toggle_description: Determines whether to block email updates when usernames don't match.
-#    When enabled (True), email updates will be blocked when the username in social auth details
-#    doesn't match the user's username. When disabled (False), email updates will proceed regardless
-#    of username mismatches. This will be used for a temporary rollout.
-# .. toggle_use_cases: temporary
-# .. toggle_creation_date: 2025-06-18
-# .. toggle_target_removal_date: 2025-08-18
-SKIP_UPDATE_EMAIL_ON_USERNAME_MISMATCH = SettingToggle("SKIP_UPDATE_EMAIL_ON_USERNAME_MISMATCH", default=False)
-
 
 # pylint: disable=unused-argument
 # The function parameters must be named exactly as they are below.
@@ -56,49 +43,17 @@ def update_email(strategy, details, user=None, *args, **kwargs):  # pylint: disa
         # Check if usernames don't match
         username_mismatch = details_username != user_username
 
-        # .. custom_attribute_name: update_email.username_mismatch
-        # .. custom_attribute_description: Tracks whether there's a mismatch between
-        #    the username in the social details and the user's actual username.
-        #    True if usernames don't match, False if they match.
-        set_custom_attribute('update_email.username_mismatch', username_mismatch)
-
-        # .. custom_attribute_name: update_email.rollout_toggle_enabled
-        # .. custom_attribute_description: Tracks whether the SKIP_UPDATE_EMAIL_ON_USERNAME_MISMATCH
-        #    toggle is enabled during this pipeline execution.
-        set_custom_attribute('update_email.rollout_toggle_enabled', SKIP_UPDATE_EMAIL_ON_USERNAME_MISMATCH.is_enabled())
-
         if username_mismatch:
-            # Log warning and set additional custom attributes for mismatches
             logger.warning(
-                "Username mismatch during email update. User username: %s, Details username: %s",
+                "Unexpected username mismatch during email update. Skipping email update for user %s. "
+                "User username: %s, Details username: %s",
+                user_username,
                 user_username,
                 details_username
             )
-            # .. custom_attribute_name: update_email.details_username
-            # .. custom_attribute_description: Records the username provided in the
-            #    social details when a mismatch occurs with the user's username.
-            set_custom_attribute('update_email.details_username', details_username)
-
-            # .. custom_attribute_name: update_email.user_username
-            # .. custom_attribute_description: Records the actual username of the user
-            #    when a mismatch occurs with the social details username.
-            set_custom_attribute('update_email.user_username', user_username)
-
-            # .. custom_attribute_name: update_email.details_has_email
-            # .. custom_attribute_description: Records whether the details contain an email
-            #    when a username mismatch occurs, to identify potential edge cases.
-            set_custom_attribute('update_email.details_has_email', bool(details.get('email')))
-
-            # Only exit if the toggle is enabled
-            if SKIP_UPDATE_EMAIL_ON_USERNAME_MISMATCH.is_enabled():
-                logger.warning(
-                    "Skipping email update for user %s due to username mismatch and "
-                    "SKIP_UPDATE_EMAIL_ON_USERNAME_MISMATCH toggle enabled",
-                    user_username
-                )
-                return  # Exit without updating email
+            return  # Exit without updating email
 
-        # Proceed with email update only if usernames match or toggle is disabled
+        # Proceed with email update only if usernames match
         email = details.get('email')
         if email and user.email != email:
             user.email = email

auth_backends/tests/test_backends.py

@@ -2,7 +2,7 @@
 import datetime
 import json
 from calendar import timegm
-from unittest.mock import patch, call
+from unittest.mock import patch
 
 import ddt
 import jwt
@@ -14,7 +14,6 @@
 from django.contrib.sessions.middleware import SessionMiddleware
 from django.core.cache import cache
 from django.test import RequestFactory
-from django.test.utils import override_settings
 from social_core.tests.backends.oauth import OAuth2Test
 
 User = get_user_model()
@@ -127,54 +126,38 @@ def test_login(self):
         self.do_login()
 
     @pytest.mark.django_db
-    @ddt.data(True, False)  # Test session cleanup with both toggle enabled and disabled
+    @ddt.data(True, False)  # Test with and without authenticated user
     @patch('auth_backends.backends.set_custom_attribute')
     @patch('auth_backends.backends.logger')
-    def test_start_with_session_cleanup(self, toggle_enabled, mock_logger, mock_set_attr):
-        """Test start method for session cleanup of existing user with toggle variation."""
-        with override_settings(ENABLE_OAUTH_SESSION_CLEANUP=toggle_enabled):
-            existing_user = User.objects.create_user(username='existing_user', email='[email protected]')
+    def test_start_with_session_cleanup(self, user_authenticated, mock_logger, mock_set_attr):
+        """Test start method for session cleanup with and without authenticated user."""
+        request = RequestFactory().get('/auth/login/edx-oauth2/')
 
-            request = RequestFactory().get('/auth/login/edx-oauth2/')
+        if user_authenticated:
+            existing_user = User.objects.create_user(username='existing_user', email='[email protected]')
             request.user = existing_user
 
-            middleware = SessionMiddleware(lambda req: None)
-            middleware.process_request(request)
-            request.session.save()
-
-            initial_session_key = request.session.session_key
-
-            self.backend.strategy.request = request
-
-            self.do_start()
-
-            if toggle_enabled:
-                self.assertNotEqual(request.session.session_key, initial_session_key)
-
-                self.assertTrue(request.user.is_anonymous)
+        middleware = SessionMiddleware(lambda req: None)
+        middleware.process_request(request)
+        request.session.save()
 
-                mock_set_attr.assert_has_calls([
-                    call('session_cleanup.toggle_enabled', True),
-                    call('session_cleanup.logout_performed', True),
-                    call('session_cleanup.logged_out_username', 'existing_user')
-                ], any_order=True)
+        initial_session_key = request.session.session_key
 
-                mock_logger.info.assert_called_with(
-                    "OAuth start: Performing session cleanup for user '%s'",
-                    'existing_user'
-                )
-            else:
-                self.assertEqual(request.session.session_key, initial_session_key)
+        self.backend.strategy.request = request
 
-                self.assertEqual(request.user, existing_user)
-                self.assertFalse(request.user.is_anonymous)
+        self.do_start()
 
-                mock_set_attr.assert_has_calls([
-                    call('session_cleanup.toggle_enabled', False),
-                    call('session_cleanup.logout_performed', False)
-                ], any_order=True)
+        mock_set_attr.assert_called_once_with('session_cleanup.logout_required', user_authenticated)
 
-                mock_logger.info.assert_not_called()
+        if user_authenticated:
+            self.assertNotEqual(request.session.session_key, initial_session_key)
+            self.assertTrue(request.user.is_anonymous)
+            mock_logger.info.assert_called_with(
+                "OAuth start: Performing session cleanup for user '%s'",
+                'existing_user'
+            )
+        else:
+            mock_logger.info.assert_not_called()
 
     def test_partial_pipeline(self):
         self.do_partial_pipeline()

auth_backends/tests/test_pipeline.py

@@ -44,11 +44,9 @@ def setUp(self):
         self.user = User.objects.create(username='test_user')
         self.strategy = load_strategy()
 
-    @patch('auth_backends.pipeline.SKIP_UPDATE_EMAIL_ON_USERNAME_MISMATCH.is_enabled')
     @patch('auth_backends.pipeline.set_custom_attribute')
-    def test_update_email(self, mock_set_attribute, mock_toggle):
+    def test_update_email(self, mock_set_attribute):
         """ Verify that user email is updated upon changing email when usernames match. """
-        mock_toggle.return_value = False
         updated_email = '[email protected]'
         self.assertNotEqual(self.user.email, updated_email)
 
@@ -60,33 +58,24 @@ def test_update_email(self, mock_set_attribute, mock_toggle):
         self.assertEqual(updated_user.email, updated_email)
         self.assertNotEqual(updated_user.email, initial_email)
 
-        mock_set_attribute.assert_any_call('update_email.username_mismatch', False)
-        mock_set_attribute.assert_any_call('update_email.rollout_toggle_enabled', False)
         self.assert_attribute_was_set(mock_set_attribute, 'update_email.email_updated', should_exist=True)
 
-    @patch('auth_backends.pipeline.SKIP_UPDATE_EMAIL_ON_USERNAME_MISMATCH.is_enabled')
     @patch('auth_backends.pipeline.set_custom_attribute')
-    def test_update_email_with_none(self, mock_set_attribute, mock_toggle):
+    def test_update_email_with_none(self, mock_set_attribute):
         """ Verify that user email is not updated if email value is None. """
-        mock_toggle.return_value = False
         old_email = self.user.email
 
         update_email(self.strategy, {'email': None, 'username': 'test_user'}, user=self.user)
 
         updated_user = User.objects.get(pk=self.user.pk)
         self.assertEqual(updated_user.email, old_email)
 
-        mock_set_attribute.assert_any_call('update_email.username_mismatch', False)
-        mock_set_attribute.assert_any_call('update_email.rollout_toggle_enabled', False)
         self.assert_attribute_was_set(mock_set_attribute, 'update_email.email_updated', should_exist=False)
 
-    @patch('auth_backends.pipeline.SKIP_UPDATE_EMAIL_ON_USERNAME_MISMATCH.is_enabled')
     @patch('auth_backends.pipeline.logger')
     @patch('auth_backends.pipeline.set_custom_attribute')
-    def test_username_mismatch_no_update_toggle_enabled(self, mock_set_attribute, mock_logger, mock_toggle):
-        """ Verify that email is not updated when usernames don't match and toggle is enabled. """
-        mock_toggle.return_value = True
-
+    def test_username_mismatch_no_update(self, mock_set_attribute, mock_logger):
+        """ Verify that email is not updated when usernames don't match. """
         old_email = self.user.email
         updated_email = '[email protected]'
 
@@ -95,49 +84,16 @@ def test_username_mismatch_no_update_toggle_enabled(self, mock_set_attribute, mo
         updated_user = User.objects.get(pk=self.user.pk)
         self.assertEqual(updated_user.email, old_email)
 
-        self.assertEqual(mock_logger.warning.call_count, 2)
-        mock_logger.warning.assert_any_call(
-            "Username mismatch during email update. User username: %s, Details username: %s",
-            'test_user', 'different_user'
-        )
-        mock_logger.warning.assert_any_call(
-            "Skipping email update for user %s due to username mismatch and "
-            "SKIP_UPDATE_EMAIL_ON_USERNAME_MISMATCH toggle enabled",
-            'test_user'
+        mock_logger.warning.assert_called_once_with(
+            "Unexpected username mismatch during email update. Skipping email update for user %s. "
+            "User username: %s, Details username: %s",
+            'test_user',
+            'test_user',
+            'different_user'
         )
 
-        mock_set_attribute.assert_any_call('update_email.username_mismatch', True)
-        mock_set_attribute.assert_any_call('update_email.rollout_toggle_enabled', True)
-        mock_set_attribute.assert_any_call('update_email.details_username', 'different_user')
-        mock_set_attribute.assert_any_call('update_email.user_username', 'test_user')
-        mock_set_attribute.assert_any_call('update_email.details_has_email', True)
         self.assert_attribute_was_set(mock_set_attribute, 'update_email.email_updated', should_exist=False)
 
-    @patch('auth_backends.pipeline.SKIP_UPDATE_EMAIL_ON_USERNAME_MISMATCH.is_enabled')
-    @patch('auth_backends.pipeline.logger')
-    @patch('auth_backends.pipeline.set_custom_attribute')
-    def test_username_mismatch_with_update_toggle_disabled(self, mock_set_attribute, mock_logger, mock_toggle):
-        """ Verify that email is updated when usernames don't match but toggle is disabled. """
-        mock_toggle.return_value = False
-
-        old_email = self.user.email
-        updated_email = '[email protected]'
-
-        update_email(self.strategy, {'email': updated_email, 'username': 'different_user'}, user=self.user)
-
-        updated_user = User.objects.get(pk=self.user.pk)
-        self.assertEqual(updated_user.email, updated_email)
-        self.assertNotEqual(updated_user.email, old_email)
-
-        mock_logger.warning.assert_called_once()
-
-        mock_set_attribute.assert_any_call('update_email.username_mismatch', True)
-        mock_set_attribute.assert_any_call('update_email.rollout_toggle_enabled', False)
-        mock_set_attribute.assert_any_call('update_email.details_username', 'different_user')
-        mock_set_attribute.assert_any_call('update_email.user_username', 'test_user')
-        mock_set_attribute.assert_any_call('update_email.details_has_email', True)
-        self.assert_attribute_was_set(mock_set_attribute, 'update_email.email_updated', should_exist=True)
-
     def assert_attribute_was_set(self, mock_set_attribute, attribute_name, should_exist=True):
         """
         Assert that a specific attribute was or was not set via set_custom_attribute.