Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 31 additions & 6 deletions common/djangoapps/student/models/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -933,14 +933,34 @@ class PendingSecondaryEmailChange(DeletableByUserValue, models.Model):
"""
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, retired 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_pending_secondary_email(cls, user_id):
"""
Retire 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 = get_retired_email_by_email(
pending_secondary_email.new_secondary_email
)
pending_secondary_email.save(update_fields=['new_secondary_email'])
pending_secondary_email.delete()
return True


class LoginFailures(models.Model):
"""
Expand Down Expand Up @@ -1690,16 +1710,21 @@ def retire_recovery_email(cls, user_id):
Retire user's recovery/secondary email as part of GDPR Phase I.
Returns 'True'

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.

:param user_id: int
:return: bool
"""
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 = get_retired_email_by_email(account_recovery.secondary_email)
account_recovery.is_active = False
account_recovery.save(update_fields=['secondary_email', 'is_active'])
account_recovery.delete()

return True

Expand Down
24 changes: 24 additions & 0 deletions common/djangoapps/student/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
ManualEnrollmentAudit,
PendingEmailChange,
PendingNameChange,
PendingSecondaryEmailChange,
UserAttribute,
UserCelebration,
UserProfile
Expand Down Expand Up @@ -745,6 +746,29 @@ 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_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_pending_secondary_email(user_id=user.id)

assert len(PendingSecondaryEmailChange.objects.filter(user_id=user.id)) == 0

def test_redact_pending_secondary_email_when_no_record(self):
"""Assert retirement cleanup returns True when no pending secondary row exists."""
user = UserFactory()
assert PendingSecondaryEmailChange.redact_pending_secondary_email(user_id=user.id) is True


@ddt.ddt
class TestUserPostSaveCallback(SharedModuleStoreTestCase):
"""
Expand Down
7 changes: 6 additions & 1 deletion common/djangoapps/student/views/management.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@
UserSignupSource,
UserStanding,
create_comments_service_user,
email_exists_or_retired
email_exists_or_retired,
get_retired_email_by_email,
)
from common.djangoapps.student.signals import REFUND_ORDER
from common.djangoapps.util.db import outer_atomic
Expand Down Expand Up @@ -862,6 +863,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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
ManualEnrollmentAudit,
PendingEmailChange,
PendingNameChange,
PendingSecondaryEmailChange,
Registration,
SocialLink,
UserProfile,
Expand Down Expand Up @@ -235,6 +236,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
Expand Down Expand Up @@ -1394,6 +1413,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')
Expand Down Expand Up @@ -1500,6 +1531,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()

Expand Down
8 changes: 7 additions & 1 deletion openedx/core/djangoapps/user_api/accounts/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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_pending_secondary_email(user.id)


def username_suffix_generator(suffix_length=4):
Expand Down
11 changes: 11 additions & 0 deletions openedx/core/djangoapps/user_api/accounts/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,13 @@
from common.djangoapps.track import segment
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,
Expand Down Expand Up @@ -1099,6 +1101,7 @@ def post(self, request):
retirement = UserRetirementStatus.get_retirement_for_retirement_action(username)
RevisionPluginRevision.retire_user(retirement.user)
ArticleRevision.retire_user(retirement.user)
PendingNameChange.objects.filter(user=retirement.user).update(new_name="redacted", rationale="redacted")
PendingNameChange.delete_by_user_value(retirement.user, field="user")
ManualEnrollmentAudit.retire_manual_enrollments(retirement.user, retirement.retired_email)

Expand Down Expand Up @@ -1195,8 +1198,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_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")
Expand All @@ -1214,6 +1224,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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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_pending_secondary_email(user.id)
except KeyError:
error_message = f'Username not specified {user}'
logger.error(error_message)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,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 ...models import UserRetirementStatus
from openedx.core.djangoapps.user_api.accounts.tests.retirement_helpers import ( # lint-amnesty, pylint: disable=unused-import, wrong-import-order
setup_retirement_states
Expand Down Expand Up @@ -105,3 +106,19 @@ 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
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()
Loading