Merge pull request #19114 from edx/ddumesnil/unicode_fix

Adding normalization to password reset

common/djangoapps/student/tests/test_reset_password.py

@@ -3,11 +3,12 @@ Test the various password reset flows
 """
 import json
 import re
+import unicodedata
 import unittest
 
 import ddt
 from django.conf import settings
-from django.contrib.auth.hashers import UNUSABLE_PASSWORD_PREFIX
+from django.contrib.auth.hashers import UNUSABLE_PASSWORD_PREFIX, make_password
 from django.contrib.auth.models import User
 from django.contrib.auth.tokens import default_token_generator
 from django.core.cache import cache
@@ -24,7 +25,9 @@ from provider.oauth2 import models as dop_models
 from openedx.core.djangoapps.oauth_dispatch.tests import factories as dot_factories
 from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
 from openedx.core.djangoapps.user_api.models import UserRetirementRequest
-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.djangolib.testing.utils import CacheIsolationTestCase
 from student.tests.factories import UserFactory
 from student.tests.test_email import mock_render_to_string
@@ -351,6 +354,28 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
                 self.user.refresh_from_db()
                 assert not self.user.is_active
 
+    def test_password_reset_normalize_password(self):
+        """
+        Tests that if we provide a not properly normalized password, it is saved using our normalization
+        method of NFKC.
+        In this test, the input password is u'p\u212bssword'. It should be normalized to u'p\xc5ssword'
+        """
+        with PASSWORD_UNICODE_NORMALIZE_FLAG.override(active=True):
+            url = reverse(
+                "password_reset_confirm",
+                kwargs={"uidb36": self.uidb36, "token": self.token}
+            )
+
+            password = u'p\u212bssword'
+            request_params = {'new_password1': password, 'new_password2': password}
+            confirm_request = self.request_factory.post(url, data=request_params)
+            response = password_reset_confirm_wrapper(confirm_request, self.uidb36, self.token)
+
+            user = User.objects.get(pk=self.user.pk)
+            salt_val = user.password.split('$')[1]
+            expected_user_password = make_password(unicodedata.normalize('NFKC', u'p\u212bssword'), salt_val)
+            self.assertEqual(expected_user_password, user.password)
+
     @override_settings(AUTH_PASSWORD_VALIDATORS=[
         create_validator_config('util.password_policy_validators.MinimumLengthValidator', {'min_length': 2}),
         create_validator_config('util.password_policy_validators.MaximumLengthValidator', {'max_length': 10})

common/djangoapps/student/views/management.py

@@ -57,7 +57,9 @@ from openedx.core.djangoapps.programs.models import ProgramsApiConfig
 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.theming.helpers import get_current_site
-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 UserNotFound, UserAPIInternalError
 from openedx.core.djangoapps.user_api.models import UserRetirementRequest
 from openedx.core.djangoapps.user_api.preferences import api as preferences_api
@@ -94,7 +96,7 @@ from student.text_me_the_app import TextMeTheAppFragmentView
 from util.bad_request_rate_limiter import BadRequestRateLimiter
 from util.db import outer_atomic
 from util.json_request import JsonResponse
-from util.password_policy_validators import validate_password
+from util.password_policy_validators import normalize_password, validate_password
 
 log = logging.getLogger("edx.student")
 
@@ -827,6 +829,17 @@ def password_reset_confirm_wrapper(request, uidb36=None, token=None):
         )
 
     if request.method == 'POST':
+        if PASSWORD_UNICODE_NORMALIZE_FLAG.is_enabled():
+            # We have to make a copy of request.POST because it is a QueryDict object which is immutable until copied.
+            # We have to use request.POST because the password_reset_confirm method takes in the request and a user's
+            # password is set to the request.POST['new_password1'] field. We have to also normalize the new_password2
+            # field so it passes the equivalence check that new_password1 == new_password2
+            # In order to switch out of having to do this copy, we would want to move the normalize_password code into
+            # a custom User model's set_password method to ensure it is always happening upon calling set_password.
+            request.POST = request.POST.copy()
+            request.POST['new_password1'] = normalize_password(request.POST['new_password1'])
+            request.POST['new_password2'] = normalize_password(request.POST['new_password2'])
+
         password = request.POST['new_password1']
 
         try: