Skip to content

Commit 7536eff

Browse files
fix: ensure PII is cleared from historical certificate records during retirement (#38671)
When ENABLE_REDACT_HISTORICAL_PII_RETIREMENT is enabled, GeneratedCertificate's django-simple-history table (certificates_historicalgeneratedcertificate) will also have the user's name redacted as part of user retirement.
1 parent 29bdbc1 commit 7536eff

6 files changed

Lines changed: 96 additions & 15 deletions

File tree

lms/djangoapps/certificates/api.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
from common.djangoapps.student.models import CourseEnrollment
2525
from lms.djangoapps.branding import api as branding_api
2626
from lms.djangoapps.certificates.config import AUTO_CERTIFICATE_GENERATION as _AUTO_CERTIFICATE_GENERATION
27+
from lms.djangoapps.certificates.config import REDACT_CERTIFICATES_HISTORICAL_PII
2728
from lms.djangoapps.certificates.data import CertificateStatuses, GeneratedCertificateData
2829
from lms.djangoapps.certificates.generation_handler import generate_certificate_task as _generate_certificate_task
2930
from lms.djangoapps.certificates.generation_handler import is_on_certificate_allowlist as _is_on_certificate_allowlist
@@ -977,13 +978,17 @@ def clear_pii_from_certificate_records_for_user(user):
977978
model's custom `save()` function, nor fire any Django signals (which is desired at the time of writing). There is
978979
nothing to update in our external systems by this update.
979980
981+
If the ``REDACT_CERTIFICATES_HISTORICAL_PII`` toggle is enabled, the history audit table will also be redacted.
982+
980983
Args:
981984
user (User): The User instance of the learner actively being retired.
982985
983986
Returns:
984987
None
985988
"""
986989
GeneratedCertificate.objects.filter(user=user).update(name="")
990+
if REDACT_CERTIFICATES_HISTORICAL_PII.is_enabled():
991+
GeneratedCertificate.history.filter(user=user).update(name="")
987992

988993

989994
def get_cert_history_for_course_id(course_id):

lms/djangoapps/certificates/config.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
This module contains various configuration settings via
33
waffle switches for the Certificates app.
44
"""
5-
from edx_toggles.toggles import WaffleSwitch
5+
from edx_toggles.toggles import SettingToggle, WaffleSwitch
66

77
# Namespace
88
WAFFLE_NAMESPACE = 'certificates'
@@ -14,3 +14,14 @@
1414
# .. toggle_use_cases: open_edx
1515
# .. toggle_creation_date: 2017-09-14
1616
AUTO_CERTIFICATE_GENERATION = WaffleSwitch(f"{WAFFLE_NAMESPACE}.auto_certificate_generation", __name__)
17+
18+
# .. toggle_name: REDACT_CERTIFICATES_HISTORICAL_PII
19+
# .. toggle_implementation: SettingToggle
20+
# .. toggle_default: False
21+
# .. toggle_description: Clears the `name` field in the django-simple-history audit table for
22+
# retiring users' certificate records.
23+
# .. toggle_use_cases: open_edx
24+
# .. toggle_creation_date: 2026-05-29
25+
REDACT_CERTIFICATES_HISTORICAL_PII = SettingToggle(
26+
"REDACT_CERTIFICATES_HISTORICAL_PII", default=False, module_name=__name__
27+
)

lms/djangoapps/certificates/management/commands/purge_pii_from_generatedcertificates.py

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""
22
A management command, designed to be run once by Open edX Operators, to obfuscate learner PII from the
3-
`Certificates_GeneratedCertificate` table that should have been purged during learner retirement.
3+
`certificates_generatedcertificate` and `certificates_historicalgeneratedcertificate` tables that should have been
4+
purged during learner retirement.
45
56
A fix has been included in the retirement pipeline to properly purge this data during learner retirement. This can be
67
used to purge PII from accounts that have already been retired.
@@ -11,6 +12,7 @@
1112
from django.contrib.auth import get_user_model
1213
from django.core.management.base import BaseCommand
1314

15+
from lms.djangoapps.certificates.config import REDACT_CERTIFICATES_HISTORICAL_PII
1416
from lms.djangoapps.certificates.models import GeneratedCertificate
1517
from openedx.core.djangoapps.user_api.api import get_retired_user_ids
1618

@@ -20,11 +22,12 @@
2022

2123
class Command(BaseCommand):
2224
"""
23-
This management command performs a bulk update on `GeneratedCertificate` instances. This means that it will not
24-
invoke the custom save() function defined as part of the `GeneratedCertificate` model, and thus will not emit any
25-
Django signals throughout the system after the update occurs. This is desired behavior. We are using this
26-
management command to purge remnant PII, retired elsewhere in the system, that should have already been removed
27-
from the Certificates tables. We don't need updates to propogate to external systems (like the Credentials IDA).
25+
This management command performs bulk updates on `GeneratedCertificate` instances and their django-simple-history
26+
audit rows in `certificates_historicalgeneratedcertificate`. This means that it will not invoke the custom save()
27+
function defined as part of the `GeneratedCertificate` model, and thus will not emit any Django signals throughout
28+
the system after the update occurs. This is desired behavior. We are using this management command to purge remnant
29+
PII, retired elsewhere in the system, that should have already been removed from the Certificates tables. We don't
30+
need updates to propogate to external systems (like the Credentials IDA).
2831
2932
This management command functions by requesting a list of learners' user_ids whom have completed their journey
3033
through the retirement pipeline. The `get_retired_user_ids` utility function is responsible for filtering out any
@@ -41,8 +44,8 @@ class Command(BaseCommand):
4144
"""
4245

4346
help = """
44-
Purges learners' full names from the `Certificates_GeneratedCertificate` table if their account has been
45-
successfully retired.
47+
Purges learners' full names from the `certificates_generatedcertificate` table and conditionally the
48+
`certificates_historicalgeneratedcertificate` table if their account has been successfully retired.
4649
"""
4750

4851
def add_arguments(self, parser):
@@ -59,6 +62,11 @@ def handle(self, *args, **options):
5962
f"Purging `name` from the certificate records of the following users: {retired_user_ids}"
6063
)
6164
GeneratedCertificate.objects.filter(user_id__in=retired_user_ids).update(name="")
65+
if REDACT_CERTIFICATES_HISTORICAL_PII.is_enabled():
66+
log.info(
67+
f"Purging `name` from the historical certificate records of the following users: {retired_user_ids}"
68+
)
69+
GeneratedCertificate.history.filter(user_id__in=retired_user_ids).update(name="")
6270
else:
6371
log.info(
6472
"DRY RUN: running this management command would purge `name` data from the following users: "

lms/djangoapps/certificates/management/commands/tests/test_purge_pii_from_generatedcertificates.py

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@
22
Tests for the `purge_pii_from_generatedcertificates` management command.
33
"""
44

5-
5+
import ddt
66
from django.core.management import call_command
7+
from django.test import override_settings
78
from testfixtures import LogCapture
89

910
from common.djangoapps.student.tests.factories import UserFactory
@@ -20,6 +21,7 @@
2021
from xmodule.modulestore.tests.factories import CourseFactory
2122

2223

24+
@ddt.ddt
2325
class PurgePiiFromCertificatesTests(ModuleStoreTestCase):
2426
"""
2527
Tests for the `purge_pii_from_generatedcertificates` management command.
@@ -72,7 +74,8 @@ def setUp(self):
7274
)
7375
UserRetirementRequestFactory(user=self.user_retired)
7476

