From 9a178e35f7ac1f41b1d16efd59b7c5cce9dd9a70 Mon Sep 17 00:00:00 2001 From: ktyagiapphelix2u Date: Thu, 23 Apr 2026 09:42:58 +0000 Subject: [PATCH 1/4] fix: Redact SSO PII before deletion --- .../djangoapps/user_api/accounts/__init__.py | 3 + .../djangoapps/user_api/accounts/signals.py | 34 ++++++++- .../user_api/accounts/tests/test_utils.py | 73 ++++++++++++++++++- .../djangoapps/user_api/accounts/utils.py | 34 ++++++++- .../management/commands/retire_user.py | 7 +- .../management/tests/test_retire_user.py | 49 +++++++++++++ 6 files changed, 194 insertions(+), 6 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/__init__.py b/openedx/core/djangoapps/user_api/accounts/__init__.py index faed01a6bb8d..e80aabe4169d 100644 --- a/openedx/core/djangoapps/user_api/accounts/__init__.py +++ b/openedx/core/djangoapps/user_api/accounts/__init__.py @@ -6,6 +6,9 @@ from django.utils.text import format_lazy from django.utils.translation import gettext_lazy as _ +# Import signals to ensure they are registered +from . import signals # noqa: F401, pylint: disable=unused-import + # The maximum length for the bio ("about me") account field BIO_MAX_LENGTH = 300 diff --git a/openedx/core/djangoapps/user_api/accounts/signals.py b/openedx/core/djangoapps/user_api/accounts/signals.py index be7bf75d99ee..d66fba35aace 100644 --- a/openedx/core/djangoapps/user_api/accounts/signals.py +++ b/openedx/core/djangoapps/user_api/accounts/signals.py @@ -2,8 +2,15 @@ Django Signal related functionality for user_api accounts """ +import logging -from django.dispatch import Signal +from django.db.models.signals import pre_delete +from django.dispatch import Signal, receiver +from social_django.models import UserSocialAuth + +from .utils import redact_user_social_auth_pii + +logger = logging.getLogger(__name__) # Signal to retire a user from LMS-initiated mailings (course mailings, etc) # providing_args=["user"] @@ -16,3 +23,28 @@ # Signal to retire LMS misc information # providing_args=["user"] USER_RETIRE_LMS_MISC = Signal() + + +@receiver(pre_delete, sender=UserSocialAuth) +def redact_social_auth_pii_before_deletion(sender, instance, **kwargs): # pylint: disable=unused-argument + """ + Signal handler to redact PII from UserSocialAuth records before deletion. + + This ensures that when SSO records are deleted (either via user retirement, manual unlinking, + or any other method), PII is redacted first. This prevents soft-deleted records in Snowflake + from retaining sensitive user information. + + Note: We call redact_user_social_auth_pii which saves the redacted data before the actual + deletion happens. This is intentional - when Snowflake syncs, it will capture the redacted + state before marking the record as deleted. + """ + try: + redact_user_social_auth_pii(instance) + except Exception as e: # pylint: disable=broad-except + # Log the error but don't prevent the deletion + logger.exception( + "Failed to redact PII for UserSocialAuth before deletion: user_id=%s, provider=%s, error=%s", + instance.user_id, + instance.provider, + str(e) + ) 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 77ac4ab1ba80..7c5f781b308f 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py @@ -6,10 +6,15 @@ from completion.test_utils import CompletionWaffleTestMixin from django.test import TestCase from django.test.utils import override_settings +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.utils import retrieve_last_sitewide_block_completed +from openedx.core.djangoapps.user_api.accounts.utils import ( + REDACTED_SOCIAL_AUTH_UID, + redact_user_social_auth_pii, + retrieve_last_sitewide_block_completed, +) from openedx.core.djangolib.testing.utils import skip_unless_lms from xmodule.modulestore.tests.django_utils import ( SharedModuleStoreTestCase, # lint-amnesty, pylint: disable=wrong-import-order @@ -133,3 +138,69 @@ def test_retrieve_last_sitewide_block_completed(self): ) assert empty_block_url is None + + +@skip_unless_lms +class RedactUserSocialAuthPIITest(TestCase): + """ + Tests for SSO PII redaction before deletion. + """ + + def setUp(self): + super().setUp() + self.user = UserFactory.create(username='testuser', email='testuser@example.com') + + def create_social_auth(self, provider='google-oauth2', uid='user@example.com', extra_data=None): + if extra_data is None: + extra_data = { + 'email': 'user@example.com', + 'name': 'Test User', + 'id': '123456789', + } + return UserSocialAuth.objects.create( + user=self.user, + provider=provider, + uid=uid, + extra_data=extra_data, + ) + + def test_redact_user_social_auth_pii(self): + social_auth = self.create_social_auth() + + redact_user_social_auth_pii(social_auth) + social_auth.refresh_from_db() + + assert social_auth.uid == REDACTED_SOCIAL_AUTH_UID + assert social_auth.extra_data == {} + + def test_redact_user_social_auth_pii_idempotent(self): + social_auth = self.create_social_auth() + + redact_user_social_auth_pii(social_auth) + redact_user_social_auth_pii(social_auth) + social_auth.refresh_from_db() + + assert social_auth.uid == REDACTED_SOCIAL_AUTH_UID + assert social_auth.extra_data == {} + + def test_redact_multiple_sso_providers(self): + google_auth = self.create_social_auth( + provider='google-oauth2', + uid='google@example.com', + extra_data={'email': 'google@example.com', 'name': 'Google User'} + ) + saml_auth = self.create_social_auth( + provider='tpa-saml', + uid='saml@example.com', + extra_data={'email': 'saml@example.com', 'name': 'SAML User', 'uid': 'saml-uid'} + ) + + redact_user_social_auth_pii(google_auth) + redact_user_social_auth_pii(saml_auth) + google_auth.refresh_from_db() + saml_auth.refresh_from_db() + + assert google_auth.uid == REDACTED_SOCIAL_AUTH_UID + assert google_auth.extra_data == {} + assert saml_auth.uid == REDACTED_SOCIAL_AUTH_UID + assert saml_auth.extra_data == {} diff --git a/openedx/core/djangoapps/user_api/accounts/utils.py b/openedx/core/djangoapps/user_api/accounts/utils.py index ab70b8f15c8d..b6225502fbd9 100644 --- a/openedx/core/djangoapps/user_api/accounts/utils.py +++ b/openedx/core/djangoapps/user_api/accounts/utils.py @@ -25,6 +25,7 @@ ENABLE_SECONDARY_EMAIL_FEATURE_SWITCH = 'enable_secondary_email_feature' LOGGER = logging.getLogger(__name__) +REDACTED_SOCIAL_AUTH_UID = 'redacted@redacted.com' def validate_social_link(platform_name, new_social_link): @@ -196,6 +197,33 @@ def is_secondary_email_feature_enabled(): return waffle.switch_is_active(ENABLE_SECONDARY_EMAIL_FEATURE_SWITCH) +def redact_user_social_auth_pii(user_social_auth): + """ + Redact PII from a UserSocialAuth record before deletion. + + Snowflake can retain deleted source rows as soft-deleted records, so sensitive + fields should be overwritten before deletion. + """ + if not user_social_auth or not user_social_auth.pk: + return + + update_fields = {} + + if user_social_auth.uid != REDACTED_SOCIAL_AUTH_UID: + update_fields['uid'] = REDACTED_SOCIAL_AUTH_UID + if user_social_auth.extra_data: + update_fields['extra_data'] = {} + + if not update_fields: + return + + UserSocialAuth.objects.filter(pk=user_social_auth.pk).update(**update_fields) + + # Keep instance in sync in case the caller reuses it. + for field_name, value in update_fields.items(): + setattr(user_social_auth, field_name, value) + + def create_retirement_request_and_deactivate_account(user): """ Adds user to retirement queue, unlinks social auth accounts, changes user passwords @@ -204,8 +232,10 @@ def create_retirement_request_and_deactivate_account(user): # Add user to retirement queue. UserRetirementStatus.create_retirement(user) - # Unlink LMS social auth accounts - UserSocialAuth.objects.filter(user_id=user.id).delete() + # Redact and unlink LMS social auth accounts + for social_auth in UserSocialAuth.objects.filter(user_id=user.id): + redact_user_social_auth_pii(social_auth) + social_auth.delete() # 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 1b3b497ea5c7..ca13d479d86b 100644 --- a/openedx/core/djangoapps/user_api/management/commands/retire_user.py +++ b/openedx/core/djangoapps/user_api/management/commands/retire_user.py @@ -11,6 +11,7 @@ from openedx.core.djangolib.oauth2_retirement_utils import retire_dot_oauth2_models from ...models import BulkUserRetirementConfig, UserRetirementStatus +from ...accounts.utils import redact_user_social_auth_pii logger = logging.getLogger(__name__) @@ -144,8 +145,10 @@ def handle(self, *args, **options): for user in users: # Add user to retirement queue. UserRetirementStatus.create_retirement(user) - # Unlink LMS social auth accounts - UserSocialAuth.objects.filter(user_id=user.id).delete() + # Redact and unlink LMS social auth accounts + for social_auth in UserSocialAuth.objects.filter(user_id=user.id): + redact_user_social_auth_pii(social_auth) + social_auth.delete() # 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 6de2287fc4ba..696c903e4b88 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 @@ -5,10 +5,12 @@ import csv import os +from unittest import mock import pytest from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user from django.core.management import CommandError, call_command +from social_django.models import UserSocialAuth from common.djangoapps.student.tests.factories import UserFactory # lint-amnesty, pylint: disable=wrong-import-order from openedx.core.djangoapps.user_api.accounts.tests.retirement_helpers import ( # lint-amnesty, pylint: disable=unused-import, wrong-import-order @@ -107,3 +109,50 @@ def test_retire_with_username_email_userfile(setup_retirement_states): # lint-a with pytest.raises(CommandError, match=r'You cannot use userfile option with username and user_email'): call_command('retire_user', user_file=user_file, username=username, user_email=user_email) remove_user_file() + + +@skip_unless_lms +def test_retire_user_redacts_sso_pii_before_deletion(setup_retirement_states): # lint-amnesty, pylint: disable=redefined-outer-name, unused-argument + user = UserFactory.create(username='sso-user', email='sso-user@example.com') + social_auth = UserSocialAuth.objects.create( + user=user, + provider='google-oauth2', + uid='sso-user@example.com', + extra_data={ + 'email': 'sso-user@example.com', + 'name': 'SSO Test User', + 'id': '123456789', + } + ) + social_auth_id = social_auth.id + + call_command('retire_user', username=user.username, user_email=user.email) + + assert not UserSocialAuth.objects.filter(id=social_auth_id).exists() + 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_calls_redaction_for_each_social_auth(setup_retirement_states): # lint-amnesty, pylint: disable=redefined-outer-name, unused-argument + user = UserFactory.create(username='multi-sso-user', email='multi-sso@example.com') + UserSocialAuth.objects.create( + user=user, + provider='google-oauth2', + uid='google-multi@example.com', + extra_data={'email': 'google-multi@example.com', 'name': 'Google User'} + ) + UserSocialAuth.objects.create( + user=user, + provider='tpa-saml', + uid='saml-multi@example.com', + extra_data={'email': 'saml-multi@example.com', 'name': 'SAML User', 'uid': 'saml-123'} + ) + + with mock.patch( + 'openedx.core.djangoapps.user_api.management.commands.retire_user.redact_user_social_auth_pii' + ) as mock_redact: + call_command('retire_user', username=user.username, user_email=user.email) + + assert mock_redact.call_count == 2 From 8d57698164ff1c8883bfa8660786e212077253a9 Mon Sep 17 00:00:00 2001 From: ktyagiapphelix2u Date: Thu, 23 Apr 2026 09:48:18 +0000 Subject: [PATCH 2/4] fix: Redact SSO PII before deletion --- openedx/core/djangoapps/user_api/accounts/signals.py | 4 ++-- .../djangoapps/user_api/management/commands/retire_user.py | 2 +- .../djangoapps/user_api/management/tests/test_retire_user.py | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/signals.py b/openedx/core/djangoapps/user_api/accounts/signals.py index d66fba35aace..fa71459bcd80 100644 --- a/openedx/core/djangoapps/user_api/accounts/signals.py +++ b/openedx/core/djangoapps/user_api/accounts/signals.py @@ -29,11 +29,11 @@ def redact_social_auth_pii_before_deletion(sender, instance, **kwargs): # pylint: disable=unused-argument """ Signal handler to redact PII from UserSocialAuth records before deletion. - + This ensures that when SSO records are deleted (either via user retirement, manual unlinking, or any other method), PII is redacted first. This prevents soft-deleted records in Snowflake from retaining sensitive user information. - + Note: We call redact_user_social_auth_pii which saves the redacted data before the actual deletion happens. This is intentional - when Snowflake syncs, it will capture the redacted state before marking the record as deleted. 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 ca13d479d86b..c3b103a08061 100644 --- a/openedx/core/djangoapps/user_api/management/commands/retire_user.py +++ b/openedx/core/djangoapps/user_api/management/commands/retire_user.py @@ -10,8 +10,8 @@ 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 ...models import BulkUserRetirementConfig, UserRetirementStatus from ...accounts.utils import redact_user_social_auth_pii +from ...models import BulkUserRetirementConfig, UserRetirementStatus logger = logging.getLogger(__name__) diff --git a/openedx/core/djangoapps/user_api/management/tests/test_retire_user.py b/openedx/core/djangoapps/user_api/management/tests/test_retire_user.py index 696c903e4b88..3a81cca9c199 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 @@ -112,7 +112,7 @@ def test_retire_with_username_email_userfile(setup_retirement_states): # lint-a @skip_unless_lms -def test_retire_user_redacts_sso_pii_before_deletion(setup_retirement_states): # lint-amnesty, pylint: disable=redefined-outer-name, unused-argument +def test_retire_user_redacts_sso_pii_before_deletion(setup_retirement_states): # lint-amnesty, pylint: disable=redefined-outer-name, unused-argument # noqa: F811 user = UserFactory.create(username='sso-user', email='sso-user@example.com') social_auth = UserSocialAuth.objects.create( user=user, @@ -135,7 +135,7 @@ def test_retire_user_redacts_sso_pii_before_deletion(setup_retirement_states): @skip_unless_lms -def test_retire_user_calls_redaction_for_each_social_auth(setup_retirement_states): # lint-amnesty, pylint: disable=redefined-outer-name, unused-argument +def test_retire_user_calls_redaction_for_each_social_auth(setup_retirement_states): # lint-amnesty, pylint: disable=redefined-outer-name, unused-argument # noqa: F811 user = UserFactory.create(username='multi-sso-user', email='multi-sso@example.com') UserSocialAuth.objects.create( user=user, From 2688ac8596388a4d4ae3368c922f2d126d144c48 Mon Sep 17 00:00:00 2001 From: ktyagiapphelix2u Date: Thu, 23 Apr 2026 09:59:36 +0000 Subject: [PATCH 3/4] fix: Redact SSO PII before deletion --- .../djangoapps/user_api/accounts/tests/test_utils.py | 12 ++++++++++++ .../user_api/management/tests/test_retire_user.py | 6 ++++++ 2 files changed, 18 insertions(+) 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 7c5f781b308f..879f3aece738 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py @@ -151,6 +151,9 @@ def setUp(self): self.user = UserFactory.create(username='testuser', email='testuser@example.com') def create_social_auth(self, provider='google-oauth2', uid='user@example.com', extra_data=None): + """ + Helper method to create UserSocialAuth instances for testing. + """ if extra_data is None: extra_data = { 'email': 'user@example.com', @@ -165,6 +168,9 @@ def create_social_auth(self, provider='google-oauth2', uid='user@example.com', e ) def test_redact_user_social_auth_pii(self): + """ + Test that redact_user_social_auth_pii correctly redacts uid and extra_data fields. + """ social_auth = self.create_social_auth() redact_user_social_auth_pii(social_auth) @@ -174,6 +180,9 @@ def test_redact_user_social_auth_pii(self): assert social_auth.extra_data == {} def test_redact_user_social_auth_pii_idempotent(self): + """ + Test that calling redact_user_social_auth_pii multiple times is idempotent. + """ social_auth = self.create_social_auth() redact_user_social_auth_pii(social_auth) @@ -184,6 +193,9 @@ def test_redact_user_social_auth_pii_idempotent(self): assert social_auth.extra_data == {} def test_redact_multiple_sso_providers(self): + """ + Test that redaction works correctly for multiple SSO providers (Google OAuth and SAML). + """ google_auth = self.create_social_auth( provider='google-oauth2', uid='google@example.com', 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 3a81cca9c199..a3da6abcf1cb 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 @@ -113,6 +113,9 @@ def test_retire_with_username_email_userfile(setup_retirement_states): # lint-a @skip_unless_lms def test_retire_user_redacts_sso_pii_before_deletion(setup_retirement_states): # lint-amnesty, pylint: disable=redefined-outer-name, unused-argument # noqa: F811 + """ + Test that SSO PII is redacted before UserSocialAuth records are deleted during retirement. + """ user = UserFactory.create(username='sso-user', email='sso-user@example.com') social_auth = UserSocialAuth.objects.create( user=user, @@ -136,6 +139,9 @@ def test_retire_user_redacts_sso_pii_before_deletion(setup_retirement_states): @skip_unless_lms def test_retire_user_calls_redaction_for_each_social_auth(setup_retirement_states): # lint-amnesty, pylint: disable=redefined-outer-name, unused-argument # noqa: F811 + """ + Test that redact_user_social_auth_pii is called for each UserSocialAuth record during retirement. + """ user = UserFactory.create(username='multi-sso-user', email='multi-sso@example.com') UserSocialAuth.objects.create( user=user, From ff4b57e28cb279b70b69dd8eb0e7d451002574bd Mon Sep 17 00:00:00 2001 From: ktyagiapphelix2u Date: Thu, 23 Apr 2026 10:26:01 +0000 Subject: [PATCH 4/4] fix: Redact SSO PII before deletion --- openedx/core/djangoapps/user_api/accounts/signals.py | 6 +++--- openedx/core/djangoapps/user_api/accounts/utils.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/signals.py b/openedx/core/djangoapps/user_api/accounts/signals.py index fa71459bcd80..cf8b01a128c4 100644 --- a/openedx/core/djangoapps/user_api/accounts/signals.py +++ b/openedx/core/djangoapps/user_api/accounts/signals.py @@ -31,11 +31,11 @@ def redact_social_auth_pii_before_deletion(sender, instance, **kwargs): # pylin Signal handler to redact PII from UserSocialAuth records before deletion. This ensures that when SSO records are deleted (either via user retirement, manual unlinking, - or any other method), PII is redacted first. This prevents soft-deleted records in Snowflake - from retaining sensitive user information. + or any other method), PII is redacted first. This prevents downstream systems that maintain + soft-deleted records from retaining sensitive user information. Note: We call redact_user_social_auth_pii which saves the redacted data before the actual - deletion happens. This is intentional - when Snowflake syncs, it will capture the redacted + deletion happens. This is intentional - downstream systems will capture the redacted state before marking the record as deleted. """ try: diff --git a/openedx/core/djangoapps/user_api/accounts/utils.py b/openedx/core/djangoapps/user_api/accounts/utils.py index b6225502fbd9..4179aac93988 100644 --- a/openedx/core/djangoapps/user_api/accounts/utils.py +++ b/openedx/core/djangoapps/user_api/accounts/utils.py @@ -201,7 +201,7 @@ def redact_user_social_auth_pii(user_social_auth): """ Redact PII from a UserSocialAuth record before deletion. - Snowflake can retain deleted source rows as soft-deleted records, so sensitive + Downstream systems can retain deleted source rows as soft-deleted records, so sensitive fields should be overwritten before deletion. """ if not user_social_auth or not user_social_auth.pk: