From b59386107bfff98d5481d63011227c0e52bbd49d Mon Sep 17 00:00:00 2001 From: Akanshu Aich Date: Tue, 19 May 2026 15:01:41 +0000 Subject: [PATCH 01/26] fix: Redact uid field and delete records for retired users from support_historicalusersocialauth --- .../user_api/accounts/tests/test_utils.py | 113 ++++++++++++++++++ .../djangoapps/user_api/accounts/utils.py | 25 ++++ .../djangoapps/user_api/accounts/views.py | 6 +- 3 files changed, 143 insertions(+), 1 deletion(-) 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..2c74c226c66c 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py @@ -5,16 +5,21 @@ import ddt from completion import models from completion.test_utils import CompletionWaffleTestMixin +from django.apps import apps from django.db import connection 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, + REDACTED_SOCIAL_AUTH_UID_SUFFIX, + redact_and_delete_historical_social_auth, redact_and_delete_social_auth, retrieve_last_sitewide_block_completed, ) @@ -242,3 +247,111 @@ def test_redact_and_delete_redacts_multiple_sso_records(self): 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.HistoricalUserSocialAuth = apps.get_model('support', 'HistoricalUserSocialAuth') + + def _create_historical_record(self, provider='google-oauth2', uid='user@example.com', extra_data=None): + """ + Create a HistoricalUserSocialAuth record directly (bypassing ORM signals). + """ + extra_data = extra_data or {'email': uid, 'name': 'Test User'} + return self.HistoricalUserSocialAuth.objects.create( + user=self.user, + id=1, + provider=provider, + uid=uid, + extra_data=extra_data, + created=timezone.now(), + modified=timezone.now(), + history_date=timezone.now(), + history_type='+', + ) + + def test_redacts_uid_before_deleting(self): + """ + redact_and_delete_historical_social_auth should UPDATE uid before DELETE + so any downstream consumer sees only the sanitised value. + """ + record = self._create_historical_record(uid='private@example.com') + history_id = record.history_id + + with CaptureQueriesContext(connection) as ctx: + redact_and_delete_historical_social_auth(self.user.id) + + assert_update_before_delete( + [query['sql'] for query in ctx], + table='support_historicalusersocialauth', + ) + assert not self.HistoricalUserSocialAuth.objects.filter(history_id=history_id).exists() + + def test_redacted_uid_format(self): + """ + The redacted uid written before deletion must follow REDACTED_SOCIAL_AUTH_UID_PREFIX/SUFFIX. + This is verified by intercepting the UPDATE before the DELETE fires. + """ + record = self._create_historical_record(uid='private@example.com') + history_id = record.history_id + + original_delete = self.HistoricalUserSocialAuth.objects.filter( + user_id=self.user.id + ).delete + + deleted = [] + + def capture_delete(): + deleted.append(True) + + qs = self.HistoricalUserSocialAuth.objects.filter(user_id=self.user.id) + qs.update( + **{ + 'uid': f'{REDACTED_SOCIAL_AUTH_UID_PREFIX}{history_id}{REDACTED_SOCIAL_AUTH_UID_SUFFIX}', + 'extra_data': {}, + } + ) + refreshed = self.HistoricalUserSocialAuth.objects.get(history_id=history_id) + assert refreshed.uid == f'{REDACTED_SOCIAL_AUTH_UID_PREFIX}{history_id}{REDACTED_SOCIAL_AUTH_UID_SUFFIX}' + assert refreshed.extra_data == {} + + def test_deletes_all_records_for_user(self): + """ + All HistoricalUserSocialAuth rows for the user are deleted, records for + other users are left untouched. + """ + self._create_historical_record(provider='google-oauth2', uid='google@example.com') + self._create_historical_record(provider='tpa-saml', uid='saml@example.com') + + other_user = UserFactory.create(username='otheruser', email='other@example.com') + other_record = self.HistoricalUserSocialAuth.objects.create( + user=other_user, + id=2, + provider='google-oauth2', + uid='other@example.com', + extra_data={}, + created=timezone.now(), + modified=timezone.now(), + history_date=timezone.now(), + history_type='+', + ) + + redact_and_delete_historical_social_auth(self.user.id) + + assert not self.HistoricalUserSocialAuth.objects.filter(user=self.user).exists() + assert self.HistoricalUserSocialAuth.objects.filter(history_id=other_record.history_id).exists() + + def test_no_op_when_no_records_exist(self): + """ + redact_and_delete_historical_social_auth should not raise when the user + has no HistoricalUserSocialAuth records. + """ + redact_and_delete_historical_social_auth(self.user.id) + assert not self.HistoricalUserSocialAuth.objects.filter(user=self.user).exists() diff --git a/openedx/core/djangoapps/user_api/accounts/utils.py b/openedx/core/djangoapps/user_api/accounts/utils.py index 083f45ecc9e0..2a6eaf8f839e 100644 --- a/openedx/core/djangoapps/user_api/accounts/utils.py +++ b/openedx/core/djangoapps/user_api/accounts/utils.py @@ -10,6 +10,7 @@ import waffle # pylint: disable=invalid-django-waffle-import from completion.models import BlockCompletion from completion.waffle import ENABLE_COMPLETION_TRACKING_SWITCH +from django.apps import apps from django.conf import settings from django.db.models import CharField, Value from django.db.models.functions import Cast, Concat @@ -226,6 +227,30 @@ 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. + + HistoricalUserSocialAuth rows are django-simple-history snapshots of every UserSocialAuth + change. They are not touched by the standard UserSocialAuth retirement step, leaving raw + email addresses stored in the ``uid`` field indefinitely. + + Redacting before deleting ensures any downstream consumer only ever sees the sanitised + value before the rows are removed. + """ + HistoricalUserSocialAuth = apps.get_model('support', 'HistoricalUserSocialAuth') + historical_queryset = HistoricalUserSocialAuth.objects.filter(user_id=user_id) + historical_queryset.update( + uid=Concat( + Value(REDACTED_SOCIAL_AUTH_UID_PREFIX), + Cast('history_id', output_field=CharField()), + 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) From caf150a0224064457c688ef7d2ec91396faa2191 Mon Sep 17 00:00:00 2001 From: Akanshu Aich Date: Tue, 19 May 2026 16:13:02 +0000 Subject: [PATCH 02/26] fix: fixed tests and utility lookup error --- .../user_api/accounts/tests/test_utils.py | 86 +++++++------------ .../djangoapps/user_api/accounts/utils.py | 9 +- 2 files changed, 41 insertions(+), 54 deletions(-) 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 2c74c226c66c..2cf4a0bb8711 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py @@ -1,5 +1,6 @@ """ Unit tests for custom UserProfile properties. """ +import unittest.mock from contextlib import contextmanager import ddt @@ -258,14 +259,12 @@ class RedactAndDeleteHistoricalSocialAuthTest(TestCase): def setUp(self): super().setUp() self.user = UserFactory.create(username='testuser', email='testuser@example.com') - self.HistoricalUserSocialAuth = apps.get_model('support', 'HistoricalUserSocialAuth') + self.historical_social_auth_model = apps.get_model('support', 'HistoricalUserSocialAuth') def _create_historical_record(self, provider='google-oauth2', uid='user@example.com', extra_data=None): - """ - Create a HistoricalUserSocialAuth record directly (bypassing ORM signals). - """ + """Create a HistoricalUserSocialAuth record directly, bypassing ORM signals.""" extra_data = extra_data or {'email': uid, 'name': 'Test User'} - return self.HistoricalUserSocialAuth.objects.create( + return self.historical_social_auth_model.objects.create( user=self.user, id=1, provider=provider, @@ -277,61 +276,42 @@ def _create_historical_record(self, provider='google-oauth2', uid='user@example. history_type='+', ) - def test_redacts_uid_before_deleting(self): - """ - redact_and_delete_historical_social_auth should UPDATE uid before DELETE - so any downstream consumer sees only the sanitised value. - """ - record = self._create_historical_record(uid='private@example.com') - history_id = record.history_id - - with CaptureQueriesContext(connection) as ctx: - redact_and_delete_historical_social_auth(self.user.id) - - assert_update_before_delete( - [query['sql'] for query in ctx], - table='support_historicalusersocialauth', - ) - assert not self.HistoricalUserSocialAuth.objects.filter(history_id=history_id).exists() - def test_redacted_uid_format(self): """ - The redacted uid written before deletion must follow REDACTED_SOCIAL_AUTH_UID_PREFIX/SUFFIX. - This is verified by intercepting the UPDATE before the DELETE fires. + uid must follow the redacted-before-delete-{history_id}@safe.com format and extra_data + must be cleared. A pre_delete signal re-fetches the row at deletion time to confirm the + UPDATE was written before the DELETE fired. """ record = self._create_historical_record(uid='private@example.com') history_id = record.history_id + expected_uid = f'{REDACTED_SOCIAL_AUTH_UID_PREFIX}{history_id}{REDACTED_SOCIAL_AUTH_UID_SUFFIX}' - original_delete = self.HistoricalUserSocialAuth.objects.filter( - user_id=self.user.id - ).delete + signal_capture = {} - deleted = [] + def capture_uid_at_delete(sender, instance, **kwargs): # pylint: disable=unused-argument + redacted_row = self.historical_social_auth_model.objects.get(history_id=instance.history_id) + signal_capture['uid'] = redacted_row.uid + signal_capture['extra_data'] = redacted_row.extra_data - def capture_delete(): - deleted.append(True) + pre_delete.connect(capture_uid_at_delete, sender=self.historical_social_auth_model) + try: + redact_and_delete_historical_social_auth(self.user.id) + finally: + pre_delete.disconnect(capture_uid_at_delete, sender=self.historical_social_auth_model) - qs = self.HistoricalUserSocialAuth.objects.filter(user_id=self.user.id) - qs.update( - **{ - 'uid': f'{REDACTED_SOCIAL_AUTH_UID_PREFIX}{history_id}{REDACTED_SOCIAL_AUTH_UID_SUFFIX}', - 'extra_data': {}, - } + assert signal_capture['uid'] == expected_uid, ( + f"uid at delete time was {signal_capture['uid']!r}, expected {expected_uid!r}" ) - refreshed = self.HistoricalUserSocialAuth.objects.get(history_id=history_id) - assert refreshed.uid == f'{REDACTED_SOCIAL_AUTH_UID_PREFIX}{history_id}{REDACTED_SOCIAL_AUTH_UID_SUFFIX}' - assert refreshed.extra_data == {} + assert signal_capture['extra_data'] == {} + assert not self.historical_social_auth_model.objects.filter(history_id=history_id).exists() def test_deletes_all_records_for_user(self): - """ - All HistoricalUserSocialAuth rows for the user are deleted, records for - other users are left untouched. - """ + """All rows for the retired user are deleted; other users' rows are untouched.""" self._create_historical_record(provider='google-oauth2', uid='google@example.com') self._create_historical_record(provider='tpa-saml', uid='saml@example.com') other_user = UserFactory.create(username='otheruser', email='other@example.com') - other_record = self.HistoricalUserSocialAuth.objects.create( + other_record = self.historical_social_auth_model.objects.create( user=other_user, id=2, provider='google-oauth2', @@ -345,13 +325,13 @@ def test_deletes_all_records_for_user(self): redact_and_delete_historical_social_auth(self.user.id) - assert not self.HistoricalUserSocialAuth.objects.filter(user=self.user).exists() - assert self.HistoricalUserSocialAuth.objects.filter(history_id=other_record.history_id).exists() + 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() - def test_no_op_when_no_records_exist(self): - """ - redact_and_delete_historical_social_auth should not raise when the user - has no HistoricalUserSocialAuth records. - """ - redact_and_delete_historical_social_auth(self.user.id) - assert not self.HistoricalUserSocialAuth.objects.filter(user=self.user).exists() + def test_skips_gracefully_when_support_app_not_installed(self): + """Returns without error when the support app is not installed.""" + with unittest.mock.patch( + 'openedx.core.djangoapps.user_api.accounts.utils.apps.get_model', + side_effect=LookupError('support'), + ): + redact_and_delete_historical_social_auth(self.user.id) diff --git a/openedx/core/djangoapps/user_api/accounts/utils.py b/openedx/core/djangoapps/user_api/accounts/utils.py index 2a6eaf8f839e..4e832e4afc57 100644 --- a/openedx/core/djangoapps/user_api/accounts/utils.py +++ b/openedx/core/djangoapps/user_api/accounts/utils.py @@ -238,7 +238,14 @@ def redact_and_delete_historical_social_auth(user_id): Redacting before deleting ensures any downstream consumer only ever sees the sanitised value before the rows are removed. """ - HistoricalUserSocialAuth = apps.get_model('support', 'HistoricalUserSocialAuth') + try: + HistoricalUserSocialAuth = apps.get_model('support', 'HistoricalUserSocialAuth') + except LookupError: + LOGGER.info( + 'redact_and_delete_historical_social_auth: support app not installed, skipping for user_id=%s', + user_id, + ) + return historical_queryset = HistoricalUserSocialAuth.objects.filter(user_id=user_id) historical_queryset.update( uid=Concat( From bf1db7b418330022f05dd1724ff786b19c07256c Mon Sep 17 00:00:00 2001 From: Akanshu Aich Date: Tue, 19 May 2026 22:19:10 +0530 Subject: [PATCH 03/26] fix: Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- openedx/core/djangoapps/user_api/accounts/utils.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/utils.py b/openedx/core/djangoapps/user_api/accounts/utils.py index 4e832e4afc57..d9a5fecad57c 100644 --- a/openedx/core/djangoapps/user_api/accounts/utils.py +++ b/openedx/core/djangoapps/user_api/accounts/utils.py @@ -235,8 +235,9 @@ def redact_and_delete_historical_social_auth(user_id): change. They are not touched by the standard UserSocialAuth retirement step, leaving raw email addresses stored in the ``uid`` field indefinitely. - Redacting before deleting ensures any downstream consumer only ever sees the sanitised - value before the rows are removed. + Redacting before deleting ensures deletion-time handlers or other observers in the same + process/transaction see sanitised values before the rows are removed. It does not imply + that other transactions will observe the intermediate redacted state before deletion. """ try: HistoricalUserSocialAuth = apps.get_model('support', 'HistoricalUserSocialAuth') From 721cd94372f7de3e5e876e25c22a80f794e7dabc Mon Sep 17 00:00:00 2001 From: Akanshu Aich Date: Tue, 19 May 2026 16:53:17 +0000 Subject: [PATCH 04/26] fix: modified annotations --- openedx/core/djangoapps/user_api/accounts/tests/test_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 2cf4a0bb8711..f113d3ea3445 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py @@ -262,7 +262,7 @@ def setUp(self): self.historical_social_auth_model = apps.get_model('support', 'HistoricalUserSocialAuth') def _create_historical_record(self, provider='google-oauth2', uid='user@example.com', extra_data=None): - """Create a HistoricalUserSocialAuth record directly, bypassing ORM signals.""" + """Create a HistoricalUserSocialAuth record directly for test setup.""" extra_data = extra_data or {'email': uid, 'name': 'Test User'} return self.historical_social_auth_model.objects.create( user=self.user, From 6563246f43b2cd4a05a43d6b7ea21d56efcec59d Mon Sep 17 00:00:00 2001 From: Akanshu Aich Date: Tue, 19 May 2026 22:31:16 +0530 Subject: [PATCH 05/26] fix: Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../core/djangoapps/user_api/accounts/tests/test_utils.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 f113d3ea3445..f05da4ec9258 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py @@ -259,7 +259,10 @@ class RedactAndDeleteHistoricalSocialAuthTest(TestCase): def setUp(self): super().setUp() self.user = UserFactory.create(username='testuser', email='testuser@example.com') - self.historical_social_auth_model = apps.get_model('support', 'HistoricalUserSocialAuth') + try: + self.historical_social_auth_model = apps.get_model('support', 'HistoricalUserSocialAuth') + except LookupError: + self.skipTest("support.HistoricalUserSocialAuth is not available in this test environment") def _create_historical_record(self, provider='google-oauth2', uid='user@example.com', extra_data=None): """Create a HistoricalUserSocialAuth record directly for test setup.""" From 7d01b68e996876718d7be92b6cc5fc0c59eeb751 Mon Sep 17 00:00:00 2001 From: Akanshu Aich Date: Tue, 19 May 2026 23:23:57 +0530 Subject: [PATCH 06/26] fix: Remove logging for missing support app --- openedx/core/djangoapps/user_api/accounts/utils.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/utils.py b/openedx/core/djangoapps/user_api/accounts/utils.py index d9a5fecad57c..bdd090619e21 100644 --- a/openedx/core/djangoapps/user_api/accounts/utils.py +++ b/openedx/core/djangoapps/user_api/accounts/utils.py @@ -242,10 +242,6 @@ def redact_and_delete_historical_social_auth(user_id): try: HistoricalUserSocialAuth = apps.get_model('support', 'HistoricalUserSocialAuth') except LookupError: - LOGGER.info( - 'redact_and_delete_historical_social_auth: support app not installed, skipping for user_id=%s', - user_id, - ) return historical_queryset = HistoricalUserSocialAuth.objects.filter(user_id=user_id) historical_queryset.update( From 8b9de0052f2b3975fb073dc182fb2cf880be3bf3 Mon Sep 17 00:00:00 2001 From: Akanshu Aich Date: Tue, 19 May 2026 18:24:58 +0000 Subject: [PATCH 07/26] fix: added logger and dynamic testing --- .../user_api/accounts/tests/test_utils.py | 13 +++++-------- openedx/core/djangoapps/user_api/accounts/utils.py | 4 ++++ 2 files changed, 9 insertions(+), 8 deletions(-) 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 f05da4ec9258..6744e98b52e8 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py @@ -259,17 +259,14 @@ class RedactAndDeleteHistoricalSocialAuthTest(TestCase): def setUp(self): super().setUp() self.user = UserFactory.create(username='testuser', email='testuser@example.com') - try: - self.historical_social_auth_model = apps.get_model('support', 'HistoricalUserSocialAuth') - except LookupError: - self.skipTest("support.HistoricalUserSocialAuth is not available in this test environment") + self.historical_social_auth_model = apps.get_model('support', 'HistoricalUserSocialAuth') - def _create_historical_record(self, provider='google-oauth2', uid='user@example.com', extra_data=None): + 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.""" extra_data = extra_data or {'email': uid, 'name': 'Test User'} return self.historical_social_auth_model.objects.create( user=self.user, - id=1, + id=source_id, provider=provider, uid=uid, extra_data=extra_data, @@ -310,8 +307,8 @@ def capture_uid_at_delete(sender, instance, **kwargs): # pylint: disable=unused def test_deletes_all_records_for_user(self): """All rows for the retired user are deleted; other users' rows are untouched.""" - self._create_historical_record(provider='google-oauth2', uid='google@example.com') - self._create_historical_record(provider='tpa-saml', uid='saml@example.com') + 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( diff --git a/openedx/core/djangoapps/user_api/accounts/utils.py b/openedx/core/djangoapps/user_api/accounts/utils.py index bdd090619e21..b2edf317e2ab 100644 --- a/openedx/core/djangoapps/user_api/accounts/utils.py +++ b/openedx/core/djangoapps/user_api/accounts/utils.py @@ -242,6 +242,10 @@ def redact_and_delete_historical_social_auth(user_id): try: HistoricalUserSocialAuth = apps.get_model('support', 'HistoricalUserSocialAuth') except LookupError: + LOGGER.debug( + 'redact_and_delete_historical_social_auth: support app not installed, skipping for user_id=%s', + user_id, + ) return historical_queryset = HistoricalUserSocialAuth.objects.filter(user_id=user_id) historical_queryset.update( From c19fe2ff506a127d6d4b415efe16896e4c6d9d96 Mon Sep 17 00:00:00 2001 From: Akanshu Aich Date: Tue, 19 May 2026 18:45:09 +0000 Subject: [PATCH 08/26] fix: added skip test lookup error --- .../core/djangoapps/user_api/accounts/tests/test_utils.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 6744e98b52e8..6ea9294abef3 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py @@ -259,7 +259,10 @@ class RedactAndDeleteHistoricalSocialAuthTest(TestCase): def setUp(self): super().setUp() self.user = UserFactory.create(username='testuser', email='testuser@example.com') - self.historical_social_auth_model = apps.get_model('support', 'HistoricalUserSocialAuth') + try: + self.historical_social_auth_model = apps.get_model('support', 'HistoricalUserSocialAuth') + except LookupError: + self.skipTest('support.HistoricalUserSocialAuth is not available in this test environment') 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.""" From a411cb5a05ab190ae4c42c3c4b204ddb9cb98e00 Mon Sep 17 00:00:00 2001 From: Akanshu Aich Date: Tue, 19 May 2026 18:56:54 +0000 Subject: [PATCH 09/26] fix: added extra_data none check --- openedx/core/djangoapps/user_api/accounts/tests/test_utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 6ea9294abef3..669cd62582f1 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py @@ -266,7 +266,8 @@ def setUp(self): 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.""" - extra_data = extra_data or {'email': uid, 'name': 'Test User'} + 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, From f813c0adab4a286c9c2ec4d594a1cd6a19981075 Mon Sep 17 00:00:00 2001 From: Akanshu Aich Date: Wed, 20 May 2026 08:08:23 +0000 Subject: [PATCH 10/26] fix: added sql tests --- .../user_api/accounts/tests/test_utils.py | 38 +++++++++++++------ .../djangoapps/user_api/accounts/utils.py | 19 ++++------ 2 files changed, 35 insertions(+), 22 deletions(-) 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 669cd62582f1..a8d62640999b 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py @@ -265,7 +265,9 @@ def setUp(self): self.skipTest('support.HistoricalUserSocialAuth is not available in this test environment') 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.""" + """ + 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( @@ -283,8 +285,8 @@ def _create_historical_record(self, provider='google-oauth2', uid='user@example. def test_redacted_uid_format(self): """ uid must follow the redacted-before-delete-{history_id}@safe.com format and extra_data - must be cleared. A pre_delete signal re-fetches the row at deletion time to confirm the - UPDATE was written before the DELETE fired. + must be cleared. Verified at two levels: SQL ordering via CaptureQueriesContext (UPDATE + precedes DELETE, history_id-based filtering) and uid value via pre_delete signal. """ record = self._create_historical_record(uid='private@example.com') history_id = record.history_id @@ -293,16 +295,29 @@ def test_redacted_uid_format(self): signal_capture = {} def capture_uid_at_delete(sender, instance, **kwargs): # pylint: disable=unused-argument + # Re-fetch from DB to read the value written by UPDATE, not the stale in-memory instance. redacted_row = self.historical_social_auth_model.objects.get(history_id=instance.history_id) signal_capture['uid'] = redacted_row.uid signal_capture['extra_data'] = redacted_row.extra_data pre_delete.connect(capture_uid_at_delete, sender=self.historical_social_auth_model) try: - redact_and_delete_historical_social_auth(self.user.id) + with CaptureQueriesContext(connection) as ctx: + redact_and_delete_historical_social_auth(self.user.id) finally: pre_delete.disconnect(capture_uid_at_delete, sender=self.historical_social_auth_model) + # Verify SQL-level: UPDATE before DELETE, ID-based filtering. + assert_update_before_delete( + [query['sql'] for query in ctx], + table='support_historicalusersocialauth', + ) + sql_list = [query['sql'].upper() for query in ctx] + assert any('HISTORY_ID' in sql and 'UPDATE' in sql for sql in sql_list), ( + 'UPDATE must filter by history_id' + ) + + # Verify value-level: correct redacted uid and cleared extra_data. assert signal_capture['uid'] == expected_uid, ( f"uid at delete time was {signal_capture['uid']!r}, expected {expected_uid!r}" ) @@ -310,7 +325,9 @@ def capture_uid_at_delete(sender, instance, **kwargs): # pylint: disable=unused assert not self.historical_social_auth_model.objects.filter(history_id=history_id).exists() def test_deletes_all_records_for_user(self): - """All rows for the retired user are deleted; other users' rows are untouched.""" + """ + All rows for the retired user are deleted; other users' rows are untouched. + """ 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) @@ -332,10 +349,9 @@ def test_deletes_all_records_for_user(self): 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() - def test_skips_gracefully_when_support_app_not_installed(self): - """Returns without error when the support app is not installed.""" - with unittest.mock.patch( - 'openedx.core.djangoapps.user_api.accounts.utils.apps.get_model', - side_effect=LookupError('support'), - ): + def test_skips_gracefully_when_history_model_unavailable(self): + """ + Returns without error when UserSocialAuth has no history model registered. + """ + with unittest.mock.patch.object(UserSocialAuth, 'history', None): redact_and_delete_historical_social_auth(self.user.id) diff --git a/openedx/core/djangoapps/user_api/accounts/utils.py b/openedx/core/djangoapps/user_api/accounts/utils.py index b2edf317e2ab..586cbc05d9ee 100644 --- a/openedx/core/djangoapps/user_api/accounts/utils.py +++ b/openedx/core/djangoapps/user_api/accounts/utils.py @@ -10,7 +10,6 @@ import waffle # pylint: disable=invalid-django-waffle-import from completion.models import BlockCompletion from completion.waffle import ENABLE_COMPLETION_TRACKING_SWITCH -from django.apps import apps from django.conf import settings from django.db.models import CharField, Value from django.db.models.functions import Cast, Concat @@ -232,22 +231,20 @@ def redact_and_delete_historical_social_auth(user_id): Redact PII from all HistoricalUserSocialAuth records for the given user, then delete them. HistoricalUserSocialAuth rows are django-simple-history snapshots of every UserSocialAuth - change. They are not touched by the standard UserSocialAuth retirement step, leaving raw - email addresses stored in the ``uid`` field indefinitely. + change. They are not touched by the standard UserSocialAuth retirement step, leaving raw + email addresses stored in the uid field indefinitely. - Redacting before deleting ensures deletion-time handlers or other observers in the same - process/transaction see sanitised values before the rows are removed. It does not imply - that other transactions will observe the intermediate redacted state before deletion. + Redacting before deleting ensures deletion-time handlers see sanitised values before + the rows are removed. """ - try: - HistoricalUserSocialAuth = apps.get_model('support', 'HistoricalUserSocialAuth') - except LookupError: + historical_social_auth_model = getattr(getattr(UserSocialAuth, 'history', None), 'model', None) + if historical_social_auth_model is None: LOGGER.debug( - 'redact_and_delete_historical_social_auth: support app not installed, skipping for user_id=%s', + 'redact_and_delete_historical_social_auth: UserSocialAuth has no history model, skipping for user_id=%s', user_id, ) return - historical_queryset = HistoricalUserSocialAuth.objects.filter(user_id=user_id) + historical_queryset = historical_social_auth_model.objects.filter(user_id=user_id) historical_queryset.update( uid=Concat( Value(REDACTED_SOCIAL_AUTH_UID_PREFIX), From 632871cf8e084fc0eb900357cefffca087d53e4f Mon Sep 17 00:00:00 2001 From: Akanshu Aich Date: Wed, 20 May 2026 09:52:35 +0000 Subject: [PATCH 11/26] fix: corrected annotations --- .../djangoapps/user_api/accounts/tests/test_utils.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) 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 a8d62640999b..ac4ecf1ac995 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py @@ -286,7 +286,7 @@ def test_redacted_uid_format(self): """ uid must follow the redacted-before-delete-{history_id}@safe.com format and extra_data must be cleared. Verified at two levels: SQL ordering via CaptureQueriesContext (UPDATE - precedes DELETE, history_id-based filtering) and uid value via pre_delete signal. + precedes DELETE, history_id embedded in SET expression) and uid value via pre_delete signal. """ record = self._create_historical_record(uid='private@example.com') history_id = record.history_id @@ -307,14 +307,17 @@ def capture_uid_at_delete(sender, instance, **kwargs): # pylint: disable=unused finally: pre_delete.disconnect(capture_uid_at_delete, sender=self.historical_social_auth_model) - # Verify SQL-level: UPDATE before DELETE, ID-based filtering. + # Verify SQL-level: UPDATE precedes DELETE on the correct table. + table_name = self.historical_social_auth_model._meta.db_table assert_update_before_delete( [query['sql'] for query in ctx], - table='support_historicalusersocialauth', + table=table_name, ) + # Verify history_id appears in the SET expression of the UPDATE, not just the WHERE clause, + # since rows are filtered by user_id and history_id is used to build the unique redacted uid. sql_list = [query['sql'].upper() for query in ctx] assert any('HISTORY_ID' in sql and 'UPDATE' in sql for sql in sql_list), ( - 'UPDATE must filter by history_id' + 'UPDATE must incorporate history_id into the redacted uid SET expression' ) # Verify value-level: correct redacted uid and cleared extra_data. From c726942dda89740c0437d475be66f18e42f7b4f8 Mon Sep 17 00:00:00 2001 From: Akanshu Aich Date: Wed, 20 May 2026 11:30:16 +0000 Subject: [PATCH 12/26] fix: added textfield instead of charfield --- openedx/core/djangoapps/user_api/accounts/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/utils.py b/openedx/core/djangoapps/user_api/accounts/utils.py index 586cbc05d9ee..c00131fffa5e 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 @@ -248,7 +248,7 @@ def redact_and_delete_historical_social_auth(user_id): historical_queryset.update( uid=Concat( Value(REDACTED_SOCIAL_AUTH_UID_PREFIX), - Cast('history_id', output_field=CharField()), + Cast('history_id', output_field=TextField()), Value(REDACTED_SOCIAL_AUTH_UID_SUFFIX), ), extra_data={}, From 0decc17926dbccca2e0017ab63e79b9a171dc6a1 Mon Sep 17 00:00:00 2001 From: Akanshu Aich Date: Thu, 21 May 2026 07:55:53 +0000 Subject: [PATCH 13/26] fix: added the valid docstring --- openedx/core/djangoapps/user_api/accounts/tests/test_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 ac4ecf1ac995..b1a4c10fe4f8 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,4 @@ -""" Unit tests for custom UserProfile properties. """ +"""Unit tests for user account utility functions, including social links, completion helpers, and social-auth PII redaction utilities.""" import unittest.mock from contextlib import contextmanager From a5f35d8b011f4927a86ddccef070e8caf6f24b4b Mon Sep 17 00:00:00 2001 From: Akanshu Aich Date: Thu, 21 May 2026 11:05:19 +0000 Subject: [PATCH 14/26] fix: added implementation in retre user management command --- openedx/core/djangoapps/user_api/accounts/utils.py | 4 +++- .../djangoapps/user_api/management/commands/retire_user.py | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/utils.py b/openedx/core/djangoapps/user_api/accounts/utils.py index c00131fffa5e..1cdd1d0bdc73 100644 --- a/openedx/core/djangoapps/user_api/accounts/utils.py +++ b/openedx/core/djangoapps/user_api/accounts/utils.py @@ -239,7 +239,7 @@ def redact_and_delete_historical_social_auth(user_id): """ historical_social_auth_model = getattr(getattr(UserSocialAuth, 'history', None), 'model', None) if historical_social_auth_model is None: - LOGGER.debug( + LOGGER.warning( 'redact_and_delete_historical_social_auth: UserSocialAuth has no history model, skipping for user_id=%s', user_id, ) @@ -266,6 +266,8 @@ def create_retirement_request_and_deactivate_account(user): # Redact and unlink LMS social auth accounts. redact_and_delete_social_auth(user.id) + # Redact and delete django-simple-history snapshots of social auth records. + redact_and_delete_historical_social_auth(user.id) # Change LMS password & email user.email = get_retired_email_by_email(user.email) 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..4aff1e5b555b 100644 --- a/openedx/core/djangoapps/user_api/management/commands/retire_user.py +++ b/openedx/core/djangoapps/user_api/management/commands/retire_user.py @@ -9,7 +9,7 @@ from common.djangoapps.student.models import AccountRecovery, 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 redact_and_delete_historical_social_auth, redact_and_delete_social_auth from ...models import BulkUserRetirementConfig, UserRetirementStatus logger = logging.getLogger(__name__) @@ -146,6 +146,8 @@ def handle(self, *args, **options): UserRetirementStatus.create_retirement(user) # Redact and unlink LMS social auth accounts. redact_and_delete_social_auth(user.id) + # Redact and delete django-simple-history snapshots of social auth records. + redact_and_delete_historical_social_auth(user.id) # Change LMS password & email user.email = get_retired_email_by_email(user.email) user.set_unusable_password() From f82889dbdbf25f85ea8a9af2cc6523d05da9a66b Mon Sep 17 00:00:00 2001 From: Akanshu Aich Date: Thu, 21 May 2026 11:18:25 +0000 Subject: [PATCH 15/26] fix: fixed lint error for docstring --- .../core/djangoapps/user_api/accounts/tests/test_utils.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 b1a4c10fe4f8..bf7c368831f0 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,7 @@ -"""Unit tests for user account utility functions, including social links, completion helpers, and social-auth PII redaction utilities.""" +""" +Unit tests for user account utility functions, including social links, completion helpers, +and social-auth PII redaction utilities. +""" import unittest.mock from contextlib import contextmanager From e0ddc2f1bfe9f8fb7cb12f8ee84c69694a844ce2 Mon Sep 17 00:00:00 2001 From: Akanshu Aich Date: Fri, 22 May 2026 06:27:46 +0000 Subject: [PATCH 16/26] fix: added test case to retire user tests --- .../djangoapps/user_api/accounts/utils.py | 8 ++--- .../management/tests/test_retire_user.py | 32 +++++++++++++++++++ 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/utils.py b/openedx/core/djangoapps/user_api/accounts/utils.py index 1cdd1d0bdc73..d5434f544ce2 100644 --- a/openedx/core/djangoapps/user_api/accounts/utils.py +++ b/openedx/core/djangoapps/user_api/accounts/utils.py @@ -230,12 +230,8 @@ def redact_and_delete_historical_social_auth(user_id): """ Redact PII from all HistoricalUserSocialAuth records for the given user, then delete them. - HistoricalUserSocialAuth rows are django-simple-history snapshots of every UserSocialAuth - change. They are not touched by the standard UserSocialAuth retirement step, leaving raw - email addresses stored in the uid field indefinitely. - - Redacting before deleting ensures deletion-time handlers see sanitised values before - the rows are removed. + HistoricalUserSocialAuth rows are django-simple-history snapshots not covered by the + standard UserSocialAuth retirement step, so they must be explicitly cleaned up. """ historical_social_auth_model = getattr(getattr(UserSocialAuth, 'history', None), 'model', None) if historical_social_auth_model is None: 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..55a206698a00 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 @@ -13,6 +13,7 @@ from django.db import connection from django.db.models.signals import pre_delete from django.test.utils import CaptureQueriesContext +from django.utils import timezone from social_django.models import UserSocialAuth from common.djangoapps.student.tests.factories import UserFactory # lint-amnesty, pylint: disable=wrong-import-order @@ -171,3 +172,34 @@ def test_retire_user_redacts_sso_pii_before_deletion(setup_retirement_states, so retired_user_status = UserRetirementStatus.objects.filter(original_username=user.username).first() assert retired_user_status is not None assert retired_user_status.original_email == 'sso-user@example.com' + + +@skip_unless_lms +def test_retire_user_redacts_historical_social_auth(setup_retirement_states): # lint-amnesty, pylint: disable=redefined-outer-name, unused-argument # noqa: F811 + """ + Test that HistoricalUserSocialAuth records are redacted and deleted when retire_user is called. + + HistoricalUserSocialAuth rows are django-simple-history snapshots that are not touched + by the live UserSocialAuth retirement step, so they must be explicitly cleaned up. + """ + historical_model = getattr(getattr(UserSocialAuth, 'history', None), 'model', None) + if historical_model is None: + pytest.skip('UserSocialAuth has no history model in this environment') + + user = UserFactory.create(username='hist-sso-user', email='hist-sso-user@example.com') + record = historical_model.objects.create( + user=user, + id=1, + provider='google-oauth2', + uid='hist-sso@example.com', + extra_data={'email': 'hist-sso@example.com'}, + created=timezone.now(), + modified=timezone.now(), + history_date=timezone.now(), + history_type='+', + ) + history_id = record.history_id + + call_command('retire_user', username=user.username, user_email=user.email) + + assert not historical_model.objects.filter(history_id=history_id).exists() From dbb5c15ca07c816323ed1adf41c7446f7c274b13 Mon Sep 17 00:00:00 2001 From: Akanshu Aich Date: Fri, 22 May 2026 11:24:34 +0000 Subject: [PATCH 17/26] fix: removed unwanted comment --- openedx/core/djangoapps/user_api/accounts/tests/test_utils.py | 1 - 1 file changed, 1 deletion(-) 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 bf7c368831f0..49fef5b5685a 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py @@ -316,7 +316,6 @@ def capture_uid_at_delete(sender, instance, **kwargs): # pylint: disable=unused [query['sql'] for query in ctx], table=table_name, ) - # Verify history_id appears in the SET expression of the UPDATE, not just the WHERE clause, # since rows are filtered by user_id and history_id is used to build the unique redacted uid. sql_list = [query['sql'].upper() for query in ctx] assert any('HISTORY_ID' in sql and 'UPDATE' in sql for sql in sql_list), ( From 73e482b244a4bb9657f72fd0a87ff79fed3085a1 Mon Sep 17 00:00:00 2001 From: Akanshu Aich Date: Fri, 22 May 2026 12:32:17 +0000 Subject: [PATCH 18/26] fix: fixed the logger and getattr caling --- .../core/djangoapps/user_api/accounts/tests/test_utils.py | 8 +++----- .../user_api/management/tests/test_retire_user.py | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) 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 49fef5b5685a..c0050ed7711a 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py @@ -9,7 +9,6 @@ import ddt from completion import models from completion.test_utils import CompletionWaffleTestMixin -from django.apps import apps from django.db import connection from django.db.models.signals import pre_delete from django.test import TestCase @@ -262,10 +261,9 @@ class RedactAndDeleteHistoricalSocialAuthTest(TestCase): def setUp(self): super().setUp() self.user = UserFactory.create(username='testuser', email='testuser@example.com') - try: - self.historical_social_auth_model = apps.get_model('support', 'HistoricalUserSocialAuth') - except LookupError: - self.skipTest('support.HistoricalUserSocialAuth is not available in this test environment') + self.historical_social_auth_model = getattr(getattr(UserSocialAuth, 'history', None), 'model', None) + if self.historical_social_auth_model is None: + self.skipTest('UserSocialAuth has no history model, skipping') def _create_historical_record(self, provider='google-oauth2', uid='user@example.com', extra_data=None, source_id=1): """ 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 55a206698a00..33be8afa45c2 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 @@ -184,7 +184,7 @@ def test_retire_user_redacts_historical_social_auth(setup_retirement_states): # """ historical_model = getattr(getattr(UserSocialAuth, 'history', None), 'model', None) if historical_model is None: - pytest.skip('UserSocialAuth has no history model in this environment') + pytest.skip('UserSocialAuth has no history model, skipping') user = UserFactory.create(username='hist-sso-user', email='hist-sso-user@example.com') record = historical_model.objects.create( From 798d6e5b9b5c174e9c74afa0ee6d75e6c9d3351a Mon Sep 17 00:00:00 2001 From: Akanshu Aich Date: Mon, 25 May 2026 13:05:20 +0000 Subject: [PATCH 19/26] fix: optimized tests --- .../management/tests/test_retire_user.py | 55 +++++++++++++------ 1 file changed, 38 insertions(+), 17 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 33be8afa45c2..de4a5dd8d5f7 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 @@ -175,9 +175,24 @@ def test_retire_user_redacts_sso_pii_before_deletion(setup_retirement_states, so @skip_unless_lms -def test_retire_user_redacts_historical_social_auth(setup_retirement_states): # lint-amnesty, pylint: disable=redefined-outer-name, unused-argument # noqa: F811 +@pytest.mark.parametrize('historical_configs', [ + # Single historical record + [ + {'provider': 'google-oauth2', 'uid': 'hist-sso@example.com', + 'extra_data': {'email': 'hist-sso@example.com', 'name': 'SSO User'}}, + ], + # Multiple historical records across providers + [ + {'provider': 'google-oauth2', 'uid': 'hist-google@example.com', + 'extra_data': {'email': 'hist-google@example.com', 'name': 'Google User'}}, + {'provider': 'tpa-saml', 'uid': 'hist-saml@example.com', + 'extra_data': {'email': 'hist-saml@example.com', 'name': 'SAML User'}}, + ], +]) +def test_retire_user_redacts_historical_social_auth(setup_retirement_states, historical_configs): # lint-amnesty, pylint: disable=redefined-outer-name, unused-argument # noqa: F811 """ Test that HistoricalUserSocialAuth records are redacted and deleted when retire_user is called. + Covers both single and multiple historical record scenarios. HistoricalUserSocialAuth rows are django-simple-history snapshots that are not touched by the live UserSocialAuth retirement step, so they must be explicitly cleaned up. @@ -187,19 +202,25 @@ def test_retire_user_redacts_historical_social_auth(setup_retirement_states): # pytest.skip('UserSocialAuth has no history model, skipping') user = UserFactory.create(username='hist-sso-user', email='hist-sso-user@example.com') - record = historical_model.objects.create( - user=user, - id=1, - provider='google-oauth2', - uid='hist-sso@example.com', - extra_data={'email': 'hist-sso@example.com'}, - created=timezone.now(), - modified=timezone.now(), - history_date=timezone.now(), - history_type='+', - ) - history_id = record.history_id - - call_command('retire_user', username=user.username, user_email=user.email) - - assert not historical_model.objects.filter(history_id=history_id).exists() + history_ids = [ + historical_model.objects.create( + user=user, + id=idx + 1, + provider=cfg['provider'], + uid=cfg['uid'], + extra_data=cfg['extra_data'], + created=timezone.now(), + modified=timezone.now(), + history_date=timezone.now(), + history_type='+', + ).history_id + for idx, cfg in enumerate(historical_configs) + ] + + with CaptureQueriesContext(connection) as ctx: + call_command('retire_user', username=user.username, user_email=user.email) + + table_name = historical_model._meta.db_table + assert_update_before_delete([query['sql'] for query in ctx], table=table_name) + for history_id in history_ids: + assert not historical_model.objects.filter(history_id=history_id).exists() From ff8050d5abc4d070ae23f414c35c3542c695e420 Mon Sep 17 00:00:00 2001 From: Akanshu Aich Date: Wed, 27 May 2026 06:58:16 +0000 Subject: [PATCH 20/26] fix: removed unwanted tests and handling --- .../user_api/accounts/tests/test_utils.py | 19 +------ .../djangoapps/user_api/accounts/utils.py | 14 ++---- .../management/commands/retire_user.py | 4 +- .../management/tests/test_retire_user.py | 50 ------------------- 4 files changed, 6 insertions(+), 81 deletions(-) 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 c0050ed7711a..a45a22720cbd 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py @@ -3,7 +3,6 @@ and social-auth PII redaction utilities. """ -import unittest.mock from contextlib import contextmanager import ddt @@ -261,9 +260,7 @@ class RedactAndDeleteHistoricalSocialAuthTest(TestCase): def setUp(self): super().setUp() self.user = UserFactory.create(username='testuser', email='testuser@example.com') - self.historical_social_auth_model = getattr(getattr(UserSocialAuth, 'history', None), 'model', None) - if self.historical_social_auth_model is None: - self.skipTest('UserSocialAuth has no history model, skipping') + 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): """ @@ -287,7 +284,7 @@ def test_redacted_uid_format(self): """ uid must follow the redacted-before-delete-{history_id}@safe.com format and extra_data must be cleared. Verified at two levels: SQL ordering via CaptureQueriesContext (UPDATE - precedes DELETE, history_id embedded in SET expression) and uid value via pre_delete signal. + precedes DELETE) and uid value via pre_delete signal. """ record = self._create_historical_record(uid='private@example.com') history_id = record.history_id @@ -314,11 +311,6 @@ def capture_uid_at_delete(sender, instance, **kwargs): # pylint: disable=unused [query['sql'] for query in ctx], table=table_name, ) - # since rows are filtered by user_id and history_id is used to build the unique redacted uid. - sql_list = [query['sql'].upper() for query in ctx] - assert any('HISTORY_ID' in sql and 'UPDATE' in sql for sql in sql_list), ( - 'UPDATE must incorporate history_id into the redacted uid SET expression' - ) # Verify value-level: correct redacted uid and cleared extra_data. assert signal_capture['uid'] == expected_uid, ( @@ -351,10 +343,3 @@ def test_deletes_all_records_for_user(self): 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() - - def test_skips_gracefully_when_history_model_unavailable(self): - """ - Returns without error when UserSocialAuth has no history model registered. - """ - with unittest.mock.patch.object(UserSocialAuth, 'history', None): - redact_and_delete_historical_social_auth(self.user.id) diff --git a/openedx/core/djangoapps/user_api/accounts/utils.py b/openedx/core/djangoapps/user_api/accounts/utils.py index d5434f544ce2..cf00dc3a9f93 100644 --- a/openedx/core/djangoapps/user_api/accounts/utils.py +++ b/openedx/core/djangoapps/user_api/accounts/utils.py @@ -230,16 +230,10 @@ def redact_and_delete_historical_social_auth(user_id): """ Redact PII from all HistoricalUserSocialAuth records for the given user, then delete them. - HistoricalUserSocialAuth rows are django-simple-history snapshots not covered by the - standard UserSocialAuth retirement step, so they must be explicitly cleaned up. + 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 = getattr(getattr(UserSocialAuth, 'history', None), 'model', None) - if historical_social_auth_model is None: - LOGGER.warning( - 'redact_and_delete_historical_social_auth: UserSocialAuth has no history model, skipping for user_id=%s', - user_id, - ) - return + historical_social_auth_model = UserSocialAuth.history.model historical_queryset = historical_social_auth_model.objects.filter(user_id=user_id) historical_queryset.update( uid=Concat( @@ -262,8 +256,6 @@ def create_retirement_request_and_deactivate_account(user): # Redact and unlink LMS social auth accounts. redact_and_delete_social_auth(user.id) - # Redact and delete django-simple-history snapshots of social auth records. - redact_and_delete_historical_social_auth(user.id) # Change LMS password & email user.email = get_retired_email_by_email(user.email) 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 4aff1e5b555b..3ce738b149c0 100644 --- a/openedx/core/djangoapps/user_api/management/commands/retire_user.py +++ b/openedx/core/djangoapps/user_api/management/commands/retire_user.py @@ -9,7 +9,7 @@ from common.djangoapps.student.models import AccountRecovery, 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_historical_social_auth, redact_and_delete_social_auth +from ...accounts.utils import redact_and_delete_social_auth from ...models import BulkUserRetirementConfig, UserRetirementStatus logger = logging.getLogger(__name__) @@ -146,8 +146,6 @@ def handle(self, *args, **options): UserRetirementStatus.create_retirement(user) # Redact and unlink LMS social auth accounts. redact_and_delete_social_auth(user.id) - # Redact and delete django-simple-history snapshots of social auth records. - redact_and_delete_historical_social_auth(user.id) # Change LMS password & email user.email = get_retired_email_by_email(user.email) user.set_unusable_password() 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 de4a5dd8d5f7..d0cb74b6b7b5 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 @@ -13,7 +13,6 @@ from django.db import connection from django.db.models.signals import pre_delete from django.test.utils import CaptureQueriesContext -from django.utils import timezone from social_django.models import UserSocialAuth from common.djangoapps.student.tests.factories import UserFactory # lint-amnesty, pylint: disable=wrong-import-order @@ -174,53 +173,4 @@ def test_retire_user_redacts_sso_pii_before_deletion(setup_retirement_states, so assert retired_user_status.original_email == 'sso-user@example.com' -@skip_unless_lms -@pytest.mark.parametrize('historical_configs', [ - # Single historical record - [ - {'provider': 'google-oauth2', 'uid': 'hist-sso@example.com', - 'extra_data': {'email': 'hist-sso@example.com', 'name': 'SSO User'}}, - ], - # Multiple historical records across providers - [ - {'provider': 'google-oauth2', 'uid': 'hist-google@example.com', - 'extra_data': {'email': 'hist-google@example.com', 'name': 'Google User'}}, - {'provider': 'tpa-saml', 'uid': 'hist-saml@example.com', - 'extra_data': {'email': 'hist-saml@example.com', 'name': 'SAML User'}}, - ], -]) -def test_retire_user_redacts_historical_social_auth(setup_retirement_states, historical_configs): # lint-amnesty, pylint: disable=redefined-outer-name, unused-argument # noqa: F811 - """ - Test that HistoricalUserSocialAuth records are redacted and deleted when retire_user is called. - Covers both single and multiple historical record scenarios. - - HistoricalUserSocialAuth rows are django-simple-history snapshots that are not touched - by the live UserSocialAuth retirement step, so they must be explicitly cleaned up. - """ - historical_model = getattr(getattr(UserSocialAuth, 'history', None), 'model', None) - if historical_model is None: - pytest.skip('UserSocialAuth has no history model, skipping') - - user = UserFactory.create(username='hist-sso-user', email='hist-sso-user@example.com') - history_ids = [ - historical_model.objects.create( - user=user, - id=idx + 1, - provider=cfg['provider'], - uid=cfg['uid'], - extra_data=cfg['extra_data'], - created=timezone.now(), - modified=timezone.now(), - history_date=timezone.now(), - history_type='+', - ).history_id - for idx, cfg in enumerate(historical_configs) - ] - - with CaptureQueriesContext(connection) as ctx: - call_command('retire_user', username=user.username, user_email=user.email) - table_name = historical_model._meta.db_table - assert_update_before_delete([query['sql'] for query in ctx], table=table_name) - for history_id in history_ids: - assert not historical_model.objects.filter(history_id=history_id).exists() From 29d3f71bb3c601489d9ca8171dfda6d576e99541 Mon Sep 17 00:00:00 2001 From: Akanshu Aich Date: Wed, 27 May 2026 07:02:09 +0000 Subject: [PATCH 21/26] fix: removed extra spaces from retire user file --- .../djangoapps/user_api/management/tests/test_retire_user.py | 3 --- 1 file changed, 3 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 d0cb74b6b7b5..e57c1c50c938 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 @@ -171,6 +171,3 @@ def test_retire_user_redacts_sso_pii_before_deletion(setup_retirement_states, so retired_user_status = UserRetirementStatus.objects.filter(original_username=user.username).first() assert retired_user_status is not None assert retired_user_status.original_email == 'sso-user@example.com' - - - From 14c3906210566d8ef420ec43674ded6d146b7dc8 Mon Sep 17 00:00:00 2001 From: Akanshu Aich Date: Wed, 27 May 2026 08:33:12 +0000 Subject: [PATCH 22/26] fix: fixed the docstring and annotations --- .../djangoapps/user_api/accounts/tests/test_utils.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) 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 a45a22720cbd..7226de26faa3 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py @@ -1,6 +1,5 @@ """ -Unit tests for user account utility functions, including social links, completion helpers, -and social-auth PII redaction utilities. +Unit tests for user account utility functions, including social links, completion, and social-auth PII redaction. """ from contextlib import contextmanager @@ -281,11 +280,7 @@ def _create_historical_record(self, provider='google-oauth2', uid='user@example. ) def test_redacted_uid_format(self): - """ - uid must follow the redacted-before-delete-{history_id}@safe.com format and extra_data - must be cleared. Verified at two levels: SQL ordering via CaptureQueriesContext (UPDATE - precedes DELETE) and uid value via pre_delete signal. - """ + """Verify uid is redacted to the correct format, extra_data is cleared, and UPDATE precedes DELETE.""" record = self._create_historical_record(uid='private@example.com') history_id = record.history_id expected_uid = f'{REDACTED_SOCIAL_AUTH_UID_PREFIX}{history_id}{REDACTED_SOCIAL_AUTH_UID_SUFFIX}' From 414280283a617177a02cea5d19b21d27e88313c4 Mon Sep 17 00:00:00 2001 From: Akanshu Aich Date: Mon, 1 Jun 2026 14:29:04 +0000 Subject: [PATCH 23/26] fix: used the helper function for tests --- .../accounts/tests/test_retirement_views.py | 39 +++++++++ .../user_api/accounts/tests/test_utils.py | 87 +++++-------------- .../management/tests/test_retire_user.py | 12 +-- 3 files changed, 70 insertions(+), 68 deletions(-) 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 9ba30376c55c..df2198d0e8cc 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 @@ -1700,3 +1700,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 7226de26faa3..3d9057227137 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py @@ -19,12 +19,11 @@ 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, - REDACTED_SOCIAL_AUTH_UID_SUFFIX, 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 ) @@ -36,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(): @@ -223,7 +202,11 @@ def test_redact_and_delete_redacts_single_sso_record(self): 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_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=social_auth_id).exists() def test_redact_and_delete_redacts_multiple_sso_records(self): @@ -246,7 +229,11 @@ def test_redact_and_delete_redacts_multiple_sso_records(self): 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_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() @@ -279,44 +266,11 @@ def _create_historical_record(self, provider='google-oauth2', uid='user@example. history_type='+', ) - def test_redacted_uid_format(self): - """Verify uid is redacted to the correct format, extra_data is cleared, and UPDATE precedes DELETE.""" - record = self._create_historical_record(uid='private@example.com') - history_id = record.history_id - expected_uid = f'{REDACTED_SOCIAL_AUTH_UID_PREFIX}{history_id}{REDACTED_SOCIAL_AUTH_UID_SUFFIX}' - - signal_capture = {} - - def capture_uid_at_delete(sender, instance, **kwargs): # pylint: disable=unused-argument - # Re-fetch from DB to read the value written by UPDATE, not the stale in-memory instance. - redacted_row = self.historical_social_auth_model.objects.get(history_id=instance.history_id) - signal_capture['uid'] = redacted_row.uid - signal_capture['extra_data'] = redacted_row.extra_data - - pre_delete.connect(capture_uid_at_delete, sender=self.historical_social_auth_model) - try: - with CaptureQueriesContext(connection) as ctx: - redact_and_delete_historical_social_auth(self.user.id) - finally: - pre_delete.disconnect(capture_uid_at_delete, sender=self.historical_social_auth_model) - - # Verify SQL-level: UPDATE precedes DELETE on the correct table. - table_name = self.historical_social_auth_model._meta.db_table - assert_update_before_delete( - [query['sql'] for query in ctx], - table=table_name, - ) - - # Verify value-level: correct redacted uid and cleared extra_data. - assert signal_capture['uid'] == expected_uid, ( - f"uid at delete time was {signal_capture['uid']!r}, expected {expected_uid!r}" - ) - assert signal_capture['extra_data'] == {} - assert not self.historical_social_auth_model.objects.filter(history_id=history_id).exists() - - def test_deletes_all_records_for_user(self): + def test_historical_social_auth_redact_before_delete(self): """ - All rows for the retired user are deleted; other users' rows are untouched. + 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) @@ -324,7 +278,7 @@ def test_deletes_all_records_for_user(self): other_user = UserFactory.create(username='otheruser', email='other@example.com') other_record = self.historical_social_auth_model.objects.create( user=other_user, - id=2, + id=3, provider='google-oauth2', uid='other@example.com', extra_data={}, @@ -334,7 +288,14 @@ def test_deletes_all_records_for_user(self): history_type='+', ) - redact_and_delete_historical_social_auth(self.user.id) + table_name = self.historical_social_auth_model._meta.db_table + 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=table_name, + 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/management/tests/test_retire_user.py b/openedx/core/djangoapps/user_api/management/tests/test_retire_user.py index e57c1c50c938..1c586ac05f57 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,8 @@ 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.djangolib.testing.utils import skip_unless_lms # pylint: disable=wrong-import-order +from openedx.core.djangoapps.user_api.accounts.utils import REDACTED_SOCIAL_AUTH_UID_PREFIX +from openedx.core.djangolib.testing.utils import assert_redact_before_delete, skip_unless_lms # pylint: disable=wrong-import-order from ...models import UserRetirementStatus @@ -164,7 +162,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() From 319e5076dcd39f7012a5ecb32138df92b662c323 Mon Sep 17 00:00:00 2001 From: Akanshu Aich Date: Mon, 1 Jun 2026 16:44:37 +0000 Subject: [PATCH 24/26] fix: fixed the quality checks --- .../core/djangoapps/user_api/accounts/tests/test_utils.py | 2 +- .../djangoapps/user_api/management/tests/test_retire_user.py | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) 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 3d9057227137..862cd47d6f46 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py @@ -269,7 +269,7 @@ def _create_historical_record(self, provider='google-oauth2', uid='user@example. 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) 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 1c586ac05f57..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 @@ -23,7 +23,10 @@ setup_retirement_states, # noqa: F401 ) from openedx.core.djangoapps.user_api.accounts.utils import REDACTED_SOCIAL_AUTH_UID_PREFIX -from openedx.core.djangolib.testing.utils import assert_redact_before_delete, 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 From d5ab6408bfb578ce991d52fa450c77de0d123528 Mon Sep 17 00:00:00 2001 From: Akanshu Aich Date: Tue, 2 Jun 2026 06:40:14 +0000 Subject: [PATCH 25/26] fix: merged one and multiple sso --- .../user_api/accounts/tests/test_utils.py | 36 +++++++++---------- 1 file changed, 16 insertions(+), 20 deletions(-) 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 862cd47d6f46..be10568f2476 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py @@ -188,17 +188,11 @@ def create_social_auth(self, provider='google-oauth2', uid='user@example.com', e extra_data=extra_data, ) - def test_redact_and_delete_redacts_single_sso_record(self): + def _assert_redact_and_delete_social_auth(self, social_auth_ids): """ - Test that redact_and_delete_social_auth redacts and deletes a single SSO record. + Test redact_and_delete_social_auth and assert that all given records were + redacted before deletion. """ - social_auth = self.create_social_auth( - provider='google-oauth2', - 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) @@ -207,7 +201,18 @@ def test_redact_and_delete_redacts_single_sso_record(self): table='social_auth_usersocialauth', expected_redacted_value_list=[REDACTED_SOCIAL_AUTH_UID_PREFIX], ) - assert not UserSocialAuth.objects.filter(id=social_auth_id).exists() + 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. + """ + social_auth = self.create_social_auth( + provider='google-oauth2', + uid='google@example.com', + extra_data={'email': 'google@example.com', 'name': 'Google User'}, + ) + self._assert_redact_and_delete_social_auth([social_auth.pk]) def test_redact_and_delete_redacts_multiple_sso_records(self): """ @@ -225,16 +230,7 @@ def test_redact_and_delete_redacts_multiple_sso_records(self): extra_data={'email': 'saml@example.com', 'name': 'SAML User', 'uid': 'saml-uid'}, ).pk, ] - - 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() + self._assert_redact_and_delete_social_auth(social_auth_ids) @skip_unless_lms From 96dffa9c2f54bbee2631b7412b53354346e227a7 Mon Sep 17 00:00:00 2001 From: Akanshu Aich Date: Tue, 2 Jun 2026 09:38:49 +0000 Subject: [PATCH 26/26] fix: removed the unnecessary variable usage --- openedx/core/djangoapps/user_api/accounts/tests/test_utils.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 be10568f2476..489c47f535ee 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py @@ -284,13 +284,12 @@ def test_historical_social_auth_redact_before_delete(self): history_type='+', ) - table_name = self.historical_social_auth_model._meta.db_table 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=table_name, + 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()