fix: retirement PII leaks by redacting pending secondary email/name data

Author: ktyagiapphelix2u

common/djangoapps/student/tests/test_models.py

@@ -740,16 +740,17 @@ def test_retire_recovery_email(self):
         AccountRecoveryFactory(user=user, secondary_email='recovery@example.com')
         assert len(AccountRecovery.objects.filter(user_id=user.id)) == 1
 
+        from django.db.models.signals import pre_delete
         saved_emails = []
 
-        original_save = AccountRecovery.save
-
-        def capture_save(instance, *args, **kwargs):
+        def capture_pre_delete(sender, instance, **kwargs):
             saved_emails.append(instance.secondary_email)
-            original_save(instance, *args, **kwargs)
 
-        with mock.patch.object(AccountRecovery, 'save', side_effect=capture_save):
+        pre_delete.connect(capture_pre_delete, sender=AccountRecovery)
+        try:
             AccountRecovery.retire_recovery_email(user_id=user.id)
+        finally:
+            pre_delete.disconnect(capture_pre_delete, sender=AccountRecovery)
 
         # Record must be gone
         assert len(AccountRecovery.objects.filter(user_id=user.id)) == 0
@@ -772,16 +773,17 @@ def test_redact_and_delete_pending_secondary_email(self):
         )
         assert len(PendingSecondaryEmailChange.objects.filter(user_id=user.id)) == 1
 
+        from django.db.models.signals import pre_delete
         saved_emails = []
 
-        original_save = PendingSecondaryEmailChange.save
-
-        def capture_save(instance, *args, **kwargs):
+        def capture_pre_delete(sender, instance, **kwargs):
             saved_emails.append(instance.new_secondary_email)
-            original_save(instance, *args, **kwargs)
 
-        with mock.patch.object(PendingSecondaryEmailChange, 'save', side_effect=capture_save):
+        pre_delete.connect(capture_pre_delete, sender=PendingSecondaryEmailChange)
+        try:
             PendingSecondaryEmailChange.redact_and_delete_pending_secondary_email(user_id=user.id)
+        finally:
+            pre_delete.disconnect(capture_pre_delete, sender=PendingSecondaryEmailChange)
 
         # Record must be gone
         assert len(PendingSecondaryEmailChange.objects.filter(user_id=user.id)) == 0

openedx/core/djangoapps/user_api/accounts/views.py

@@ -1154,7 +1154,6 @@ def post(self, request):
             self.retire_entitlement_support_detail(user)
 
             # Retire misc. models that may contain PII of this user
-            PendingEmailChange.redact_pending_email_by_user_value(user, field="user")
             PendingEmailChange.delete_by_user_value(user, field="user")
             UserOrgTag.delete_by_user_value(user, field="user")
             PendingSecondaryEmailChange.redact_and_delete_pending_secondary_email(user.id)

openedx/core/djangoapps/user_api/management/tests/test_retire_user.py

@@ -122,14 +122,15 @@ def test_retire_user_cleans_pending_secondary_email(setup_retirement_states):  #
     assert PendingSecondaryEmailChange.objects.filter(user=user).exists()
 
     saved_emails = []
-    original_save = PendingSecondaryEmailChange.save
 
-    def capture_save(instance, *args, **kwargs):
+    def capture_pre_delete(sender, instance, **kwargs):
         saved_emails.append(instance.new_secondary_email)
-        original_save(instance, *args, **kwargs)
 
-    with mock.patch.object(PendingSecondaryEmailChange, 'save', side_effect=capture_save):
+    pre_delete.connect(capture_pre_delete, sender=PendingSecondaryEmailChange)
+    try:
         call_command('retire_user', username=user.username, user_email=user.email)
+    finally:
+        pre_delete.disconnect(capture_pre_delete, sender=PendingSecondaryEmailChange)
 
     # Record must be deleted
     assert not PendingSecondaryEmailChange.objects.filter(user=user).exists()