fix:bug in third party authentication email and name update

CHANGELOG.rst

@@ -12,7 +12,16 @@ Change Log
 Unreleased
 ----------
 
-*
+
+[4.6.0] - 2025-06-18
+--------------------
+
+Changed
+~~~~~~~
+
+* Improved test coverage by replacing MagicMock with real load_strategy() implementation.
+* Fixed email update handling in authentication pipeline.
+* Added logging for email updates.
 
 Added
 ~~~~~~~

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.5.0'  # pragma: no cover
+__version__ = '4.6.0'  # pragma: no cover

auth_backends/pipeline.py

@@ -1,12 +1,27 @@
 """Python-social-auth pipeline functions.
 
-For more info visit http://python-social-auth.readthedocs.org/en/latest/pipeline.html.
+For more info visit https://python-social-auth.readthedocs.io/en/latest/pipeline.html.
 """
+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.
@@ -33,9 +48,63 @@ def get_user_if_exists(strategy, details, user=None, *args, **kwargs):  # pylint
 
 def update_email(strategy, details, user=None, *args, **kwargs):  # pylint: disable=keyword-arg-before-vararg
     """Update the user's email address using data from provider."""
+
     if user:
-        email = details.get('email')
+        # Get usernames for comparison, using defensive coding
+        details_username = details.get('username')
+        user_username = getattr(user, 'username', None)
+        # 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",
+                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
+
+        # Proceed with email update only if usernames match or toggle is disabled
+        email = details.get('email')
         if email and user.email != email:
             user.email = email
             strategy.storage.user.changed(user)
+
+            # .. custom_attribute_name: update_email.email_updated
+            # .. custom_attribute_description: Indicates that the user's email was
+            #    actually updated during this pipeline execution.
+            set_custom_attribute('update_email.email_updated', True)

auth_backends/tests/test_pipeline.py

@@ -1,5 +1,6 @@
 """ Tests for pipelines. """
 
+from unittest.mock import patch
 from django.contrib.auth import get_user_model
 from django.test import TestCase
 from social_django.utils import load_strategy
@@ -40,20 +41,124 @@ class UpdateEmailPipelineTests(TestCase):
 
     def setUp(self):
         super().setUp()
-        self.user = User.objects.create()
+        self.user = User.objects.create(username='test_user')
+        self.strategy = load_strategy()
 
-    def test_update_email(self):
-        """ Verify that user email is updated upon changing email. """
+    @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):
+        """ Verify that user email is updated upon changing email when usernames match. """
+        mock_toggle.return_value = False
         updated_email = 'updated@example.com'
         self.assertNotEqual(self.user.email, updated_email)
 
-        update_email(load_strategy(), {'email': updated_email}, user=self.user)
-        self.user = User.objects.get(username=self.user.username)
-        self.assertEqual(self.user.email, updated_email)
+        initial_email = self.user.email
 
-    def test_update_email_with_none(self):
+        update_email(self.strategy, {'email': updated_email, 'username': 'test_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, 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):
         """ Verify that user email is not updated if email value is None. """
+        mock_toggle.return_value = False
         old_email = self.user.email
-        update_email(load_strategy(), {'email': None}, user=self.user)
-        self.user = User.objects.get(username=self.user.username)
-        self.assertEqual(self.user.email, old_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
+
+        old_email = self.user.email
+        updated_email = 'updated@example.com'
+
+        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, 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_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 = 'updated@example.com'
+
+        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.
+
+        Args:
+            mock_set_attribute: The mocked set_custom_attribute function
+            attribute_name: The name of the attribute to check
+            should_exist: If True, assert the attribute was set; if False, assert it wasn't
+        """
+        matching_calls = [
+            call for call in mock_set_attribute.call_args_list
+            if call[0][0] == attribute_name
+        ]
+
+        if should_exist:
+            self.assertGreater(
+                len(matching_calls), 0,
+                f"Expected '{attribute_name}' to be set, but it wasn't"
+            )
+        else:
+            self.assertEqual(
+                len(matching_calls), 0,
+                f"Expected '{attribute_name}' not to be set, but it was: {matching_calls}"
+            )

requirements/base.in

@@ -2,6 +2,8 @@
 -c constraints.txt
 
 Django
+edx-django-utils
+edx-toggles
 pyjwt[crypto]>=2.1.0       # depends on newer jwt.decode and jwt.encode functionality
 six
 social-auth-app-django

requirements/base.txt

@@ -6,39 +6,79 @@
 #
 asgiref==3.8.1
     # via django
-certifi==2025.1.31
+certifi==2025.4.26
     # via requests
 cffi==1.17.1
-    # via cryptography
-charset-normalizer==3.4.1
+    # via
+    #   cryptography
+    #   pynacl
+charset-normalizer==3.4.2
     # via requests
-cryptography==44.0.2
+click==8.2.1
+    # via
+    #   code-annotations
+    #   edx-django-utils
+code-annotations==2.3.0
+    # via edx-toggles
+cryptography==45.0.3
     # via
     #   pyjwt
     #   social-auth-core
 defusedxml==0.7.1
     # via
     #   python3-openid
     #   social-auth-core
-django==4.2.20
+django==4.2.22
     # via
     #   -c requirements/common_constraints.txt
     #   -r requirements/base.in
+    #   django-crum
+    #   django-waffle
+    #   edx-django-utils
+    #   edx-toggles
     #   social-auth-app-django
+django-crum==0.7.9
+    # via
+    #   edx-django-utils
+    #   edx-toggles
+django-waffle==4.2.0
+    # via
+    #   edx-django-utils
+    #   edx-toggles
+edx-django-utils==8.0.0
+    # via
+    #   -r requirements/base.in
+    #   edx-toggles
+edx-toggles==5.3.0
+    # via -r requirements/base.in
 idna==3.10
     # via requests
+jinja2==3.1.6
+    # via code-annotations
+markupsafe==3.0.2
+    # via jinja2
 oauthlib==3.2.2
     # via
     #   requests-oauthlib
     #   social-auth-core
+pbr==6.1.1
+    # via stevedore
+psutil==7.0.0
+    # via edx-django-utils
 pycparser==2.22
     # via cffi
 pyjwt[crypto]==2.10.1
     # via
     #   -r requirements/base.in
     #   social-auth-core
+pynacl==1.5.0
+    # via edx-django-utils
+python-slugify==8.0.4
+    # via code-annotations
 python3-openid==3.2.0
     # via social-auth-core
+pyyaml==6.0.2
+    # via code-annotations
 requests==2.32.3
     # via
     #   requests-oauthlib
@@ -49,13 +89,22 @@ six==1.17.0
     # via -r requirements/base.in
 social-auth-app-django==5.4.3
     # via -r requirements/base.in
-social-auth-core==4.5.6
+social-auth-core==4.6.1
     # via
     #   -r requirements/base.in
     #   social-auth-app-django
 sqlparse==0.5.3
     # via django
+stevedore==5.4.1
+    # via
+    #   code-annotations
+    #   edx-django-utils
+text-unidecode==1.3
+    # via python-slugify
 urllib3==2.2.3
     # via
     #   -c requirements/common_constraints.txt
     #   requests
+
+# The following packages are considered to be unsafe in a requirements file:
+# setuptools