Merge pull request #13480 from edx/aj/fix-password-rest-confirm

Show error on password reset form

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):

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)
 

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)

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(
+        # Visit the activation link again.
-            activation_link,
+        response = self.client.get(activation_link)
-            {'new_password1': self.OLD_PASSWORD, 'new_password2': self.OLD_PASSWORD},
-            follow=True
-        )
         self.assertEqual(response.status_code, 200)
         self.assertContains(response, "This password reset link is invalid. It may have been used already.")