Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@
from django.contrib.sites.models import Site
from django.core import mail
from django.core.cache import cache
from django.db import connection
from django.test import TestCase
from django.test.utils import CaptureQueriesContext
from django.urls import reverse
from enterprise.models import (
EnterpriseCourseEnrollment,
Expand Down Expand Up @@ -1078,9 +1080,91 @@ def cleanup_and_assert_status(self, data=None, expected_status=status.HTTP_204_N
assert response.status_code == expected_status
return response

def _assert_redacted_update_delete_queries(self, queries, redacted_username, redacted_email, redacted_name):
"""
Helper method to verify UPDATE and DELETE queries contain correct field-value assignments.

Args:
queries: List of captured query dicts from CaptureQueriesContext
redacted_username: Expected redacted username value
redacted_email: Expected redacted email value
redacted_name: Expected redacted name value
"""
update_queries = [q for q in queries if 'UPDATE' in q['sql'] and 'user_api_userretirementstatus' in q['sql']]
delete_queries = [q for q in queries if 'DELETE' in q['sql'] and 'user_api_userretirementstatus' in q['sql']]

# Should have 9 UPDATE and 9 DELETE queries
assert len(update_queries) == 9, f"Expected 9 UPDATE queries, found {len(update_queries)}"
assert len(delete_queries) == 9, f"Expected 9 DELETE queries, found {len(delete_queries)}"

# Verify UPDATE queries contain the redacted values
for update_query in update_queries:
sql = update_query['sql'].upper()
sql_lower = update_query['sql']
# Check that SET clause contains the redacted values
assert redacted_username in sql_lower, (
f"UPDATE query missing redacted username '{redacted_username}': {sql_lower}"
)
assert redacted_email in sql_lower, f"UPDATE query missing redacted email '{redacted_email}': {sql_lower}"
assert redacted_name in sql_lower, f"UPDATE query missing redacted name '{redacted_name}': {sql_lower}"
# Verify it's an UPDATE on the correct table
assert 'original_username' in sql_lower or 'original_email' in sql_lower, (
f"UPDATE query doesn't appear to update retirement fields: {sql_lower}"
)

def test_simple_success(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there is any difference between what is being tested here, and what is tested in test_redaction_before_deletion, right? It's the same test, but you are just using two different names and asserting on different things. I think I would just delete test_simple_success and rename test_redaction_before_deletion to test_default_redacted_values. I think test_default_redacted_values already has all the assertions you need. Thoughts?

"""
Test basic cleanup with default redacted values.
"""
# Verify redaction happens (records exist before cleanup)
assert UserRetirementStatus.objects.count() == 9

# Make the cleanup request
self.cleanup_and_assert_status()
assert not UserRetirementStatus.objects.all()

# Records should be deleted after redaction
retirements = UserRetirementStatus.objects.all()
assert retirements.count() == 0

def test_redaction_before_deletion(self):
"""
Verify that redaction (UPDATE) happens before deletion (DELETE).
Captures actual SQL queries to ensure UPDATE queries contain redacted values.
"""
with CaptureQueriesContext(connection) as context:
self.cleanup_and_assert_status()

# Verify records are deleted after redaction
retirements = UserRetirementStatus.objects.all()
assert retirements.count() == 0

# Verify UPDATE and DELETE queries with default 'redacted' value
self._assert_redacted_update_delete_queries(context.captured_queries, 'redacted', 'redacted', 'redacted')

def test_custom_redacted_values(self):
"""Test that custom redacted values are applied before deletion."""
custom_username = 'username-redacted-12345'
custom_email = 'email-redacted-67890'
custom_name = 'name-redacted-abcde'

data = {
'usernames': self.usernames,
'redacted_username': custom_username,
'redacted_email': custom_email,
'redacted_name': custom_name
}

with CaptureQueriesContext(connection) as context:
self.cleanup_and_assert_status(data=data)

# Verify records are deleted after redaction
retirements = UserRetirementStatus.objects.all()
assert retirements.count() == 0

# Verify UPDATE and DELETE queries with custom redacted values
self._assert_redacted_update_delete_queries(
context.captured_queries, custom_username, custom_email, custom_name
)

def test_leaves_other_users(self):
remaining_usernames = []
Expand Down
21 changes: 18 additions & 3 deletions openedx/core/djangoapps/user_api/accounts/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -1024,14 +1024,20 @@ def cleanup(self, request):

```
{
'usernames': ['user1', 'user2', ...]
'usernames': ['user1', 'user2', ...],
'redacted_username': 'Value to store in username field',
'redacted_email': 'Value to store in email field',
'redacted_name': 'Value to store in name field'
}
```

Deletes a batch of retirement requests by username.
Redacts a batch of retirement requests by redacting PII fields.
"""
try:
usernames = request.data["usernames"]
redacted_username = request.data.get("redacted_username", "redacted")
redacted_email = request.data.get("redacted_email", "redacted")
redacted_name = request.data.get("redacted_name", "redacted")

if not isinstance(usernames, list):
raise TypeError("Usernames should be an array.")
Expand All @@ -1045,7 +1051,16 @@ def cleanup(self, request):
if len(usernames) != len(retirements):
raise UserRetirementStatus.DoesNotExist("Not all usernames exist in the COMPLETE state.")

retirements.delete()
# Redact PII fields first, then delete. In case an ETL tool is syncing data
# to a downstream data warehouse, and treats the deletes as soft-deletes,
# the data will have first been redacted, protecting the sensitive PII.
for retirement in retirements:
retirement.original_username = redacted_username
retirement.original_email = redacted_email
retirement.original_name = redacted_name
retirement.save()
retirement.delete()

return Response(status=status.HTTP_204_NO_CONTENT)
except (RetirementStateError, UserRetirementStatus.DoesNotExist, TypeError) as exc:
return Response(str(exc), status=status.HTTP_400_BAD_REQUEST)
Expand Down
Loading