diff --git a/openedx/core/djangoapps/user_authn/views/login.py b/openedx/core/djangoapps/user_authn/views/login.py index 4b24c44ad2..6f17cb9c3e 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/openedx/core/djangoapps/user_authn/views/login.py @@ -6,6 +6,7 @@ Much of this file was broken out from views.py, previous history can be found th import json import logging +import hashlib import six from django.conf import settings @@ -103,10 +104,8 @@ def _get_user_by_email(request): try: return User.objects.get(email=email) except User.DoesNotExist: - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - AUDIT_LOG.warning(u"Login failed - Unknown user email") - else: - AUDIT_LOG.warning(u"Login failed - Unknown user email: {0}".format(email)) + digest = hashlib.shake_128(email.encode('utf-8')).hexdigest(16) # pylint: disable=too-many-function-args + AUDIT_LOG.warning(f"Login failed - Unknown user email {digest}") def _check_excessive_login_attempts(user): @@ -161,15 +160,9 @@ def _log_and_raise_inactive_user_auth_error(unauthenticated_user): Depending on Django version we can get here a couple of ways, but this takes care of logging an auth attempt by an inactive user, re-sending the activation email, and raising an error with the correct message. """ - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - AUDIT_LOG.warning( - u"Login failed - Account not active for user.id: {0}, resending activation".format( - unauthenticated_user.id) - ) - else: - AUDIT_LOG.warning(u"Login failed - Account not active for user {0}, resending activation".format( - unauthenticated_user.username) - ) + AUDIT_LOG.warning( + f"Login failed - Account not active for user.id: {unauthenticated_user.id}, resending activation" + ) profile = UserProfile.objects.get(user=unauthenticated_user) compose_and_send_activation_email(unauthenticated_user, profile) @@ -224,11 +217,8 @@ def _handle_failed_authentication(user, authenticated_user): # if we didn't find this username earlier, the account for this email # doesn't exist, and doesn't have a corresponding password - if settings.FEATURES['SQUELCH_PII_IN_LOGS']: - loggable_id = user.id if user else "" - AUDIT_LOG.warning(u"Login failed - password for user.id: {0} is invalid".format(loggable_id)) - else: - AUDIT_LOG.warning(u"Login failed - password for {0} is invalid".format(user.email)) + loggable_id = user.id if user else "" + AUDIT_LOG.warning(f"Login failed - password for user.id: {loggable_id} is invalid") if user and LoginFailures.is_feature_enabled(): blocked_threshold, failure_count = LoginFailures.check_user_reset_password_threshold(user) 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 dcbcb4e50f..338248bb49 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_login.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_login.py @@ -5,6 +5,7 @@ Tests for student activation and login import datetime +import hashlib import json import unicodedata @@ -200,6 +201,8 @@ class LoginTest(SiteMixin, CacheIsolationTestCase): def test_login_fail_no_user_exists(self): nonexistent_email = u'not_a_user@edx.org' + # pylint: disable=too-many-function-args + email_hash = hashlib.shake_128(nonexistent_email.encode('utf-8')).hexdigest(16) response, mock_audit_log = self._login_response( nonexistent_email, self.password, @@ -207,7 +210,7 @@ class LoginTest(SiteMixin, CacheIsolationTestCase): self._assert_response( response, success=False, value=self.LOGIN_FAILED_WARNING, status_code=400 ) - self._assert_audit_log(mock_audit_log, 'warning', [u'Login failed', u'Unknown user email', nonexistent_email]) + self._assert_audit_log(mock_audit_log, 'warning', [u'Login failed', u'Unknown user email', email_hash]) @patch.dict("django.conf.settings.FEATURES", {'SQUELCH_PII_IN_LOGS': True}) def test_login_fail_no_user_exists_no_pii(self): @@ -227,16 +230,17 @@ class LoginTest(SiteMixin, CacheIsolationTestCase): ) self._assert_response(response, success=False, value=self.LOGIN_FAILED_WARNING) self._assert_audit_log(mock_audit_log, 'warning', - [u'Login failed', u'password for', self.user_email, u'invalid']) + [u'Login failed', u'password for', str(self.user.id), u'invalid']) @patch.dict("django.conf.settings.FEATURES", {'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) - self._assert_audit_log(mock_audit_log, 'warning', [u'Login failed', u'password for', u'invalid']) + self._assert_audit_log( + mock_audit_log, 'warning', [u'Login failed', u'password for', str(self.user.id), u'invalid'] + ) self._assert_not_in_audit_log(mock_audit_log, 'warning', [self.user_email]) - @patch.dict("django.conf.settings.FEATURES", {'SQUELCH_PII_IN_LOGS': True}) def test_login_not_activated_no_pii(self): # De-activate the user self.user.is_active = False @@ -273,22 +277,26 @@ class LoginTest(SiteMixin, CacheIsolationTestCase): the system does *not* send account activation email notification to the user. """ nonexistent_email = 'incorrect@email.com' + # pylint: disable=too-many-function-args + email_hash = hashlib.shake_128(nonexistent_email.encode('utf-8')).hexdigest(16) self.user.is_active = False self.user.save() response, mock_audit_log = self._login_response(nonexistent_email, 'incorrect_password') self.assertFalse(mock_inactive_user_email_and_error.called) self._assert_response(response, success=False, value=self.LOGIN_FAILED_WARNING) - self._assert_audit_log(mock_audit_log, 'warning', [u'Login failed', u'Unknown user email', nonexistent_email]) + self._assert_audit_log(mock_audit_log, 'warning', [u'Login failed', u'Unknown user email', email_hash]) def test_login_unicode_email(self): unicode_email = self.user_email + six.unichr(40960) + # pylint: disable=too-many-function-args + email_hash = hashlib.shake_128(unicode_email.encode('utf-8')).hexdigest(16) response, mock_audit_log = self._login_response( unicode_email, self.password, ) self._assert_response(response, success=False) - self._assert_audit_log(mock_audit_log, 'warning', [u'Login failed', unicode_email]) + self._assert_audit_log(mock_audit_log, 'warning', [u'Login failed', email_hash]) def test_login_unicode_password(self): unicode_password = self.password + six.unichr(1972) @@ -298,7 +306,7 @@ class LoginTest(SiteMixin, CacheIsolationTestCase): ) self._assert_response(response, success=False) self._assert_audit_log(mock_audit_log, 'warning', - [u'Login failed', u'password for', self.user_email, u'invalid']) + [u'Login failed', u'password for', str(self.user.id), u'invalid']) def test_logout_logging(self): response, _ = self._login_response(self.user_email, self.password)