Merge branch 'master' into pwnage101/ENT-11563

Author: kiram15

lms/envs/common.py

@@ -3288,12 +3288,8 @@ def _should_send_certificate_events(settings):
 #    'pipeline' (list of dotted-path strings to PipelineStep subclasses).
 OPEN_EDX_FILTERS_CONFIG = {
     "org.openedx.learning.account.settings.read_only_fields.requested.v1": {
-        "fail_silently": False,
-        "pipeline": ["enterprise.filters.accounts.AccountSettingsEnterpriseReadOnlyFieldsStep"],
-    },
-    "org.openedx.learning.grade.context.requested.v1": {
-        "fail_silently": False,
-        "pipeline": ["enterprise.filters.grades.GradeEventContextEnricher"],
+        "fail_silently": True,
+        "pipeline": ["enterprise.filters.accounts.AccountSettingsReadOnlyFieldsStep"],
     },
     "org.openedx.learning.dashboard.render.started.v1": {
         "fail_silently": False,

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
 

openedx/core/djangoapps/user_api/accounts/signals.py

@@ -2,8 +2,24 @@
 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 REDACTED_SOCIAL_AUTH_UID_PREFIX, REDACTED_SOCIAL_AUTH_UID_SUFFIX, redact_and_delete_social_auth
+
+logger = logging.getLogger(__name__)
+
+
+def get_redacted_social_auth_uid(pk):
+    """
+    Return the redacted uid for a UserSocialAuth record.
+
+    This must match the format used in redact_and_delete_social_auth.
+    """
+    return f'{REDACTED_SOCIAL_AUTH_UID_PREFIX}{pk}{REDACTED_SOCIAL_AUTH_UID_SUFFIX}'
 
 # Signal to retire a user from LMS-initiated mailings (course mailings, etc)
 # providing_args=["user"]
@@ -16,3 +32,24 @@
 # 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
+    """
+    Safety-net signal handler that redacts PII on any UserSocialAuth before deletion.
+
+    Records deleted via ``redact_and_delete_social_auth`` will already be redacted;
+    this handler is a fallback for any missed deletion path.
+    """
+    redacted_uid = get_redacted_social_auth_uid(instance.pk)
+
+    # Safety-net in case the record wasn't redacted before delete.
+    if instance.extra_data or instance.uid != redacted_uid:
+        logger.warning(
+            'Social auth link for user_id=%s, provider=%s was deleted without first being redacted.'
+            ' Redacting in pre_delete.',
+            instance.user_id,
+            instance.provider,
+        )
+        redact_and_delete_social_auth(instance.user_id, skip_delete=True)

openedx/core/djangoapps/user_api/accounts/tests/test_signals.py

@@ -0,0 +1,76 @@
+"""
+Tests for user_api accounts signals.
+"""
+
+import logging
+from unittest.mock import patch
+
+from django.test import TestCase
+from social_django.models import UserSocialAuth
+
+from common.djangoapps.student.tests.factories import UserFactory
+from openedx.core.djangoapps.user_api.accounts.signals import get_redacted_social_auth_uid
+from openedx.core.djangolib.testing.utils import skip_unless_lms
+
+
+@skip_unless_lms
+class RedactSocialAuthPIIOnDeleteSignalTest(TestCase):
+    """
+    Tests for the redact_social_auth_pii_before_deletion pre_delete signal handler.
+    """
+
+    def setUp(self):
+        super().setUp()
+        self.user = UserFactory.create(username='testuser', email='testuser@example.com')
+
+    def _create_social_auth(self, uid='user@example.com', extra_data=None):
+        if extra_data is None:
+            extra_data = {'email': 'user@example.com', 'name': 'Test User'}
+        return UserSocialAuth.objects.create(
+            user=self.user,
+            provider='google-oauth2',
+            uid=uid,
+            extra_data=extra_data,
+        )
+
+    def test_get_redacted_social_auth_uid_format(self):
+        """
+        Test that get_redacted_social_auth_uid returns the expected string format.
+
+        This is the single source of truth for the redacted uid format.
+        """
+        assert get_redacted_social_auth_uid(42) == 'redacted-before-delete-42@safe.com'
+        assert get_redacted_social_auth_uid(1) == 'redacted-before-delete-1@safe.com'
+
+    @patch('openedx.core.djangoapps.user_api.accounts.signals.redact_and_delete_social_auth')
+    def test_signal_warns_and_redacts_when_not_already_redacted(self, mock_redact):
+        """
+        When a UserSocialAuth is deleted without prior redaction, the signal handler
+        should log a warning and call redact_and_delete_social_auth with skip_delete=True.
+        """
+        social_auth = self._create_social_auth()
+
+        with self.assertLogs(
+            'openedx.core.djangoapps.user_api.accounts.signals', level=logging.WARNING
+        ) as log_ctx:
+            social_auth.delete()
+
+        mock_redact.assert_called_once_with(self.user.id, skip_delete=True)
+        assert any('was deleted without first being redacted' in msg for msg in log_ctx.output)
+
+    @patch('openedx.core.djangoapps.user_api.accounts.signals.redact_and_delete_social_auth')
+    def test_signal_skips_warning_and_redaction_when_already_redacted(self, mock_redact):
+        """
+        When a UserSocialAuth is already redacted before deletion, the signal handler
+        should not log a warning and should not call redact_and_delete_social_auth.
+        """
+        social_auth = self._create_social_auth()
+        social_auth.uid = get_redacted_social_auth_uid(social_auth.pk)
+        social_auth.extra_data = {}
+        social_auth.save(update_fields=['uid', 'extra_data'])
+        social_auth_id = social_auth.id
+
+        social_auth.delete()
+
+        mock_redact.assert_not_called()
+        assert not UserSocialAuth.objects.filter(id=social_auth_id).exists()

openedx/core/djangoapps/user_api/accounts/tests/test_utils.py

@@ -1,15 +1,23 @@
 """ Unit tests for custom UserProfile properties. """
 
+from contextlib import contextmanager
 
 import ddt
 from completion import models
 from completion.test_utils import CompletionWaffleTestMixin
+from django.db import connection
+from django.db.models.signals import pre_delete
 from django.test import TestCase
-from django.test.utils import override_settings
+from django.test.utils import CaptureQueriesContext, 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.signals import redact_social_auth_pii_before_deletion
+from openedx.core.djangoapps.user_api.accounts.utils import (
+    redact_and_delete_social_auth,
+    retrieve_last_sitewide_block_completed,
+)
 from openedx.core.djangolib.testing.utils import skip_unless_lms
 from xmodule.modulestore.tests.django_utils import (
     SharedModuleStoreTestCase,  # pylint: disable=wrong-import-order
@@ -22,6 +30,39 @@
 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():
+    """
+    Temporarily disconnect the fallback signal so tests exercise the helper path.
+    """
+    pre_delete.disconnect(redact_social_auth_pii_before_deletion, sender=UserSocialAuth)
+    try:
+        yield
+    finally:
+        pre_delete.connect(redact_social_auth_pii_before_deletion, sender=UserSocialAuth)
+
+
 @ddt.ddt
 class UserAccountSettingsTest(TestCase):
     """Unit tests for setting Social Media Links."""
@@ -133,3 +174,71 @@ def test_retrieve_last_sitewide_block_completed(self):
                )
 
         assert empty_block_url is None
+
+
+@ddt.ddt
+@skip_unless_lms
+class RedactAndDeleteSocialAuthTest(TestCase):
+    """
+    Tests for the redact_and_delete_social_auth utility function.
+    """
+
+    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):
+        """
+        Helper method to create UserSocialAuth instances for testing.
+        """
+        extra_data = extra_data or {
+            'email': f'{provider}@example.com',
+            'name': f'{provider.capitalize()} User',
+            'id': '123456789',
+        }
+        return UserSocialAuth.objects.create(
+            user=self.user,
+            provider=provider,
+            uid=uid,
+            extra_data=extra_data,
+        )
+
+    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'},
+        )
+        social_auth_id = social_auth.pk
+
+        with disconnected_social_auth_redaction_signal(), CaptureQueriesContext(connection) as ctx:
+            redact_and_delete_social_auth(self.user.id)
+
+        assert_update_before_delete([query['sql'] for query in ctx])
+        assert not UserSocialAuth.objects.filter(id=social_auth_id).exists()
+
+    def test_redact_and_delete_redacts_multiple_sso_records(self):
+        """
+        Test that redact_and_delete_social_auth redacts and deletes all SSO records for a user.
+        """
+        social_auth_ids = [
+            self.create_social_auth(
+                provider='google-oauth2',
+                uid='google@example.com',
+                extra_data={'email': 'google@example.com', 'name': 'Google User'},
+            ).pk,
+            self.create_social_auth(
+                provider='tpa-saml',
+                uid='saml@example.com',
+                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_update_before_delete([query['sql'] for query in ctx])
+        assert not UserSocialAuth.objects.filter(id__in=social_auth_ids).exists()

openedx/core/djangoapps/user_api/accounts/utils.py

@@ -11,6 +11,8 @@
 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.functions import Cast, Concat
 from django.utils.translation import gettext as _
 from edx_django_utils.user import generate_password
 from social_django.models import UserSocialAuth
@@ -23,6 +25,10 @@
 
 from ..models import UserRetirementStatus
 
+# Prefix and suffix used to build a per-record redacted uid for UserSocialAuth.
+REDACTED_SOCIAL_AUTH_UID_PREFIX = 'redacted-before-delete-'
+REDACTED_SOCIAL_AUTH_UID_SUFFIX = '@safe.com'
+
 ENABLE_SECONDARY_EMAIL_FEATURE_SWITCH = 'enable_secondary_email_feature'
 LOGGER = logging.getLogger(__name__)
 
@@ -196,6 +202,30 @@ def is_secondary_email_feature_enabled():
     return waffle.switch_is_active(ENABLE_SECONDARY_EMAIL_FEATURE_SWITCH)
 
 
+def redact_and_delete_social_auth(user_id, skip_delete=False):
+    """
+    Redact PII from all UserSocialAuth records for the given user, then delete them.
+
+    Downstream copies of data may use soft-deletes, and redacting before deleting
+    ensures PII for retired users (or future retirements) is not retained.
+
+    ``skip_delete`` should only be set to True when called from the pre_delete signal
+    handler, where deletion is already in progress.
+    """
+    social_auth_queryset = UserSocialAuth.objects.filter(user_id=user_id)
+    # Important: this redacted uid must match the format used by ``get_redacted_social_auth_uid()``.
+    social_auth_queryset.update(
+        uid=Concat(
+            Value(REDACTED_SOCIAL_AUTH_UID_PREFIX),
+            Cast('id', output_field=CharField()),
+            Value(REDACTED_SOCIAL_AUTH_UID_SUFFIX),
+        ),
+        extra_data={},
+    )
+    if not skip_delete:
+        social_auth_queryset.delete()
+
+
 def create_retirement_request_and_deactivate_account(user):
     """
     Adds user to retirement queue, unlinks social auth accounts, changes user passwords
@@ -204,8 +234,8 @@ 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.
+    redact_and_delete_social_auth(user.id)
 
     # Change LMS password & email
     user.email = get_retired_email_by_email(user.email)

openedx/core/djangoapps/user_api/management/commands/retire_user.py

@@ -5,11 +5,11 @@
 from django.contrib.auth.models import User  # pylint: disable=imported-auth-user
 from django.core.management.base import BaseCommand, CommandError
 from django.db import transaction
-from social_django.models import UserSocialAuth
 
 from common.djangoapps.student.models import AccountRecovery, Registration, get_retired_email_by_email
 from openedx.core.djangolib.oauth2_retirement_utils import retire_dot_oauth2_models
 
+from ...accounts.utils import redact_and_delete_social_auth
 from ...models import BulkUserRetirementConfig, UserRetirementStatus
 
 logger = logging.getLogger(__name__)
@@ -144,8 +144,8 @@ 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.
+                    redact_and_delete_social_auth(user.id)
                     # Change LMS password & email
                     user.email = get_retired_email_by_email(user.email)
                     user.set_unusable_password()