From c9d20bf4b49962f38f6f086cc80d5026e8c16832 Mon Sep 17 00:00:00 2001 From: Tarun Tak Date: Wed, 17 Jun 2026 11:16:03 +0000 Subject: [PATCH 01/21] feat: replace user identity strings with user IDs --- cms/djangoapps/course_creators/admin.py | 12 ++++-- common/djangoapps/student/emails.py | 10 ++++- .../student/models/course_enrollment.py | 10 ++++- common/djangoapps/student/models/user.py | 5 ++- common/djangoapps/student/views/management.py | 37 +++++++++++++++---- lms/djangoapps/bulk_user_retirement/views.py | 11 +++++- .../commands/goal_reminder_email.py | 5 ++- lms/djangoapps/courseware/model_data.py | 8 +++- lms/djangoapps/courseware/views/views.py | 5 ++- lms/djangoapps/instructor/views/api.py | 5 ++- .../views/instructor_task_helpers.py | 10 ++++- .../commands/manual_verifications.py | 21 +++++++++-- .../core/djangoapps/user_authn/views/login.py | 5 ++- 13 files changed, 115 insertions(+), 29 deletions(-) diff --git a/cms/djangoapps/course_creators/admin.py b/cms/djangoapps/course_creators/admin.py index 75f29c6ca669..5f9f713f0213 100644 --- a/cms/djangoapps/course_creators/admin.py +++ b/cms/djangoapps/course_creators/admin.py @@ -158,8 +158,11 @@ def send_user_notification_callback(sender, **kwargs): # pylint: disable=unused try: user.email_user(subject, message, studio_request_email) - except: # pylint: disable=bare-except - log.warning("Unable to send course creator status e-mail to %s", user.email) + except: # lint-amnesty, pylint: disable=bare-except + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + log.warning("Unable to send course creator status e-mail to user ID %s", user.id) + else: + log.warning("Unable to send course creator status e-mail to %s", user.email) @receiver(send_admin_notification, sender=CourseCreator) @@ -185,7 +188,10 @@ def send_admin_notification_callback(sender, **kwargs): # pylint: disable=unuse fail_silently=False ) except SMTPException: - log.warning("Failure sending 'pending state' e-mail for %s to %s", user.email, studio_request_email) + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + log.warning("Failure sending 'pending state' e-mail for user ID %s to [REDACTED]", user.id) + else: + log.warning("Failure sending 'pending state' e-mail for %s to %s", user.email, studio_request_email) @receiver(m2m_changed, sender=CourseCreator.organizations.through) diff --git a/common/djangoapps/student/emails.py b/common/djangoapps/student/emails.py index 6b4641722de6..91cb340fee1c 100644 --- a/common/djangoapps/student/emails.py +++ b/common/djangoapps/student/emails.py @@ -27,8 +27,14 @@ def send_proctoring_requirements_email(context): user_context={'full_name': user.profile.name} ) ace.send(msg) - log.info('Proctoring requirements email sent to user: %r', user.username) + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + log.info('Proctoring requirements email sent to user ID: %r', user.id) + else: + log.info('Proctoring requirements email sent to user: %r', user.username) return True except Exception: # pylint: disable=broad-except - log.exception('Could not send email for proctoring requirements to user %s', user.username) + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + log.exception('Could not send email for proctoring requirements to user ID %s', user.id) + else: + log.exception('Could not send email for proctoring requirements to user %s', user.username) return False diff --git a/common/djangoapps/student/models/course_enrollment.py b/common/djangoapps/student/models/course_enrollment.py index cbf257b77f46..4c3f3f76f200 100644 --- a/common/djangoapps/student/models/course_enrollment.py +++ b/common/djangoapps/student/models/course_enrollment.py @@ -738,8 +738,11 @@ def enroll(cls, user, course_key, mode=None, check_access=False, can_upgrade=Fal course_key=course_key, ) if check_access: - log.warning("User %s failed to enroll in non-existent course %s", user.username, str(course_key)) - raise NonExistentCourseError # pylint: disable=raise-missing-from # noqa: B904 + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + log.warning("User %s failed to enroll in non-existent course %s", user.id, str(course_key)) + else: + log.warning("User %s failed to enroll in non-existent course %s", user.username, str(course_key)) + raise NonExistentCourseError # lint-amnesty, pylint: disable=raise-missing-from if check_access: if cls.is_enrollment_closed(user, course) and not can_upgrade: @@ -829,6 +832,9 @@ def enroll_by_email(cls, email, course_id, mode=None, ignore_errors=True): return cls.enroll(user, course_id, mode) except User.DoesNotExist: err_msg = "Tried to enroll email {} into course {}, but user not found" + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + log.error(err_msg.format('[PII_REDACTED]', course_id)) + else: log.error(err_msg.format(email, course_id)) if ignore_errors: return None diff --git a/common/djangoapps/student/models/user.py b/common/djangoapps/student/models/user.py index 4789f0e47c38..c5c4edbd7422 100644 --- a/common/djangoapps/student/models/user.py +++ b/common/djangoapps/student/models/user.py @@ -887,7 +887,10 @@ def activate(self): # pylint: disable=missing-function-docstring self.activation_timestamp = datetime.utcnow() self.save() USER_ACCOUNT_ACTIVATED.send_robust(self.__class__, user=self.user) - log.info('User %s (%s) account is successfully activated.', self.user.username, self.user.email) + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + log.info('User %s account is successfully activated.', self.user.id) + else: + log.info('User %s (%s) account is successfully activated.', self.user.username, self.user.email) class PendingNameChange(DeletableByUserValue, models.Model): # noqa: DJ008 diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index 5609afd49ddc..3638fc15a9f1 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -489,7 +489,10 @@ def change_enrollment(request, check_access=True): except UnenrollmentNotAllowed as exc: return HttpResponseBadRequest(str(exc)) - log.info("User %s unenrolled from %s; sending REFUND_ORDER", user.username, course_id) + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + log.info("User %s unenrolled from %s; sending REFUND_ORDER", user.id, course_id) + else: + log.info("User %s unenrolled from %s; sending REFUND_ORDER", user.username, course_id) REFUND_ORDER.send(sender=None, course_enrollment=enrollment) return HttpResponse() else: @@ -557,11 +560,17 @@ def disable_account_ajax(request): if account_action == 'disable': user_account.account_status = UserStanding.ACCOUNT_DISABLED context['message'] = _("Successfully disabled {}'s account").format(username) - log.info("%s disabled %s's account", request.user, username) + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + log.info("%s disabled user %s's account", request.user.id, '[PII_REDACTED]') + else: + log.info("%s disabled %s's account", request.user, username) elif account_action == 'reenable': user_account.account_status = UserStanding.ACCOUNT_ENABLED context['message'] = _("Successfully reenabled {}'s account").format(username) - log.info("%s reenabled %s's account", request.user, username) + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + log.info("%s reenabled user %s's account", request.user.id, '[PII_REDACTED]') + else: + log.info("%s reenabled %s's account", request.user, username) else: context['message'] = _("Unexpected account status") return JsonResponse(context, status=400) @@ -847,11 +856,17 @@ def do_email_change_request(user, new_email, activation_key=None, secondary_emai try: ace.send(msg) - log.info("Email activation link sent to user [%s].", new_email) + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + log.info("Email activation link sent for user ID: [%s].", user.id) + else: + log.info("Email activation link sent to user [%s].", new_email) except Exception: from_address = configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL) - log.error('Unable to send email activation link to user from "%s"', from_address, exc_info=True) - raise ValueError(_('Unable to send email activation link. Please try again later.')) # pylint: disable=raise-missing-from # noqa: B904 + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + log.error('Unable to send email activation link from a redacted address', exc_info=True) + else: + log.error('Unable to send email activation link to user from "%s"', from_address, exc_info=True) + raise ValueError(_('Unable to send email activation link. Please try again later.')) # lint-amnesty, pylint: disable=raise-missing-from if not secondary_email_change_request: # When the email address change is complete, a "edx.user.settings.changed" event will be emitted. @@ -960,7 +975,10 @@ def confirm_email_change(request, key): try: ace.send(msg) except Exception: # pylint: disable=broad-except - log.warning('Unable to send confirmation email to old address', exc_info=True) + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + log.warning('Unable to send confirmation email to old address [REDACTED]', exc_info=True) + else: + log.warning('Unable to send confirmation email to old address', exc_info=True) response = render_to_response("email_change_failed.html", {'email': user.email}) transaction.set_rollback(True) return response @@ -976,7 +994,10 @@ def confirm_email_change(request, key): try: ace.send(msg) except Exception: # pylint: disable=broad-except - log.warning('Unable to send confirmation email to new address', exc_info=True) + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + log.warning('Unable to send confirmation email to new address [REDACTED]', exc_info=True) + else: + log.warning('Unable to send confirmation email to new address', exc_info=True) response = render_to_response("email_change_failed.html", {'email': user.email}) transaction.set_rollback(True) return response diff --git a/lms/djangoapps/bulk_user_retirement/views.py b/lms/djangoapps/bulk_user_retirement/views.py index 28f23651fb65..32b2a4c60d4b 100644 --- a/lms/djangoapps/bulk_user_retirement/views.py +++ b/lms/djangoapps/bulk_user_retirement/views.py @@ -5,6 +5,7 @@ from django.contrib.auth import get_user_model from django.db import transaction +from django.conf import settings from rest_framework import permissions, status from rest_framework.response import Response from rest_framework.views import APIView @@ -56,11 +57,17 @@ def post(self, request, **kwargs): # pylint: disable=unused-argument user_to_retire = User.objects.get(username=username) with transaction.atomic(): create_retirement_request_and_deactivate_account(user_to_retire) - log.info(f'The user "{username}" has been added to the retirement pipeline \ + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + log.info('A user has been added to the retirement pipeline') + else: + log.info(f'The user "{username}" has been added to the retirement pipeline \') by "{request.user}"') except User.DoesNotExist: - log.exception(f'The user "{username}" does not exist.') + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + log.exception('A user does not exist for bulk retirement.') + else: + log.exception(f'The user "{username}" does not exist.') failed_user_retirements.append(username) except Exception as exc: # pylint: disable=broad-except diff --git a/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py b/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py index b9aac5e8efaa..e89f419d46a4 100644 --- a/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py +++ b/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py @@ -50,7 +50,10 @@ def send_ace_message(goal, session_id): """ user = goal.user if not user.has_usable_password(): - log.info(f'Goal Reminder User is disabled {user.username} course {goal.course_key}') + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + log.info(f'Goal Reminder User is disabled user ID {user.id} course {goal.course_key}') + else: + log.info(f'Goal Reminder User is disabled {user.username} course {goal.course_key}') return False try: course = CourseOverview.get_from_id(goal.course_key) diff --git a/lms/djangoapps/courseware/model_data.py b/lms/djangoapps/courseware/model_data.py index ed5cde367ae1..e881cbb453a6 100644 --- a/lms/djangoapps/courseware/model_data.py +++ b/lms/djangoapps/courseware/model_data.py @@ -28,6 +28,7 @@ from collections import defaultdict, namedtuple from django.db import DatabaseError, IntegrityError, transaction +from django.conf import settings from opaque_keys.edx.asides import AsideUsageKeyV1, AsideUsageKeyV2 from opaque_keys.edx.block_types import BlockTypeKeyV1 from opaque_keys.edx.keys import LearningContextKey @@ -405,8 +406,11 @@ def set_many(self, kv_dict): pending_updates ) except DatabaseError: - log.exception("Saving user state failed for %s", self.user.username) - raise KeyValueMultiSaveError([]) # pylint: disable=raise-missing-from # noqa: B904 + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + log.exception("Saving user state failed for user ID %s", self.user.id) + else: + log.exception("Saving user state failed for %s", self.user.username) + raise KeyValueMultiSaveError([]) # lint-amnesty, pylint: disable=raise-missing-from finally: self._cache.update(pending_updates) diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 960be2fd7849..39b08cb3e53a 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -1479,7 +1479,10 @@ def generate_user_cert(request, course_id): return HttpResponseBadRequest(str(e)) if not is_course_passed(student, course): - log.info("User %s has not passed the course: %s", student.username, course_id) + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + log.info("User ID %s has not passed the course: %s", student.id, course_id) + else: + log.info("User %s has not passed the course: %s", student.username, course_id) return HttpResponseBadRequest(_("Your certificate will be available when you pass the course.")) certificate_status = certs_api.certificate_downloadable_status(student, course.id) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 9b629fd807c7..44dadf23ddec 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -462,7 +462,10 @@ def post(self, request, course_id): # pylint: disable=too-many-statements warnings.append({ 'username': username, 'email': email, 'response': warning_message }) - log.warning('email %s already exist', email) + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + log.warning('email [REDACTED] already exist') + else: + log.warning('email %s already exist', email) else: log.info( "user already exists with username '%s' and email '%s'", diff --git a/lms/djangoapps/instructor/views/instructor_task_helpers.py b/lms/djangoapps/instructor/views/instructor_task_helpers.py index e50687da3a77..904c0784f58e 100644 --- a/lms/djangoapps/instructor/views/instructor_task_helpers.py +++ b/lms/djangoapps/instructor/views/instructor_task_helpers.py @@ -10,6 +10,8 @@ from django.utils.translation import gettext as _ from django.utils.translation import ngettext +from django.conf import settings + from lms.djangoapps.bulk_email.models import CourseEmail from lms.djangoapps.instructor_task.models import InstructorTaskSchedule from lms.djangoapps.instructor_task.views import get_task_completion_info @@ -55,7 +57,10 @@ def extract_email_features(email_task): try: task_input_information = json.loads(email_task.task_input) except ValueError: - log.error("Could not parse task input as valid json; task input: %s", email_task.task_input) + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + log.error("Could not parse task input as valid json; task input: [REDACTED]") + else: + log.error("Could not parse task input as valid json; task input: %s", email_task.task_input) return email_error_information() email = CourseEmail.objects.get(id=task_input_information['email_id']) @@ -82,6 +87,9 @@ def extract_email_features(email_task): try: task_output = json.loads(email_task.task_output) except ValueError: + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + log.error("Could not parse task output as valid json; task output: [REDACTED]") + else: log.error("Could not parse task output as valid json; task output: %s", email_task.task_output) else: if 'succeeded' in task_output and task_output['succeeded'] > 0: diff --git a/lms/djangoapps/verify_student/management/commands/manual_verifications.py b/lms/djangoapps/verify_student/management/commands/manual_verifications.py index bb84fabb2713..bdaf82f0b4d9 100644 --- a/lms/djangoapps/verify_student/management/commands/manual_verifications.py +++ b/lms/djangoapps/verify_student/management/commands/manual_verifications.py @@ -9,6 +9,7 @@ from django.contrib.auth.models import User # pylint: disable=imported-auth-user from django.core.management.base import BaseCommand, CommandError +from django.conf import settings from lms.djangoapps.verify_student.models import ManualVerification from lms.djangoapps.verify_student.utils import earliest_allowed_verification_date @@ -53,7 +54,10 @@ def handle(self, *args, **options): if single_email: successfully_verified = self._add_user_to_manual_verification(single_email) if successfully_verified is False: - log.error(f'Manual verification of {single_email} failed') + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + log.error('Manual verification of [REDACTED_EMAIL] failed') + else: + log.error(f'Manual verification of {single_email} failed') return email_ids_file = options['email_ids_file'] @@ -70,7 +74,10 @@ def handle(self, *args, **options): len(failed_emails), total_emails )) - log.error(f'Failed emails:{pformat(failed_emails)}') + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + log.error('Failed emails: [REDACTED]') + else: + log.error(f'Failed emails:{pformat(failed_emails)}') else: log.info(f'Successfully generated manual verification for {total_emails} emails.') @@ -122,7 +129,10 @@ def _add_users_to_manual_verification(self, email_ids): status='approved', )) else: - log.info(f'Skipping email {user.email}, existing verification found.') + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + log.info(f'Skipping user ID {user.id}, existing verification found.') + else: + log.info(f'Skipping email {user.email}, existing verification found.') ManualVerification.objects.bulk_create(verifications_to_create) failed_emails = set(email_ids) - set(users.values_list('email', flat=True)) return list(failed_emails) @@ -147,5 +157,8 @@ def _add_user_to_manual_verification(self, email_id): ) return True except User.DoesNotExist: - log.error(f'Tried to verify email {email_id}, but user not found') + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + log.error('Tried to verify email [REDACTED_EMAIL], but user not found') + else: + log.error(f'Tried to verify email {email_id}, but user not found') return False diff --git a/openedx/core/djangoapps/user_authn/views/login.py b/openedx/core/djangoapps/user_authn/views/login.py index 6a012d8d212d..9f06322e5af0 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/openedx/core/djangoapps/user_authn/views/login.py @@ -199,7 +199,10 @@ def _enforce_password_policy_compliance(request, user): # pylint: disable=missi if LoginFailures.is_feature_enabled(): LoginFailures.increment_lockout_counter(user) - AUDIT_LOG.info("Password reset initiated for email %s.", user.email) + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + AUDIT_LOG.info("Password reset initiated for user ID %s.", user.id) + else: + AUDIT_LOG.info("Password reset initiated for email %s.", user.email) tracker.emit( PASSWORD_RESET_INITIATED, { From a06f530e363ef026dba36118cac13a4fcb138f45 Mon Sep 17 00:00:00 2001 From: Tarun Tak Date: Thu, 18 Jun 2026 07:19:50 +0000 Subject: [PATCH 02/21] fix: corrected lint error --- common/djangoapps/student/models/course_enrollment.py | 6 +++--- .../management/commands/manual_verifications.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/common/djangoapps/student/models/course_enrollment.py b/common/djangoapps/student/models/course_enrollment.py index 4c3f3f76f200..82c887167a7d 100644 --- a/common/djangoapps/student/models/course_enrollment.py +++ b/common/djangoapps/student/models/course_enrollment.py @@ -833,9 +833,9 @@ def enroll_by_email(cls, email, course_id, mode=None, ignore_errors=True): except User.DoesNotExist: err_msg = "Tried to enroll email {} into course {}, but user not found" if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.error(err_msg.format('[PII_REDACTED]', course_id)) - else: - log.error(err_msg.format(email, course_id)) + log.error(err_msg.format('[PII_REDACTED]', course_id)) + else: + log.error(err_msg.format(email, course_id)) if ignore_errors: return None raise diff --git a/lms/djangoapps/verify_student/management/commands/manual_verifications.py b/lms/djangoapps/verify_student/management/commands/manual_verifications.py index bdaf82f0b4d9..73f3f90bb515 100644 --- a/lms/djangoapps/verify_student/management/commands/manual_verifications.py +++ b/lms/djangoapps/verify_student/management/commands/manual_verifications.py @@ -7,9 +7,9 @@ import os from pprint import pformat +from django.conf import settings from django.contrib.auth.models import User # pylint: disable=imported-auth-user from django.core.management.base import BaseCommand, CommandError -from django.conf import settings from lms.djangoapps.verify_student.models import ManualVerification from lms.djangoapps.verify_student.utils import earliest_allowed_verification_date From 35f8781393b39ea4ce48697ce708a34132a3f2b0 Mon Sep 17 00:00:00 2001 From: Tarun Tak Date: Fri, 19 Jun 2026 06:13:59 +0000 Subject: [PATCH 03/21] fix: resolved pylint issue --- lms/djangoapps/courseware/model_data.py | 2 +- .../instructor/views/instructor_task_helpers.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lms/djangoapps/courseware/model_data.py b/lms/djangoapps/courseware/model_data.py index e881cbb453a6..dcd4c16bc6ef 100644 --- a/lms/djangoapps/courseware/model_data.py +++ b/lms/djangoapps/courseware/model_data.py @@ -27,8 +27,8 @@ from abc import ABCMeta, abstractmethod from collections import defaultdict, namedtuple -from django.db import DatabaseError, IntegrityError, transaction from django.conf import settings +from django.db import DatabaseError, IntegrityError, transaction from opaque_keys.edx.asides import AsideUsageKeyV1, AsideUsageKeyV2 from opaque_keys.edx.block_types import BlockTypeKeyV1 from opaque_keys.edx.keys import LearningContextKey diff --git a/lms/djangoapps/instructor/views/instructor_task_helpers.py b/lms/djangoapps/instructor/views/instructor_task_helpers.py index 904c0784f58e..e45edfa5d5a3 100644 --- a/lms/djangoapps/instructor/views/instructor_task_helpers.py +++ b/lms/djangoapps/instructor/views/instructor_task_helpers.py @@ -87,10 +87,10 @@ def extract_email_features(email_task): try: task_output = json.loads(email_task.task_output) except ValueError: - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.error("Could not parse task output as valid json; task output: [REDACTED]") - else: - log.error("Could not parse task output as valid json; task output: %s", email_task.task_output) + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + log.error("Could not parse task output as valid json; task output: [REDACTED]") + else: + log.error("Could not parse task output as valid json; task output: %s", email_task.task_output) else: if 'succeeded' in task_output and task_output['succeeded'] > 0: num_emails = task_output['succeeded'] From b426fd341ff506c89cf58b1b0c8533274f6335b2 Mon Sep 17 00:00:00 2001 From: Tarun Tak Date: Fri, 19 Jun 2026 06:29:12 +0000 Subject: [PATCH 04/21] fix: lint issue fixed --- lms/djangoapps/bulk_user_retirement/views.py | 6 ++++-- lms/djangoapps/instructor/views/instructor_task_helpers.py | 3 +-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/bulk_user_retirement/views.py b/lms/djangoapps/bulk_user_retirement/views.py index 32b2a4c60d4b..ec1ddc4f38f1 100644 --- a/lms/djangoapps/bulk_user_retirement/views.py +++ b/lms/djangoapps/bulk_user_retirement/views.py @@ -60,8 +60,10 @@ def post(self, request, **kwargs): # pylint: disable=unused-argument if settings.FEATURES['SQUELCH_PII_IN_LOGS']: log.info('A user has been added to the retirement pipeline') else: - log.info(f'The user "{username}" has been added to the retirement pipeline \') - by "{request.user}"') + log.info('The user "%s" has been added to the retirement pipeline by "%s"', + username, + request.user, + ) except User.DoesNotExist: if settings.FEATURES['SQUELCH_PII_IN_LOGS']: diff --git a/lms/djangoapps/instructor/views/instructor_task_helpers.py b/lms/djangoapps/instructor/views/instructor_task_helpers.py index e45edfa5d5a3..f9d17e37e95d 100644 --- a/lms/djangoapps/instructor/views/instructor_task_helpers.py +++ b/lms/djangoapps/instructor/views/instructor_task_helpers.py @@ -7,11 +7,10 @@ import json import logging +from django.conf import settings from django.utils.translation import gettext as _ from django.utils.translation import ngettext -from django.conf import settings - from lms.djangoapps.bulk_email.models import CourseEmail from lms.djangoapps.instructor_task.models import InstructorTaskSchedule from lms.djangoapps.instructor_task.views import get_task_completion_info From b08bbd7d49dc8562de0c56d30516e29d8820397d Mon Sep 17 00:00:00 2001 From: Tarun Tak Date: Fri, 19 Jun 2026 06:44:51 +0000 Subject: [PATCH 05/21] fix: lint issue fixed --- common/djangoapps/student/models/course_enrollment.py | 4 ++-- common/djangoapps/student/views/management.py | 4 ++-- lms/djangoapps/bulk_user_retirement/views.py | 2 +- lms/djangoapps/courseware/model_data.py | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/common/djangoapps/student/models/course_enrollment.py b/common/djangoapps/student/models/course_enrollment.py index 82c887167a7d..f422f627b844 100644 --- a/common/djangoapps/student/models/course_enrollment.py +++ b/common/djangoapps/student/models/course_enrollment.py @@ -731,7 +731,7 @@ def enroll(cls, user, course_key, mode=None, check_access=False, can_upgrade=Fal course_key=course.id, display_name=course.display_name, ) - except CourseOverview.DoesNotExist: + except CourseOverview.DoesNotExist as err: # This is here to preserve legacy behavior which allowed enrollment in courses # announced before the start of content creation. course_data = CourseData( @@ -742,7 +742,7 @@ def enroll(cls, user, course_key, mode=None, check_access=False, can_upgrade=Fal log.warning("User %s failed to enroll in non-existent course %s", user.id, str(course_key)) else: log.warning("User %s failed to enroll in non-existent course %s", user.username, str(course_key)) - raise NonExistentCourseError # lint-amnesty, pylint: disable=raise-missing-from + raise NonExistentCourseError from err # lint-amnesty, pylint: disable=raise-missing-from if check_access: if cls.is_enrollment_closed(user, course) and not can_upgrade: diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index 3638fc15a9f1..aff76e116c81 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -860,13 +860,13 @@ def do_email_change_request(user, new_email, activation_key=None, secondary_emai log.info("Email activation link sent for user ID: [%s].", user.id) else: log.info("Email activation link sent to user [%s].", new_email) - except Exception: + except Exception as err: from_address = configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL) if settings.FEATURES['SQUELCH_PII_IN_LOGS']: log.error('Unable to send email activation link from a redacted address', exc_info=True) else: log.error('Unable to send email activation link to user from "%s"', from_address, exc_info=True) - raise ValueError(_('Unable to send email activation link. Please try again later.')) # lint-amnesty, pylint: disable=raise-missing-from + raise ValueError(_('Unable to send email activation link. Please try again later.')) from err # lint-amnesty, pylint: disable=raise-missing-from if not secondary_email_change_request: # When the email address change is complete, a "edx.user.settings.changed" event will be emitted. diff --git a/lms/djangoapps/bulk_user_retirement/views.py b/lms/djangoapps/bulk_user_retirement/views.py index ec1ddc4f38f1..69219226d067 100644 --- a/lms/djangoapps/bulk_user_retirement/views.py +++ b/lms/djangoapps/bulk_user_retirement/views.py @@ -3,9 +3,9 @@ """ import logging +from django.conf import settings from django.contrib.auth import get_user_model from django.db import transaction -from django.conf import settings from rest_framework import permissions, status from rest_framework.response import Response from rest_framework.views import APIView diff --git a/lms/djangoapps/courseware/model_data.py b/lms/djangoapps/courseware/model_data.py index dcd4c16bc6ef..53447dde1b92 100644 --- a/lms/djangoapps/courseware/model_data.py +++ b/lms/djangoapps/courseware/model_data.py @@ -405,12 +405,12 @@ def set_many(self, kv_dict): self.user.username, pending_updates ) - except DatabaseError: + except DatabaseError as err: if settings.FEATURES['SQUELCH_PII_IN_LOGS']: log.exception("Saving user state failed for user ID %s", self.user.id) else: log.exception("Saving user state failed for %s", self.user.username) - raise KeyValueMultiSaveError([]) # lint-amnesty, pylint: disable=raise-missing-from + raise KeyValueMultiSaveError([]) from err # lint-amnesty, pylint: disable=raise-missing-from finally: self._cache.update(pending_updates) From 5e2a0f7d5b57a361ffb1822aacf2277fa633cdd6 Mon Sep 17 00:00:00 2001 From: Tarun Tak Date: Fri, 19 Jun 2026 07:12:43 +0000 Subject: [PATCH 06/21] fix: lint issue fixed --- common/djangoapps/student/views/management.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index aff76e116c81..c6441738496d 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -915,7 +915,7 @@ def activate_secondary_email(request, key): @ensure_csrf_cookie -def confirm_email_change(request, key): +def confirm_email_change(request, key): # pylint: disable=too-many-statements """ User requested a new e-mail. This is called when the activation link is clicked. We confirm with the old e-mail, and update From 780ae84b9db5c84492f3943ecd3ebddbeaa1ff5e Mon Sep 17 00:00:00 2001 From: Tarun Tak Date: Wed, 24 Jun 2026 04:43:08 +0000 Subject: [PATCH 07/21] fix: updated the log message with user id --- cms/djangoapps/course_creators/admin.py | 3 +- .../student/models/course_enrollment.py | 77 +++++++++++++------ common/djangoapps/student/views/management.py | 10 +-- lms/djangoapps/bulk_user_retirement/views.py | 5 +- lms/djangoapps/instructor/views/api.py | 15 ++-- .../views/instructor_task_helpers.py | 4 +- .../commands/manual_verifications.py | 6 +- 7 files changed, 79 insertions(+), 41 deletions(-) diff --git a/cms/djangoapps/course_creators/admin.py b/cms/djangoapps/course_creators/admin.py index 5f9f713f0213..adc621b24e36 100644 --- a/cms/djangoapps/course_creators/admin.py +++ b/cms/djangoapps/course_creators/admin.py @@ -172,6 +172,7 @@ def send_admin_notification_callback(sender, **kwargs): # pylint: disable=unuse """ user = kwargs['user'] + # studio_request_email is a system email address, not PII, which can safely be logged. studio_request_email = settings.FEATURES.get('STUDIO_REQUEST_EMAIL', '') context = {'user_name': user.username, 'user_email': user.email} @@ -189,7 +190,7 @@ def send_admin_notification_callback(sender, **kwargs): # pylint: disable=unuse ) except SMTPException: if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.warning("Failure sending 'pending state' e-mail for user ID %s to [REDACTED]", user.id) + log.warning("Failure sending 'pending state' e-mail for user ID %s to %s", user.id, studio_request_email) else: log.warning("Failure sending 'pending state' e-mail for %s to %s", user.email, studio_request_email) diff --git a/common/djangoapps/student/models/course_enrollment.py b/common/djangoapps/student/models/course_enrollment.py index f422f627b844..d2a5aa3f7aad 100644 --- a/common/djangoapps/student/models/course_enrollment.py +++ b/common/djangoapps/student/models/course_enrollment.py @@ -664,12 +664,20 @@ def emit_event(self, event_name, enterprise_uuid=None): except Exception: # pylint: disable=broad-except if event_name and self.course_id: - log.exception( - 'Unable to emit event %s for user %s and course %s', - event_name, - self.user.username, - self.course_id, - ) + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + log.exception( + 'Unable to emit event %s for user %s and course %s', + event_name, + self.user.id, + self.course_id, + ) + else: + log.exception( + 'Unable to emit event %s for user %s and course %s', + event_name, + self.user.username, + self.course_id, + ) @classmethod def enroll(cls, user, course_key, mode=None, check_access=False, can_upgrade=False, enterprise_uuid=None): @@ -746,28 +754,51 @@ def enroll(cls, user, course_key, mode=None, check_access=False, can_upgrade=Fal if check_access: if cls.is_enrollment_closed(user, course) and not can_upgrade: - log.warning( - "User %s failed to enroll in course %s because enrollment is closed (can_upgrade=%s).", - user.username, - str(course_key), - can_upgrade, - ) + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + log.warning( + "User %s failed to enroll in course %s because enrollment is closed (can_upgrade=%s).", + user.id, + str(course_key), + can_upgrade, + ) + else: + log.warning( + "User %s failed to enroll in course %s because enrollment is closed (can_upgrade=%s).", + user.username, + str(course_key), + can_upgrade, + ) raise EnrollmentClosedError if cls.objects.is_course_full(course): + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + log.warning( + "Course %s has reached its maximum enrollment of %d learners. User %s failed to enroll.", + str(course_key), + course.max_student_enrollments_allowed, + user.id, + ) + else: + log.warning( + "Course %s has reached its maximum enrollment of %d learners. User %s failed to enroll.", + str(course_key), + course.max_student_enrollments_allowed, + user.username, + ) + raise CourseFullError + if cls.is_enrolled(user, course_key): + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: log.warning( - "Course %s has reached its maximum enrollment of %d learners. User %s failed to enroll.", - str(course_key), - course.max_student_enrollments_allowed, + "User %s attempted to enroll in %s, but they were already enrolled", + user.id, + str(course_key) + ) + else: + log.warning( + "User %s attempted to enroll in %s, but they were already enrolled", user.username, + str(course_key) ) - raise CourseFullError - if cls.is_enrolled(user, course_key): - log.warning( - "User %s attempted to enroll in %s, but they were already enrolled", - user.username, - str(course_key) - ) if check_access: raise AlreadyEnrolledError @@ -833,7 +864,7 @@ def enroll_by_email(cls, email, course_id, mode=None, ignore_errors=True): except User.DoesNotExist: err_msg = "Tried to enroll email {} into course {}, but user not found" if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.error(err_msg.format('[PII_REDACTED]', course_id)) + log.error("Tried to enroll a redacted email into course %s, but user not found", course_id) else: log.error(err_msg.format(email, course_id)) if ignore_errors: diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index c6441738496d..3e35065ffdcd 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -561,14 +561,14 @@ def disable_account_ajax(request): user_account.account_status = UserStanding.ACCOUNT_DISABLED context['message'] = _("Successfully disabled {}'s account").format(username) if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.info("%s disabled user %s's account", request.user.id, '[PII_REDACTED]') + log.info("User %s disabled user %s's account", request.user.id, user.id) else: log.info("%s disabled %s's account", request.user, username) elif account_action == 'reenable': user_account.account_status = UserStanding.ACCOUNT_ENABLED context['message'] = _("Successfully reenabled {}'s account").format(username) if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.info("%s reenabled user %s's account", request.user.id, '[PII_REDACTED]') + log.info("User %s reenabled user %s's account", request.user.id, user.id) else: log.info("%s reenabled %s's account", request.user, username) else: @@ -863,7 +863,7 @@ def do_email_change_request(user, new_email, activation_key=None, secondary_emai except Exception as err: from_address = configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL) if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.error('Unable to send email activation link from a redacted address', exc_info=True) + log.error('Unable to send email activation link for user %s from a redacted address', user.id, exc_info=True) else: log.error('Unable to send email activation link to user from "%s"', from_address, exc_info=True) raise ValueError(_('Unable to send email activation link. Please try again later.')) from err # lint-amnesty, pylint: disable=raise-missing-from @@ -976,7 +976,7 @@ def confirm_email_change(request, key): # pylint: disable=too-many-statements ace.send(msg) except Exception: # pylint: disable=broad-except if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.warning('Unable to send confirmation email to old address [REDACTED]', exc_info=True) + log.warning('Unable to send confirmation email to old address for user %s', user.id, exc_info=True) else: log.warning('Unable to send confirmation email to old address', exc_info=True) response = render_to_response("email_change_failed.html", {'email': user.email}) @@ -995,7 +995,7 @@ def confirm_email_change(request, key): # pylint: disable=too-many-statements ace.send(msg) except Exception: # pylint: disable=broad-except if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.warning('Unable to send confirmation email to new address [REDACTED]', exc_info=True) + log.warning('Unable to send confirmation email to new address for user %s', user.id, exc_info=True) else: log.warning('Unable to send confirmation email to new address', exc_info=True) response = render_to_response("email_change_failed.html", {'email': user.email}) diff --git a/lms/djangoapps/bulk_user_retirement/views.py b/lms/djangoapps/bulk_user_retirement/views.py index 69219226d067..dcb655c5dddc 100644 --- a/lms/djangoapps/bulk_user_retirement/views.py +++ b/lms/djangoapps/bulk_user_retirement/views.py @@ -58,7 +58,10 @@ def post(self, request, **kwargs): # pylint: disable=unused-argument with transaction.atomic(): create_retirement_request_and_deactivate_account(user_to_retire) if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.info('A user has been added to the retirement pipeline') + log.info('User %s added to retirement pipeline by user %s', + user_to_retire.id, + request.user.id + ) else: log.info('The user "%s" has been added to the retirement pipeline by "%s"', username, diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 44dadf23ddec..abdbdf73257c 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -463,15 +463,18 @@ def post(self, request, course_id): # pylint: disable=too-many-statements 'username': username, 'email': email, 'response': warning_message }) if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.warning('email [REDACTED] already exist') + log.warning('email for user %s already exist', user.id) else: log.warning('email %s already exist', email) else: - log.info( - "user already exists with username '%s' and email '%s'", - username, - email - ) + if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + log.info('user already exists with user ID %s', user.id) + else: + log.info( + "user already exists with username '%s' and email '%s'", + username, + email + ) # enroll a user if it is not already enrolled. if not is_user_enrolled_in_course(user, course_id): diff --git a/lms/djangoapps/instructor/views/instructor_task_helpers.py b/lms/djangoapps/instructor/views/instructor_task_helpers.py index f9d17e37e95d..9aedc2ef713e 100644 --- a/lms/djangoapps/instructor/views/instructor_task_helpers.py +++ b/lms/djangoapps/instructor/views/instructor_task_helpers.py @@ -57,7 +57,7 @@ def extract_email_features(email_task): task_input_information = json.loads(email_task.task_input) except ValueError: if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.error("Could not parse task input as valid json; task input: [REDACTED]") + log.error("Could not parse task input as valid json; task input is redacted") else: log.error("Could not parse task input as valid json; task input: %s", email_task.task_input) return email_error_information() @@ -87,7 +87,7 @@ def extract_email_features(email_task): task_output = json.loads(email_task.task_output) except ValueError: if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.error("Could not parse task output as valid json; task output: [REDACTED]") + log.error("Could not parse task output as valid json; task output is redacted") else: log.error("Could not parse task output as valid json; task output: %s", email_task.task_output) else: diff --git a/lms/djangoapps/verify_student/management/commands/manual_verifications.py b/lms/djangoapps/verify_student/management/commands/manual_verifications.py index 73f3f90bb515..8c0d77ace462 100644 --- a/lms/djangoapps/verify_student/management/commands/manual_verifications.py +++ b/lms/djangoapps/verify_student/management/commands/manual_verifications.py @@ -55,7 +55,7 @@ def handle(self, *args, **options): successfully_verified = self._add_user_to_manual_verification(single_email) if successfully_verified is False: if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.error('Manual verification of [REDACTED_EMAIL] failed') + log.error('Manual verification of a redacted email failed') else: log.error(f'Manual verification of {single_email} failed') return @@ -75,7 +75,7 @@ def handle(self, *args, **options): total_emails )) if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.error('Failed emails: [REDACTED]') + log.error('Failed emails are redacted') else: log.error(f'Failed emails:{pformat(failed_emails)}') else: @@ -158,7 +158,7 @@ def _add_user_to_manual_verification(self, email_id): return True except User.DoesNotExist: if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.error('Tried to verify email [REDACTED_EMAIL], but user not found') + log.error('Tried to verify a redacted email, but user not found') else: log.error(f'Tried to verify email {email_id}, but user not found') return False From 546747c352401b9ae6e8688d52c93f2035abae3c Mon Sep 17 00:00:00 2001 From: Tarun Tak Date: Wed, 24 Jun 2026 06:10:11 +0000 Subject: [PATCH 08/21] fix: linter issue resolved --- common/djangoapps/student/views/management.py | 6 +++++- lms/djangoapps/bulk_user_retirement/views.py | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index 3e35065ffdcd..c8f8a2930c8c 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -863,7 +863,11 @@ def do_email_change_request(user, new_email, activation_key=None, secondary_emai except Exception as err: from_address = configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL) if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.error('Unable to send email activation link for user %s from a redacted address', user.id, exc_info=True) + log.error( + 'Unable to send email activation link for user %s from a redacted address', + user.id, + exc_info=True, + ) else: log.error('Unable to send email activation link to user from "%s"', from_address, exc_info=True) raise ValueError(_('Unable to send email activation link. Please try again later.')) from err # lint-amnesty, pylint: disable=raise-missing-from diff --git a/lms/djangoapps/bulk_user_retirement/views.py b/lms/djangoapps/bulk_user_retirement/views.py index dcb655c5dddc..00ce5f092079 100644 --- a/lms/djangoapps/bulk_user_retirement/views.py +++ b/lms/djangoapps/bulk_user_retirement/views.py @@ -58,8 +58,8 @@ def post(self, request, **kwargs): # pylint: disable=unused-argument with transaction.atomic(): create_retirement_request_and_deactivate_account(user_to_retire) if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.info('User %s added to retirement pipeline by user %s', - user_to_retire.id, + log.info('User %s added to retirement pipeline by user %s', + user_to_retire.id, request.user.id ) else: From fc1bf2f480e40f235db943afb954dd2d1ea68ffd Mon Sep 17 00:00:00 2001 From: Tarun Tak Date: Thu, 2 Jul 2026 11:01:32 +0000 Subject: [PATCH 09/21] fix: standardize PII squelching logic across logs --- cms/djangoapps/course_creators/admin.py | 14 +-- .../course_creators/tests/test_admin.py | 58 +++++++++++ .../student/models/course_enrollment.py | 96 +++++++------------ common/djangoapps/student/tests/tests.py | 55 +++++++++++ common/djangoapps/student/views/management.py | 18 +--- lms/djangoapps/bulk_user_retirement/views.py | 18 ++-- .../commands/goal_reminder_email.py | 6 +- lms/djangoapps/courseware/views/views.py | 6 +- 8 files changed, 168 insertions(+), 103 deletions(-) diff --git a/cms/djangoapps/course_creators/admin.py b/cms/djangoapps/course_creators/admin.py index 4c4237ee7cea..cbaeb329b479 100644 --- a/cms/djangoapps/course_creators/admin.py +++ b/cms/djangoapps/course_creators/admin.py @@ -168,11 +168,9 @@ def send_user_notification_callback(sender, **kwargs): # pylint: disable=unused try: user.email_user(subject, message, studio_request_email) - except: # lint-amnesty, pylint: disable=bare-except - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.warning("Unable to send course creator status e-mail to user ID %s", user.id) - else: - log.warning("Unable to send course creator status e-mail to %s", user.email) + except: # pylint: disable=bare-except + user_identifier_for_log = f"user ID {user.id}" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.email + log.warning("Unable to send course creator status e-mail to %s", user_identifier_for_log) @receiver(send_admin_notification, sender=CourseCreator) @@ -199,10 +197,8 @@ def send_admin_notification_callback(sender, **kwargs): # pylint: disable=unuse fail_silently=False ) except SMTPException: - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.warning("Failure sending 'pending state' e-mail for user ID %s to %s", user.id, studio_request_email) - else: - log.warning("Failure sending 'pending state' e-mail for %s to %s", user.email, studio_request_email) + user_identifier_for_log = f"user ID {user.id}" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.email + log.warning("Failure sending 'pending state' e-mail for %s to %s", user_identifier_for_log, studio_request_email) @receiver(m2m_changed, sender=CourseCreator.organizations.through) diff --git a/cms/djangoapps/course_creators/tests/test_admin.py b/cms/djangoapps/course_creators/tests/test_admin.py index 5d0508b5822b..026d3b67a668 100644 --- a/cms/djangoapps/course_creators/tests/test_admin.py +++ b/cms/djangoapps/course_creators/tests/test_admin.py @@ -3,6 +3,7 @@ """ +from smtplib import SMTPException from unittest import mock from django.contrib.admin.sites import AdminSite @@ -169,3 +170,60 @@ def test_change_permission(self): self.request.user = self.user self.assertFalse(self.creator_admin.has_change_permission(self.request)) # noqa: PT009 + + @mock.patch('cms.djangoapps.course_creators.admin.log') + @mock.patch('django.contrib.auth.models.User.email_user') + def test_send_user_notification_error_logging(self, mock_email_user, mock_log): + """ + Test that email_user raising an exception logs the correct message based on SQUELCH_PII_IN_LOGS setting. + """ + mock_email_user.side_effect = Exception("SMTP error") + + with self.settings(SQUELCH_PII_IN_LOGS=True): + with mock.patch.dict('django.conf.settings.FEATURES', self.enable_creator_group_patch): + self._change_state(CourseCreator.GRANTED) + mock_log.warning.assert_any_call( + "Unable to send course creator status e-mail to %s", + f"user ID {self.user.id}" + ) + + mock_log.reset_mock() + + with self.settings(SQUELCH_PII_IN_LOGS=False): + with mock.patch.dict('django.conf.settings.FEATURES', self.enable_creator_group_patch): + self._change_state(CourseCreator.DENIED) + self._change_state(CourseCreator.GRANTED) + mock_log.warning.assert_any_call( + "Unable to send course creator status e-mail to %s", + self.user.email + ) + + @mock.patch('cms.djangoapps.course_creators.admin.log') + @mock.patch('cms.djangoapps.course_creators.admin.send_mail') + def test_send_admin_notification_error_logging(self, mock_send_mail, mock_log): + """ + Test that send_mail raising SMTPException logs the correct message based on SQUELCH_PII_IN_LOGS setting. + """ + mock_send_mail.side_effect = SMTPException("SMTP error") + + with self.settings(SQUELCH_PII_IN_LOGS=True): + with mock.patch.dict('django.conf.settings.FEATURES', self.enable_creator_group_patch): + self._change_state(CourseCreator.PENDING) + mock_log.warning.assert_any_call( + "Failure sending 'pending state' e-mail for %s to %s", + f"user ID {self.user.id}", + self.studio_request_email + ) + + mock_log.reset_mock() + + with self.settings(SQUELCH_PII_IN_LOGS=False): + with mock.patch.dict('django.conf.settings.FEATURES', self.enable_creator_group_patch): + self._change_state(CourseCreator.UNREQUESTED) + self._change_state(CourseCreator.PENDING) + mock_log.warning.assert_any_call( + "Failure sending 'pending state' e-mail for %s to %s", + self.user.email, + self.studio_request_email + ) + diff --git a/common/djangoapps/student/models/course_enrollment.py b/common/djangoapps/student/models/course_enrollment.py index d2a5aa3f7aad..640126af332a 100644 --- a/common/djangoapps/student/models/course_enrollment.py +++ b/common/djangoapps/student/models/course_enrollment.py @@ -664,20 +664,13 @@ def emit_event(self, event_name, enterprise_uuid=None): except Exception: # pylint: disable=broad-except if event_name and self.course_id: - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.exception( - 'Unable to emit event %s for user %s and course %s', - event_name, - self.user.id, - self.course_id, - ) - else: - log.exception( - 'Unable to emit event %s for user %s and course %s', - event_name, - self.user.username, - self.course_id, - ) + user_identifier_for_log = self.user.id if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else self.user.username + log.exception( + 'Unable to emit event %s for user %s and course %s', + event_name, + user_identifier_for_log, + self.course_id, + ) @classmethod def enroll(cls, user, course_key, mode=None, check_access=False, can_upgrade=False, enterprise_uuid=None): @@ -746,59 +739,37 @@ def enroll(cls, user, course_key, mode=None, check_access=False, can_upgrade=Fal course_key=course_key, ) if check_access: - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.warning("User %s failed to enroll in non-existent course %s", user.id, str(course_key)) - else: - log.warning("User %s failed to enroll in non-existent course %s", user.username, str(course_key)) - raise NonExistentCourseError from err # lint-amnesty, pylint: disable=raise-missing-from + user_identifier_for_log = user.id if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.username + log.warning("User %s failed to enroll in non-existent course %s", user_identifier_for_log, str(course_key)) + raise NonExistentCourseError from err if check_access: if cls.is_enrollment_closed(user, course) and not can_upgrade: - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.warning( - "User %s failed to enroll in course %s because enrollment is closed (can_upgrade=%s).", - user.id, - str(course_key), - can_upgrade, - ) - else: - log.warning( - "User %s failed to enroll in course %s because enrollment is closed (can_upgrade=%s).", - user.username, - str(course_key), - can_upgrade, - ) + user_identifier_for_log = user.id if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.username + log.warning( + "User %s failed to enroll in course %s because enrollment is closed (can_upgrade=%s).", + user_identifier_for_log, + str(course_key), + can_upgrade, + ) raise EnrollmentClosedError if cls.objects.is_course_full(course): - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.warning( - "Course %s has reached its maximum enrollment of %d learners. User %s failed to enroll.", - str(course_key), - course.max_student_enrollments_allowed, - user.id, - ) - else: - log.warning( - "Course %s has reached its maximum enrollment of %d learners. User %s failed to enroll.", - str(course_key), - course.max_student_enrollments_allowed, - user.username, - ) - raise CourseFullError - if cls.is_enrolled(user, course_key): - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + user_identifier_for_log = user.id if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.username log.warning( - "User %s attempted to enroll in %s, but they were already enrolled", - user.id, - str(course_key) - ) - else: - log.warning( - "User %s attempted to enroll in %s, but they were already enrolled", - user.username, - str(course_key) + "Course %s has reached its maximum enrollment of %d learners. User %s failed to enroll.", + str(course_key), + course.max_student_enrollments_allowed, + user_identifier_for_log, ) + raise CourseFullError + if cls.is_enrolled(user, course_key): + user_identifier_for_log = user.id if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.username + log.warning( + "User %s attempted to enroll in %s, but they were already enrolled", + user_identifier_for_log, + str(course_key) + ) if check_access: raise AlreadyEnrolledError @@ -862,11 +833,8 @@ def enroll_by_email(cls, email, course_id, mode=None, ignore_errors=True): user = User.objects.get(email=email) return cls.enroll(user, course_id, mode) except User.DoesNotExist: - err_msg = "Tried to enroll email {} into course {}, but user not found" - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.error("Tried to enroll a redacted email into course %s, but user not found", course_id) - else: - log.error(err_msg.format(email, course_id)) + email_for_log = "a redacted email" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else f"email {email}" + log.error("Tried to enroll %s into course %s, but user not found", email_for_log, course_id) if ignore_errors: return None raise diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index c7a0b4c5037c..b0607283abf4 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -9,6 +9,7 @@ from zoneinfo import ZoneInfo import ddt +import pytest from config_models.models import cache from django.conf import settings from django.contrib.auth.models import AnonymousUser, User # pylint: disable=imported-auth-user @@ -28,6 +29,7 @@ AnonymousUserId, CourseEnrollment, LinkedInAddToProfileConfiguration, + NonExistentCourseError, UserAttribute, anonymous_id_for_user, unique_id_for_user, @@ -882,6 +884,59 @@ def test_enrollment_by_email(self): CourseEnrollment.unenroll_by_email("not_jack@fake.edx.org", course_id) self.assert_no_events_were_emitted() + @skip_unless_lms + @patch('common.djangoapps.student.models.course_enrollment.log') + def test_enroll_non_existent_course_squelch_logs(self, mock_log): + user = UserFactory.create(username="squelchy", email="squelchy@example.com") + course_id = CourseLocator("edX", "NoExist", "2013") + + with self.settings(SQUELCH_PII_IN_LOGS=True): + with pytest.raises(NonExistentCourseError): + CourseEnrollment.enroll(user, course_id, check_access=True) + mock_log.warning.assert_any_call( + "User %s failed to enroll in non-existent course %s", + user.id, + str(course_id) + ) + + mock_log.reset_mock() + + with self.settings(SQUELCH_PII_IN_LOGS=False): + with pytest.raises(NonExistentCourseError): + CourseEnrollment.enroll(user, course_id, check_access=True) + mock_log.warning.assert_any_call( + "User %s failed to enroll in non-existent course %s", + user.username, + str(course_id) + ) + + @skip_unless_lms + @patch('common.djangoapps.student.models.course_enrollment.log') + def test_enroll_by_email_non_existent_user_squelch_logs(self, mock_log): + course_id = CourseLocator("edX", "Test101", "2013") + CourseOverviewFactory.create(id=course_id) + email = "non_existent_user@example.com" + + with self.settings(SQUELCH_PII_IN_LOGS=True): + with pytest.raises(User.DoesNotExist): + CourseEnrollment.enroll_by_email(email, course_id, ignore_errors=False) + mock_log.error.assert_any_call( + "Tried to enroll %s into course %s, but user not found", + "a redacted email", + course_id + ) + + mock_log.reset_mock() + + with self.settings(SQUELCH_PII_IN_LOGS=False): + with pytest.raises(User.DoesNotExist): + CourseEnrollment.enroll_by_email(email, course_id, ignore_errors=False) + mock_log.error.assert_any_call( + "Tried to enroll %s into course %s, but user not found", + f"email {email}", + course_id + ) + @skip_unless_lms def test_enrollment_multiple_classes(self): user = UserFactory(username="rusty", email="rusty@fake.edx.org") diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index c8f8a2930c8c..3f78d5e47b2d 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -862,15 +862,8 @@ def do_email_change_request(user, new_email, activation_key=None, secondary_emai log.info("Email activation link sent to user [%s].", new_email) except Exception as err: from_address = configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL) - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.error( - 'Unable to send email activation link for user %s from a redacted address', - user.id, - exc_info=True, - ) - else: - log.error('Unable to send email activation link to user from "%s"', from_address, exc_info=True) - raise ValueError(_('Unable to send email activation link. Please try again later.')) from err # lint-amnesty, pylint: disable=raise-missing-from + log.error('Unable to send email activation link to user from "%s"', from_address, exc_info=True) + raise ValueError(_('Unable to send email activation link. Please try again later.')) from err if not secondary_email_change_request: # When the email address change is complete, a "edx.user.settings.changed" event will be emitted. @@ -978,11 +971,8 @@ def confirm_email_change(request, key): # pylint: disable=too-many-statements # Send it to the old email... try: ace.send(msg) - except Exception: # pylint: disable=broad-except - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.warning('Unable to send confirmation email to old address for user %s', user.id, exc_info=True) - else: - log.warning('Unable to send confirmation email to old address', exc_info=True) + except Exception: + log.warning('Unable to send confirmation email to old address', exc_info=True) response = render_to_response("email_change_failed.html", {'email': user.email}) transaction.set_rollback(True) return response diff --git a/lms/djangoapps/bulk_user_retirement/views.py b/lms/djangoapps/bulk_user_retirement/views.py index 00ce5f092079..ad855024de29 100644 --- a/lms/djangoapps/bulk_user_retirement/views.py +++ b/lms/djangoapps/bulk_user_retirement/views.py @@ -52,27 +52,29 @@ def post(self, request, **kwargs): # pylint: disable=unused-argument successful_user_retirements, failed_user_retirements = [], [] - for username in usernames_to_retire: + for index, username in enumerate(usernames_to_retire): try: user_to_retire = User.objects.get(username=username) with transaction.atomic(): create_retirement_request_and_deactivate_account(user_to_retire) - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.info('User %s added to retirement pipeline by user %s', + if getattr(settings, 'SQUELCH_PII_IN_LOGS', False): + log.info('User %s added to retirement pipeline by user %s at index %s', user_to_retire.id, - request.user.id + request.user.id, + index, ) else: - log.info('The user "%s" has been added to the retirement pipeline by "%s"', + log.info('The user "%s" has been added to the retirement pipeline by "%s" at index %s', username, request.user, + index, ) except User.DoesNotExist: - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.exception('A user does not exist for bulk retirement.') + if getattr(settings, 'SQUELCH_PII_IN_LOGS', False): + log.exception('Bulk retirement user at index %s does not exist.', index) else: - log.exception(f'The user "{username}" does not exist.') + log.exception('The user "%s" does not exist.', username) failed_user_retirements.append(username) except Exception as exc: # pylint: disable=broad-except diff --git a/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py b/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py index e89f419d46a4..0b21cbc42f90 100644 --- a/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py +++ b/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py @@ -50,10 +50,8 @@ def send_ace_message(goal, session_id): """ user = goal.user if not user.has_usable_password(): - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.info(f'Goal Reminder User is disabled user ID {user.id} course {goal.course_key}') - else: - log.info(f'Goal Reminder User is disabled {user.username} course {goal.course_key}') + user_identifier_for_log = f"ID {user.id}" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.username + log.info('Goal Reminder User is disabled user %s course %s', user_identifier_for_log, goal.course_key) return False try: course = CourseOverview.get_from_id(goal.course_key) diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 09844b0b15a0..2ad1649ccc38 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -1479,10 +1479,8 @@ def generate_user_cert(request, course_id): return HttpResponseBadRequest(str(e)) if not is_course_passed(student, course): - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.info("User ID %s has not passed the course: %s", student.id, course_id) - else: - log.info("User %s has not passed the course: %s", student.username, course_id) + user_identifier_for_log = f"ID {student.id}" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else student.username + log.info("User %s has not passed the course: %s", user_identifier_for_log, course_id) return HttpResponseBadRequest(_("Your certificate will be available when you pass the course.")) certificate_status = certs_api.certificate_downloadable_status(student, course.id) From 6d99f4aa621d2366f0e84346d9365f4bfb444ca4 Mon Sep 17 00:00:00 2001 From: Tarun Tak Date: Thu, 2 Jul 2026 12:19:37 +0000 Subject: [PATCH 10/21] fix: standardize PII squelching logic across logs --- cms/djangoapps/course_creators/admin.py | 14 ++++++++-- .../course_creators/tests/test_admin.py | 1 - .../student/models/course_enrollment.py | 28 ++++++++++++++----- common/djangoapps/student/views/management.py | 2 +- lms/djangoapps/courseware/views/views.py | 4 ++- 5 files changed, 36 insertions(+), 13 deletions(-) diff --git a/cms/djangoapps/course_creators/admin.py b/cms/djangoapps/course_creators/admin.py index cbaeb329b479..89a296e35f1d 100644 --- a/cms/djangoapps/course_creators/admin.py +++ b/cms/djangoapps/course_creators/admin.py @@ -169,7 +169,9 @@ def send_user_notification_callback(sender, **kwargs): # pylint: disable=unused try: user.email_user(subject, message, studio_request_email) except: # pylint: disable=bare-except - user_identifier_for_log = f"user ID {user.id}" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.email + user_identifier_for_log = ( + f"user ID {user.id}" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.email + ) log.warning("Unable to send course creator status e-mail to %s", user_identifier_for_log) @@ -197,8 +199,14 @@ def send_admin_notification_callback(sender, **kwargs): # pylint: disable=unuse fail_silently=False ) except SMTPException: - user_identifier_for_log = f"user ID {user.id}" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.email - log.warning("Failure sending 'pending state' e-mail for %s to %s", user_identifier_for_log, studio_request_email) + user_identifier_for_log = ( + f"user ID {user.id}" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.email + ) + log.warning( + "Failure sending 'pending state' e-mail for %s to %s", + user_identifier_for_log, + studio_request_email, + ) @receiver(m2m_changed, sender=CourseCreator.organizations.through) diff --git a/cms/djangoapps/course_creators/tests/test_admin.py b/cms/djangoapps/course_creators/tests/test_admin.py index 026d3b67a668..0071dc9ca470 100644 --- a/cms/djangoapps/course_creators/tests/test_admin.py +++ b/cms/djangoapps/course_creators/tests/test_admin.py @@ -226,4 +226,3 @@ def test_send_admin_notification_error_logging(self, mock_send_mail, mock_log): self.user.email, self.studio_request_email ) - diff --git a/common/djangoapps/student/models/course_enrollment.py b/common/djangoapps/student/models/course_enrollment.py index 640126af332a..cde2824e5dc4 100644 --- a/common/djangoapps/student/models/course_enrollment.py +++ b/common/djangoapps/student/models/course_enrollment.py @@ -664,7 +664,9 @@ def emit_event(self, event_name, enterprise_uuid=None): except Exception: # pylint: disable=broad-except if event_name and self.course_id: - user_identifier_for_log = self.user.id if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else self.user.username + user_identifier_for_log = ( + self.user.id if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else self.user.username + ) log.exception( 'Unable to emit event %s for user %s and course %s', event_name, @@ -739,13 +741,21 @@ def enroll(cls, user, course_key, mode=None, check_access=False, can_upgrade=Fal course_key=course_key, ) if check_access: - user_identifier_for_log = user.id if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.username - log.warning("User %s failed to enroll in non-existent course %s", user_identifier_for_log, str(course_key)) + user_identifier_for_log = ( + user.id if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.username + ) + log.warning( + "User %s failed to enroll in non-existent course %s", + user_identifier_for_log, + str(course_key), + ) raise NonExistentCourseError from err if check_access: if cls.is_enrollment_closed(user, course) and not can_upgrade: - user_identifier_for_log = user.id if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.username + user_identifier_for_log = ( + user.id if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.username + ) log.warning( "User %s failed to enroll in course %s because enrollment is closed (can_upgrade=%s).", user_identifier_for_log, @@ -755,7 +765,9 @@ def enroll(cls, user, course_key, mode=None, check_access=False, can_upgrade=Fal raise EnrollmentClosedError if cls.objects.is_course_full(course): - user_identifier_for_log = user.id if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.username + user_identifier_for_log = ( + user.id if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.username + ) log.warning( "Course %s has reached its maximum enrollment of %d learners. User %s failed to enroll.", str(course_key), @@ -764,11 +776,13 @@ def enroll(cls, user, course_key, mode=None, check_access=False, can_upgrade=Fal ) raise CourseFullError if cls.is_enrolled(user, course_key): - user_identifier_for_log = user.id if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.username + user_identifier_for_log = ( + user.id if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.username + ) log.warning( "User %s attempted to enroll in %s, but they were already enrolled", user_identifier_for_log, - str(course_key) + str(course_key), ) if check_access: raise AlreadyEnrolledError diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index 3f78d5e47b2d..eb9f69dcd9f5 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -971,7 +971,7 @@ def confirm_email_change(request, key): # pylint: disable=too-many-statements # Send it to the old email... try: ace.send(msg) - except Exception: + except Exception: # pylint: disable=broad-exception-caught log.warning('Unable to send confirmation email to old address', exc_info=True) response = render_to_response("email_change_failed.html", {'email': user.email}) transaction.set_rollback(True) diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 2ad1649ccc38..38af78f4b9c0 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -1479,7 +1479,9 @@ def generate_user_cert(request, course_id): return HttpResponseBadRequest(str(e)) if not is_course_passed(student, course): - user_identifier_for_log = f"ID {student.id}" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else student.username + user_identifier_for_log = ( + f"ID {student.id}" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else student.username + ) log.info("User %s has not passed the course: %s", user_identifier_for_log, course_id) return HttpResponseBadRequest(_("Your certificate will be available when you pass the course.")) From 244c3cefd2e9ce39ead97a5d76adad1827454d21 Mon Sep 17 00:00:00 2001 From: Tarun Tak Date: Thu, 2 Jul 2026 17:12:17 +0000 Subject: [PATCH 11/21] fix: standardize PII squelching logic across logs --- common/djangoapps/student/emails.py | 12 ++---- common/djangoapps/student/models/user.py | 20 +++++----- common/djangoapps/student/views/management.py | 37 ++++++++++--------- lms/djangoapps/courseware/model_data.py | 9 +++-- lms/djangoapps/instructor/views/api.py | 15 +++----- .../views/instructor_task_helpers.py | 4 +- .../commands/manual_verifications.py | 8 ++-- 7 files changed, 52 insertions(+), 53 deletions(-) diff --git a/common/djangoapps/student/emails.py b/common/djangoapps/student/emails.py index 91cb340fee1c..1d3d848e65ef 100644 --- a/common/djangoapps/student/emails.py +++ b/common/djangoapps/student/emails.py @@ -27,14 +27,10 @@ def send_proctoring_requirements_email(context): user_context={'full_name': user.profile.name} ) ace.send(msg) - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.info('Proctoring requirements email sent to user ID: %r', user.id) - else: - log.info('Proctoring requirements email sent to user: %r', user.username) + user_identifier_for_log = f"ID: {user.id}" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.username + log.info('Proctoring requirements email sent to user %s', user_identifier_for_log) return True except Exception: # pylint: disable=broad-except - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.exception('Could not send email for proctoring requirements to user ID %s', user.id) - else: - log.exception('Could not send email for proctoring requirements to user %s', user.username) + user_identifier_for_log = f"ID {user.id}" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.username + log.exception('Could not send email for proctoring requirements to user %s', user_identifier_for_log) return False diff --git a/common/djangoapps/student/models/user.py b/common/djangoapps/student/models/user.py index c5c4edbd7422..e30d000ffc7c 100644 --- a/common/djangoapps/student/models/user.py +++ b/common/djangoapps/student/models/user.py @@ -887,7 +887,7 @@ def activate(self): # pylint: disable=missing-function-docstring self.activation_timestamp = datetime.utcnow() self.save() USER_ACCOUNT_ACTIVATED.send_robust(self.__class__, user=self.user) - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + if getattr(settings, 'SQUELCH_PII_IN_LOGS', False): log.info('User %s account is successfully activated.', self.user.id) else: log.info('User %s (%s) account is successfully activated.', self.user.username, self.user.email) @@ -1316,10 +1316,11 @@ def log_successful_login(sender, request, user, **kwargs): # pylint: disable=un 'event_type': "login", } ) - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - AUDIT_LOG.info(f"Login success - user.id: {user.id}") - else: - AUDIT_LOG.info(f"Login success - {user.username} ({user.email})") + user_identifier_for_log = ( + f"user.id: {user.id}" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) + else f"{user.username} ({user.email})" + ) + AUDIT_LOG.info(f"Login success - {user_identifier_for_log}") @receiver(user_logged_out) @@ -1333,10 +1334,11 @@ def log_successful_logout(sender, request, user, **kwargs): # pylint: disable=u 'event_type': "logout", } ) - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - AUDIT_LOG.info(f'Logout - user.id: {request.user.id}') # pylint: disable=logging-format-interpolation - else: - AUDIT_LOG.info(f'Logout - {request.user}') # pylint: disable=logging-format-interpolation + user_identifier_for_log = ( + f"user.id: {request.user.id}" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) + else f"{request.user}" + ) + AUDIT_LOG.info(f'Logout - {user_identifier_for_log}') # pylint: disable=logging-format-interpolation if request.user.id: segment.track(request.user.id, 'edx.bi.user.account.logout') diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index eb9f69dcd9f5..e875a9a5efa3 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -489,10 +489,8 @@ def change_enrollment(request, check_access=True): except UnenrollmentNotAllowed as exc: return HttpResponseBadRequest(str(exc)) - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.info("User %s unenrolled from %s; sending REFUND_ORDER", user.id, course_id) - else: - log.info("User %s unenrolled from %s; sending REFUND_ORDER", user.username, course_id) + user_identifier_for_log = user.id if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.username + log.info("User %s unenrolled from %s; sending REFUND_ORDER", user_identifier_for_log, course_id) REFUND_ORDER.send(sender=None, course_enrollment=enrollment) return HttpResponse() else: @@ -560,14 +558,14 @@ def disable_account_ajax(request): if account_action == 'disable': user_account.account_status = UserStanding.ACCOUNT_DISABLED context['message'] = _("Successfully disabled {}'s account").format(username) - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + if getattr(settings, 'SQUELCH_PII_IN_LOGS', False): log.info("User %s disabled user %s's account", request.user.id, user.id) else: log.info("%s disabled %s's account", request.user, username) elif account_action == 'reenable': user_account.account_status = UserStanding.ACCOUNT_ENABLED context['message'] = _("Successfully reenabled {}'s account").format(username) - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + if getattr(settings, 'SQUELCH_PII_IN_LOGS', False): log.info("User %s reenabled user %s's account", request.user.id, user.id) else: log.info("%s reenabled %s's account", request.user, username) @@ -856,14 +854,15 @@ def do_email_change_request(user, new_email, activation_key=None, secondary_emai try: ace.send(msg) - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.info("Email activation link sent for user ID: [%s].", user.id) - else: - log.info("Email activation link sent to user [%s].", new_email) + user_identifier_for_log = ( + f"for user ID: [{user.id}]" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) + else f"to user [{new_email}]" + ) + log.info("Email activation link sent %s.", user_identifier_for_log) except Exception as err: from_address = configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL) log.error('Unable to send email activation link to user from "%s"', from_address, exc_info=True) - raise ValueError(_('Unable to send email activation link. Please try again later.')) from err + raise ValueError(_('Unable to send email activation link. Please try again later.')) from err # lint-amnesty, pylint: disable=raise-missing-from if not secondary_email_change_request: # When the email address change is complete, a "edx.user.settings.changed" event will be emitted. @@ -971,8 +970,15 @@ def confirm_email_change(request, key): # pylint: disable=too-many-statements # Send it to the old email... try: ace.send(msg) - except Exception: # pylint: disable=broad-exception-caught - log.warning('Unable to send confirmation email to old address', exc_info=True) + except Exception: # pylint: disable=broad-except + if getattr(settings, 'SQUELCH_PII_IN_LOGS', False): + log.warning( + 'Unable to send confirmation email to old address for user %s', + user.id, + exc_info=True, + ) + else: + log.warning('Unable to send confirmation email to old address', exc_info=True) response = render_to_response("email_change_failed.html", {'email': user.email}) transaction.set_rollback(True) return response @@ -988,10 +994,7 @@ def confirm_email_change(request, key): # pylint: disable=too-many-statements try: ace.send(msg) except Exception: # pylint: disable=broad-except - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.warning('Unable to send confirmation email to new address for user %s', user.id, exc_info=True) - else: - log.warning('Unable to send confirmation email to new address', exc_info=True) + log.warning('Unable to send confirmation email to new address', exc_info=True) response = render_to_response("email_change_failed.html", {'email': user.email}) transaction.set_rollback(True) return response diff --git a/lms/djangoapps/courseware/model_data.py b/lms/djangoapps/courseware/model_data.py index 53447dde1b92..64a0fcae546c 100644 --- a/lms/djangoapps/courseware/model_data.py +++ b/lms/djangoapps/courseware/model_data.py @@ -406,10 +406,11 @@ def set_many(self, kv_dict): pending_updates ) except DatabaseError as err: - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.exception("Saving user state failed for user ID %s", self.user.id) - else: - log.exception("Saving user state failed for %s", self.user.username) + user_identifier_for_log = ( + f"user ID {self.user.id}" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) + else self.user.username + ) + log.exception("Saving user state failed for %s", user_identifier_for_log) raise KeyValueMultiSaveError([]) from err # lint-amnesty, pylint: disable=raise-missing-from finally: self._cache.update(pending_updates) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index abdbdf73257c..2077056c8111 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -462,19 +462,16 @@ def post(self, request, course_id): # pylint: disable=too-many-statements warnings.append({ 'username': username, 'email': email, 'response': warning_message }) - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + if getattr(settings, 'SQUELCH_PII_IN_LOGS', False): log.warning('email for user %s already exist', user.id) else: log.warning('email %s already exist', email) else: - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - log.info('user already exists with user ID %s', user.id) - else: - log.info( - "user already exists with username '%s' and email '%s'", - username, - email - ) + user_identifier_for_log = ( + f"user ID {user.id}" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) + else f"username '{username}' and email '{email}'" + ) + log.info('user already exists with %s', user_identifier_for_log) # enroll a user if it is not already enrolled. if not is_user_enrolled_in_course(user, course_id): diff --git a/lms/djangoapps/instructor/views/instructor_task_helpers.py b/lms/djangoapps/instructor/views/instructor_task_helpers.py index 9aedc2ef713e..3c538fcb37d0 100644 --- a/lms/djangoapps/instructor/views/instructor_task_helpers.py +++ b/lms/djangoapps/instructor/views/instructor_task_helpers.py @@ -56,7 +56,7 @@ def extract_email_features(email_task): try: task_input_information = json.loads(email_task.task_input) except ValueError: - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + if getattr(settings, 'SQUELCH_PII_IN_LOGS', False): log.error("Could not parse task input as valid json; task input is redacted") else: log.error("Could not parse task input as valid json; task input: %s", email_task.task_input) @@ -86,7 +86,7 @@ def extract_email_features(email_task): try: task_output = json.loads(email_task.task_output) except ValueError: - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + if getattr(settings, 'SQUELCH_PII_IN_LOGS', False): log.error("Could not parse task output as valid json; task output is redacted") else: log.error("Could not parse task output as valid json; task output: %s", email_task.task_output) diff --git a/lms/djangoapps/verify_student/management/commands/manual_verifications.py b/lms/djangoapps/verify_student/management/commands/manual_verifications.py index 8c0d77ace462..26c866dcfb4e 100644 --- a/lms/djangoapps/verify_student/management/commands/manual_verifications.py +++ b/lms/djangoapps/verify_student/management/commands/manual_verifications.py @@ -54,7 +54,7 @@ def handle(self, *args, **options): if single_email: successfully_verified = self._add_user_to_manual_verification(single_email) if successfully_verified is False: - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + if getattr(settings, 'SQUELCH_PII_IN_LOGS', False): log.error('Manual verification of a redacted email failed') else: log.error(f'Manual verification of {single_email} failed') @@ -74,7 +74,7 @@ def handle(self, *args, **options): len(failed_emails), total_emails )) - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + if getattr(settings, 'SQUELCH_PII_IN_LOGS', False): log.error('Failed emails are redacted') else: log.error(f'Failed emails:{pformat(failed_emails)}') @@ -129,7 +129,7 @@ def _add_users_to_manual_verification(self, email_ids): status='approved', )) else: - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + if getattr(settings, 'SQUELCH_PII_IN_LOGS', False): log.info(f'Skipping user ID {user.id}, existing verification found.') else: log.info(f'Skipping email {user.email}, existing verification found.') @@ -157,7 +157,7 @@ def _add_user_to_manual_verification(self, email_id): ) return True except User.DoesNotExist: - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + if getattr(settings, 'SQUELCH_PII_IN_LOGS', False): log.error('Tried to verify a redacted email, but user not found') else: log.error(f'Tried to verify email {email_id}, but user not found') From c1faadca127027f4e3b18d67648e3836c11fd492 Mon Sep 17 00:00:00 2001 From: Tarun Tak Date: Thu, 2 Jul 2026 17:39:10 +0000 Subject: [PATCH 12/21] fix: standardize PII squelching logic across logs --- common/djangoapps/student/views/management.py | 13 +++---------- openedx/core/djangoapps/user_authn/views/login.py | 2 +- .../djangoapps/user_authn/views/tests/test_login.py | 8 ++++---- 3 files changed, 8 insertions(+), 15 deletions(-) diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index e875a9a5efa3..847946c605cf 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -862,7 +862,7 @@ def do_email_change_request(user, new_email, activation_key=None, secondary_emai except Exception as err: from_address = configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL) log.error('Unable to send email activation link to user from "%s"', from_address, exc_info=True) - raise ValueError(_('Unable to send email activation link. Please try again later.')) from err # lint-amnesty, pylint: disable=raise-missing-from + raise ValueError(_('Unable to send email activation link. Please try again later.')) from err if not secondary_email_change_request: # When the email address change is complete, a "edx.user.settings.changed" event will be emitted. @@ -911,7 +911,7 @@ def activate_secondary_email(request, key): @ensure_csrf_cookie -def confirm_email_change(request, key): # pylint: disable=too-many-statements +def confirm_email_change(request, key): """ User requested a new e-mail. This is called when the activation link is clicked. We confirm with the old e-mail, and update @@ -971,14 +971,7 @@ def confirm_email_change(request, key): # pylint: disable=too-many-statements try: ace.send(msg) except Exception: # pylint: disable=broad-except - if getattr(settings, 'SQUELCH_PII_IN_LOGS', False): - log.warning( - 'Unable to send confirmation email to old address for user %s', - user.id, - exc_info=True, - ) - else: - log.warning('Unable to send confirmation email to old address', exc_info=True) + log.warning('Unable to send confirmation email to old address', exc_info=True) response = render_to_response("email_change_failed.html", {'email': user.email}) transaction.set_rollback(True) return response diff --git a/openedx/core/djangoapps/user_authn/views/login.py b/openedx/core/djangoapps/user_authn/views/login.py index 9f06322e5af0..848923db31b1 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/openedx/core/djangoapps/user_authn/views/login.py @@ -199,7 +199,7 @@ def _enforce_password_policy_compliance(request, user): # pylint: disable=missi if LoginFailures.is_feature_enabled(): LoginFailures.increment_lockout_counter(user) - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: + if getattr(settings, 'SQUELCH_PII_IN_LOGS', False): AUDIT_LOG.info("Password reset initiated for user ID %s.", user.id) else: AUDIT_LOG.info("Password reset initiated for email %s.", user.email) diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_login.py b/openedx/core/djangoapps/user_authn/views/tests/test_login.py index 4e4d0a9e8894..af5f3937ca3b 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_login.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_login.py @@ -314,7 +314,7 @@ def test_enterprise_in_url( self._assert_response(response, success=True) self._assert_redirect_url(response, settings.LMS_ROOT_URL + expected_redirect + next_url) - @patch.dict("django.conf.settings.FEATURES", {'SQUELCH_PII_IN_LOGS': True}) + @override_settings(SQUELCH_PII_IN_LOGS=True) def test_login_success_no_pii(self): response, mock_audit_log = self._login_response( self.user_email, self.password, patched_audit_log='common.djangoapps.student.models.user.AUDIT_LOG' @@ -346,7 +346,7 @@ def test_login_fail_no_user_exists(self): ) self._assert_audit_log(mock_audit_log, 'warning', ['Login failed', 'Unknown user email', email_hash]) - @patch.dict("django.conf.settings.FEATURES", {'SQUELCH_PII_IN_LOGS': True}) + @override_settings(SQUELCH_PII_IN_LOGS=True) def test_login_fail_no_user_exists_no_pii(self): nonexistent_email = 'not_a_user@edx.org' response, mock_audit_log = self._login_response( @@ -366,7 +366,7 @@ def test_login_fail_wrong_password(self): self._assert_audit_log(mock_audit_log, 'warning', ['Login failed', 'password for', str(self.user.id), 'invalid']) - @patch.dict("django.conf.settings.FEATURES", {'SQUELCH_PII_IN_LOGS': True}) + @override_settings(SQUELCH_PII_IN_LOGS=True) def test_login_fail_wrong_password_no_pii(self): response, mock_audit_log = self._login_response(self.user_email, 'wrong_password') self._assert_response(response, success=False, value=self.LOGIN_FAILED_WARNING) @@ -536,7 +536,7 @@ def test_unicode_mktg_cookie_names(self): } assert_dict_contains_subset(self, expected, response.context_data) - @patch.dict("django.conf.settings.FEATURES", {'SQUELCH_PII_IN_LOGS': True}) + @override_settings(SQUELCH_PII_IN_LOGS=True) def test_logout_logging_no_pii(self): response, _ = self._login_response(self.user_email, self.password) self._assert_response(response, success=True) From bef58f88912cde00cce8d0737b705ddd1f038a64 Mon Sep 17 00:00:00 2001 From: Tarun Tak Date: Thu, 2 Jul 2026 18:24:34 +0000 Subject: [PATCH 13/21] fix: fixed unit test cases --- lms/djangoapps/instructor/tests/test_api.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index d68633c819c6..be10a7b0c391 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -718,9 +718,8 @@ def test_email_and_username_already_exist(self, info_log): # test the log for email that's send to new created user. info_log.assert_called_with( - "user already exists with username '%s' and email '%s'", - 'test_student_1', - 'test_student@example.com' + 'user already exists with %s', + "username 'test_student_1' and email 'test_student@example.com'" ) def test_file_upload_type_not_csv(self): From 466a4e8b697c0983d9b61862ebf5a66eb93019e5 Mon Sep 17 00:00:00 2001 From: Tarun Tak Date: Thu, 9 Jul 2026 16:17:40 +0000 Subject: [PATCH 14/21] fix: removed linters --- lms/djangoapps/courseware/model_data.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/model_data.py b/lms/djangoapps/courseware/model_data.py index 64a0fcae546c..62cd8852782a 100644 --- a/lms/djangoapps/courseware/model_data.py +++ b/lms/djangoapps/courseware/model_data.py @@ -411,7 +411,7 @@ def set_many(self, kv_dict): else self.user.username ) log.exception("Saving user state failed for %s", user_identifier_for_log) - raise KeyValueMultiSaveError([]) from err # lint-amnesty, pylint: disable=raise-missing-from + raise KeyValueMultiSaveError([]) from err finally: self._cache.update(pending_updates) From d3382206f9dcc01860729cc901dc1694354dfffd Mon Sep 17 00:00:00 2001 From: Tarun Tak Date: Tue, 28 Jul 2026 09:38:40 +0000 Subject: [PATCH 15/21] fix: simplify user identifier logging across multiple modules --- common/djangoapps/student/emails.py | 4 ++-- .../student/models/course_enrollment.py | 4 ++-- common/djangoapps/student/models/user.py | 4 ++-- common/djangoapps/student/views/management.py | 7 ++----- lms/djangoapps/bulk_user_retirement/views.py | 6 ++---- .../management/commands/goal_reminder_email.py | 2 +- lms/djangoapps/courseware/model_data.py | 2 +- lms/djangoapps/courseware/views/views.py | 2 +- .../views/instructor_task_helpers.py | 10 ++-------- .../commands/manual_verifications.py | 18 ++++++------------ .../core/djangoapps/user_authn/views/login.py | 6 ++---- 11 files changed, 23 insertions(+), 42 deletions(-) diff --git a/common/djangoapps/student/emails.py b/common/djangoapps/student/emails.py index 1d3d848e65ef..1615c16234f6 100644 --- a/common/djangoapps/student/emails.py +++ b/common/djangoapps/student/emails.py @@ -27,10 +27,10 @@ def send_proctoring_requirements_email(context): user_context={'full_name': user.profile.name} ) ace.send(msg) - user_identifier_for_log = f"ID: {user.id}" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.username + user_identifier_for_log = user.id if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.username log.info('Proctoring requirements email sent to user %s', user_identifier_for_log) return True except Exception: # pylint: disable=broad-except - user_identifier_for_log = f"ID {user.id}" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.username + user_identifier_for_log = user.id if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.username log.exception('Could not send email for proctoring requirements to user %s', user_identifier_for_log) return False diff --git a/common/djangoapps/student/models/course_enrollment.py b/common/djangoapps/student/models/course_enrollment.py index 7103b4b45579..02f1bd4bac9c 100644 --- a/common/djangoapps/student/models/course_enrollment.py +++ b/common/djangoapps/student/models/course_enrollment.py @@ -847,8 +847,8 @@ def enroll_by_email(cls, email, course_id, mode=None, ignore_errors=True): user = User.objects.get(email=email) return cls.enroll(user, course_id, mode) except User.DoesNotExist: - email_for_log = "a redacted email" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else f"email {email}" - log.error("Tried to enroll %s into course %s, but user not found", email_for_log, course_id) + email_for_log = "[REDACTED]" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else email + log.error("Tried to enroll email %s into course %s, but user not found", email_for_log, course_id) if ignore_errors: return None raise diff --git a/common/djangoapps/student/models/user.py b/common/djangoapps/student/models/user.py index f8a5abe1e584..f9873e4b60cc 100644 --- a/common/djangoapps/student/models/user.py +++ b/common/djangoapps/student/models/user.py @@ -1317,7 +1317,7 @@ def log_successful_login(sender, request, user, **kwargs): # pylint: disable=un } ) user_identifier_for_log = ( - f"user.id: {user.id}" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) + user.id if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else f"{user.username} ({user.email})" ) AUDIT_LOG.info(f"Login success - {user_identifier_for_log}") @@ -1335,7 +1335,7 @@ def log_successful_logout(sender, request, user, **kwargs): # pylint: disable=u } ) user_identifier_for_log = ( - f"user.id: {request.user.id}" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) + request.user.id if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else f"{request.user}" ) AUDIT_LOG.info(f'Logout - {user_identifier_for_log}') # pylint: disable=logging-format-interpolation diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index 27daf5926ad9..d19cb94836b3 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -854,11 +854,8 @@ def do_email_change_request(user, new_email, activation_key=None, secondary_emai try: ace.send(msg) - user_identifier_for_log = ( - f"for user ID: [{user.id}]" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) - else f"to user [{new_email}]" - ) - log.info("Email activation link sent %s.", user_identifier_for_log) + user_identifier_for_log = user.id if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else new_email + log.info("Email activation link sent to user [%s].", user_identifier_for_log) except Exception as err: from_address = configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL) log.error('Unable to send email activation link to user from "%s"', from_address, exc_info=True) diff --git a/lms/djangoapps/bulk_user_retirement/views.py b/lms/djangoapps/bulk_user_retirement/views.py index ad855024de29..81ebc8f2180f 100644 --- a/lms/djangoapps/bulk_user_retirement/views.py +++ b/lms/djangoapps/bulk_user_retirement/views.py @@ -71,10 +71,8 @@ def post(self, request, **kwargs): # pylint: disable=unused-argument ) except User.DoesNotExist: - if getattr(settings, 'SQUELCH_PII_IN_LOGS', False): - log.exception('Bulk retirement user at index %s does not exist.', index) - else: - log.exception('The user "%s" does not exist.', username) + user_identifier_for_log = f"index {index}" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else username + log.exception('Bulk retirement user %s does not exist.', user_identifier_for_log) failed_user_retirements.append(username) except Exception as exc: # pylint: disable=broad-except diff --git a/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py b/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py index 0b21cbc42f90..36bbac87c54b 100644 --- a/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py +++ b/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py @@ -50,7 +50,7 @@ def send_ace_message(goal, session_id): """ user = goal.user if not user.has_usable_password(): - user_identifier_for_log = f"ID {user.id}" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.username + user_identifier_for_log = user.id if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.username log.info('Goal Reminder User is disabled user %s course %s', user_identifier_for_log, goal.course_key) return False try: diff --git a/lms/djangoapps/courseware/model_data.py b/lms/djangoapps/courseware/model_data.py index 62cd8852782a..5de089821616 100644 --- a/lms/djangoapps/courseware/model_data.py +++ b/lms/djangoapps/courseware/model_data.py @@ -407,7 +407,7 @@ def set_many(self, kv_dict): ) except DatabaseError as err: user_identifier_for_log = ( - f"user ID {self.user.id}" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) + self.user.id if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else self.user.username ) log.exception("Saving user state failed for %s", user_identifier_for_log) diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index a743c377c621..94866efc9410 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -1480,7 +1480,7 @@ def generate_user_cert(request, course_id): if not is_course_passed(student, course): user_identifier_for_log = ( - f"ID {student.id}" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else student.username + student.id if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else student.username ) log.info("User %s has not passed the course: %s", user_identifier_for_log, course_id) return HttpResponseBadRequest(_("Your certificate will be available when you pass the course.")) diff --git a/lms/djangoapps/instructor/views/instructor_task_helpers.py b/lms/djangoapps/instructor/views/instructor_task_helpers.py index 3c538fcb37d0..f07b1f38e98b 100644 --- a/lms/djangoapps/instructor/views/instructor_task_helpers.py +++ b/lms/djangoapps/instructor/views/instructor_task_helpers.py @@ -56,10 +56,7 @@ def extract_email_features(email_task): try: task_input_information = json.loads(email_task.task_input) except ValueError: - if getattr(settings, 'SQUELCH_PII_IN_LOGS', False): - log.error("Could not parse task input as valid json; task input is redacted") - else: - log.error("Could not parse task input as valid json; task input: %s", email_task.task_input) + log.error("Could not parse task input as valid json; task input: %s", email_task.task_input) return email_error_information() email = CourseEmail.objects.get(id=task_input_information['email_id']) @@ -86,10 +83,7 @@ def extract_email_features(email_task): try: task_output = json.loads(email_task.task_output) except ValueError: - if getattr(settings, 'SQUELCH_PII_IN_LOGS', False): - log.error("Could not parse task output as valid json; task output is redacted") - else: - log.error("Could not parse task output as valid json; task output: %s", email_task.task_output) + log.error("Could not parse task output as valid json; task output: %s", email_task.task_output) else: if 'succeeded' in task_output and task_output['succeeded'] > 0: num_emails = task_output['succeeded'] diff --git a/lms/djangoapps/verify_student/management/commands/manual_verifications.py b/lms/djangoapps/verify_student/management/commands/manual_verifications.py index 26c866dcfb4e..73fcb0f3f908 100644 --- a/lms/djangoapps/verify_student/management/commands/manual_verifications.py +++ b/lms/djangoapps/verify_student/management/commands/manual_verifications.py @@ -54,10 +54,8 @@ def handle(self, *args, **options): if single_email: successfully_verified = self._add_user_to_manual_verification(single_email) if successfully_verified is False: - if getattr(settings, 'SQUELCH_PII_IN_LOGS', False): - log.error('Manual verification of a redacted email failed') - else: - log.error(f'Manual verification of {single_email} failed') + user_identifier_for_log = '[REDACTED]' if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else single_email + log.error('Manual verification of %s failed', user_identifier_for_log) return email_ids_file = options['email_ids_file'] @@ -74,10 +72,8 @@ def handle(self, *args, **options): len(failed_emails), total_emails )) - if getattr(settings, 'SQUELCH_PII_IN_LOGS', False): - log.error('Failed emails are redacted') - else: - log.error(f'Failed emails:{pformat(failed_emails)}') + failed_emails_for_log = '[REDACTED]' if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else pformat(failed_emails) + log.error('Failed emails:%s', failed_emails_for_log) else: log.info(f'Successfully generated manual verification for {total_emails} emails.') @@ -157,8 +153,6 @@ def _add_user_to_manual_verification(self, email_id): ) return True except User.DoesNotExist: - if getattr(settings, 'SQUELCH_PII_IN_LOGS', False): - log.error('Tried to verify a redacted email, but user not found') - else: - log.error(f'Tried to verify email {email_id}, but user not found') + email_for_log = '[REDACTED]' if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else email_id + log.error('Tried to verify email %s, but user not found', email_for_log) return False diff --git a/openedx/core/djangoapps/user_authn/views/login.py b/openedx/core/djangoapps/user_authn/views/login.py index 848923db31b1..8629b1708914 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/openedx/core/djangoapps/user_authn/views/login.py @@ -199,10 +199,8 @@ def _enforce_password_policy_compliance(request, user): # pylint: disable=missi if LoginFailures.is_feature_enabled(): LoginFailures.increment_lockout_counter(user) - if getattr(settings, 'SQUELCH_PII_IN_LOGS', False): - AUDIT_LOG.info("Password reset initiated for user ID %s.", user.id) - else: - AUDIT_LOG.info("Password reset initiated for email %s.", user.email) + user_identifier_for_log = user.id if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.email + AUDIT_LOG.info("Password reset initiated for email %s.", user_identifier_for_log) tracker.emit( PASSWORD_RESET_INITIATED, { From 9c3ee2d8a71f62c3159c67ed20716d25b78d5d39 Mon Sep 17 00:00:00 2001 From: Tarun Tak Date: Tue, 28 Jul 2026 12:46:20 +0000 Subject: [PATCH 16/21] fix: linter issue --- .../course_creators/tests/test_admin.py | 69 ++++++++++--------- lms/djangoapps/bulk_user_retirement/views.py | 4 +- .../views/instructor_task_helpers.py | 1 - .../commands/manual_verifications.py | 8 ++- 4 files changed, 46 insertions(+), 36 deletions(-) diff --git a/cms/djangoapps/course_creators/tests/test_admin.py b/cms/djangoapps/course_creators/tests/test_admin.py index 32a4d75073a7..9b6aa3fe312d 100644 --- a/cms/djangoapps/course_creators/tests/test_admin.py +++ b/cms/djangoapps/course_creators/tests/test_admin.py @@ -56,6 +56,7 @@ def setUp(self): 'user_name': 'test_user', 'user_email': 'test_user+courses@edx.org', } + self.enable_creator_group_patch = {'ENABLE_CREATOR_GROUP': True} @override_settings(ENABLE_CREATOR_GROUP=True, STUDIO_REQUEST_EMAIL='mark@marky.mark') @mock.patch('django.contrib.auth.models.User.email_user') @@ -175,24 +176,26 @@ def test_send_user_notification_error_logging(self, mock_email_user, mock_log): """ mock_email_user.side_effect = Exception("SMTP error") - with self.settings(SQUELCH_PII_IN_LOGS=True): - with mock.patch.dict('django.conf.settings.FEATURES', self.enable_creator_group_patch): - self._change_state(CourseCreator.GRANTED) - mock_log.warning.assert_any_call( - "Unable to send course creator status e-mail to %s", - f"user ID {self.user.id}" - ) + with self.settings(SQUELCH_PII_IN_LOGS=True), mock.patch.dict( + 'django.conf.settings.FEATURES', self.enable_creator_group_patch + ): + self._change_state(CourseCreator.GRANTED) + mock_log.warning.assert_any_call( + "Unable to send course creator status e-mail to %s", + f"user ID {self.user.id}" + ) mock_log.reset_mock() - with self.settings(SQUELCH_PII_IN_LOGS=False): - with mock.patch.dict('django.conf.settings.FEATURES', self.enable_creator_group_patch): - self._change_state(CourseCreator.DENIED) - self._change_state(CourseCreator.GRANTED) - mock_log.warning.assert_any_call( - "Unable to send course creator status e-mail to %s", - self.user.email - ) + with self.settings(SQUELCH_PII_IN_LOGS=False), mock.patch.dict( + 'django.conf.settings.FEATURES', self.enable_creator_group_patch + ): + self._change_state(CourseCreator.DENIED) + self._change_state(CourseCreator.GRANTED) + mock_log.warning.assert_any_call( + "Unable to send course creator status e-mail to %s", + self.user.email + ) @mock.patch('cms.djangoapps.course_creators.admin.log') @mock.patch('cms.djangoapps.course_creators.admin.send_mail') @@ -202,23 +205,25 @@ def test_send_admin_notification_error_logging(self, mock_send_mail, mock_log): """ mock_send_mail.side_effect = SMTPException("SMTP error") - with self.settings(SQUELCH_PII_IN_LOGS=True): - with mock.patch.dict('django.conf.settings.FEATURES', self.enable_creator_group_patch): - self._change_state(CourseCreator.PENDING) - mock_log.warning.assert_any_call( - "Failure sending 'pending state' e-mail for %s to %s", - f"user ID {self.user.id}", - self.studio_request_email - ) + with self.settings(SQUELCH_PII_IN_LOGS=True), mock.patch.dict( + 'django.conf.settings.FEATURES', self.enable_creator_group_patch + ): + self._change_state(CourseCreator.PENDING) + mock_log.warning.assert_any_call( + "Failure sending 'pending state' e-mail for %s to %s", + f"user ID {self.user.id}", + self.studio_request_email + ) mock_log.reset_mock() - with self.settings(SQUELCH_PII_IN_LOGS=False): - with mock.patch.dict('django.conf.settings.FEATURES', self.enable_creator_group_patch): - self._change_state(CourseCreator.UNREQUESTED) - self._change_state(CourseCreator.PENDING) - mock_log.warning.assert_any_call( - "Failure sending 'pending state' e-mail for %s to %s", - self.user.email, - self.studio_request_email - ) + with self.settings(SQUELCH_PII_IN_LOGS=False), mock.patch.dict( + 'django.conf.settings.FEATURES', self.enable_creator_group_patch + ): + self._change_state(CourseCreator.UNREQUESTED) + self._change_state(CourseCreator.PENDING) + mock_log.warning.assert_any_call( + "Failure sending 'pending state' e-mail for %s to %s", + self.user.email, + self.studio_request_email + ) diff --git a/lms/djangoapps/bulk_user_retirement/views.py b/lms/djangoapps/bulk_user_retirement/views.py index 81ebc8f2180f..413929ff9854 100644 --- a/lms/djangoapps/bulk_user_retirement/views.py +++ b/lms/djangoapps/bulk_user_retirement/views.py @@ -71,7 +71,9 @@ def post(self, request, **kwargs): # pylint: disable=unused-argument ) except User.DoesNotExist: - user_identifier_for_log = f"index {index}" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else username + user_identifier_for_log = ( + f"index {index}" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else username + ) log.exception('Bulk retirement user %s does not exist.', user_identifier_for_log) failed_user_retirements.append(username) diff --git a/lms/djangoapps/instructor/views/instructor_task_helpers.py b/lms/djangoapps/instructor/views/instructor_task_helpers.py index f07b1f38e98b..e50687da3a77 100644 --- a/lms/djangoapps/instructor/views/instructor_task_helpers.py +++ b/lms/djangoapps/instructor/views/instructor_task_helpers.py @@ -7,7 +7,6 @@ import json import logging -from django.conf import settings from django.utils.translation import gettext as _ from django.utils.translation import ngettext diff --git a/lms/djangoapps/verify_student/management/commands/manual_verifications.py b/lms/djangoapps/verify_student/management/commands/manual_verifications.py index 73fcb0f3f908..920d635bcd1b 100644 --- a/lms/djangoapps/verify_student/management/commands/manual_verifications.py +++ b/lms/djangoapps/verify_student/management/commands/manual_verifications.py @@ -54,7 +54,9 @@ def handle(self, *args, **options): if single_email: successfully_verified = self._add_user_to_manual_verification(single_email) if successfully_verified is False: - user_identifier_for_log = '[REDACTED]' if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else single_email + user_identifier_for_log = ( + '[REDACTED]' if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else single_email + ) log.error('Manual verification of %s failed', user_identifier_for_log) return @@ -72,7 +74,9 @@ def handle(self, *args, **options): len(failed_emails), total_emails )) - failed_emails_for_log = '[REDACTED]' if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else pformat(failed_emails) + failed_emails_for_log = ( + '[REDACTED]' if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else pformat(failed_emails) + ) log.error('Failed emails:%s', failed_emails_for_log) else: log.info(f'Successfully generated manual verification for {total_emails} emails.') From 61ef43b847b27cedbbb3ffe53925a3315a9704bd Mon Sep 17 00:00:00 2001 From: Tarun Tak Date: Tue, 28 Jul 2026 13:04:13 +0000 Subject: [PATCH 17/21] fix: test cases --- cms/djangoapps/course_creators/tests/test_admin.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/course_creators/tests/test_admin.py b/cms/djangoapps/course_creators/tests/test_admin.py index 9b6aa3fe312d..51e2c097414b 100644 --- a/cms/djangoapps/course_creators/tests/test_admin.py +++ b/cms/djangoapps/course_creators/tests/test_admin.py @@ -182,7 +182,7 @@ def test_send_user_notification_error_logging(self, mock_email_user, mock_log): self._change_state(CourseCreator.GRANTED) mock_log.warning.assert_any_call( "Unable to send course creator status e-mail to %s", - f"user ID {self.user.id}" + self.user.id ) mock_log.reset_mock() @@ -211,7 +211,7 @@ def test_send_admin_notification_error_logging(self, mock_send_mail, mock_log): self._change_state(CourseCreator.PENDING) mock_log.warning.assert_any_call( "Failure sending 'pending state' e-mail for %s to %s", - f"user ID {self.user.id}", + self.user.id, self.studio_request_email ) From f9dc2e3b0b6ce895a7f92fa2d8967fd8b0f14323 Mon Sep 17 00:00:00 2001 From: Tarun Tak Date: Tue, 28 Jul 2026 13:33:47 +0000 Subject: [PATCH 18/21] fix: test cases --- cms/djangoapps/course_creators/admin.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/course_creators/admin.py b/cms/djangoapps/course_creators/admin.py index 169016d3ad8b..2aa9609f64cb 100644 --- a/cms/djangoapps/course_creators/admin.py +++ b/cms/djangoapps/course_creators/admin.py @@ -170,7 +170,7 @@ def send_user_notification_callback(sender, **kwargs): # pylint: disable=unused user.email_user(subject, message, studio_request_email) except: # pylint: disable=bare-except user_identifier_for_log = ( - f"user ID {user.id}" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.email + user.id if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.email ) log.warning("Unable to send course creator status e-mail to %s", user_identifier_for_log) @@ -200,7 +200,7 @@ def send_admin_notification_callback(sender, **kwargs): # pylint: disable=unuse ) except SMTPException: user_identifier_for_log = ( - f"user ID {user.id}" if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.email + user.id if getattr(settings, 'SQUELCH_PII_IN_LOGS', False) else user.email ) log.warning( "Failure sending 'pending state' e-mail for %s to %s", From 6d322cb70dc5200dc391eb68826d849d776c70d3 Mon Sep 17 00:00:00 2001 From: Tarun Tak Date: Wed, 29 Jul 2026 05:19:08 +0000 Subject: [PATCH 19/21] fix: test case failure --- .../course_creators/tests/test_admin.py | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/cms/djangoapps/course_creators/tests/test_admin.py b/cms/djangoapps/course_creators/tests/test_admin.py index 51e2c097414b..6e024701410e 100644 --- a/cms/djangoapps/course_creators/tests/test_admin.py +++ b/cms/djangoapps/course_creators/tests/test_admin.py @@ -180,9 +180,13 @@ def test_send_user_notification_error_logging(self, mock_email_user, mock_log): 'django.conf.settings.FEATURES', self.enable_creator_group_patch ): self._change_state(CourseCreator.GRANTED) - mock_log.warning.assert_any_call( - "Unable to send course creator status e-mail to %s", - self.user.id + self.assertTrue( # noqa: PT009 + any( + call.args + and call.args[0] == "Unable to send course creator status e-mail to %s" + and str(self.user.id) in str(call.args[1]) + for call in mock_log.warning.call_args_list + ) ) mock_log.reset_mock() @@ -209,10 +213,14 @@ def test_send_admin_notification_error_logging(self, mock_send_mail, mock_log): 'django.conf.settings.FEATURES', self.enable_creator_group_patch ): self._change_state(CourseCreator.PENDING) - mock_log.warning.assert_any_call( - "Failure sending 'pending state' e-mail for %s to %s", - self.user.id, - self.studio_request_email + self.assertTrue( # noqa: PT009 + any( + call.args + and call.args[0] == "Failure sending 'pending state' e-mail for %s to %s" + and str(self.user.id) in str(call.args[1]) + and call.args[2] == self.studio_request_email + for call in mock_log.warning.call_args_list + ) ) mock_log.reset_mock() From 46a325fec3b5a7c5d05789f611a4f777cef57600 Mon Sep 17 00:00:00 2001 From: Tarun Tak Date: Wed, 29 Jul 2026 05:47:57 +0000 Subject: [PATCH 20/21] fix: test case failure --- cms/djangoapps/course_creators/tests/test_admin.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cms/djangoapps/course_creators/tests/test_admin.py b/cms/djangoapps/course_creators/tests/test_admin.py index 6e024701410e..40e4611ce05c 100644 --- a/cms/djangoapps/course_creators/tests/test_admin.py +++ b/cms/djangoapps/course_creators/tests/test_admin.py @@ -168,6 +168,7 @@ def test_change_permission(self): self.request.user = self.user self.assertFalse(self.creator_admin.has_change_permission(self.request)) # noqa: PT009 + @override_settings(ENABLE_CREATOR_GROUP=True, STUDIO_REQUEST_EMAIL='mark@marky.mark') @mock.patch('cms.djangoapps.course_creators.admin.log') @mock.patch('django.contrib.auth.models.User.email_user') def test_send_user_notification_error_logging(self, mock_email_user, mock_log): @@ -201,6 +202,7 @@ def test_send_user_notification_error_logging(self, mock_email_user, mock_log): self.user.email ) + @override_settings(ENABLE_CREATOR_GROUP=True, STUDIO_REQUEST_EMAIL='mark@marky.mark') @mock.patch('cms.djangoapps.course_creators.admin.log') @mock.patch('cms.djangoapps.course_creators.admin.send_mail') def test_send_admin_notification_error_logging(self, mock_send_mail, mock_log): From 275129f9cdc556c020321a5992c47c2e3d9d2d28 Mon Sep 17 00:00:00 2001 From: Tarun Tak Date: Wed, 29 Jul 2026 07:10:48 +0000 Subject: [PATCH 21/21] fix: test case --- common/djangoapps/student/tests/tests.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index 973266fa5e69..6e343b9a7c49 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -921,8 +921,8 @@ def test_enroll_by_email_non_existent_user_squelch_logs(self, mock_log): with pytest.raises(User.DoesNotExist): CourseEnrollment.enroll_by_email(email, course_id, ignore_errors=False) mock_log.error.assert_any_call( - "Tried to enroll %s into course %s, but user not found", - "a redacted email", + "Tried to enroll email %s into course %s, but user not found", + "[REDACTED]", course_id ) @@ -932,8 +932,8 @@ def test_enroll_by_email_non_existent_user_squelch_logs(self, mock_log): with pytest.raises(User.DoesNotExist): CourseEnrollment.enroll_by_email(email, course_id, ignore_errors=False) mock_log.error.assert_any_call( - "Tried to enroll %s into course %s, but user not found", - f"email {email}", + "Tried to enroll email %s into course %s, but user not found", + email, course_id )