diff --git a/common/djangoapps/student/models/user.py b/common/djangoapps/student/models/user.py index 8cd1fcef7aa4..c8f471ecfe7d 100644 --- a/common/djangoapps/student/models/user.py +++ b/common/djangoapps/student/models/user.py @@ -903,7 +903,7 @@ class PendingEmailChange(DeletableByUserValue, models.Model): """ This model keeps track of pending requested changes to a user's email address. - .. pii: Contains new_email, retired in AccountRetirementView + .. pii: Contains new_email, redacted then deleted in AccountRetirementView .. pii_types: email_address .. pii_retirement: local_api """ @@ -911,6 +911,26 @@ class PendingEmailChange(DeletableByUserValue, models.Model): new_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_pending_email_by_user_value(cls, value, field): + """ + Redact pending email change fields for records matching ``field=value``. + + This method is intended for retirement flows where downstream replication + may keep soft-deleted snapshots of these rows. + """ + filter_kwargs = {field: value} + records_matching_user_value = cls.objects.filter(**filter_kwargs) + + if not records_matching_user_value.exists(): + return False + + for record in records_matching_user_value: + record.new_email = get_retired_email_by_email(record.new_email) + record.save(update_fields=['new_email']) + + return True + def request_change(self, email): """Request a change to a user's email. diff --git a/common/djangoapps/student/tests/test_models.py b/common/djangoapps/student/tests/test_models.py index 02df1a6714c6..a433a1ac2e5a 100644 --- a/common/djangoapps/student/tests/test_models.py +++ b/common/djangoapps/student/tests/test_models.py @@ -30,7 +30,8 @@ PendingNameChange, UserAttribute, UserCelebration, - UserProfile + UserProfile, + get_retired_email_by_email, ) from common.djangoapps.student.models_api import confirm_name_change, do_name_change_request, get_name from common.djangoapps.student.tests.factories import AccountRecoveryFactory, CourseEnrollmentFactory, UserFactory @@ -593,11 +594,34 @@ def test_delete_by_user_removes_pending_email_change(self): assert record_was_deleted assert 0 == len(PendingEmailChange.objects.all()) + def test_redact_by_user_redacts_pending_email_change_fields(self): + original_new_email = self.email_change.new_email + original_activation_key = self.email_change.activation_key + expected_retired_email = get_retired_email_by_email(original_new_email) + + record_was_redacted = PendingEmailChange.redact_pending_email_by_user_value(self.user, field='user') + + assert record_was_redacted + self.email_change.refresh_from_db() + assert self.email_change.new_email == expected_retired_email + assert self.email_change.activation_key == original_activation_key + def test_delete_by_user_no_effect_for_user_with_no_email_change(self): record_was_deleted = PendingEmailChange.delete_by_user_value(self.user2, field='user') assert not record_was_deleted assert 1 == len(PendingEmailChange.objects.all()) + def test_redact_by_user_no_effect_for_user_with_no_email_change(self): + original_new_email = self.email_change.new_email + original_activation_key = self.email_change.activation_key + + record_was_redacted = PendingEmailChange.redact_pending_email_by_user_value(self.user2, field='user') + + assert not record_was_redacted + self.email_change.refresh_from_db() + assert self.email_change.new_email == original_new_email + assert self.email_change.activation_key == original_activation_key + class TestCourseEnrollmentAllowed(ModuleStoreTestCase): # lint-amnesty, pylint: disable=missing-class-docstring diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py index 5dc1f170c7f6..92e07ae8963a 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py @@ -1466,7 +1466,13 @@ def test_retire_user_where_username_not_provided(self): @mock.patch('openedx.core.djangoapps.user_api.accounts.views.get_profile_image_names') @mock.patch('openedx.core.djangoapps.user_api.accounts.views.remove_profile_images') - def test_retire_user(self, mock_remove_profile_images, mock_get_profile_image_names): + @mock.patch('openedx.core.djangoapps.user_api.accounts.views.PendingEmailChange.redact_pending_email_by_user_value') + def test_retire_user( + self, + mock_redact_pending_email, + mock_remove_profile_images, + mock_get_profile_image_names, + ): data = {'username': self.original_username} self.post_and_assert_status(data) @@ -1497,6 +1503,7 @@ def test_retire_user(self, mock_remove_profile_images, mock_get_profile_image_na self._entitlement_support_detail_assertions() + mock_redact_pending_email.assert_called_once_with(self.test_user, field="user") assert not PendingEmailChange.objects.filter(user=self.test_user).exists() assert not UserOrgTag.objects.filter(user=self.test_user).exists() @@ -1509,6 +1516,24 @@ def test_retire_user_twice_idempotent(self): fake_completed_retirement(self.test_user) self.post_and_assert_status(data) + @mock.patch('openedx.core.djangoapps.user_api.accounts.views.PendingEmailChange.delete_by_user_value') + def test_retire_user_redacts_pending_email_before_delete(self, mock_delete_pending_email): + pending_email_before_retirement = PendingEmailChange.objects.get(user=self.test_user).new_email + expected_retired_pending_email = get_retired_email_by_email(pending_email_before_retirement) + + def _assert_redacted_then_delete(value, field): + pending_record = PendingEmailChange.objects.get(user=self.test_user) + assert pending_record.new_email == expected_retired_pending_email + pending_record.delete() + return True + + mock_delete_pending_email.side_effect = _assert_redacted_then_delete + + data = {'username': self.original_username} + self.post_and_assert_status(data) + + assert not PendingEmailChange.objects.filter(user=self.test_user).exists() + @mock.patch('openedx.core.djangoapps.user_api.accounts.views.USER_RETIRE_LMS_CRITICAL') def test_retirement_sends_critical_signal_with_retirement_data(self, mock_signal): """ diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index 796aaa3090a3..98cd654b0cf7 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -1193,7 +1193,10 @@ def post(self, request): self.retire_entitlement_support_detail(user) - # Retire misc. models that may contain PII of this user + # Retire misc. models that may contain PII of this user. + # Redact pending primary email fields before delete because + # downstream replication can preserve soft-deleted snapshots. + 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")