Merge pull request #26460 from edx/feanil/always_squelch

feat: Update the user_authn app to not log PII by default.

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 "<unknown>"
-            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 "<unknown>"
+        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)

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)