Merge pull request #19051 from edx/cstenson/unicode_normalization

Add unicode normalization to passwords.

common/djangoapps/student/helpers.py

@@ -35,6 +35,7 @@ from lms.djangoapps.verify_student.utils import is_verification_expiring_soon, v
 from openedx.core.djangoapps.certificates.api import certificates_viewable_for_course
 from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
 from openedx.core.djangoapps.theming import helpers as theming_helpers
+from openedx.core.djangoapps.user_api.config.waffle import PASSWORD_UNICODE_NORMALIZE_FLAG
 from openedx.core.djangoapps.theming.helpers import get_themes
 from openedx.core.djangoapps.user_authn.utils import is_safe_login_or_logout_redirect
 from student.models import (
@@ -47,6 +48,7 @@ from student.models import (
     email_exists_or_retired,
     username_exists_or_retired
 )
+from util.password_policy_validators import normalize_password
 
 
 # Enumeration of per-course verification statuses
@@ -412,6 +414,8 @@ def authenticate_new_user(request, username, password):
     logged in until they close the browser. They can't log in again until they click
     the activation link from the email.
     """
+    if PASSWORD_UNICODE_NORMALIZE_FLAG.is_enabled():
+        password = normalize_password(password)
     backend = load_backend(NEW_USER_AUTH_BACKEND)
     user = backend.authenticate(request=request, username=username, password=password)
     user.backend = NEW_USER_AUTH_BACKEND
@@ -610,7 +614,10 @@ def do_create_account(form, custom_form=None):
         email=form.cleaned_data["email"],
         is_active=False
     )
-    user.set_password(form.cleaned_data["password"])
+    password = form.cleaned_data["password"]
+    if PASSWORD_UNICODE_NORMALIZE_FLAG.is_enabled():
+        password = normalize_password(password)
+    user.set_password(password)
     registration = Registration()
 
     # TODO: Rearrange so that if part of the process fails, the whole process fails.

common/djangoapps/util/password_policy_validators.py

@@ -7,7 +7,6 @@ from __future__ import unicode_literals
 import logging
 import unicodedata
 
-from django.conf import settings
 from django.contrib.auth.password_validation import (
     get_default_password_validators,
     validate_password as django_validate_password,
@@ -15,6 +14,7 @@ from django.contrib.auth.password_validation import (
 )
 from django.core.exceptions import ValidationError
 from django.utils.translation import ugettext as _, ungettext
+from openedx.core.djangoapps.user_api.config.waffle import PASSWORD_UNICODE_NORMALIZE_FLAG
 from six import text_type
 
 log = logging.getLogger(__name__)
@@ -89,6 +89,14 @@ def password_validators_restrictions():
     return complexity_restrictions
 
 
+def normalize_password(password):
+    """
+    Normalize all passwords to 'NFKC' across the platform to prevent mismatched hash strings when comparing entered
+    passwords on login. See LEARNER-4283 for more context.
+    """
+    return unicodedata.normalize('NFKC', password)
+
+
 def validate_password(password, user=None):
     """
     EdX's custom password validator for passwords. This function performs the
@@ -117,6 +125,8 @@ def validate_password(password, user=None):
             # no reason to get into weeds
             raise ValidationError([_('Invalid password.')])
 
+    if PASSWORD_UNICODE_NORMALIZE_FLAG.is_enabled():
+        password = normalize_password(password)
     django_validate_password(password, user)
 
 

common/djangoapps/util/tests/test_password_policy_validators.py

@@ -1,6 +1,5 @@
 # -*- coding: utf-8 -*-
 """Tests for util.password_policy_validators module."""
-
 import mock
 import unittest
 
@@ -10,13 +9,15 @@ from django.contrib.auth.models import User
 from django.core.exceptions import ValidationError
 from django.test.utils import override_settings
 
+from openedx.core.djangoapps.user_api.config.waffle import PASSWORD_UNICODE_NORMALIZE_FLAG
+from openedx.core.djangolib.testing.utils import CacheIsolationTestCase
 from util.password_policy_validators import (
     create_validator_config, validate_password, password_validators_instruction_texts,
 )
 
 
 @ddt
-class PasswordPolicyValidatorsTestCase(unittest.TestCase):
+class PasswordPolicyValidatorsTestCase(CacheIsolationTestCase):
     """
     Tests for password validator utility functions
 
@@ -25,7 +26,6 @@ class PasswordPolicyValidatorsTestCase(unittest.TestCase):
         2) requiring multiple instances of the check (also checks proper plural message)
         3) successful check
     """
-
     def validation_errors_checker(self, password, msg, user=None):
         """
         This helper function is used to check the proper error messages are
@@ -61,6 +61,20 @@ class PasswordPolicyValidatorsTestCase(unittest.TestCase):
         # Test badly encoded password
         self.validation_errors_checker(b'\xff\xff', 'Invalid password.')
 
+    def test_password_unicode_normalization(self):
+        """ Tests that validate_password normalizes passwords """
+        # s ̣ ̇ (s with combining dot below and combining dot above)
+        not_normalized_password = u'\u0073\u0323\u0307'
+        self.assertEqual(len(not_normalized_password), 3)
+        # When the flag is not set, the validation should succeed since len > 2
+        self.validation_errors_checker(not_normalized_password, None)
+
+        # When we normalize we expect the not_normalized password to fail
+        # because it should be normalized to u'\u1E69' -> ṩ
+        with PASSWORD_UNICODE_NORMALIZE_FLAG.override(active=True):
+            self.validation_errors_checker(not_normalized_password,
+                                           'This password is too short. It must contain at least 2 characters.')
+
     @data(
         ([create_validator_config('util.password_policy_validators.MinimumLengthValidator', {'min_length': 2})],
             'at least 2 characters.'),

openedx/core/djangoapps/user_api/accounts/api.py

@@ -18,11 +18,16 @@ from student.models import User, UserProfile, Registration, email_exists_or_reti
 from student import forms as student_forms
 from student import views as student_views
 from util.model_utils import emit_setting_changed_event
-from util.password_policy_validators import validate_password
+from util.password_policy_validators import validate_password, normalize_password
 
 from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
 from openedx.core.djangoapps.user_api import errors, accounts, forms, helpers
-from openedx.core.djangoapps.user_api.config.waffle import PREVENT_AUTH_USER_WRITES, SYSTEM_MAINTENANCE_MSG, waffle
+from openedx.core.djangoapps.user_api.config.waffle import (
+    PASSWORD_UNICODE_NORMALIZE_FLAG,
+    PREVENT_AUTH_USER_WRITES,
+    SYSTEM_MAINTENANCE_MSG,
+    waffle,
+)
 from openedx.core.djangoapps.user_api.errors import (
     AccountUpdateError,
     AccountValidationError,
@@ -331,6 +336,8 @@ def create_account(username, password, email):
 
     # Create the user account, setting them to "inactive" until they activate their account.
     user = User(username=username, email=email, is_active=False)
+    if PASSWORD_UNICODE_NORMALIZE_FLAG.is_enabled():
+        password = normalize_password(password)
     user.set_password(password)
 
     try:

openedx/core/djangoapps/user_api/accounts/tests/test_api.py

@@ -5,11 +5,13 @@ Most of the functionality is covered in test_views.py.
 """
 
 import re
+import unicodedata
 
 import ddt
 import pytest
 from dateutil.parser import parse as parse_datetime
 from django.conf import settings
+from django.contrib.auth.hashers import make_password
 from django.contrib.auth.models import User
 from django.core import mail
 from django.test import TestCase
@@ -27,18 +29,23 @@ from openedx.core.djangoapps.user_api.accounts.api import (
     request_password_change,
     update_account_settings
 )
+from openedx.core.djangoapps.user_api.accounts.tests.retirement_helpers import (  # pylint: disable=unused-import
+    RetirementTestCase,
+    fake_requested_retirement,
+    setup_retirement_states
+)
 from openedx.core.djangoapps.user_api.accounts.tests.testutils import (
     INVALID_EMAILS,
     INVALID_PASSWORDS,
     INVALID_USERNAMES,
     VALID_USERNAMES_UNICODE
 )
-from openedx.core.djangoapps.user_api.accounts.tests.retirement_helpers import (  # pylint: disable=unused-import
-    RetirementTestCase,
-    fake_requested_retirement,
-    setup_retirement_states
+from openedx.core.djangoapps.user_api.config.waffle import (
+    PASSWORD_UNICODE_NORMALIZE_FLAG,
+    PREVENT_AUTH_USER_WRITES,
+    SYSTEM_MAINTENANCE_MSG,
+    waffle
 )
-from openedx.core.djangoapps.user_api.config.waffle import PREVENT_AUTH_USER_WRITES, SYSTEM_MAINTENANCE_MSG, waffle
 from openedx.core.djangoapps.user_api.errors import (
     AccountEmailInvalid,
     AccountPasswordInvalid,
@@ -51,6 +58,7 @@ from openedx.core.djangoapps.user_api.errors import (
     UserNotAuthorized,
     UserNotFound
 )
+from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag
 from openedx.core.djangolib.testing.utils import skip_unless_lms
 from openedx.core.lib.tests import attr
 from student.models import PendingEmailChange
@@ -317,7 +325,6 @@ class AccountSettingsOnCreationTest(TestCase):
     def test_create_account(self):
         # Create a new account, which should have empty account settings by default.
         create_account(self.USERNAME, self.PASSWORD, self.EMAIL)
-
         # Retrieve the account settings
         user = User.objects.get(username=self.USERNAME)
         request = RequestFactory().get("/api/user/v1/accounts/")
@@ -354,6 +361,22 @@ class AccountSettingsOnCreationTest(TestCase):
             'extended_profile': [],
         })
 
+    @override_waffle_flag(PASSWORD_UNICODE_NORMALIZE_FLAG, active=True)
+    def test_normalize_password(self):
+        """
+        Test that unicode normalization on passwords is happening when a user is created.
+        """
+        # Set user password to NFKD format so that we can test that it is normalized to
+        # NFKC format upon account creation.
+        create_account(self.USERNAME, unicodedata.normalize('NFKD', u'Ṗŕệṿïệẅ Ṯệẍt'), self.EMAIL)
+
+        user = User.objects.get(username=self.USERNAME)
+
+        salt_val = user.password.split('$')[1]
+
+        expected_user_password = make_password(unicodedata.normalize('NFKC', u'Ṗŕệṿïệẅ Ṯệẍt'), salt_val)
+        self.assertEqual(expected_user_password, user.password)
+
 
 @attr(shard=2)
 @pytest.mark.django_db

openedx/core/djangoapps/user_api/config/waffle.py

@@ -5,10 +5,12 @@ from __future__ import absolute_import
 
 from django.utils.translation import ugettext_lazy as _
 
-from openedx.core.djangoapps.waffle_utils import WaffleSwitchNamespace
+from openedx.core.djangoapps.waffle_utils import WaffleSwitchNamespace, WaffleFlagNamespace, WaffleFlag
 
 SYSTEM_MAINTENANCE_MSG = _(u'System maintenance in progress. Please try again later.')
 WAFFLE_NAMESPACE = u'user_api'
+_WAFFLE_FLAG_NAMESPACE = WaffleFlagNamespace(WAFFLE_NAMESPACE)
+PASSWORD_UNICODE_NORMALIZE_FLAG = WaffleFlag(_WAFFLE_FLAG_NAMESPACE, u'password_unicode_normalize')
 
 # Switches
 PREVENT_AUTH_USER_WRITES = u'prevent_auth_user_writes'

openedx/core/djangoapps/user_authn/views/login.py

@@ -27,6 +27,7 @@ from openedx.core.djangoapps.external_auth.models import ExternalAuthMap
 from openedx.core.djangoapps.password_policy import compliance as password_policy_compliance
 from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
 from openedx.core.djangoapps.util.user_messages import PageLevelMessages
+from openedx.core.djangoapps.user_api.config.waffle import PASSWORD_UNICODE_NORMALIZE_FLAG
 from student.models import (
     LoginFailures,
     PasswordHistory,
@@ -36,6 +37,7 @@ from student.forms import send_password_reset_email_for_user
 import third_party_auth
 from third_party_auth import pipeline, provider
 from util.json_request import JsonResponse
+from util.password_policy_validators import normalize_password
 
 log = logging.getLogger("edx.student")
 AUDIT_LOG = logging.getLogger("audit")
@@ -211,10 +213,14 @@ def _authenticate_first_party(request, unauthenticated_user):
     username = unauthenticated_user.username if unauthenticated_user else ""
 
     try:
+        password = request.POST['password']
+        if PASSWORD_UNICODE_NORMALIZE_FLAG.is_enabled():
+            password = normalize_password(password)
         return authenticate(
             username=username,
-            password=request.POST['password'],
-            request=request)
+            password=password,
+            request=request
+        )
 
     # This occurs when there are too many attempts from the same IP address
     except RateLimitException:

openedx/core/djangoapps/user_authn/views/tests/test_login.py

@@ -1,35 +1,43 @@
+#coding:utf-8
 """
 Tests for student activation and login
 """
 import json
+import unicodedata
 import unittest
 
+import ddt
 from django.conf import settings
 from django.contrib.auth.models import User
-from django.core.cache import cache
 from django.core import mail
-from django.urls import NoReverseMatch, reverse
+from django.core.cache import cache
 from django.http import HttpResponse, HttpResponseBadRequest
 from django.test.client import Client
 from django.test.utils import override_settings
+from django.urls import NoReverseMatch, reverse
 from mock import patch
 from six import text_type
 
 from openedx.core.djangoapps.external_auth.models import ExternalAuthMap
-from openedx.core.djangoapps.user_api.config.waffle import PREVENT_AUTH_USER_WRITES, waffle
-from openedx.core.djangoapps.user_authn.cookies import jwt_cookies
-from openedx.core.djangoapps.user_authn.tests.utils import setup_login_oauth_client
-from openedx.core.djangoapps.user_authn.waffle import JWT_COOKIES_FLAG
 from openedx.core.djangoapps.password_policy.compliance import (
     NonCompliantPasswordException,
     NonCompliantPasswordWarning
 )
+from openedx.core.djangoapps.user_api.config.waffle import (
+    PASSWORD_UNICODE_NORMALIZE_FLAG,
+    PREVENT_AUTH_USER_WRITES,
+    waffle
+)
+from openedx.core.djangoapps.user_authn.cookies import jwt_cookies
+from openedx.core.djangoapps.user_authn.tests.utils import setup_login_oauth_client
+from openedx.core.djangoapps.user_authn.waffle import JWT_COOKIES_FLAG
 from openedx.core.djangolib.testing.utils import CacheIsolationTestCase
 from student.tests.factories import RegistrationFactory, UserFactory, UserProfileFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
 
 
+@ddt.ddt
 class LoginTest(CacheIsolationTestCase):
     """
     Test login_user() view
@@ -482,6 +490,33 @@ class LoginTest(CacheIsolationTestCase):
             self.assertIn('Test warning', self.client.session['_messages'])
         self.assertTrue(response_content.get('success'))
 
+    @ddt.data(
+        ('test_password', 'test_password', True, True),
+        (unicodedata.normalize('NFKD', u'Ṗŕệṿïệẅ Ṯệẍt'),
+         unicodedata.normalize('NFKC', u'Ṗŕệṿïệẅ Ṯệẍt'), False, True),
+        (unicodedata.normalize('NFKC', u'Ṗŕệṿïệẅ Ṯệẍt'),
+         unicodedata.normalize('NFKD', u'Ṗŕệṿïệẅ Ṯệẍt'), True, True),
+        (unicodedata.normalize('NFKD', u'Ṗŕệṿïệẅ Ṯệẍt'),
+         unicodedata.normalize('NFKD', u'Ṗŕệṿïệẅ Ṯệẍt'), False, True),
+        ('test_password', 'test_password', True, False),
+        (unicodedata.normalize('NFKD', u'Ṗŕệṿïệẅ Ṯệẍt'),
+         unicodedata.normalize('NFKC', u'Ṗŕệṿïệẅ Ṯệẍt'), False, False),
+        (unicodedata.normalize('NFKC', u'Ṗŕệṿïệẅ Ṯệẍt'),
+         unicodedata.normalize('NFKD', u'Ṗŕệṿïệẅ Ṯệẍt'), False, False),
+        (unicodedata.normalize('NFKD', u'Ṗŕệṿïệẅ Ṯệẍt'),
+         unicodedata.normalize('NFKD', u'Ṗŕệṿïệẅ Ṯệẍt'), True, False),
+    )
+    @ddt.unpack
+    def test_password_unicode_normalization_login(self, password, password_entered, login_success, waffle_flag):
+        """
+        Tests unicode normalization on user's passwords on login.
+        """
+        with PASSWORD_UNICODE_NORMALIZE_FLAG.override(active=waffle_flag):
+            self.user.set_password(password)
+            self.user.save()
+            response, _ = self._login_response(self.user.email, password_entered)
+            self._assert_response(response, success=login_success)
+
     def _login_response(self, email, password, patched_audit_log=None, extra_post_params=None):
         """
         Post the login info

openedx/core/djangoapps/user_authn/views/tests/test_register.py

@@ -4,6 +4,7 @@ import json
 import unittest
 from datetime import datetime
 from importlib import import_module
+import unicodedata
 
 import ddt
 import mock
@@ -14,6 +15,7 @@ from django.urls import reverse
 from django.test import TestCase, TransactionTestCase
 from django.test.client import RequestFactory
 from django.test.utils import override_settings
+from django.contrib.auth.hashers import make_password
 
 from django_comment_common.models import ForumsConfig
 from notification_prefs import NOTIFICATION_PREF_KEY
@@ -23,12 +25,15 @@ from openedx.core.djangoapps.user_authn.views.register import (
     _skip_activation_email,
 )
 from openedx.core.djangoapps.external_auth.models import ExternalAuthMap
+from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag
 from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY
 from openedx.core.djangoapps.site_configuration.tests.mixins import SiteMixin
 from openedx.core.djangoapps.user_api.accounts import (
     USERNAME_BAD_LENGTH_MSG, USERNAME_INVALID_CHARS_ASCII, USERNAME_INVALID_CHARS_UNICODE
 )
-from openedx.core.djangoapps.user_api.config.waffle import PREVENT_AUTH_USER_WRITES, waffle
+from openedx.core.djangoapps.user_api.config.waffle import (
+    PASSWORD_UNICODE_NORMALIZE_FLAG, PREVENT_AUTH_USER_WRITES, waffle
+)
 from openedx.core.djangoapps.user_api.preferences.api import get_user_preference
 from student.models import UserAttribute
 from student.tests.factories import UserFactory
@@ -130,6 +135,23 @@ class TestCreateAccount(SiteMixin, TestCase):
         user = User.objects.get(username=self.username)
         return user.profile
 
+    @override_waffle_flag(PASSWORD_UNICODE_NORMALIZE_FLAG, active=True)
+    def test_create_account_and_normalize_password(self):
+        """
+        Test that unicode normalization on passwords is happening when a user registers.
+        """
+        # Set user password to NFKD format so that we can test that it is normalized to
+        # NFKC format upon account creation.
+        self.params['password'] = unicodedata.normalize('NFKD', u'Ṗŕệṿïệẅ Ṯệẍt')
+        response = self.client.post(self.url, self.params)
+        self.assertEqual(response.status_code, 200)
+
+        user = User.objects.get(username=self.username)
+        salt_val = user.password.split('$')[1]
+
+        expected_user_password = make_password(unicodedata.normalize('NFKC', u'Ṗŕệṿïệẅ Ṯệẍt'), salt_val)
+        self.assertEqual(expected_user_password, user.password)
+
     def test_marketing_cookie(self):
         response = self.client.post(self.url, self.params)
         self.assertEqual(response.status_code, 200)