fix: bug in third party authentication email and name update

Author: Akanshu-2u

auth_backends/pipeline.py

@@ -2,9 +2,12 @@
 
 For more info visit http://python-social-auth.readthedocs.org/en/latest/pipeline.html.
 """
+import logging
 from django.contrib.auth import get_user_model
 
+from edx_django_utils.monitoring import set_custom_attribute
 
+logger = logging.getLogger(__name__)
 User = get_user_model()
 
 
@@ -34,8 +37,38 @@ 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 match
+        username_match = 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 authentication details and the user's actual username.
+        #    True if usernames don't match, False if they match.
+        set_custom_attribute('update_email.username_mismatch', not username_match)
+
+        if not username_match:
+            # 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
+            #    authentication details when a mismatch occurs with the user's username.
+            set_custom_attribute('update_email.details_username', str(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 authentication details username.
+            set_custom_attribute('update_email.user_username', str(user_username))
+            return  # Exit without updating email
+
+        # Proceed with email update only if usernames match
+        email = details.get('email')
         if email and user.email != email:
             user.email = email
             strategy.storage.user.changed(user)

auth_backends/tests/test_pipeline.py

@@ -1,8 +1,8 @@
 """ Tests for pipelines. """
 
+from unittest.mock import patch, MagicMock
 from django.contrib.auth import get_user_model
 from django.test import TestCase
-from social_django.utils import load_strategy
 
 from auth_backends.pipeline import get_user_if_exists, update_email
 
@@ -40,20 +40,67 @@ class UpdateEmailPipelineTests(TestCase):
 
     def setUp(self):
         super().setUp()
-        self.user = User.objects.create()
+        self.user = User.objects.create(username='test_user')
+        self.strategy = MagicMock()
 
-    def test_update_email(self):
-        """ Verify that user email is updated upon changing email. """
+        # Make strategy.storage.user.changed actually save the user
+        def save_user(user):
+            user.save()
+
+        self.strategy.storage.user.changed.side_effect = save_user
+
+    @patch('auth_backends.pipeline.set_custom_attribute')
+    def test_update_email(self, mock_set_attribute):
+        """ Verify that user email is updated upon changing email when usernames match. """
         updated_email = 'updated@example.com'
         self.assertNotEqual(self.user.email, updated_email)
 
-        update_email(load_strategy(), {'email': updated_email}, user=self.user)
+        # Provide matching username in details
+        update_email(self.strategy, {'email': updated_email, 'username': 'test_user'}, user=self.user)
+
+        # Verify email was updated
         self.user = User.objects.get(username=self.user.username)
         self.assertEqual(self.user.email, updated_email)
+        self.strategy.storage.user.changed.assert_called_once_with(self.user)
+
+        # Verify custom attribute was set correctly
+        mock_set_attribute.assert_called_with('update_email.username_mismatch', False)
 
-    def test_update_email_with_none(self):
+    @patch('auth_backends.pipeline.set_custom_attribute')
+    def test_update_email_with_none(self, mock_set_attribute):
         """ Verify that user email is not updated if email value is None. """
         old_email = self.user.email
-        update_email(load_strategy(), {'email': None}, user=self.user)
+
+        # Provide matching username in details
+        update_email(self.strategy, {'email': None, 'username': 'test_user'}, user=self.user)
+
+        # Verify email was not updated
+        self.user = User.objects.get(username=self.user.username)
+        self.assertEqual(self.user.email, old_email)
+        self.strategy.storage.user.changed.assert_not_called()
+
+        # Verify custom attribute was still set
+        mock_set_attribute.assert_called_with('update_email.username_mismatch', False)
+
+    @patch('auth_backends.pipeline.logger')
+    @patch('auth_backends.pipeline.set_custom_attribute')
+    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 = 'updated@example.com'
+
+        # Provide mismatched username in details
+        update_email(self.strategy, {'email': updated_email, 'username': 'different_user'}, user=self.user)
+
+        # Verify email was not updated
         self.user = User.objects.get(username=self.user.username)
         self.assertEqual(self.user.email, old_email)
+        self.strategy.storage.user.changed.assert_not_called()
+
+        # Verify logger was called with warning
+        mock_logger.warning.assert_called_once()
+
+        # Verify all custom attributes were set correctly
+        mock_set_attribute.assert_any_call('update_email.username_mismatch', 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')

requirements/base.in

@@ -6,3 +6,4 @@ pyjwt[crypto]>=2.1.0       # depends on newer jwt.decode and jwt.encode function
 six
 social-auth-app-django
 social-auth-core           # Common auth interfaces
+edx-django-utils