75-
def test_management_command(self):
77+
@ddt.data(True, False)
78+
def test_management_command(self, redact_history_toggle_enabled):
7679
"""
7780
Verify the management command purges expected data from a GeneratedCertificate instance if a learner has
7881
successfully had their account retired.
@@ -82,13 +85,31 @@ def test_management_command(self):
8285
cert_for_retired_user = GeneratedCertificate.objects.get(user_id=self.user_retired)
8386
assert cert_for_retired_user.name == self.user_retired_name
8487

85-
call_command("purge_pii_from_generatedcertificates")
88+
with override_settings(REDACT_CERTIFICATES_HISTORICAL_PII=redact_history_toggle_enabled):
89+
call_command("purge_pii_from_generatedcertificates")
8690

8791
cert_for_active_user = GeneratedCertificate.objects.get(user_id=self.user_active)
8892
assert cert_for_active_user.name == self.user_active_name
8993
cert_for_retired_user = GeneratedCertificate.objects.get(user_id=self.user_retired)
9094
assert cert_for_retired_user.name == ""
9195

96+
active_history_names = list(
97+
GeneratedCertificate.history.filter(user=self.user_active).values_list("name", flat=True)
98+
)
99+
assert len(active_history_names) > 0
100+
assert all(n == self.user_active_name for n in active_history_names)
101+
102+
retired_history_names = list(
103+
GeneratedCertificate.history.filter(user=self.user_retired).values_list("name", flat=True)
104+
)
105+
assert len(retired_history_names) > 0
106+
if redact_history_toggle_enabled:
107+
assert all(n == "" for n in retired_history_names), "Names in the history table should have been redacted."
108+
else:
109+
assert all(n == self.user_retired_name for n in retired_history_names), (
110+
"Names in the history table should not have been redacted."
111+
)
112+
92113
def test_management_command_dry_run(self):
93114
"""
94115
Verify that the management command does not purge any data when invoked with the `--dry-run` flag
@@ -111,4 +132,10 @@ def test_management_command_dry_run(self):
111132
cert_for_retired_user = GeneratedCertificate.objects.get(user_id=self.user_retired)
112133
assert cert_for_retired_user.name == self.user_retired_name
113134

