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..f4c931f23eee 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 @@ -1707,3 +1707,42 @@ def test_retire_user_twice_idempotent(self): self.post_and_assert_status(data) fake_completed_retirement(self.test_user) self.post_and_assert_status(data) + + @mock.patch('openedx.core.djangoapps.user_api.accounts.views.USER_RETIRE_MAILINGS') + @mock.patch('openedx.core.djangoapps.user_api.accounts.views.USER_RETIRE_LMS_MISC') + @mock.patch('openedx.core.djangoapps.user_api.accounts.views.redact_and_delete_historical_social_auth') + @mock.patch('openedx.core.djangoapps.user_api.accounts.views.CreditRequirementStatus.retire_user') + @mock.patch('openedx.core.djangoapps.user_api.accounts.views.ApiAccessRequest.retire_user') + @mock.patch('openedx.core.djangoapps.user_api.accounts.views.CreditRequest.retire_user') + @mock.patch('openedx.core.djangoapps.user_api.accounts.views.ManualEnrollmentAudit.retire_manual_enrollments') + @mock.patch('openedx.core.djangoapps.user_api.accounts.views.PendingNameChange.delete_by_user_value') + @mock.patch('openedx.core.djangoapps.user_api.accounts.views.ArticleRevision.retire_user') + @mock.patch('openedx.core.djangoapps.user_api.accounts.views.RevisionPluginRevision.retire_user') + def test_retire_misc_calls_all_retirement_steps( + self, + mock_revision_plugin, + mock_article_revision, + mock_pending_name, + mock_manual_enroll, + mock_credit_request, + mock_api_access, + mock_credit_req_status, + mock_redact_historical, + mock_lms_misc_signal, + mock_mailings_signal, + ): + """ + Ensure that all retirement steps in the retire_misc view are invoked. + """ + self.post_and_assert_status({'username': self.original_username}) + + mock_revision_plugin.assert_called_once() + mock_article_revision.assert_called_once() + mock_pending_name.assert_called_once() + mock_manual_enroll.assert_called_once() + mock_credit_request.assert_called_once() + mock_api_access.assert_called_once() + mock_credit_req_status.assert_called_once() + mock_redact_historical.assert_called_once() + mock_lms_misc_signal.send.assert_called_once() + mock_mailings_signal.send.assert_called_once() diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py b/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py index c05f30d6870d..489c47f535ee 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py @@ -1,4 +1,6 @@ -""" Unit tests for custom UserProfile properties. """ +""" +Unit tests for user account utility functions, including social links, completion, and social-auth PII redaction. +""" from contextlib import contextmanager @@ -9,16 +11,19 @@ from django.db.models.signals import pre_delete from django.test import TestCase from django.test.utils import CaptureQueriesContext, override_settings +from django.utils import timezone from social_django.models import UserSocialAuth from common.djangoapps.student.models import CourseEnrollment from common.djangoapps.student.tests.factories import UserFactory from openedx.core.djangoapps.user_api.accounts.signals import redact_social_auth_pii_before_deletion from openedx.core.djangoapps.user_api.accounts.utils import ( + REDACTED_SOCIAL_AUTH_UID_PREFIX, + redact_and_delete_historical_social_auth, redact_and_delete_social_auth, retrieve_last_sitewide_block_completed, ) -from openedx.core.djangolib.testing.utils import skip_unless_lms +from openedx.core.djangolib.testing.utils import assert_redact_before_delete, skip_unless_lms from xmodule.modulestore.tests.django_utils import ( SharedModuleStoreTestCase, # pylint: disable=wrong-import-order ) @@ -30,26 +35,6 @@ from ..utils import format_social_link, validate_social_link -def assert_update_before_delete(sql_list, num_redact_delete_pairs=1, table='social_auth_usersocialauth'): - """ - Assert that UPDATE and DELETE queries for ``table`` occur in consecutive pairs. - """ - table_key = table.upper() - expected_sql_list = [ - sql for sql in sql_list - if table_key in sql.upper() and ('UPDATE' in sql.upper() or 'DELETE' in sql.upper()) - ] - assert len(expected_sql_list) == num_redact_delete_pairs * 2, ( - f'Expected {num_redact_delete_pairs * 2} UPDATE/DELETE queries on {table}, ' - f'got {len(expected_sql_list)}' - ) - - for index in range(0, len(expected_sql_list), 2): - update_sql = expected_sql_list[index] - delete_sql = expected_sql_list[index + 1] - assert 'UPDATE' in update_sql.upper(), f'Expected UPDATE at position {index} for {table}' - assert 'DELETE' in delete_sql.upper(), f'Expected DELETE at position {index + 1} for {table}' - # Use a context manager to guarantee signal reconnection between tests. @contextmanager def disconnected_social_auth_redaction_signal(): @@ -203,6 +188,21 @@ def create_social_auth(self, provider='google-oauth2', uid='user@example.com', e extra_data=extra_data, ) + def _assert_redact_and_delete_social_auth(self, social_auth_ids): + """ + Test redact_and_delete_social_auth and assert that all given records were + redacted before deletion. + """ + with disconnected_social_auth_redaction_signal(), CaptureQueriesContext(connection) as ctx: + redact_and_delete_social_auth(self.user.id) + + assert_redact_before_delete( + [query['sql'] for query in ctx], + table='social_auth_usersocialauth', + expected_redacted_value_list=[REDACTED_SOCIAL_AUTH_UID_PREFIX], + ) + assert not UserSocialAuth.objects.filter(id__in=social_auth_ids).exists() + def test_redact_and_delete_redacts_single_sso_record(self): """ Test that redact_and_delete_social_auth redacts and deletes a single SSO record. @@ -212,13 +212,7 @@ def test_redact_and_delete_redacts_single_sso_record(self): uid='google@example.com', extra_data={'email': 'google@example.com', 'name': 'Google User'}, ) - social_auth_id = social_auth.pk - - with disconnected_social_auth_redaction_signal(), CaptureQueriesContext(connection) as ctx: - redact_and_delete_social_auth(self.user.id) - - assert_update_before_delete([query['sql'] for query in ctx]) - assert not UserSocialAuth.objects.filter(id=social_auth_id).exists() + self._assert_redact_and_delete_social_auth([social_auth.pk]) def test_redact_and_delete_redacts_multiple_sso_records(self): """ @@ -236,9 +230,67 @@ def test_redact_and_delete_redacts_multiple_sso_records(self): extra_data={'email': 'saml@example.com', 'name': 'SAML User', 'uid': 'saml-uid'}, ).pk, ] + self._assert_redact_and_delete_social_auth(social_auth_ids) - with disconnected_social_auth_redaction_signal(), CaptureQueriesContext(connection) as ctx: - redact_and_delete_social_auth(self.user.id) - assert_update_before_delete([query['sql'] for query in ctx]) - assert not UserSocialAuth.objects.filter(id__in=social_auth_ids).exists() +@skip_unless_lms +class RedactAndDeleteHistoricalSocialAuthTest(TestCase): + """ + Tests for the redact_and_delete_historical_social_auth utility function. + """ + + def setUp(self): + super().setUp() + self.user = UserFactory.create(username='testuser', email='testuser@example.com') + self.historical_social_auth_model = UserSocialAuth.history.model + + def _create_historical_record(self, provider='google-oauth2', uid='user@example.com', extra_data=None, source_id=1): + """ + Create a HistoricalUserSocialAuth record directly for test setup. + """ + if extra_data is None: + extra_data = {'email': uid, 'name': 'Test User'} + return self.historical_social_auth_model.objects.create( + user=self.user, + id=source_id, + provider=provider, + uid=uid, + extra_data=extra_data, + created=timezone.now(), + modified=timezone.now(), + history_date=timezone.now(), + history_type='+', + ) + + def test_historical_social_auth_redact_before_delete(self): + """ + Ensure HistoricalUserSocialAuth records are properly redacted and deleted for retirement. + + The fields uid (email format) and extra_data must be redacted before delete. + """ + self._create_historical_record(provider='google-oauth2', uid='google@example.com', source_id=1) + self._create_historical_record(provider='tpa-saml', uid='saml@example.com', source_id=2) + + other_user = UserFactory.create(username='otheruser', email='other@example.com') + other_record = self.historical_social_auth_model.objects.create( + user=other_user, + id=3, + provider='google-oauth2', + uid='other@example.com', + extra_data={}, + created=timezone.now(), + modified=timezone.now(), + history_date=timezone.now(), + history_type='+', + ) + + with CaptureQueriesContext(connection) as ctx: + redact_and_delete_historical_social_auth(self.user.id) + + assert_redact_before_delete( + [query['sql'] for query in ctx], + table=self.historical_social_auth_model._meta.db_table, + expected_redacted_value_list=[REDACTED_SOCIAL_AUTH_UID_PREFIX], + ) + assert not self.historical_social_auth_model.objects.filter(user=self.user).exists() + assert self.historical_social_auth_model.objects.filter(history_id=other_record.history_id).exists() diff --git a/openedx/core/djangoapps/user_api/accounts/utils.py b/openedx/core/djangoapps/user_api/accounts/utils.py index 083f45ecc9e0..cf00dc3a9f93 100644 --- a/openedx/core/djangoapps/user_api/accounts/utils.py +++ b/openedx/core/djangoapps/user_api/accounts/utils.py @@ -11,7 +11,7 @@ from completion.models import BlockCompletion from completion.waffle import ENABLE_COMPLETION_TRACKING_SWITCH from django.conf import settings -from django.db.models import CharField, Value +from django.db.models import CharField, TextField, Value from django.db.models.functions import Cast, Concat from django.utils.translation import gettext as _ from edx_django_utils.user import generate_password @@ -226,6 +226,26 @@ def redact_and_delete_social_auth(user_id, skip_delete=False): social_auth_queryset.delete() +def redact_and_delete_historical_social_auth(user_id): + """ + Redact PII from all HistoricalUserSocialAuth records for the given user, then delete them. + + Downstream copies of data may use soft-deletes, and redacting before deleting + ensures PII for retired users (or future retirements) is not retained. + """ + historical_social_auth_model = UserSocialAuth.history.model + historical_queryset = historical_social_auth_model.objects.filter(user_id=user_id) + historical_queryset.update( + uid=Concat( + Value(REDACTED_SOCIAL_AUTH_UID_PREFIX), + Cast('history_id', output_field=TextField()), + Value(REDACTED_SOCIAL_AUTH_UID_SUFFIX), + ), + extra_data={}, + ) + historical_queryset.delete() + + def create_retirement_request_and_deactivate_account(user): """ Adds user to retirement queue, unlinks social auth accounts, changes user passwords diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index c6515390fed1..3edc6420c074 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -62,7 +62,10 @@ from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.user_api import accounts from openedx.core.djangoapps.user_api.accounts.image_helpers import get_profile_image_names, set_has_profile_image -from openedx.core.djangoapps.user_api.accounts.utils import handle_retirement_cancellation +from openedx.core.djangoapps.user_api.accounts.utils import ( + handle_retirement_cancellation, + redact_and_delete_historical_social_auth, +) from openedx.core.djangoapps.user_authn.exceptions import AuthFailedError from openedx.core.lib.api.authentication import BearerAuthentication, BearerAuthenticationAllowInactiveUser from openedx.core.lib.api.parsers import MergePatchParser @@ -1104,6 +1107,7 @@ def post(self, request): CreditRequest.retire_user(retirement) ApiAccessRequest.retire_user(retirement.user) CreditRequirementStatus.retire_user(retirement) + redact_and_delete_historical_social_auth(retirement.user.id) # This signal allows code in higher points of LMS to retire the user as necessary USER_RETIRE_LMS_MISC.send(sender=self.__class__, user=retirement.user) 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..68ba4a1743b8 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 @@ -22,10 +22,11 @@ from openedx.core.djangoapps.user_api.accounts.tests.retirement_helpers import ( setup_retirement_states, # noqa: F401 ) -from openedx.core.djangoapps.user_api.accounts.tests.test_utils import ( - assert_update_before_delete, +from openedx.core.djangoapps.user_api.accounts.utils import REDACTED_SOCIAL_AUTH_UID_PREFIX +from openedx.core.djangolib.testing.utils import ( # pylint: disable=wrong-import-order + assert_redact_before_delete, + skip_unless_lms, ) -from openedx.core.djangolib.testing.utils import skip_unless_lms # pylint: disable=wrong-import-order from ...models import UserRetirementStatus @@ -164,7 +165,11 @@ def test_retire_user_redacts_sso_pii_before_deletion(setup_retirement_states, so with disconnected_social_auth_redaction_signal(), CaptureQueriesContext(connection) as ctx: call_command('retire_user', username=user.username, user_email=user.email) - assert_update_before_delete([query['sql'] for query in ctx]) + assert_redact_before_delete( + [query['sql'] for query in ctx], + table='social_auth_usersocialauth', + expected_redacted_value_list=[REDACTED_SOCIAL_AUTH_UID_PREFIX], + ) for auth_id in auth_ids: assert not UserSocialAuth.objects.filter(id=auth_id).exists()