diff --git a/cms/djangoapps/course_creators/admin.py b/cms/djangoapps/course_creators/admin.py index 2d7d2797c335..2aa9609f64cb 100644 --- a/cms/djangoapps/course_creators/admin.py +++ b/cms/djangoapps/course_creators/admin.py @@ -169,7 +169,10 @@ 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) + user_identifier_for_log = ( + 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) @@ -179,6 +182,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. context = course_creator_notification_context(user) studio_request_email = context['studio_request_email'] @@ -195,7 +199,14 @@ 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) + user_identifier_for_log = ( + 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 5ed7b87dc95b..40e4611ce05c 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 @@ -55,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') @@ -165,3 +167,73 @@ 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): + """ + 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), mock.patch.dict( + 'django.conf.settings.FEATURES', self.enable_creator_group_patch + ): + self._change_state(CourseCreator.GRANTED) + 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() + + 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 + ) + + @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): + """ + 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), mock.patch.dict( + 'django.conf.settings.FEATURES', self.enable_creator_group_patch + ): + self._change_state(CourseCreator.PENDING) + 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() + + 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/common/djangoapps/student/emails.py b/common/djangoapps/student/emails.py index 6b4641722de6..1615c16234f6 100644 --- a/common/djangoapps/student/emails.py +++ b/common/djangoapps/student/emails.py @@ -27,8 +27,10 @@ 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) + 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 - log.exception('Could not send email for proctoring requirements to user %s', 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 99140d051cb6..02f1bd4bac9c 100644 --- a/common/djangoapps/student/models/course_enrollment.py +++ b/common/djangoapps/student/models/course_enrollment.py @@ -664,10 +664,13 @@ 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 + ) log.exception( 'Unable to emit event %s for user %s and course %s', event_name, - self.user.username, + user_identifier_for_log, self.course_id, ) @@ -731,39 +734,55 @@ 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( 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 + 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 + ) log.warning( "User %s failed to enroll in course %s because enrollment is closed (can_upgrade=%s).", - user.username, + user_identifier_for_log, str(course_key), can_upgrade, ) 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 + ) 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, + 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.username, - str(course_key) + user_identifier_for_log, + str(course_key), ) if check_access: raise AlreadyEnrolledError @@ -828,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: - err_msg = "Tried to enroll email {} into course {}, but user not found" - log.error(err_msg.format(email, 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 4607cb40a8f7..f9873e4b60cc 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 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) class PendingNameChange(DeletableByUserValue, models.Model): # noqa: DJ008 @@ -1313,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 = ( + 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) @@ -1330,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 = ( + 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/tests/tests.py b/common/djangoapps/student/tests/tests.py index 20819b6b8859..6e343b9a7c49 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 email %s into course %s, but user not found", + "[REDACTED]", + 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 email %s into course %s, but user not found", + 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 4387f44d6c10..d19cb94836b3 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -489,7 +489,8 @@ 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) + 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: @@ -557,11 +558,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 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) - log.info("%s reenabled %s's account", request.user, username) + 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) else: context['message'] = _("Unexpected account status") return JsonResponse(context, status=400) @@ -847,11 +854,12 @@ 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) - except Exception: + 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) - raise ValueError(_('Unable to send email activation link. Please try again later.')) # pylint: disable=raise-missing-from # noqa: B904 + 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. diff --git a/lms/djangoapps/bulk_user_retirement/views.py b/lms/djangoapps/bulk_user_retirement/views.py index 28f23651fb65..413929ff9854 100644 --- a/lms/djangoapps/bulk_user_retirement/views.py +++ b/lms/djangoapps/bulk_user_retirement/views.py @@ -3,6 +3,7 @@ """ import logging +from django.conf import settings from django.contrib.auth import get_user_model from django.db import transaction from rest_framework import permissions, status @@ -51,16 +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) - log.info(f'The user "{username}" has been added to the retirement pipeline \ - by "{request.user}"') + 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, + index, + ) + else: + log.info('The user "%s" has been added to the retirement pipeline by "%s" at index %s', + username, + request.user, + index, + ) except User.DoesNotExist: - log.exception(f'The user "{username}" does not exist.') + 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 b9aac5e8efaa..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,8 @@ 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}') + 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: 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..5de089821616 100644 --- a/lms/djangoapps/courseware/model_data.py +++ b/lms/djangoapps/courseware/model_data.py @@ -27,6 +27,7 @@ from abc import ABCMeta, abstractmethod from collections import defaultdict, namedtuple +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 @@ -404,9 +405,13 @@ def set_many(self, kv_dict): self.user.username, pending_updates ) - except DatabaseError: - log.exception("Saving user state failed for %s", self.user.username) - raise KeyValueMultiSaveError([]) # pylint: disable=raise-missing-from # noqa: B904 + except DatabaseError as err: + user_identifier_for_log = ( + 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 finally: self._cache.update(pending_updates) diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 027f0d4f6591..94866efc9410 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) + user_identifier_for_log = ( + 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) diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index c4c1a6c0b050..94671db22ce5 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): diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 3b66a0dac654..4a483d41220f 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -462,13 +462,16 @@ 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 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: - 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/verify_student/management/commands/manual_verifications.py b/lms/djangoapps/verify_student/management/commands/manual_verifications.py index bb84fabb2713..920d635bcd1b 100644 --- a/lms/djangoapps/verify_student/management/commands/manual_verifications.py +++ b/lms/djangoapps/verify_student/management/commands/manual_verifications.py @@ -7,6 +7,7 @@ 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 @@ -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') + 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'] @@ -70,7 +74,10 @@ def handle(self, *args, **options): len(failed_emails), total_emails )) - 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.') @@ -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 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.') 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,6 @@ 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') + 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 6a012d8d212d..8629b1708914 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/openedx/core/djangoapps/user_authn/views/login.py @@ -199,7 +199,8 @@ 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) + 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, { 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 027d583e6a96..384dfe66be2b 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_login.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_login.py @@ -312,7 +312,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' @@ -344,7 +344,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( @@ -364,7 +364,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) @@ -534,7 +534,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)