Merge pull request #13066 from edx/adam/add-commit-to-eucalyptus

authenticate user after their reset confirmation goes through

common/djangoapps/student/forms.py

@@ -19,11 +19,7 @@ from django.template import loader
 from django.conf import settings
 from microsite_configuration import microsite
 from student.models import CourseEnrollmentAllowed
-from util.password_policy_validators import (
-    validate_password_length,
-    validate_password_complexity,
-    validate_password_dictionary,
-)
+from util.password_policy_validators import validate_password_strength
 from openedx.core.djangoapps.theming import helpers as theming_helpers
 
 
@@ -223,9 +219,7 @@ class AccountCreationForm(forms.Form):
             raise ValidationError(_("Username and password fields cannot match"))
         if self.enforce_password_policy:
             try:
-                validate_password_length(password)
-                validate_password_complexity(password)
-                validate_password_dictionary(password)
+                validate_password_strength(password)
             except ValidationError, err:
                 raise ValidationError(_("Password: ") + "; ".join(err.messages))
         return password

common/djangoapps/student/tests/test_password_policy.py

@@ -59,7 +59,7 @@ class TestPasswordPolicy(TestCase):
         obj = json.loads(response.content)
         self.assertEqual(
             obj['value'],
-            "Password: Invalid Length (must be 12 characters or less)",
+            "Password: Invalid Length (must be 12 characters or fewer)",
         )
 
     @patch.dict("django.conf.settings.PASSWORD_COMPLEXITY", {'UPPER': 3})

common/djangoapps/student/tests/test_reset_password.py

@@ -6,8 +6,8 @@ import re
 import unittest
 
 from django.core.cache import cache
+from django.core.urlresolvers import reverse
 from django.conf import settings
-from django.test import TestCase
 from django.test.client import RequestFactory
 from django.contrib.auth.models import User
 from django.contrib.auth.hashers import UNUSABLE_PASSWORD_PREFIX
@@ -29,6 +29,10 @@ from .test_microsite import fake_microsite_get_value
 from openedx.core.djangoapps.theming import helpers as theming_helpers
 
 
