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

common/djangoapps/student/models/user.py

@@ -934,14 +934,31 @@ class PendingSecondaryEmailChange(DeletableByUserValue, models.Model):  # noqa:
     """
     This model keeps track of pending requested changes to a user's secondary email address.
 
-    .. pii: Contains new_secondary_email, not currently retired
+    .. pii: Contains new_secondary_email, redacted in `DeactivateLogoutView`
     .. pii_types: email_address
-    .. pii_retirement: retained
+    .. pii_retirement: local_api
     """
     user = models.OneToOneField(User, unique=True, db_index=True, on_delete=models.CASCADE)
     new_secondary_email = models.CharField(blank=True, max_length=255, db_index=True)
     activation_key = models.CharField(('activation key'), max_length=32, unique=True, db_index=True)
 
+    @classmethod
+    def redact_and_delete_pending_secondary_email(cls, user_id):
+        """
+        Redact and delete a pending secondary email change row for a user.
+
+        Redacts the email before deletion so any downstream soft-delete mirror does
+        not retain the original secondary email address in the final row image.
+        """
+        try:
+            pending_secondary_email = cls.objects.get(user_id=user_id)
+        except cls.DoesNotExist:
+            return True
+        pending_secondary_email.new_secondary_email = f"redacted+{user_id}@redacted.com"
+        pending_secondary_email.save(update_fields=['new_secondary_email'])
+        pending_secondary_email.delete()
+        return True
+
 
 class LoginFailures(models.Model):
     """
@@ -1688,7 +1705,7 @@ class AccountRecovery(models.Model):  # noqa: DJ008
     """
     Model for storing information for user's account recovery in case of access loss.
 
-    .. pii: the field named secondary_email contains pii, retired in the `DeactivateLogoutView`
+    .. pii: the field named secondary_email contains pii, redacted in the `DeactivateLogoutView`
     .. pii_types: email_address
     .. pii_retirement: local_api
     """
@@ -1721,19 +1738,25 @@ def update_recovery_email(self, email):
     @classmethod
     def retire_recovery_email(cls, user_id):
         """
-        Retire user's recovery/secondary email as part of GDPR Phase I.
-        Returns 'True'
+        Redact and delete user's recovery/secondary email as part of GDPR Phase I.
 
-        If an AccountRecovery record is found for this user it will be deleted,
-        if it is not found it is assumed this table has no PII for the given user.
+        If an AccountRecovery record is found for this user it will be redacted and
+        deleted. If it is not found it is assumed this table has no PII for the given user.
+
+        Note: "Retire" here refers to retiring this user's data, not the recovery email
+        feature itself, which remains available for new accounts.
 
         :param user_id: int
-        :return: bool
+        :return: bool - True on success
         """
         try:
-            cls.objects.get(user_id=user_id).delete()
+            account_recovery = cls.objects.get(user_id=user_id)
         except cls.DoesNotExist:
-            pass
+            return True
+        account_recovery.secondary_email = f"redacted+{user_id}@redacted.com"
+        account_recovery.is_active = False
+        account_recovery.save(update_fields=['secondary_email', 'is_active'])
+        account_recovery.delete()
 
         return True
 

common/djangoapps/student/tests/test_models.py

@@ -27,6 +27,7 @@
     ManualEnrollmentAudit,
     PendingEmailChange,
     PendingNameChange,
+    PendingSecondaryEmailChange,
     UserAttribute,
     UserCelebration,
     UserProfile,
@@ -747,6 +748,27 @@ def test_retire_recovery_email(self):
         assert len(AccountRecovery.objects.filter(user_id=user.id)) == 0
 
 
+class TestPendingSecondaryEmailChange(TestCase):
+    """Tests for retiring PendingSecondaryEmailChange records."""
+
+    def test_redact_and_delete_pending_secondary_email(self):
+        """Assert that pending secondary email records are deleted for retired users."""
+        user = UserFactory()
+        PendingSecondaryEmailChange.objects.create(
+            user=user,
+            new_secondary_email='new-secondary@example.com',
+            activation_key='a' * 32,
+        )
+        assert len(PendingSecondaryEmailChange.objects.filter(user_id=user.id)) == 1
+        PendingSecondaryEmailChange.redact_and_delete_pending_secondary_email(user_id=user.id)
+        assert len(PendingSecondaryEmailChange.objects.filter(user_id=user.id)) == 0
+
+    def test_redact_and_delete_pending_secondary_email_when_no_record(self):
+        """Assert retirement cleanup returns True when no pending secondary row exists."""
+        user = UserFactory()
+        assert PendingSecondaryEmailChange.redact_and_delete_pending_secondary_email(user_id=user.id) is True
+
+
 @ddt.ddt
 class TestUserPostSaveCallback(SharedModuleStoreTestCase):
     """

