Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
2579a6d
fix: ensure PII is cleared from historical certificate records during…
ttak-apphelix May 28, 2026
034c4c5
Merge branch 'master' into ttak-apphelix/BOMS-590
ttak-apphelix May 28, 2026
3f2037a
feat: add waffle flag to enable historical PII retirement in certific…
ttak-apphelix May 29, 2026
672b2f0
fix: separate import statements for certificate generation configuration
ttak-apphelix May 29, 2026
cdd89a6
Merge branch 'master' into ttak-apphelix/BOMS-590
ttak-apphelix May 29, 2026
a29f936
fix: rename waffle flag for historical PII retirement to reflect reda…
ttak-apphelix May 29, 2026
21572af
fix: improve formatting in clear_pii_from_certificate_records_for_use…
ttak-apphelix May 29, 2026
446f7d8
Merge branch 'master' into ttak-apphelix/BOMS-590
ttak-apphelix Jun 5, 2026
8fc5574
fix: Enhance tests for purge_pii_from_generatedcertificates
ttak-apphelix Jun 5, 2026
194faaa
fix: Refactor dry run test for purge_pii_from_generatedcertificates t…
ttak-apphelix Jun 5, 2026
58de326
fix: Add comment to clarify PII retirement handling in HistoricalGene…
ttak-apphelix Jun 9, 2026
b09f9ad
fix: Simplify test management command by removing redundant waffle fl…
ttak-apphelix Jun 10, 2026
e7e6275
fix: Refactor test for purge_pii_from_generatedcertificates to use dd…
ttak-apphelix Jun 10, 2026
97e9d4f
Merge branch 'master' into ttak-apphelix/BOMS-590
ttak-apphelix Jun 10, 2026
ff40610
fix: Update PII redaction toggle from waffle flag to setting toggle f…
ttak-apphelix Jun 11, 2026
e47f579
fix: removed unused imports
ttak-apphelix Jun 11, 2026
9ce194e
fix: reverting to ddt implmentation
ttak-apphelix Jun 11, 2026
9270d16
fix: removed unused imports
ttak-apphelix Jun 11, 2026
4515df2
fixup! Apply suggestion from @robrap
robrap Jun 11, 2026
863daf7
fixup! Apply suggestion from @robrap
robrap Jun 11, 2026
c083ede
Merge branch 'master' into ttak-apphelix/BOMS-590
robrap Jun 11, 2026
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
5 changes: 5 additions & 0 deletions lms/djangoapps/certificates/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
from common.djangoapps.student.models import CourseEnrollment
from lms.djangoapps.branding import api as branding_api
from lms.djangoapps.certificates.config import AUTO_CERTIFICATE_GENERATION as _AUTO_CERTIFICATE_GENERATION
from lms.djangoapps.certificates.config import REDACT_CERTIFICATES_HISTORICAL_PII
from lms.djangoapps.certificates.data import CertificateStatuses, GeneratedCertificateData
from lms.djangoapps.certificates.generation_handler import generate_certificate_task as _generate_certificate_task
from lms.djangoapps.certificates.generation_handler import is_on_certificate_allowlist as _is_on_certificate_allowlist
Expand Down Expand Up @@ -977,13 +978,17 @@ def clear_pii_from_certificate_records_for_user(user):
model's custom `save()` function, nor fire any Django signals (which is desired at the time of writing). There is
nothing to update in our external systems by this update.

If the ``REDACT_CERTIFICATES_HISTORICAL_PII`` toggle is enabled, the history audit table will also be redacted.

Args:
user (User): The User instance of the learner actively being retired.

Returns:
None
"""
GeneratedCertificate.objects.filter(user=user).update(name="")
if REDACT_CERTIFICATES_HISTORICAL_PII.is_enabled():
GeneratedCertificate.history.filter(user=user).update(name="")


def get_cert_history_for_course_id(course_id):
Expand Down
13 changes: 12 additions & 1 deletion lms/djangoapps/certificates/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
This module contains various configuration settings via
waffle switches for the Certificates app.
"""
from edx_toggles.toggles import WaffleSwitch
from edx_toggles.toggles import SettingToggle, WaffleSwitch