+@unittest.skipUnless(
+    settings.ROOT_URLCONF == "lms.urls",
+    "reset password tests should only run in LMS"
+)
 @ddt.ddt
 class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
     """ Tests that clicking reset password sends email, and doesn't activate the user
@@ -50,10 +54,6 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
         self.user_bad_passwd.password = UNUSABLE_PASSWORD_PREFIX
         self.user_bad_passwd.save()
 
-    def uidb36_to_uidb64(self, uidb36=None):
-        """ Converts uidb36 into uidb64 """
-        return force_text(urlsafe_base64_encode(force_bytes(base36_to_int(uidb36 or self.uidb36))))
-
     @patch('student.views.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True))
     def test_user_bad_password_reset(self):
         """Tests password reset behavior for user with password marked UNUSABLE_PASSWORD_PREFIX"""
@@ -219,29 +219,37 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
         )
         self.assertEqual(from_addr, "no-reply@fakeuniversity.com")
 
-    @patch('student.views.password_reset_confirm')
-    def test_reset_password_bad_token(self, reset_confirm):
+    @ddt.data(
+        ('invalidUid', 'invalid_token'),
+        (None, 'invalid_token'),
+        ('invalidUid', None),
+    )
+    @ddt.unpack
+    def test_reset_password_bad_token(self, uidb36, token):
         """Tests bad token and uidb36 in password reset"""
-
-        bad_reset_req = self.request_factory.get('/password_reset_confirm/NO-OP/')
-        password_reset_confirm_wrapper(bad_reset_req, 'NO', 'OP')
-        confirm_kwargs = reset_confirm.call_args[1]
-
-        self.assertEquals(confirm_kwargs['uidb64'], self.uidb36_to_uidb64('NO'))
-
-        self.assertEquals(confirm_kwargs['token'], 'OP')
+        if uidb36 is None:
+            uidb36 = self.uidb36
+        if token is None:
+            token = self.token
+
+        bad_request = self.request_factory.get(
+            reverse(
+                "password_reset_confirm",
+                kwargs={"uidb36": uidb36, "token": token}
+            )
+        )
+        password_reset_confirm_wrapper(bad_request, uidb36, token)
         self.user = User.objects.get(pk=self.user.pk)
         self.assertFalse(self.user.is_active)
 
-    @patch('student.views.password_reset_confirm')
-    def test_reset_password_good_token(self, reset_confirm):
+    def test_reset_password_good_token(self):
         """Tests good token and uidb36 in password reset"""
-
-        good_reset_req = self.request_factory.get('/password_reset_confirm/{0}-{1}/'.format(self.uidb36, self.token))
+        url = reverse(
+            "password_reset_confirm",
+            kwargs={"uidb36": self.uidb36, "token": self.token}
+        )
+        good_reset_req = self.request_factory.get(url)
         password_reset_confirm_wrapper(good_reset_req, self.uidb36, self.token)
-        confirm_kwargs = reset_confirm.call_args[1]
-        self.assertEquals(confirm_kwargs['uidb64'], self.uidb36_to_uidb64())
-        self.assertEquals(confirm_kwargs['token'], self.token)
         self.user = User.objects.get(pk=self.user.pk)
         self.assertTrue(self.user.is_active)
 
@@ -249,20 +257,13 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
     @patch("microsite_configuration.microsite.get_value", fake_microsite_get_value)
     def test_reset_password_good_token_microsite(self, reset_confirm):
         """Tests password reset confirmation page for micro site"""
-
-        good_reset_req = self.request_factory.get('/password_reset_confirm/{0}-{1}/'.format(self.uidb36, self.token))
+        url = reverse(
+            "password_reset_confirm",
+            kwargs={"uidb36": self.uidb36, "token": self.token}
+        )
+        good_reset_req = self.request_factory.get(url)
         password_reset_confirm_wrapper(good_reset_req, self.uidb36, self.token)
         confirm_kwargs = reset_confirm.call_args[1]
         self.assertEquals(confirm_kwargs['extra_context']['platform_name'], 'Fake University')
-
-    @patch('student.views.password_reset_confirm')
-    def test_reset_password_with_reused_password(self, reset_confirm):
-        """Tests good token and uidb36 in password reset"""
-
-        good_reset_req = self.request_factory.get('/password_reset_confirm/{0}-{1}/'.format(self.uidb36, self.token))
-        password_reset_confirm_wrapper(good_reset_req, self.uidb36, self.token)
-        confirm_kwargs = reset_confirm.call_args[1]
-        self.assertEquals(confirm_kwargs['uidb64'], self.uidb36_to_uidb64())
-        self.assertEquals(confirm_kwargs['token'], self.token)
         self.user = User.objects.get(pk=self.user.pk)
         self.assertTrue(self.user.is_active)

common/djangoapps/util/password_policy_validators.py

@@ -15,6 +15,26 @@ from django.conf import settings
 import nltk
 
 
+def validate_password_strength(value):
+    """
+    This function loops through each validator defined in this file
+    and applies it to a user's proposed password
+
+    Args:
+        value: a user's proposed password
+
+    Returns: None, but raises a ValidationError if the proposed password
+        fails any one of the validators in password_validators
+    """
+    password_validators = [
+        validate_password_length,
+        validate_password_complexity,
+        validate_password_dictionary,
+    ]
+    for validator in password_validators:
+        validator(value)
+
+
 def validate_password_length(value):
     """
     Validator that enforces minimum length of a password
@@ -28,7 +48,7 @@ def validate_password_length(value):
     if min_length and len(value) < min_length:
         raise ValidationError(message.format(_("must be {0} characters or more").format(min_length)), code=code)
     elif max_length and len(value) > max_length:
-        raise ValidationError(message.format(_("must be {0} characters or less").format(max_length)), code=code)
+        raise ValidationError(message.format(_("must be {0} characters or fewer").format(max_length)), code=code)
 
 
 def validate_password_complexity(value):

lms/djangoapps/courseware/tests/test_password_history.py

