Merge pull request #15699 from edx/tasawer/learner-1494/add-password-complexity-on-reset-page

length complexity added on password reset page

common/djangoapps/student/tests/test_reset_password.py

@@ -13,6 +13,7 @@ from django.contrib.auth.tokens import default_token_generator
 from django.core.cache import cache
 from django.core.urlresolvers import reverse
 from django.test.client import RequestFactory
+from django.test.utils import override_settings
 from django.utils.http import int_to_base36
 from edx_oauth2_provider.tests.factories import AccessTokenFactory, ClientFactory, RefreshTokenFactory
 from mock import Mock, patch
@@ -287,6 +288,35 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
         self.assertEqual(resp.status_code, 200)
         self.assertFalse(User.objects.get(pk=self.user.pk).is_active)
 
+    @override_settings(PASSWORD_MIN_LENGTH=2)
+    @override_settings(PASSWORD_MAX_LENGTH=10)
+    @ddt.data(
+        {
+            'password': '1',
+            'error_message': 'Password: Invalid Length (must be 2 characters or more)',
+        },
+        {
+            'password': '01234567891',
+            'error_message': 'Password: Invalid Length (must be 10 characters or fewer)'
+        }
+    )
+    def test_password_reset_with_invalid_length(self, password_dict):
+        """Tests that if we provide password characters less then PASSWORD_MIN_LENGTH,
+        or more than PASSWORD_MAX_LENGTH, password reset will fail with error message.
+        """
+
+        url = reverse(
+            'password_reset_confirm',
+            kwargs={'uidb36': self.uidb36, 'token': self.token}
+        )
+        request_params = {'new_password1': password_dict['password'], 'new_password2': password_dict['password']}
+        confirm_request = self.request_factory.post(url, data=request_params)
+
+        # Make a password reset request with minimum/maximum passwords characters.
+        response = password_reset_confirm_wrapper(confirm_request, self.uidb36, self.token)
+
+        self.assertEqual(response.context_data['err_msg'], password_dict['error_message'])
+
     @patch('student.views.password_reset_confirm')
     @patch("openedx.core.djangoapps.site_configuration.helpers.get_value", fake_get_value)
     def test_reset_password_good_token_configuration_override(self, reset_confirm):

common/djangoapps/student/views.py

@@ -127,7 +127,7 @@ from util.bad_request_rate_limiter import BadRequestRateLimiter
 from util.db import outer_atomic
 from util.json_request import JsonResponse
 from util.milestones_helpers import get_pre_requisite_courses_not_completed
-from util.password_policy_validators import validate_password_strength
+from util.password_policy_validators import validate_password_length, validate_password_strength
 from xmodule.modulestore.django import modulestore
 
 log = logging.getLogger("edx.student")
@@ -2476,7 +2476,30 @@ def uidb36_to_uidb64(uidb36):
     return uidb64
 
 
-def validate_password(user, password):
+def validate_password(password):
+    """
+    Validate password overall strength if ENFORCE_PASSWORD_POLICY is enable
+    otherwise only validate the length of the password.
+
+    Args:
+        password: the user's proposed new password.
+
+    Returns:
+        err_msg: an error message if there's a violation of one of the password
+            checks. Otherwise, `None`.
+    """
+
+    try:
+        if settings.FEATURES.get('ENFORCE_PASSWORD_POLICY', False):
+            validate_password_strength(password)
+        else:
+            validate_password_length(password)
+
+    except ValidationError as err:
+        return _('Password: ') + '; '.join(err.messages)
+
+
+def validate_password_security_policy(user, password):
     """
     Tie in password policy enforcement as an optional level of
     security protection
@@ -2486,19 +2509,11 @@ def validate_password(user, password):
         password: the user's proposed new password.
 
     Returns:
-        is_valid_password: a boolean indicating if the new password
-            passes the validation.
         err_msg: an error message if there's a violation of one of the password
             checks. Otherwise, `None`.
     """
-    err_msg = None
-
-    if settings.FEATURES.get('ENFORCE_PASSWORD_POLICY', False):
-        try:
-            validate_password_strength(password)
-        except ValidationError as err:
-            err_msg = _('Password: ') + '; '.join(err.messages)
 
+    err_msg = None
     # also, check the password reuse policy
     if not PasswordHistory.is_allowable_password_reuse(user, password):
         if user.is_staff:
@@ -2524,9 +2539,7 @@ def validate_password(user, password):
             num_days
         ).format(num=num_days)
 
-    is_password_valid = err_msg is None
-
-    return is_password_valid, err_msg
+    return err_msg
 
 
 def password_reset_confirm_wrapper(request, uidb36=None, token=None):
@@ -2552,16 +2565,24 @@ def password_reset_confirm_wrapper(request, uidb36=None, token=None):
 
     if request.method == 'POST':
         password = request.POST['new_password1']
-        is_password_valid, password_err_msg = validate_password(user, password)
-        if not is_password_valid:
+        valid_link = False
+        error_message = validate_password_security_policy(user, password)
+        if not error_message:
+            # if security is not violated, we need to validate password
+            error_message = validate_password(password)
+            if error_message:
+                # password reset link will be valid if there is no security violation
+                valid_link = True
+
+        if error_message:
             # We have a password reset attempt which violates some security
-            # policy. Use the existing Django template to communicate that
+            # policy, or any other validation. Use the existing Django template to communicate that
             # back to the user.
             context = {
-                'validlink': False,
+                'validlink': valid_link,
                 'form': None,
                 'title': _('Password reset unsuccessful'),
-                'err_msg': password_err_msg,
+                'err_msg': error_message,
             }
             context.update(platform_name)
             return TemplateResponse(