# Namespace
WAFFLE_NAMESPACE = 'certificates'
Expand All @@ -14,3 +14,14 @@
# .. toggle_use_cases: open_edx
# .. toggle_creation_date: 2017-09-14
AUTO_CERTIFICATE_GENERATION = WaffleSwitch(f"{WAFFLE_NAMESPACE}.auto_certificate_generation", __name__)

# .. toggle_name: REDACT_CERTIFICATES_HISTORICAL_PII
# .. toggle_implementation: SettingToggle
# .. toggle_default: False
# .. toggle_description: Clears the `name` field in the django-simple-history audit table for
# retiring users' certificate records.
# .. toggle_use_cases: open_edx
# .. toggle_creation_date: 2026-05-29
REDACT_CERTIFICATES_HISTORICAL_PII = SettingToggle(
"REDACT_CERTIFICATES_HISTORICAL_PII", default=False, module_name=__name__
)
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""
A management command, designed to be run once by Open edX Operators, to obfuscate learner PII from the
`Certificates_GeneratedCertificate` table that should have been purged during learner retirement.
`certificates_generatedcertificate` and `certificates_historicalgeneratedcertificate` tables that should have been
purged during learner retirement.

A fix has been included in the retirement pipeline to properly purge this data during learner retirement. This can be
used to purge PII from accounts that have already been retired.
Expand All @@ -11,6 +12,7 @@
from django.contrib.auth import get_user_model
from django.core.management.base import BaseCommand

from lms.djangoapps.certificates.config import REDACT_CERTIFICATES_HISTORICAL_PII
from lms.djangoapps.certificates.models import GeneratedCertificate
from openedx.core.djangoapps.user_api.api import get_retired_user_ids

Expand All @@ -20,11 +22,12 @@

class Command(BaseCommand):
"""
This management command performs a bulk update on `GeneratedCertificate` instances. This means that it will not
invoke the custom save() function defined as part of the `GeneratedCertificate` model, and thus will not emit any
Django signals throughout the system after the update occurs. This is desired behavior. We are using this
management command to purge remnant PII, retired elsewhere in the system, that should have already been removed
from the Certificates tables. We don't need updates to propogate to external systems (like the Credentials IDA).
This management command performs bulk updates on `GeneratedCertificate` instances and their django-simple-history
audit rows in `certificates_historicalgeneratedcertificate`. This means that it will not invoke the custom save()
function defined as part of the `GeneratedCertificate` model, and thus will not emit any Django signals throughout
the system after the update occurs. This is desired behavior. We are using this management command to purge remnant
PII, retired elsewhere in the system, that should have already been removed from the Certificates tables. We don't
need updates to propogate to external systems (like the Credentials IDA).

This management command functions by requesting a list of learners' user_ids whom have completed their journey
through the retirement pipeline. The `get_retired_user_ids` utility function is responsible for filtering out any
Expand All @@ -41,8 +44,8 @@ class Command(BaseCommand):
"""

help = """
Purges learners' full names from the `Certificates_GeneratedCertificate` table if their account has been
successfully retired.
Purges learners' full names from the `certificates_generatedcertificate` table and conditionally the
`certificates_historicalgeneratedcertificate` table if their account has been successfully retired.
"""

def add_arguments(self, parser):
Expand All @@ -59,6 +62,11 @@ def handle(self, *args, **options):
f"Purging `name` from the certificate records of the following users: {retired_user_ids}"
)
GeneratedCertificate.objects.filter(user_id__in=retired_user_ids).update(name="")
if REDACT_CERTIFICATES_HISTORICAL_PII.is_enabled():
log.info(
f"Purging `name` from the historical certificate records of the following users: {retired_user_ids}"
)
GeneratedCertificate.history.filter(user_id__in=retired_user_ids).update(name="")
Comment thread
robrap marked this conversation as resolved.
else:
log.info(
"DRY RUN: running this management command would purge `name` data from the following users: "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
Tests for the `purge_pii_from_generatedcertificates` management command.
"""


import ddt
from django.core.management import call_command
from django.test import override_settings
from testfixtures import LogCapture

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


@ddt.ddt
class PurgePiiFromCertificatesTests(ModuleStoreTestCase):
"""
Tests for the `purge_pii_from_generatedcertificates` management command.
Expand Down Expand Up @@ -72,7 +74,8 @@ def setUp(self):
)
UserRetirementRequestFactory(user=self.user_retired)

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