common/djangoapps/student/views/management.py

@@ -84,6 +84,7 @@
     UserStanding,
     create_comments_service_user,  # noqa: F401
     email_exists_or_retired,  # noqa: F401
+    get_retired_email_by_email,
 )
 from common.djangoapps.student.signals import REFUND_ORDER, USER_EMAIL_CHANGED
 from common.djangoapps.student.toggles import should_redirect_to_courseware_after_enrollment
@@ -890,6 +891,10 @@ def activate_secondary_email(request, key):
             'secondary_email': pending_secondary_email_change.new_secondary_email
         })
 
+    pending_secondary_email_change.new_secondary_email = get_retired_email_by_email(
+        pending_secondary_email_change.new_secondary_email
+    )
+    pending_secondary_email_change.save(update_fields=['new_secondary_email'])
     pending_secondary_email_change.delete()
 
     return render_to_response("secondary_email_change_successful.html")

openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py

@@ -29,6 +29,7 @@
     ManualEnrollmentAudit,
     PendingEmailChange,
     PendingNameChange,
+    PendingSecondaryEmailChange,
     Registration,
     SocialLink,
     UserProfile,
@@ -233,6 +234,24 @@ def test_user_can_deactivate_secondary_email(self):
         # Assert that there is no longer a secondary/recovery email for test user
         assert len(AccountRecovery.objects.filter(user_id=self.test_user.id)) == 0
 
