From 3f21b0718fb25303f2a810410bd69c8e77d06bc9 Mon Sep 17 00:00:00 2001 From: Awais Jibran Date: Thu, 15 Sep 2016 15:37:25 +0500 Subject: [PATCH] Show error message on password reset if provided passwords does not match. --- .../student/tests/test_reset_password.py | 19 +++++++++++ common/djangoapps/student/views.py | 7 ++++ .../courseware/tests/test_password_history.py | 33 +++++++++++++++++-- .../student_account/test/test_views.py | 7 ++-- 4 files changed, 59 insertions(+), 7 deletions(-) diff --git a/common/djangoapps/student/tests/test_reset_password.py b/common/djangoapps/student/tests/test_reset_password.py index 0bef3d29a7..13d8243d07 100644 --- a/common/djangoapps/student/tests/test_reset_password.py +++ b/common/djangoapps/student/tests/test_reset_password.py @@ -252,6 +252,25 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): self.user = User.objects.get(pk=self.user.pk) self.assertTrue(self.user.is_active) + def test_password_reset_fail(self): + """Tests that if we provide mismatched passwords, user is not marked as active.""" + self.assertFalse(self.user.is_active) + + url = reverse( + 'password_reset_confirm', + kwargs={'uidb36': self.uidb36, 'token': self.token} + ) + request_params = {'new_password1': 'password1', 'new_password2': 'password2'} + confirm_request = self.request_factory.post(url, data=request_params) + + # Make a password reset request with mismatching passwords. + resp = password_reset_confirm_wrapper(confirm_request, self.uidb36, self.token) + + # Verify the response status code is: 200 with password reset fail and also verify that + # the user is not marked as active. + self.assertEqual(resp.status_code, 200) + self.assertFalse(User.objects.get(pk=self.user.pk).is_active) + @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 b2d1ad4771..45c8b75541 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -2242,6 +2242,13 @@ def password_reset_confirm_wrapper(request, uidb36=None, token=None): request, uidb64=uidb64, token=token, extra_context=platform_name ) + # If password reset was unsuccessful a template response is returned (status_code 200). + # Check if form is invalid then show an error to the user. + # Note if password reset was successful we get response redirect (status_code 302). + if response.status_code == 200 and not response.context_data['form'].is_valid(): + response.context_data['err_msg'] = _('Error in resetting your password. Please try again.') + return response + # get the updated user updated_user = User.objects.get(id=uid_int) diff --git a/lms/djangoapps/courseware/tests/test_password_history.py b/lms/djangoapps/courseware/tests/test_password_history.py index ec5656e993..0f9a26eb77 100644 --- a/lms/djangoapps/courseware/tests/test_password_history.py +++ b/lms/djangoapps/courseware/tests/test_password_history.py @@ -2,6 +2,7 @@ This file will test through the LMS some of the PasswordHistory features """ import json +import ddt from mock import patch from uuid import uuid4 from nose.plugins.attrib import attr @@ -23,6 +24,7 @@ from courseware.tests.helpers import LoginEnrollmentTestCase @attr(shard=1) @patch.dict("django.conf.settings.FEATURES", {'ADVANCED_SECURITY': True}) +@ddt.ddt class TestPasswordHistory(LoginEnrollmentTestCase): """ Go through some of the PasswordHistory use cases @@ -71,16 +73,18 @@ class TestPasswordHistory(LoginEnrollmentTestCase): history = PasswordHistory() history.create(user) - def assertPasswordResetError(self, response, error_message): + def assertPasswordResetError(self, response, error_message, valid_link=False): """ This method is a custom assertion that verifies that a password reset view returns an error response as expected. Args: response: response from calling a password reset endpoint error_message: message we expect to see in the response + valid_link: if the current password reset link is still valid """ - self.assertFalse(response.context_data['validlink']) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.context_data['validlink'], valid_link) self.assertIn(error_message, response.content) @patch.dict("django.conf.settings.ADVANCED_SECURITY_CONFIG", {'MIN_DAYS_FOR_STAFF_ACCOUNTS_PASSWORD_RESETS': None}) @@ -338,3 +342,28 @@ class TestPasswordHistory(LoginEnrollmentTestCase): }, follow=True) self.assertIn(success_msg, resp.content) + + @ddt.data( + ('foo', 'foobar'), + ('', ''), + ) + @ddt.unpack + def test_password_reset_form_invalid(self, password1, password2): + """ + Tests that password reset fail when providing bad passwords and error message is displayed + to the user. + """ + user_email, _ = self._setup_user() + err_msg = 'Error in resetting your password. Please try again.' + + # try to reset password, it should fail + user = User.objects.get(email=user_email) + token = default_token_generator.make_token(user) + uidb36 = int_to_base36(user.id) + + # try to do a password reset with the same password as before + resp = self.client.post('/password_reset_confirm/{0}-{1}/'.format(uidb36, token), { + 'new_password1': password1, + 'new_password2': password2, + }, follow=True) + self.assertPasswordResetError(resp, err_msg, valid_link=True) diff --git a/lms/djangoapps/student_account/test/test_views.py b/lms/djangoapps/student_account/test/test_views.py index 656c275062..1f75f64023 100644 --- a/lms/djangoapps/student_account/test/test_views.py +++ b/lms/djangoapps/student_account/test/test_views.py @@ -126,11 +126,8 @@ class StudentAccountUpdateTest(CacheIsolationTestCase, UrlResetMixin): self.assertTrue(result) # Try reusing the activation link to change the password again - response = self.client.post( - activation_link, - {'new_password1': self.OLD_PASSWORD, 'new_password2': self.OLD_PASSWORD}, - follow=True - ) + # Visit the activation link again. + response = self.client.get(activation_link) self.assertEqual(response.status_code, 200) self.assertContains(response, "This password reset link is invalid. It may have been used already.")