diff --git a/common/djangoapps/student/tests/test_reset_password.py b/common/djangoapps/student/tests/test_reset_password.py index 9b4fbbfcd6..ff69082c3e 100644 --- a/common/djangoapps/student/tests/test_reset_password.py +++ b/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): diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 3ac0e6c579..f871ee08ee 100644 --- a/common/djangoapps/student/views.py +++ b/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) - # 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(