@@ -71,6 +71,18 @@ class TestPasswordHistory(LoginEnrollmentTestCase):
         history = PasswordHistory()
         history.create(user)
 
+    def assertPasswordResetError(self, response, error_message):
+        """
+        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
+
+        """
+        self.assertFalse(response.context_data['validlink'])
+        self.assertIn(error_message, response.content)
+
     @patch.dict("django.conf.settings.ADVANCED_SECURITY_CONFIG", {'MIN_DAYS_FOR_STAFF_ACCOUNTS_PASSWORD_RESETS': None})
     @patch.dict("django.conf.settings.ADVANCED_SECURITY_CONFIG", {'MIN_DAYS_FOR_STUDENT_ACCOUNTS_PASSWORD_RESETS': None})
     def test_no_forced_password_change(self):
@@ -168,10 +180,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase):
             'new_password2': 'foo'
         }, follow=True)
 
-        self.assertIn(
-            err_msg,
-            resp.content
-        )
+        self.assertPasswordResetError(resp, err_msg)
 
         # now retry with a different password
         resp = self.client.post('/password_reset_confirm/{0}-{1}/'.format(uidb36, token), {
@@ -179,10 +188,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase):
             'new_password2': 'bar'
         }, follow=True)
 
-        self.assertIn(
-            success_msg,
-            resp.content
-        )
+        self.assertIn(success_msg, resp.content)
 
     @patch.dict("django.conf.settings.ADVANCED_SECURITY_CONFIG", {'MIN_DIFFERENT_STAFF_PASSWORDS_BEFORE_REUSE': 2})
     def test_staff_password_reset_reuse(self):
@@ -204,10 +210,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase):
             'new_password2': 'foo',
         }, follow=True)
 
-        self.assertIn(
-            err_msg,
-            resp.content
-        )
+        self.assertPasswordResetError(resp, err_msg)
 
         # now use different one
         user = User.objects.get(email=staff_email)
@@ -219,10 +222,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase):
             'new_password2': 'bar',
         }, follow=True)
 
-        self.assertIn(
-            success_msg,
-            resp.content
-        )
+        self.assertIn(success_msg, resp.content)
 
         # now try again with the first one
         user = User.objects.get(email=staff_email)
@@ -234,11 +234,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase):
             'new_password2': 'foo',
         }, follow=True)
 
-        # should be rejected
-        self.assertIn(
-            err_msg,
-            resp.content
-        )
+        self.assertPasswordResetError(resp, err_msg)
 
         # now use different one
         user = User.objects.get(email=staff_email)
@@ -250,10 +246,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase):
             'new_password2': 'baz',
         }, follow=True)
 
-        self.assertIn(
-            success_msg,
-            resp.content
-        )
+        self.assertIn(success_msg, resp.content)
 
         # now we should be able to reuse the first one
         user = User.objects.get(email=staff_email)
@@ -265,10 +258,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase):
             'new_password2': 'foo',
         }, follow=True)
 
-        self.assertIn(
-            success_msg,
-            resp.content
-        )
+        self.assertIn(success_msg, resp.content)
 
     @patch.dict("django.conf.settings.ADVANCED_SECURITY_CONFIG", {'MIN_TIME_IN_DAYS_BETWEEN_ALLOWED_RESETS': 1})
     def test_password_reset_frequency_limit(self):
@@ -308,10 +298,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase):
                 'new_password2': 'foo',
             }, follow=True)
 
-            self.assertIn(
-                success_msg,
-                resp.content
-            )
+            self.assertIn(success_msg, resp.content)
 
     @patch.dict("django.conf.settings.FEATURES", {'ENFORCE_PASSWORD_POLICY': True})
     @override_settings(PASSWORD_MIN_LENGTH=6)
@@ -350,7 +337,4 @@ class TestPasswordHistory(LoginEnrollmentTestCase):
             'new_password2': 'foofoo',
         }, follow=True)
 
-        self.assertIn(
-            success_msg,
-            resp.content
-        )
+        self.assertIn(success_msg, resp.content)