From d7a28df567eacd1458fadb82a3dbe10331f42eed Mon Sep 17 00:00:00 2001 From: ktyagiapphelix2u Date: Tue, 2 Jun 2026 09:40:14 +0000 Subject: [PATCH 1/6] fix: retirement PII leaks by redacting pending secondary email/name data --- common/djangoapps/student/models/user.py | 59 +++++++++++++---- .../djangoapps/student/tests/test_models.py | 63 ++++++++++++++++--- common/djangoapps/student/views/management.py | 4 ++ .../accounts/tests/test_retirement_views.py | 18 ++++++ .../djangoapps/user_api/accounts/utils.py | 8 ++- .../djangoapps/user_api/accounts/views.py | 7 ++- .../management/commands/retire_user.py | 8 ++- .../management/tests/test_retire_user.py | 30 ++++++++- 8 files changed, 175 insertions(+), 22 deletions(-) diff --git a/common/djangoapps/student/models/user.py b/common/djangoapps/student/models/user.py index 159c784a28eb..9e0e88b182b1 100644 --- a/common/djangoapps/student/models/user.py +++ b/common/djangoapps/student/models/user.py @@ -76,6 +76,9 @@ USER_LOGGED_OUT_EVENT_NAME = 'edx.user.logout' USER_STREAK_UPDATED_EVENT_NAME = "edx.user.celebration.streak_updated" +# Placeholder for soft-deleted pending secondary email records +PENDING_SECONDARY_EMAIL_REDACTED_VALUE = 'redact-before-delete@redacted.com' + class AnonymousUserId(models.Model): # noqa: DJ008 """ @@ -941,14 +944,37 @@ 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, redact and delete 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. + """ + pending_secondary_email_records = cls.objects.filter(user_id=user_id) + pending_secondary_email_ids = list( + pending_secondary_email_records.values_list('id', flat=True) + ) + if not pending_secondary_email_ids: + return False + + # Converting to query set by id ensures we redact and delete the appropriate records + pending_secondary_email_records_by_id = cls.objects.filter(id__in=pending_secondary_email_ids) + pending_secondary_email_records_by_id.update( + new_secondary_email=PENDING_SECONDARY_EMAIL_REDACTED_VALUE, + ) + pending_secondary_email_records_by_id.delete() + return True + class LoginFailures(models.Model): """ @@ -1695,7 +1721,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, redact and delete in the `DeactivateLogoutView` .. pii_types: email_address .. pii_retirement: local_api """ @@ -1728,19 +1754,30 @@ 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 redacted and + 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 deleted, - if it is not found it is assumed this table has no PII for the given user. + Note: In retire_recovery_email, "retire" means this is part of user retirement. + The recovery email itself remains available for use with future accounts. :param user_id: int :return: bool """ - try: - cls.objects.get(user_id=user_id).delete() - except cls.DoesNotExist: - pass + account_recovery_records = cls.objects.filter(user_id=user_id) + account_recovery_ids = list( + account_recovery_records.values_list('id', flat=True) + ) + if not account_recovery_ids: + return False + + # Converting to query set by id ensures we redact and delete the appropriate records + account_recovery_records_by_id = cls.objects.filter(id__in=account_recovery_ids) + account_recovery_records_by_id.update( + secondary_email=f'redact-before-delete+{user_id}@redacted.com', + ) + account_recovery_records_by_id.delete() return True diff --git a/common/djangoapps/student/tests/test_models.py b/common/djangoapps/student/tests/test_models.py index c4650578b32e..60f8f23b1b24 100644 --- a/common/djangoapps/student/tests/test_models.py +++ b/common/djangoapps/student/tests/test_models.py @@ -12,7 +12,7 @@ from django.core.cache import cache from django.db import connection from django.db.models.functions import Lower -from django.test import TestCase, override_settings +from django.test import override_settings, TestCase from django.test.utils import CaptureQueriesContext from edx_toggles.toggles.testutils import override_waffle_flag from freezegun import freeze_time @@ -27,8 +27,10 @@ CourseEnrollment, CourseEnrollmentAllowed, ManualEnrollmentAudit, + PENDING_SECONDARY_EMAIL_REDACTED_VALUE, PendingEmailChange, PendingNameChange, + PendingSecondaryEmailChange, UserAttribute, UserCelebration, UserProfile, @@ -758,19 +760,66 @@ class TestAccountRecovery(TestCase): def test_retire_recovery_email(self): """ - Assert that Account Record for a given user is deleted when `retire_recovery_email` is called + Assert that AccountRecovery secondary_email is redacted before the record is deleted. """ - # Create user and associated recovery email record user = UserFactory() - AccountRecoveryFactory(user=user) + AccountRecoveryFactory(user=user, secondary_email='recovery@example.com') assert len(AccountRecovery.objects.filter(user_id=user.id)) == 1 - # Retire recovery email - AccountRecovery.retire_recovery_email(user_id=user.id) + with CaptureQueriesContext(connection) as ctx: + AccountRecovery.retire_recovery_email(user_id=user.id) + + assert_redact_before_delete( + [q['sql'] for q in ctx], + table=AccountRecovery._meta.db_table, + expected_redacted_value_list=[f'redact-before-delete+{user.id}@redacted.com'], + ) - # Assert that there is no longer an AccountRecovery record for this user assert len(AccountRecovery.objects.filter(user_id=user.id)) == 0 + def test_retire_recovery_email_when_no_record(self): + """ + Assert retirement cleanup returns False when no account recovery row exists. + """ + user = UserFactory() + assert AccountRecovery.retire_recovery_email(user_id=user.id) is False + + +class TestPendingSecondaryEmailChange(TestCase): + """ + Tests for retiring PendingSecondaryEmailChange records. + """ + + def test_redact_and_delete_pending_secondary_email(self): + """ + Assert that the pending secondary email is redacted before the record is deleted. + """ + 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 + + with CaptureQueriesContext(connection) as ctx: + PendingSecondaryEmailChange.redact_and_delete_pending_secondary_email(user_id=user.id) + + assert_redact_before_delete( + [query['sql'] for query in ctx], + table=PendingSecondaryEmailChange._meta.db_table, + expected_redacted_value_list=[PENDING_SECONDARY_EMAIL_REDACTED_VALUE], + ) + + 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 False when no pending secondary row exists. + """ + user = UserFactory() + assert PendingSecondaryEmailChange.redact_and_delete_pending_secondary_email(user_id=user.id) is False + @ddt.ddt class TestUserPostSaveCallback(SharedModuleStoreTestCase): diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index 2f1d7bd535e5..5609afd49ddc 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -70,6 +70,7 @@ RecoveryEmailCreate, ) from common.djangoapps.student.models import ( # pylint: disable=unused-import + PENDING_SECONDARY_EMAIL_REDACTED_VALUE, AccountRecovery, CourseEnrollment, EnrollmentNotAllowed, @@ -890,6 +891,9 @@ def activate_secondary_email(request, key): 'secondary_email': pending_secondary_email_change.new_secondary_email }) + # Redact the pending email before deletion so downstream soft-delete mirrors do not retain the original address. + pending_secondary_email_change.new_secondary_email = PENDING_SECONDARY_EMAIL_REDACTED_VALUE + pending_secondary_email_change.save(update_fields=['new_secondary_email']) pending_secondary_email_change.delete() return render_to_response("secondary_email_change_successful.html") 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 35c8914117e6..657958fee63d 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 @@ -31,6 +31,7 @@ ManualEnrollmentAudit, PendingEmailChange, PendingNameChange, + PendingSecondaryEmailChange, Registration, SocialLink, UserProfile, @@ -1395,6 +1396,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') @@ -1486,6 +1499,7 @@ def test_retire_user(self, mock_remove_profile_images, mock_get_profile_image_na 'last_name': '', 'is_active': False, 'username': self.retired_username, + 'email': self.retired_email, } for field, expected_value in expected_user_values.items(): assert expected_value == getattr(self.test_user, field) @@ -1508,6 +1522,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() diff --git a/openedx/core/djangoapps/user_api/accounts/utils.py b/openedx/core/djangoapps/user_api/accounts/utils.py index 083f45ecc9e0..61f8092041cf 100644 --- a/openedx/core/djangoapps/user_api/accounts/utils.py +++ b/openedx/core/djangoapps/user_api/accounts/utils.py @@ -17,7 +17,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 @@ -249,6 +254,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): diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index c6515390fed1..3c873362254c 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -36,12 +36,14 @@ from wiki.models.pluginbase import RevisionPluginRevision from common.djangoapps.entitlements.models import CourseEntitlement -from common.djangoapps.student.models import ( # pylint: disable=unused-import +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, @@ -1173,6 +1175,8 @@ def post(self, request): # Retire misc. models that may contain PII of this 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) + 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") @@ -1190,6 +1194,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) diff --git a/openedx/core/djangoapps/user_api/management/commands/retire_user.py b/openedx/core/djangoapps/user_api/management/commands/retire_user.py index 3ce738b149c0..eeedfdf9fab0 100644 --- a/openedx/core/djangoapps/user_api/management/commands/retire_user.py +++ b/openedx/core/djangoapps/user_api/management/commands/retire_user.py @@ -6,7 +6,12 @@ from django.core.management.base import BaseCommand, CommandError from django.db import transaction -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 ...accounts.utils import redact_and_delete_social_auth @@ -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) diff --git a/openedx/core/djangoapps/user_api/management/tests/test_retire_user.py b/openedx/core/djangoapps/user_api/management/tests/test_retire_user.py index e57c1c50c938..e1566a1cc923 100644 --- a/openedx/core/djangoapps/user_api/management/tests/test_retire_user.py +++ b/openedx/core/djangoapps/user_api/management/tests/test_retire_user.py @@ -15,6 +15,7 @@ from django.test.utils import CaptureQueriesContext from social_django.models import UserSocialAuth +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.signals import ( redact_social_auth_pii_before_deletion, @@ -25,7 +26,10 @@ from openedx.core.djangoapps.user_api.accounts.tests.test_utils import ( assert_update_before_delete, ) -from openedx.core.djangolib.testing.utils import skip_unless_lms # pylint: disable=wrong-import-order +from openedx.core.djangolib.testing.utils import ( # pylint: disable=wrong-import-order + assert_redact_before_delete, + skip_unless_lms, +) from ...models import UserRetirementStatus @@ -133,6 +137,30 @@ def test_retire_with_username_email_userfile(setup_retirement_states): # pylint @skip_unless_lms +def test_retire_user_redacts_and_deletes_pending_secondary_email(setup_retirement_states): # lint-amnesty, pylint: disable=redefined-outer-name, unused-argument # noqa: F811 + """ + Verify pending secondary email rows are redacted before delete during retire_user. + """ + 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() + + with CaptureQueriesContext(connection) as ctx: + call_command('retire_user', username=user.username, user_email=user.email) + + assert_redact_before_delete( + [query['sql'] for query in ctx], + table='student_pendingsecondaryemailchange', + expected_redacted_value_list=['redact-before-delete@redacted.com'], + ) + + assert not PendingSecondaryEmailChange.objects.filter(user=user).exists() + + @pytest.mark.parametrize('social_auth_configs', [ # Single SSO provider [ From 87813d18bc72721032ff33db7042f3b1c6647a20 Mon Sep 17 00:00:00 2001 From: ktyagiapphelix2u Date: Tue, 2 Jun 2026 09:49:52 +0000 Subject: [PATCH 2/6] fix: retirement PII leaks by redacting pending secondary email/name data --- common/djangoapps/student/tests/test_models.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/djangoapps/student/tests/test_models.py b/common/djangoapps/student/tests/test_models.py index 60f8f23b1b24..69e6d3916e3d 100644 --- a/common/djangoapps/student/tests/test_models.py +++ b/common/djangoapps/student/tests/test_models.py @@ -12,7 +12,7 @@ from django.core.cache import cache from django.db import connection from django.db.models.functions import Lower -from django.test import override_settings, TestCase +from django.test import TestCase, override_settings from django.test.utils import CaptureQueriesContext from edx_toggles.toggles.testutils import override_waffle_flag from freezegun import freeze_time @@ -23,11 +23,11 @@ from common.djangoapps.student.models import ( ALLOWEDTOENROLL_TO_ENROLLED, IS_MARKETABLE, + PENDING_SECONDARY_EMAIL_REDACTED_VALUE, AccountRecovery, CourseEnrollment, CourseEnrollmentAllowed, ManualEnrollmentAudit, - PENDING_SECONDARY_EMAIL_REDACTED_VALUE, PendingEmailChange, PendingNameChange, PendingSecondaryEmailChange, From c1af3f04786cb154e447e33f29897ed62b4f1ff2 Mon Sep 17 00:00:00 2001 From: ktyagiapphelix2u Date: Tue, 2 Jun 2026 11:34:29 +0000 Subject: [PATCH 3/6] fix: retirement PII leaks by redacting pending secondary email/name data --- common/djangoapps/student/models/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/djangoapps/student/models/user.py b/common/djangoapps/student/models/user.py index 9e0e88b182b1..0631edd3d5d6 100644 --- a/common/djangoapps/student/models/user.py +++ b/common/djangoapps/student/models/user.py @@ -1754,7 +1754,7 @@ def update_recovery_email(self, email): @classmethod def retire_recovery_email(cls, user_id): """ - Redact and delete user's recovery/secondary email as part of GDPR Phase I. + Redact and delete user's recovery/secondary email. 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. From a2ce50a0e3146db1df3ea039bece56e23de3df5e Mon Sep 17 00:00:00 2001 From: ktyagiapphelix2u Date: Wed, 3 Jun 2026 06:24:57 +0000 Subject: [PATCH 4/6] fix: retirement PII leaks by redacting pending secondary email/name data --- .../djangoapps/user_api/accounts/utils.py | 4 +++ .../management/commands/retire_user.py | 28 ++----------------- .../management/tests/test_retire_user.py | 25 ++++------------- 3 files changed, 12 insertions(+), 45 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/utils.py b/openedx/core/djangoapps/user_api/accounts/utils.py index 61f8092041cf..c5c568219d48 100644 --- a/openedx/core/djangoapps/user_api/accounts/utils.py +++ b/openedx/core/djangoapps/user_api/accounts/utils.py @@ -19,6 +19,7 @@ from common.djangoapps.student.models import ( AccountRecovery, + PendingEmailChange, PendingSecondaryEmailChange, Registration, get_retired_email_by_email, @@ -254,6 +255,9 @@ 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) + + # Delete any pending account changes for added safety around account lock-out. + PendingEmailChange.delete_by_user_value(user, field='user') PendingSecondaryEmailChange.redact_and_delete_pending_secondary_email(user.id) diff --git a/openedx/core/djangoapps/user_api/management/commands/retire_user.py b/openedx/core/djangoapps/user_api/management/commands/retire_user.py index eeedfdf9fab0..ab1c77abf486 100644 --- a/openedx/core/djangoapps/user_api/management/commands/retire_user.py +++ b/openedx/core/djangoapps/user_api/management/commands/retire_user.py @@ -6,15 +6,7 @@ from django.core.management.base import BaseCommand, CommandError from django.db import transaction -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 ...accounts.utils import redact_and_delete_social_auth +from ...accounts.utils import create_retirement_request_and_deactivate_account from ...models import BulkUserRetirementConfig, UserRetirementStatus logger = logging.getLogger(__name__) @@ -147,23 +139,7 @@ def handle(self, *args, **options): try: with transaction.atomic(): for user in users: - # Add user to retirement queue. - UserRetirementStatus.create_retirement(user) - # Redact and unlink LMS social auth accounts. - redact_and_delete_social_auth(user.id) - # Change LMS password & email - user.email = get_retired_email_by_email(user.email) - user.set_unusable_password() - user.save() - - # TODO: Unlink social accounts & change password on each IDA. - # Remove the activation keys sent by email to the user for account activation. - Registration.objects.filter(user=user).delete() - - # 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) + create_retirement_request_and_deactivate_account(user) except KeyError: error_message = f'Username not specified {user}' logger.error(error_message) diff --git a/openedx/core/djangoapps/user_api/management/tests/test_retire_user.py b/openedx/core/djangoapps/user_api/management/tests/test_retire_user.py index e1566a1cc923..094cc9d0f720 100644 --- a/openedx/core/djangoapps/user_api/management/tests/test_retire_user.py +++ b/openedx/core/djangoapps/user_api/management/tests/test_retire_user.py @@ -6,6 +6,7 @@ import csv import os from contextlib import contextmanager +from unittest import mock import pytest from django.contrib.auth.models import User # pylint: disable=imported-auth-user @@ -15,7 +16,6 @@ from django.test.utils import CaptureQueriesContext from social_django.models import UserSocialAuth -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.signals import ( redact_social_auth_pii_before_deletion, @@ -27,7 +27,6 @@ assert_update_before_delete, ) from openedx.core.djangolib.testing.utils import ( # pylint: disable=wrong-import-order - assert_redact_before_delete, skip_unless_lms, ) @@ -137,28 +136,16 @@ def test_retire_with_username_email_userfile(setup_retirement_states): # pylint @skip_unless_lms -def test_retire_user_redacts_and_deletes_pending_secondary_email(setup_retirement_states): # lint-amnesty, pylint: disable=redefined-outer-name, unused-argument # noqa: F811 +@mock.patch('openedx.core.djangoapps.user_api.management.commands.retire_user.create_retirement_request_and_deactivate_account') +def test_retire_user_calls_shared_deactivate_helper(mock_create_retirement_request_and_deactivate_account, setup_retirement_states): # lint-amnesty, pylint: disable=redefined-outer-name, unused-argument # noqa: F811 """ - Verify pending secondary email rows are redacted before delete during retire_user. + Verify the command delegates retirement side effects to the shared helper. """ 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() - - with CaptureQueriesContext(connection) as ctx: - call_command('retire_user', username=user.username, user_email=user.email) - assert_redact_before_delete( - [query['sql'] for query in ctx], - table='student_pendingsecondaryemailchange', - expected_redacted_value_list=['redact-before-delete@redacted.com'], - ) + call_command('retire_user', username=user.username, user_email=user.email) - assert not PendingSecondaryEmailChange.objects.filter(user=user).exists() + mock_create_retirement_request_and_deactivate_account.assert_called_once_with(user) @pytest.mark.parametrize('social_auth_configs', [ From 11561c47ee2c2407cfd320c9fcb147e6c245ea7f Mon Sep 17 00:00:00 2001 From: ktyagiapphelix2u Date: Wed, 3 Jun 2026 07:18:10 +0000 Subject: [PATCH 5/6] fix: retirement PII leaks by redacting pending secondary email/name data --- .../user_api/management/commands/retire_user.py | 2 +- .../user_api/management/tests/test_retire_user.py | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/openedx/core/djangoapps/user_api/management/commands/retire_user.py b/openedx/core/djangoapps/user_api/management/commands/retire_user.py index ab1c77abf486..e6577e1c5960 100644 --- a/openedx/core/djangoapps/user_api/management/commands/retire_user.py +++ b/openedx/core/djangoapps/user_api/management/commands/retire_user.py @@ -7,7 +7,7 @@ from django.db import transaction from ...accounts.utils import create_retirement_request_and_deactivate_account -from ...models import BulkUserRetirementConfig, UserRetirementStatus +from ...models import BulkUserRetirementConfig logger = logging.getLogger(__name__) diff --git a/openedx/core/djangoapps/user_api/management/tests/test_retire_user.py b/openedx/core/djangoapps/user_api/management/tests/test_retire_user.py index 094cc9d0f720..1c1fbd4e1e40 100644 --- a/openedx/core/djangoapps/user_api/management/tests/test_retire_user.py +++ b/openedx/core/djangoapps/user_api/management/tests/test_retire_user.py @@ -136,8 +136,10 @@ def test_retire_with_username_email_userfile(setup_retirement_states): # pylint @skip_unless_lms -@mock.patch('openedx.core.djangoapps.user_api.management.commands.retire_user.create_retirement_request_and_deactivate_account') -def test_retire_user_calls_shared_deactivate_helper(mock_create_retirement_request_and_deactivate_account, setup_retirement_states): # lint-amnesty, pylint: disable=redefined-outer-name, unused-argument # noqa: F811 +@mock.patch( + 'openedx.core.djangoapps.user_api.management.commands.retire_user.create_retirement_request_and_deactivate_account' +) +def test_retire_user_calls_shared_deactivate_helper(mock_deactivate_helper, setup_retirement_states): # lint-amnesty, pylint: disable=redefined-outer-name, unused-argument # noqa: F811 """ Verify the command delegates retirement side effects to the shared helper. """ @@ -145,7 +147,7 @@ def test_retire_user_calls_shared_deactivate_helper(mock_create_retirement_reque call_command('retire_user', username=user.username, user_email=user.email) - mock_create_retirement_request_and_deactivate_account.assert_called_once_with(user) + mock_deactivate_helper.assert_called_once_with(user) @pytest.mark.parametrize('social_auth_configs', [ From 897786b874fd48d45dfb2fcb84548b6f2d06dd03 Mon Sep 17 00:00:00 2001 From: ktyagiapphelix2u Date: Thu, 4 Jun 2026 06:21:49 +0000 Subject: [PATCH 6/6] fix: retirement PII leaks by redacting pending secondary email/name data --- .../djangoapps/user_api/management/tests/test_retire_user.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/openedx/core/djangoapps/user_api/management/tests/test_retire_user.py b/openedx/core/djangoapps/user_api/management/tests/test_retire_user.py index 1c1fbd4e1e40..87d389ac3a2e 100644 --- a/openedx/core/djangoapps/user_api/management/tests/test_retire_user.py +++ b/openedx/core/djangoapps/user_api/management/tests/test_retire_user.py @@ -26,7 +26,7 @@ from openedx.core.djangoapps.user_api.accounts.tests.test_utils import ( assert_update_before_delete, ) -from openedx.core.djangolib.testing.utils import ( # pylint: disable=wrong-import-order +from openedx.core.djangolib.testing.utils import ( skip_unless_lms, ) @@ -136,10 +136,11 @@ def test_retire_with_username_email_userfile(setup_retirement_states): # pylint @skip_unless_lms +@pytest.mark.usefixtures('setup_retirement_states') @mock.patch( 'openedx.core.djangoapps.user_api.management.commands.retire_user.create_retirement_request_and_deactivate_account' ) -def test_retire_user_calls_shared_deactivate_helper(mock_deactivate_helper, setup_retirement_states): # lint-amnesty, pylint: disable=redefined-outer-name, unused-argument # noqa: F811 +def test_retire_user_calls_shared_deactivate_helper(mock_deactivate_helper): """ Verify the command delegates retirement side effects to the shared helper. """