fix: Redact SSO PII before deletion

Author: ktyagiapphelix2u

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

@@ -170,15 +170,14 @@ def setUp(self):
 
     def _assert_update_before_delete(self, sql_list, table='social_auth_usersocialauth'):
         """
-        Assert that at least one UPDATE on ``table`` precedes the DELETE in ``sql_list``.
+        Assert that an UPDATE on ``table`` runs before the DELETE on ``table``.
 
-        This verifies that PII was redacted in the database before the row was removed,
-        which matters because downstream systems may use soft-deletes and could otherwise
-        retain unredacted data.
+        This verifies that redaction happens in the helper itself, not via the
+        fallback pre_delete signal.
         """
         table_key = table.upper()
-        update_indices = [i for i, s in enumerate(sql_list) if 'UPDATE' in s.upper() and table_key in s.upper()]
-        delete_indices = [i for i, s in enumerate(sql_list) if 'DELETE' in s.upper() and table_key in s.upper()]
+        update_indices = [i for i, sql in enumerate(sql_list) if 'UPDATE' in sql.upper() and table_key in sql.upper()]
+        delete_indices = [i for i, sql in enumerate(sql_list) if 'DELETE' in sql.upper() and table_key in sql.upper()]
         assert update_indices, f'Expected at least one UPDATE on {table}'
         assert delete_indices, f'Expected at least one DELETE on {table}'
         assert update_indices[0] < delete_indices[0], 'Expected UPDATE to precede DELETE'
@@ -202,15 +201,15 @@ def create_social_auth(self, provider='google-oauth2', uid='user@example.com', e
 
     def test_redact_and_delete_issues_update_before_delete(self):
         """
-        Test that redact_and_delete_social_auth issues an UPDATE (redaction) before the DELETE.
+        Test that redact_and_delete_social_auth redacts before deleting a social auth record.
         """
         social_auth = self.create_social_auth()
         social_auth_id = social_auth.id
 
         with CaptureQueriesContext(connection) as ctx:
             redact_and_delete_social_auth(self.user.id)
 
-        self._assert_update_before_delete([q['sql'] for q in ctx])
+        self._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_providers(self):
@@ -235,6 +234,6 @@ def test_redact_and_delete_redacts_multiple_sso_providers(self):
         with CaptureQueriesContext(connection) as ctx:
             redact_and_delete_social_auth(self.user.id)
 
-        self._assert_update_before_delete([q['sql'] for q in ctx])
+        self._assert_update_before_delete([query['sql'] for query in ctx])
         for auth_id in auth_ids:
             assert not UserSocialAuth.objects.filter(id=auth_id).exists()

openedx/core/djangoapps/user_api/management/tests/test_retire_user.py

@@ -5,6 +5,7 @@
 
 import csv
 import os
+from contextlib import contextmanager
 
 import pytest
 from django.contrib.auth.models import User  # lint-amnesty, pylint: disable=imported-auth-user
@@ -29,6 +30,27 @@
 user_file = 'userfile.csv'
 
 
+@contextmanager
+def disconnected_social_auth_redaction_signal():
+    """
+    Temporarily disconnect the fallback signal so these tests exercise the command 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)
+
+
+def assert_update_before_delete(sql_list, table='social_auth_usersocialauth'):
+    table_key = table.upper()
+    update_indices = [i for i, sql in enumerate(sql_list) if 'UPDATE' in sql.upper() and table_key in sql.upper()]
+    delete_indices = [i for i, sql in enumerate(sql_list) if 'DELETE' in sql.upper() and table_key in sql.upper()]
+    assert update_indices, f'Expected at least one UPDATE on {table}'
+    assert delete_indices, f'Expected at least one DELETE on {table}'
+    assert update_indices[0] < delete_indices[0], 'Expected UPDATE to precede DELETE'
+
+
 def generate_dummy_users():
     """
     Function to generate dummy users that needs to be retired
@@ -137,21 +159,10 @@ def test_retire_user_redacts_sso_pii_before_deletion(setup_retirement_states):
     )
     social_auth_id = social_auth.id
 
-    # Disconnect the safety-net signal so we prove retire_user itself redacts first.
-    pre_delete.disconnect(redact_social_auth_pii_before_deletion, sender=UserSocialAuth)
-    try:
-        with CaptureQueriesContext(connection) as ctx:
-            call_command('retire_user', username=user.username, user_email=user.email)
-    finally:
-        pre_delete.connect(redact_social_auth_pii_before_deletion, sender=UserSocialAuth)
+    with disconnected_social_auth_redaction_signal(), CaptureQueriesContext(connection) as ctx:
+        call_command('retire_user', username=user.username, user_email=user.email)
 
-    sql_list = [q['sql'] for q in ctx]
-    table_key = 'SOCIAL_AUTH_USERSOCIALAUTH'
-    update_indices = [i for i, s in enumerate(sql_list) if 'UPDATE' in s.upper() and table_key in s.upper()]
-    delete_indices = [i for i, s in enumerate(sql_list) if 'DELETE' in s.upper() and table_key in s.upper()]
-    assert update_indices, 'Expected at least one UPDATE (redaction) on social_auth_usersocialauth'
-    assert delete_indices, 'Expected at least one DELETE on social_auth_usersocialauth'
-    assert update_indices[0] < delete_indices[0], 'Expected UPDATE (redaction) to precede DELETE'
+    assert_update_before_delete([query['sql'] for query in ctx])
 
     assert not UserSocialAuth.objects.filter(id=social_auth_id).exists()
 
@@ -184,21 +195,10 @@ def test_retire_user_redacts_each_social_auth_before_bulk_deletion(setup_retirem
     google_auth_id = google_auth.id
     saml_auth_id = saml_auth.id
 
-    # Disconnect the safety-net signal so we prove retire_user itself redacts first.
-    pre_delete.disconnect(redact_social_auth_pii_before_deletion, sender=UserSocialAuth)
-    try:
-        with CaptureQueriesContext(connection) as ctx:
-            call_command('retire_user', username=user.username, user_email=user.email)
-    finally:
-        pre_delete.connect(redact_social_auth_pii_before_deletion, sender=UserSocialAuth)
+    with disconnected_social_auth_redaction_signal(), CaptureQueriesContext(connection) as ctx:
+        call_command('retire_user', username=user.username, user_email=user.email)
 
-    sql_list = [q['sql'] for q in ctx]
-    table_key = 'SOCIAL_AUTH_USERSOCIALAUTH'
-    update_indices = [i for i, s in enumerate(sql_list) if 'UPDATE' in s.upper() and table_key in s.upper()]
-    delete_indices = [i for i, s in enumerate(sql_list) if 'DELETE' in s.upper() and table_key in s.upper()]
-    assert update_indices, 'Expected at least one UPDATE (redaction) on social_auth_usersocialauth'
-    assert delete_indices, 'Expected at least one DELETE on social_auth_usersocialauth'
-    assert update_indices[0] < delete_indices[0], 'Expected UPDATE (redaction) to precede DELETE'
+    assert_update_before_delete([query['sql'] for query in ctx])
 
     assert not UserSocialAuth.objects.filter(id=google_auth_id).exists()
     assert not UserSocialAuth.objects.filter(id=saml_auth_id).exists()