135+
retired_history_names = list(
136+
GeneratedCertificate.history.filter(user=self.user_retired).values_list("name", flat=True)
137+
)
138+
assert len(retired_history_names) > 0
139+
assert all(n == self.user_retired_name for n in retired_history_names)
140+
114141
assert logger.records[0].msg == expected_log_msg

lms/djangoapps/certificates/models.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -170,9 +170,9 @@ class GeneratedCertificate(models.Model): # noqa: DJ008
170170
"""
171171
Base model for generated course certificates
172172
173-
.. pii: PII can exist in the generated certificate linked to in this model. Certificate data is currently retained.
174-
.. pii_types: name, username
175-
.. pii_retirement: retained
173+
.. pii: PII can exist in the generated certificate linked to in this model.
174+
.. pii_types: name
175+
.. pii_retirement: local_api
176176
177177
course_id - Course run key
178178
created_date - Date and time the certificate was created
@@ -248,6 +248,10 @@ class GeneratedCertificate(models.Model): # noqa: DJ008
248248
# imports this model's code. Simple History will attempt to connect to the installed
249249
# model in the certificates app, which will fail.
250250
if 'certificates' in apps.app_configs:
251+
# The PII is retained by default, but can be removed by enabling ``REDACT_CERTIFICATES_HISTORICAL_PII``.
252+
# .. pii: The auto-generated ``HistoricalGeneratedCertificate`` table mirrors all fields of this model.
253+
# .. pii_types: name
254+
# .. pii_retirement: retained
251255
history = HistoricalRecords()
252256

253257
class Meta:

lms/djangoapps/certificates/tests/test_api.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1277,6 +1277,32 @@ def test_clear_pii_from_certificate_records(self):
12771277
assert cert_course1.name == ""
12781278
assert cert_course2.name == ""
12791279

1280+
def test_clear_pii_from_certificate_records_clears_history_table(self):
1281+
"""
1282+
Verify that `clear_pii_from_certificate_records_for_user` blanks `name` in the
1283+
django-simple-history audit table only when the ``REDACT_CERTIFICATES_HISTORICAL_PII``
1284+
setting toggle is enabled, and leaves it untouched when the toggle is disabled.
1285+
"""
1286+
with override_settings(REDACT_CERTIFICATES_HISTORICAL_PII=False):
1287+
clear_pii_from_certificate_records_for_user(self.user)
1288+
1289+
history_names = list(
1290+
GeneratedCertificate.history.filter(user=self.user).values_list("name", flat=True)
1291+
)
1292+
assert all(n == self.user_full_name for n in history_names), (
1293+
"History rows should be untouched when the waffle flag is disabled."
1294+
)
1295+
1296+
with override_settings(REDACT_CERTIFICATES_HISTORICAL_PII=True):
1297+
clear_pii_from_certificate_records_for_user(self.user)
1298+
1299+
history_names_after = list(
1300+
GeneratedCertificate.history.filter(user=self.user).values_list("name", flat=True)
1301+
)
1302+
assert all(n == "" for n in history_names_after), (
1303+
"Expected all history rows to have name blanked after retirement."
1304+
)
1305+
12801306

12811307
class GetCourseIdsForUsernameTests(TestCase):
12821308
"""

0 commit comments

Comments
 (0)