fix: Redact SSO PII before deletion by ktyagiapphelix2u · Pull Request #38425 · openedx/openedx-platform

ktyagiapphelix2u · 2026-04-23T09:43:45Z

Description

Implements automatic PII redaction for UserSocialAuth records before deletion to prevent personally identifiable information from persisting after records are removed.

Jira Ticket

https://2u-internal.atlassian.net/browse/BOMS-514

ktyagiapphelix2u · 2026-05-06T10:56:18Z

@robrap We’re dealing with multiple ways SSO records can get deleted through Django admin, user actions like unlinking accounts, bulk retirement scripts. The challenge is that we don’t control all of these paths, so we can’t reliably add PII redaction directly into each one.

Instead, we’ve set up a two-layer approach.

The first layer is a Django signal that runs automatically right before any SSO record is deleted. This acts as a safety net. No matter how the deletion is triggered whether it’s from admin, user action, the signal ensures sensitive fields like the UID and extra data are redacted. It’s centralized, consistent, so it won’t cause issues if it runs more than once.

The second layer is used only in cases we fully control, like user retirement flows. There, we proactively run a bulk redaction step before deleting records. This is much faster because it uses efficient database operations. When the delete happens afterward, the signal still fires, but it detects that the data is already redacted and simply exits without doing extra work.

Together, these two layers cover both safety and performance. The signal guarantees we never miss redaction, even in code we don’t control, while the explicit bulk step keeps large-scale operations efficient.

robrap · 2026-05-06T17:31:53Z

+
+    try:
+        update_fields = {}
+        redacted_uid = f'redacted_{instance.pk}@retired.invalid'


We should use something more generic, like redact-before-delete@safe.com (or whatever we've come up with). This is not a retired email in all cases.

Since we are using this email as a flag between the bulk retirement and here, we should be using a constant that both pieces of code make use of, to ensure they stay in sync.

I'd comment the short-circuit code below to mention why we are doing it. Something like:

These fields may have already been redacted as part of a bulk retirement, so we skip the update if it is already done to reduce query count.

[for future] I wonder if we should have generic code for models with annotated PII that automatically introduce this redaction into the pre_delete signal?

robrap · 2026-05-07T14:44:27Z

+                    social_auth_records = list(UserSocialAuth.objects.filter(user_id=user.id))
+                    for auth in social_auth_records:
+                        auth.uid = get_redacted_social_auth_uid(auth.pk)
+                        auth.extra_data = {}
+                    UserSocialAuth.objects.bulk_update(social_auth_records, ['uid', 'extra_data'])


See "Using F() Expressions" in https://websitehurdles.com/django-bulk-update/ as an example of how you could refer to the pk without an extra call.

Doing so may make it impossible to use the shared method, but you could use the constants you had started with.

robrap · 2026-05-07T14:57:42Z

                    UserRetirementStatus.create_retirement(user)
-                    # Unlink LMS social auth accounts
+                    # Redact and unlink LMS social auth accounts.
+                    social_auth_records = list(UserSocialAuth.objects.filter(user_id=user.id))


Is it possible to put UserSocialAuth.objects.filter(user_id=user.id) in a variable and call the bulk_update and delete off of this? It might make it more clear that we're working from the same set.

robrap · 2026-05-07T19:40:43Z

+    social_auth_queryset = UserSocialAuth.objects.filter(user_id=user.id)
+    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={},
+    )
+    social_auth_queryset.delete()


Move to new method in utils called redact_and_delete_social_auth(user_id). This can be called from both locations, rather than duplicating code. Docstring can remind why we are redacting before deleting.

robrap · 2026-05-07T19:41:10Z

+                    # Redact and unlink LMS social auth accounts.
+                    social_auth_queryset = UserSocialAuth.objects.filter(user_id=user.id)
+                    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={},
+                    )
+                    social_auth_queryset.delete()


See related comment about redact_and_delete_social_auth.

robrap · 2026-05-07T20:11:59Z