call_command("purge_pii_from_generatedcertificates")
with override_settings(REDACT_CERTIFICATES_HISTORICAL_PII=redact_history_toggle_enabled):
call_command("purge_pii_from_generatedcertificates")

cert_for_active_user = GeneratedCertificate.objects.get(user_id=self.user_active)
assert cert_for_active_user.name == self.user_active_name
cert_for_retired_user = GeneratedCertificate.objects.get(user_id=self.user_retired)
assert cert_for_retired_user.name == ""

active_history_names = list(
Comment thread
robrap marked this conversation as resolved.
GeneratedCertificate.history.filter(user=self.user_active).values_list("name", flat=True)
)
assert len(active_history_names) > 0
assert all(n == self.user_active_name for n in active_history_names)

retired_history_names = list(
GeneratedCertificate.history.filter(user=self.user_retired).values_list("name", flat=True)
)
assert len(retired_history_names) > 0
if redact_history_toggle_enabled:
assert all(n == "" for n in retired_history_names), "Names in the history table should have been redacted."
else:
assert all(n == self.user_retired_name for n in retired_history_names), (
"Names in the history table should not have been redacted."
)

def test_management_command_dry_run(self):
"""
Verify that the management command does not purge any data when invoked with the `--dry-run` flag
Expand All @@ -111,4 +132,10 @@ def test_management_command_dry_run(self):
cert_for_retired_user = GeneratedCertificate.objects.get(user_id=self.user_retired)
assert cert_for_retired_user.name == self.user_retired_name

retired_history_names = list(
GeneratedCertificate.history.filter(user=self.user_retired).values_list("name", flat=True)
)
assert len(retired_history_names) > 0
assert all(n == self.user_retired_name for n in retired_history_names)

assert logger.records[0].msg == expected_log_msg
10 changes: 7 additions & 3 deletions lms/djangoapps/certificates/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,9 +170,9 @@ class GeneratedCertificate(models.Model): # noqa: DJ008
"""
Base model for generated course certificates

.. pii: PII can exist in the generated certificate linked to in this model. Certificate data is currently retained.
.. pii_types: name, username
.. pii_retirement: retained
.. pii: PII can exist in the generated certificate linked to in this model.
Comment thread
robrap marked this conversation as resolved.
.. pii_types: name
.. pii_retirement: local_api
Comment thread
robrap marked this conversation as resolved.

course_id - Course run key
created_date - Date and time the certificate was created
Expand Down Expand Up @@ -248,6 +248,10 @@ class GeneratedCertificate(models.Model): # noqa: DJ008
# imports this model's code. Simple History will attempt to connect to the installed
# model in the certificates app, which will fail.
if 'certificates' in apps.app_configs:
# The PII is retained by default, but can be removed by enabling ``REDACT_CERTIFICATES_HISTORICAL_PII``.
# .. pii: The auto-generated ``HistoricalGeneratedCertificate`` table mirrors all fields of this model.
# .. pii_types: name
# .. pii_retirement: retained
history = HistoricalRecords()

class Meta:
Expand Down
26 changes: 26 additions & 0 deletions lms/djangoapps/certificates/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -1277,6 +1277,32 @@ def test_clear_pii_from_certificate_records(self):
assert cert_course1.name == ""
assert cert_course2.name == ""

def test_clear_pii_from_certificate_records_clears_history_table(self):
"""
Verify that `clear_pii_from_certificate_records_for_user` blanks `name` in the
django-simple-history audit table only when the ``REDACT_CERTIFICATES_HISTORICAL_PII``
setting toggle is enabled, and leaves it untouched when the toggle is disabled.
"""
with override_settings(REDACT_CERTIFICATES_HISTORICAL_PII=False):
clear_pii_from_certificate_records_for_user(self.user)

history_names = list(
GeneratedCertificate.history.filter(user=self.user).values_list("name", flat=True)
)
assert all(n == self.user_full_name for n in history_names), (
"History rows should be untouched when the waffle flag is disabled."
)

with override_settings(REDACT_CERTIFICATES_HISTORICAL_PII=True):
clear_pii_from_certificate_records_for_user(self.user)

history_names_after = list(
GeneratedCertificate.history.filter(user=self.user).values_list("name", flat=True)
)
assert all(n == "" for n in history_names_after), (
"Expected all history rows to have name blanked after retirement."
)


class GetCourseIdsForUsernameTests(TestCase):
"""
Expand Down
Loading