diff --git a/common/djangoapps/student/models/user.py b/common/djangoapps/student/models/user.py index 6b7cdb7e761a..423c86e656cc 100644 --- a/common/djangoapps/student/models/user.py +++ b/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 diff --git a/common/djangoapps/student/tests/test_models.py b/common/djangoapps/student/tests/test_models.py index 09a1d35fa424..3a565bcf3c36 100644 --- a/common/djangoapps/student/tests/test_models.py +++ b/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): """ diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index cf7948b448bb..2e5bb5a4d61c 100644 --- a/common/djangoapps/student/views/management.py +++ b/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") 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 dfc07b643b9d..13a3f1e814c5 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 @@ -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() diff --git a/openedx/core/djangoapps/user_api/accounts/utils.py b/openedx/core/djangoapps/user_api/accounts/utils.py index ab70b8f15c8d..6b5448eeefab 100644 --- a/openedx/core/djangoapps/user_api/accounts/utils.py +++ b/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): diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index 90d7ba70cb86..21131fbc7539 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/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) 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 1b3b497ea5c7..8ab371a1fe70 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,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) 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 6de2287fc4ba..1d29c46b3388 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 @@ -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()