+    and clears extra_data.
+    Blocks deletion if redaction fails to prevent PII leaks to downstream systems.
+    """
+    if not instance or not instance.pk:


I want to greatly simplify this safety-net method to something like the following:

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.warn('Social auth link for ... was deleted without first being redacted.') redact_and_delete_social_auth(instance.user_id, skip_delete=True)

Reuses existing call (see other comments) and sends optional argument to skip the delete step.

The optional skip_delete is a little hacky, but the docstring for the method could note that it is only to be used with the delete signal, where delete was already called.

It is also a little hacky that the first link for the user will trigger redaction for all the remaining links (because it takes user_id and not pk, but it greatly simplifies the code to reuse.

I'd also hope that we wouldn't need any additional exception handling.

robrap · 2026-05-07T20:16:17Z

+# 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'


It may make more sense for these to live in utils.py instead.

robrap · 2026-05-07T20:18:20Z

+
+def get_redacted_social_auth_uid(pk):
+    """
+    Return the redacted uid for a UserSocialAuth record. Single source of truth for this format.


The following is a possible update for this docstring.

Suggested change

Return the redacted uid for a UserSocialAuth record. Single source of truth for this format.

Return the redacted uid for a UserSocialAuth record.

This must match the format used in redact_and_delete_social_auth.

ktyagiapphelix2u added 4 commits April 23, 2026 09:42

fix: Redact SSO PII before deletion

9a178e3

fix: Redact SSO PII before deletion

8d57698

fix: Redact SSO PII before deletion

2688ac8

fix: Redact SSO PII before deletion

ff4b57e

ktyagiapphelix2u marked this pull request as ready for review April 23, 2026 11:29

ktyagiapphelix2u requested a review from a team as a code owner April 23, 2026 11:29

vgulati-apphelix reviewed Apr 27, 2026

View reviewed changes

Comment thread openedx/core/djangoapps/user_api/accounts/tests/test_utils.py Outdated

vgulati-apphelix reviewed Apr 27, 2026

View reviewed changes

Comment thread openedx/core/djangoapps/user_api/accounts/tests/test_utils.py Outdated

Akanshu-2u reviewed Apr 27, 2026

View reviewed changes

Comment thread openedx/core/djangoapps/user_api/accounts/utils.py Outdated

Akanshu-2u reviewed Apr 27, 2026

View reviewed changes

Comment thread openedx/core/djangoapps/user_api/accounts/signals.py Outdated

Akanshu-2u reviewed Apr 27, 2026

View reviewed changes

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

ktyagiapphelix2u added 2 commits April 28, 2026 05:45

fix: Redact SSO PII before deletion

417aa3d

fix: Redact SSO PII before deletion

542b5be

Akanshu-2u mentioned this pull request Apr 28, 2026

fix: retirement PII leaks by redacting pending secondary email/name data #38427

Open

Akanshu-2u approved these changes Apr 28, 2026

View reviewed changes

robrap reviewed Apr 29, 2026

View reviewed changes

Comment thread openedx/core/djangoapps/user_api/management/commands/retire_user.py Outdated

ktyagiapphelix2u added 2 commits May 4, 2026 06:14

fix: Redact SSO PII before deletion

1b46be6

fix: Redact SSO PII before deletion

74d655b

robrap reviewed May 5, 2026

View reviewed changes

Comment thread openedx/core/djangoapps/user_api/accounts/signals.py Outdated

ktyagiapphelix2u added 2 commits May 6, 2026 06:18

fix: Redact SSO PII before deletion

08b491f

fix: Redact SSO PII before deletion

bbb5643

robrap reviewed May 6, 2026

View reviewed changes

ktyagiapphelix2u added 3 commits May 7, 2026 05:45

fix: Redact SSO PII before deletion

07b82ff

fix: Redact SSO PII before deletion

15bcdc0

fix: Redact SSO PII before deletion

2a9fba8

robrap reviewed May 7, 2026

View reviewed changes

fix: Redact SSO PII before deletion

dd7ac9c

robrap reviewed May 7, 2026

View reviewed changes

Conversation

ktyagiapphelix2u commented Apr 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Jira Ticket

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ktyagiapphelix2u commented May 6, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ktyagiapphelix2u commented Apr 23, 2026 •

edited

Loading