+    def test_user_can_deactivate_pending_secondary_email_change(self):
+        """
+        Verify that pending secondary email change records are removed when a user retires.
+        """
+        PendingSecondaryEmailChange.objects.create(
+            user=self.test_user,
+            new_secondary_email='pending-secondary@example.com',
+            activation_key='b' * 32,
+        )
+        assert len(PendingSecondaryEmailChange.objects.filter(user_id=self.test_user.id)) == 1
+
+        self.client.login(username=self.test_user.username, password=self.test_password)
+        headers = build_jwt_headers(self.test_user)
+        response = self.client.post(self.url, self.build_post(self.test_password), **headers)
+        assert response.status_code == status.HTTP_204_NO_CONTENT
+
+        assert len(PendingSecondaryEmailChange.objects.filter(user_id=self.test_user.id)) == 0
+
     def test_password_mismatch(self):
         """
         Verify that the user submitting a mismatched password results in
@@ -1293,6 +1312,18 @@ def setUp(self):
         UserOrgTagFactory.create(user=self.test_user, key='foo', value='bar')
         UserOrgTagFactory.create(user=self.test_user, key='cat', value='dog')
 
+        # Secondary email setup
+        PendingSecondaryEmailChange.objects.create(
+            user=self.test_user,
+            new_secondary_email='pending_secondary@example.com',
+            activation_key='test_activation_key_123'
+        )
+        AccountRecovery.objects.create(
+            user=self.test_user,
+            secondary_email='confirmed_secondary@example.com',
+            is_active=True
+        )
+
         CourseEnrollmentAllowedFactory.create(email=self.original_email)
 
         self.course_key = CourseKey.from_string('course-v1:edX+DemoX+Demo_Course')
@@ -1399,6 +1430,10 @@ def test_retire_user(self, mock_remove_profile_images, mock_get_profile_image_na
         assert not PendingEmailChange.objects.filter(user=self.test_user).exists()
         assert not UserOrgTag.objects.filter(user=self.test_user).exists()
 
+        # Verify secondary email models were cleaned
+        assert not PendingSecondaryEmailChange.objects.filter(user=self.test_user).exists()
+        assert not AccountRecovery.objects.filter(user=self.test_user).exists()
+
         assert not CourseEnrollmentAllowed.objects.filter(email=self.original_email).exists()
         assert not UnregisteredLearnerCohortAssignments.objects.filter(email=self.original_email).exists()
 

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

@@ -15,7 +15,12 @@
 from edx_django_utils.user import generate_password
 from social_django.models import UserSocialAuth
 
-from common.djangoapps.student.models import AccountRecovery, Registration, get_retired_email_by_email
+from common.djangoapps.student.models import (
+    AccountRecovery,
+    PendingSecondaryEmailChange,
+    Registration,
+    get_retired_email_by_email,
+)
 from openedx.core.djangoapps.site_configuration.models import SiteConfiguration
 from openedx.core.djangoapps.theming.helpers import get_config_value_from_site_or_settings, get_current_site
 from openedx.core.djangolib.oauth2_retirement_utils import retire_dot_oauth2_models
@@ -219,6 +224,7 @@ def create_retirement_request_and_deactivate_account(user):
     # Delete OAuth tokens associated with the user.
     retire_dot_oauth2_models(user)
     AccountRecovery.retire_recovery_email(user.id)
+    PendingSecondaryEmailChange.redact_and_delete_pending_secondary_email(user.id)
 
 
 def username_suffix_generator(suffix_length=4):

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

@@ -37,11 +37,13 @@
 
 from common.djangoapps.entitlements.models import CourseEntitlement
 from common.djangoapps.student.models import (  # lint-amnesty, pylint: disable=unused-import
+    AccountRecovery,
     CourseEnrollmentAllowed,
     LoginFailures,
     ManualEnrollmentAudit,
     PendingEmailChange,
     PendingNameChange,
+    PendingSecondaryEmailChange,
     User,
     UserProfile,
     get_potentially_retired_user_by_username,
@@ -1152,8 +1154,15 @@ def post(self, request):
             self.retire_entitlement_support_detail(user)
 
             # Retire misc. models that may contain PII of this user
+            # Redact pending email change before deletion to prevent plaintext sync to Snowflake
+            pending_email = PendingEmailChange.objects.filter(user=user).first()
+            if pending_email:
+                pending_email.new_email = get_retired_email_by_email(pending_email.new_email)
+                pending_email.save(update_fields=['new_email'])
             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)
+            AccountRecovery.retire_recovery_email(user.id)
 
             # Retire any objects linked to the user via their original email
             CourseEnrollmentAllowed.delete_by_user_value(original_email, field="email")
@@ -1171,6 +1180,7 @@ def post(self, request):
             user.last_name = ""
             user.is_active = False
             user.username = retired_username
+            user.email = retired_email
             user.save()
         except UserRetirementStatus.DoesNotExist:
             return Response(status=status.HTTP_404_NOT_FOUND)

openedx/core/djangoapps/user_api/management/commands/retire_user.py

@@ -7,7 +7,12 @@
 from django.db import transaction
 from social_django.models import UserSocialAuth
 
-from common.djangoapps.student.models import AccountRecovery, Registration, get_retired_email_by_email
+from common.djangoapps.student.models import (
+    AccountRecovery,
+    PendingSecondaryEmailChange,
+    Registration,
+    get_retired_email_by_email,
+)
 from openedx.core.djangolib.oauth2_retirement_utils import retire_dot_oauth2_models
 
 from ...models import BulkUserRetirementConfig, UserRetirementStatus
@@ -158,6 +163,7 @@ def handle(self, *args, **options):
                     # Delete OAuth tokens associated with the user.
                     retire_dot_oauth2_models(user)
                     AccountRecovery.retire_recovery_email(user.id)
+                    PendingSecondaryEmailChange.redact_and_delete_pending_secondary_email(user.id)
         except KeyError:
             error_message = f'Username not specified {user}'
             logger.error(error_message)

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

@@ -10,6 +10,7 @@
 from django.contrib.auth.models import User  # lint-amnesty, pylint: disable=imported-auth-user
 from django.core.management import CommandError, call_command
 
+from common.djangoapps.student.models import PendingSecondaryEmailChange
 from common.djangoapps.student.tests.factories import UserFactory  # lint-amnesty, pylint: disable=wrong-import-order
 from openedx.core.djangoapps.user_api.accounts.tests.retirement_helpers import (  # lint-amnesty, pylint: disable=unused-import, wrong-import-order
     setup_retirement_states,  # noqa: F401
@@ -107,3 +108,18 @@ def test_retire_with_username_email_userfile(setup_retirement_states):  # lint-a
     with pytest.raises(CommandError, match=r'You cannot use userfile option with username and user_email'):
         call_command('retire_user', user_file=user_file, username=username, user_email=user_email)
     remove_user_file()
+
+
+@skip_unless_lms
+def test_retire_user_cleans_pending_secondary_email(setup_retirement_states):  # lint-amnesty, pylint: disable=redefined-outer-name, unused-argument  # noqa: F811
+    user = UserFactory.create(username='user-cleanup', email='user-cleanup@example.com')
+    PendingSecondaryEmailChange.objects.create(
+        user=user,
+        new_secondary_email='pending-secondary@example.com',
+        activation_key='c' * 32,
+    )
+    assert PendingSecondaryEmailChange.objects.filter(user=user).exists()
+
+    call_command('retire_user', username=user.username, user_email=user.email)
+
+    assert not PendingSecondaryEmailChange.objects.filter(user=user).exists()