Skip to content

Commit 5e6a070

Browse files
fix: redact pending primary email before retirement deletion
1 parent 759a9bb commit 5e6a070

4 files changed

Lines changed: 66 additions & 3 deletions

File tree

common/djangoapps/student/models/user.py

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -904,14 +904,30 @@ class PendingEmailChange(DeletableByUserValue, models.Model): # noqa: DJ008
904904
"""
905905
This model keeps track of pending requested changes to a user's email address.
906906
907-
.. pii: Contains new_email, retired in AccountRetirementView
907+
.. pii: Contains new_email, redacted then deleted in AccountRetirementView
908908
.. pii_types: email_address
909909
.. pii_retirement: local_api
910910
"""
911911
user = models.OneToOneField(User, unique=True, db_index=True, on_delete=models.CASCADE)
912912
new_email = models.CharField(blank=True, max_length=255, db_index=True)
913913
activation_key = models.CharField(('activation key'), max_length=32, unique=True, db_index=True)
914914

915+
@classmethod
916+
def redact_pending_email_by_user_value(cls, value, field):
917+
"""
918+
Redact pending email change fields for records matching ``field=value``.
919+
This method is intended for retirement flows where downstream systems
920+
may keep soft-deleted snapshots of these rows.
921+
"""
922+
filter_kwargs = {field: value}
923+
records_matching_user_value = cls.objects.filter(**filter_kwargs)
924+
if not records_matching_user_value.exists():
925+
return False
926+
for record in records_matching_user_value:
927+
record.new_email = get_retired_email_by_email(record.new_email)
928+
record.save(update_fields=['new_email'])
929+
return True
930+
915931
def request_change(self, email):
916932
"""Request a change to a user's email.
917933

common/djangoapps/student/tests/test_models.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
UserAttribute,
3131
UserCelebration,
3232
UserProfile,
33+
get_retired_email_by_email,
3334
)
3435
from common.djangoapps.student.models_api import confirm_name_change, do_name_change_request, get_name
3536
from common.djangoapps.student.tests.factories import AccountRecoveryFactory, CourseEnrollmentFactory, UserFactory
@@ -600,6 +601,25 @@ def test_delete_by_user_no_effect_for_user_with_no_email_change(self):
600601
assert not record_was_deleted
601602
assert 1 == len(PendingEmailChange.objects.all())
602603

604+
def test_redact_by_user_redacts_pending_email_change_fields(self):
605+
original_new_email = self.email_change.new_email
606+
original_activation_key = self.email_change.activation_key
607+
expected_retired_email = get_retired_email_by_email(original_new_email)
608+
record_was_redacted = PendingEmailChange.redact_pending_email_by_user_value(self.user, field='user')
609+
assert record_was_redacted
610+
self.email_change.refresh_from_db()
611+
assert self.email_change.new_email == expected_retired_email
612+
assert self.email_change.activation_key == original_activation_key
613+
614+
def test_redact_by_user_no_effect_for_user_with_no_email_change(self):
615+
original_new_email = self.email_change.new_email
616+
original_activation_key = self.email_change.activation_key
617+
record_was_redacted = PendingEmailChange.redact_pending_email_by_user_value(self.user2, field='user')
618+
assert not record_was_redacted
619+
self.email_change.refresh_from_db()
620+
assert self.email_change.new_email == original_new_email
621+
assert self.email_change.activation_key == original_activation_key
622+
603623

604624
class TestCourseEnrollmentAllowed(ModuleStoreTestCase): # lint-amnesty, pylint: disable=missing-class-docstring
605625

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

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1365,7 +1365,13 @@ def test_retire_user_where_username_not_provided(self):
13651365

13661366
@mock.patch('openedx.core.djangoapps.user_api.accounts.views.get_profile_image_names')
13671367
@mock.patch('openedx.core.djangoapps.user_api.accounts.views.remove_profile_images')
1368-
def test_retire_user(self, mock_remove_profile_images, mock_get_profile_image_names):
1368+
@mock.patch('openedx.core.djangoapps.user_api.accounts.views.PendingEmailChange.redact_pending_email_by_user_value')
1369+
def test_retire_user(
1370+
self,
1371+
mock_redact_pending_email,
1372+
mock_remove_profile_images,
1373+
mock_get_profile_image_names,
1374+
):
13691375
data = {'username': self.original_username}
13701376
self.post_and_assert_status(data)
13711377

@@ -1396,6 +1402,7 @@ def test_retire_user(self, mock_remove_profile_images, mock_get_profile_image_na
13961402

13971403
self._entitlement_support_detail_assertions()
13981404

1405+
mock_redact_pending_email.assert_called_once_with(self.test_user, field="user")
13991406
assert not PendingEmailChange.objects.filter(user=self.test_user).exists()
14001407
assert not UserOrgTag.objects.filter(user=self.test_user).exists()
14011408

@@ -1408,6 +1415,23 @@ def test_retire_user_twice_idempotent(self):
14081415
fake_completed_retirement(self.test_user)
14091416
self.post_and_assert_status(data)
14101417

1418+
@mock.patch('openedx.core.djangoapps.user_api.accounts.views.PendingEmailChange.delete_by_user_value')
1419+
def test_retire_user_redacts_pending_email_before_delete(self, mock_delete_pending_email):
1420+
pending_email_record = PendingEmailChange.objects.get(user=self.test_user)
1421+
pending_email_before_retirement = pending_email_record.new_email
1422+
expected_retired_pending_email = get_retired_email_by_email(pending_email_before_retirement)
1423+
1424+
def _assert_redacted_then_delete(value, field):
1425+
pending_record = PendingEmailChange.objects.get(user=self.test_user)
1426+
assert pending_record.new_email == expected_retired_pending_email
1427+
pending_record.delete()
1428+
return True
1429+
1430+
mock_delete_pending_email.side_effect = _assert_redacted_then_delete
1431+
data = {'username': self.original_username}
1432+
self.post_and_assert_status(data)
1433+
assert not PendingEmailChange.objects.filter(user=self.test_user).exists()
1434+
14111435
@mock.patch('openedx.core.djangoapps.user_api.accounts.views.USER_RETIRE_LMS_CRITICAL')
14121436
def test_retirement_sends_critical_signal_with_retirement_data(self, mock_signal):
14131437
"""

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1151,7 +1151,10 @@ def post(self, request):
11511151

11521152
self.retire_entitlement_support_detail(user)
11531153

1154-
# Retire misc. models that may contain PII of this user
1154+
# Retire misc. models that may contain PII of this user.
1155+
# Redact pending email before delete because downstream systems
1156+
# may preserve soft-deleted snapshots.
1157+
PendingEmailChange.redact_pending_email_by_user_value(user, field="user")
11551158
PendingEmailChange.delete_by_user_value(user, field="user")
11561159
UserOrgTag.delete_by_user_value(user, field="user")
11571160

0 commit comments

Comments
 (0)