Merge pull request #2781 from edx/cdodge/password-history-enforcements

Add some optional policies around password resets, such as password reus...

cms/envs/aws.py

@@ -274,3 +274,6 @@ SESSION_INACTIVITY_TIMEOUT_IN_SECONDS = AUTH_TOKENS.get("SESSION_INACTIVITY_TIME
 
 ##### X-Frame-Options response header settings #####
 X_FRAME_OPTIONS = ENV_TOKENS.get('X_FRAME_OPTIONS', X_FRAME_OPTIONS)
+
+##### ADVANCED_SECURITY_CONFIG #####
+ADVANCED_SECURITY_CONFIG = ENV_TOKENS.get('ADVANCED_SECURITY_CONFIG', {})

cms/envs/common.py

@@ -96,6 +96,9 @@ FEATURES = {
 
     # Prevent concurrent logins per user
     'PREVENT_CONCURRENT_LOGINS': False,
+
+    # Turn off Advanced Security by default
+    'ADVANCED_SECURITY': False,
 }
 ENABLE_JASMINE = False
 
@@ -580,6 +583,7 @@ OPTIONAL_APPS = (
     'openassessment.xblock'
 )
 
+
 for app_name in OPTIONAL_APPS:
     # First attempt to only find the module rather than actually importing it,
     # to avoid circular references - only try to import if it can't be found
@@ -592,3 +596,7 @@ for app_name in OPTIONAL_APPS:
         except ImportError:
             continue
     INSTALLED_APPS += (app_name,)
+
+### ADVANCED_SECURITY_CONFIG
+# Empty by default
+ADVANCED_SECURITY_CONFIG = {}

common/djangoapps/student/migrations/0032_auto__add_loginfailures.py

@@ -113,6 +113,8 @@ class Migration(SchemaMigration):
         },
         'student.userprofile': {
             'Meta': {'object_name': 'UserProfile', 'db_table': "'auth_userprofile'"},
+            'city': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}),
+            'country': ('django_countries.fields.CountryField', [], {'max_length': '2', 'null': 'True', 'blank': 'True'}),
             'allow_certificate': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
             'courseware': ('django.db.models.fields.CharField', [], {'default': "'course.xml'", 'max_length': '255', 'blank': 'True'}),
             'gender': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '6', 'null': 'True', 'blank': 'True'}),
@@ -144,4 +146,4 @@ class Migration(SchemaMigration):
         }
     }
 
-    complete_apps = ['student']
\ No newline at end of file
+    complete_apps = ['student']

common/djangoapps/student/models.py

@@ -21,14 +21,15 @@ from collections import defaultdict
 from dogapi import dog_stats_api
 
 from django.conf import settings
+from django.utils import timezone
 from django.contrib.auth.models import User
+from django.contrib.auth.hashers import make_password
 from django.contrib.auth.signals import user_logged_in, user_logged_out
 from django.db import models, IntegrityError
 from django.db.models import Count
 from django.db.models.signals import post_save
 from django.dispatch import receiver, Signal
 import django.dispatch
-from django.forms import ModelForm, forms
 from django.core.exceptions import ObjectDoesNotExist
 from django.utils.translation import ugettext_noop
 from django_countries import CountryField
@@ -315,6 +316,185 @@ EVENT_NAME_ENROLLMENT_ACTIVATED = 'edx.course.enrollment.activated'
 EVENT_NAME_ENROLLMENT_DEACTIVATED = 'edx.course.enrollment.deactivated'
 
 
+class PasswordHistory(models.Model):
+    """
+    This model will keep track of past passwords that a user has used
+    as well as providing contraints (e.g. can't reuse passwords)
+    """
+    user = models.ForeignKey(User)
+    password = models.CharField(max_length=128)
+    time_set = models.DateTimeField(default=timezone.now)
+
+    def create(self, user):
+        """
+        This will copy over the current password, if any of the configuration has been turned on
+        """
+
+        if not (PasswordHistory.is_student_password_reuse_restricted() or
+                PasswordHistory.is_staff_password_reuse_restricted() or
+                PasswordHistory.is_password_reset_frequency_restricted() or
+                PasswordHistory.is_staff_forced_password_reset_enabled() or
+                PasswordHistory.is_student_forced_password_reset_enabled()):
+
+            return
+
+        self.user = user
+        self.password = user.password
+        self.save()
+
+    @classmethod
+    def is_student_password_reuse_restricted(cls):
+        """
+        Returns whether the configuration which limits password reuse has been turned on
+        """
+        return settings.FEATURES['ADVANCED_SECURITY'] and \
+            settings.ADVANCED_SECURITY_CONFIG.get(
+                'MIN_DIFFERENT_STUDENT_PASSWORDS_BEFORE_REUSE', 0
+            ) > 0
+
+    @classmethod
+    def is_staff_password_reuse_restricted(cls):
+        """
+        Returns whether the configuration which limits password reuse has been turned on
+        """
+        return settings.FEATURES['ADVANCED_SECURITY'] and \
+            settings.ADVANCED_SECURITY_CONFIG.get(
+                'MIN_DIFFERENT_STAFF_PASSWORDS_BEFORE_REUSE', 0
+            ) > 0
+
+    @classmethod
+    def is_password_reset_frequency_restricted(cls):
+        """
+        Returns whether the configuration which limits the password reset frequency has been turned on
+        """
+        return settings.FEATURES['ADVANCED_SECURITY'] and \
+            settings.ADVANCED_SECURITY_CONFIG.get(
+                'MIN_TIME_IN_DAYS_BETWEEN_ALLOWED_RESETS', None
+            )
+
+    @classmethod
+    def is_staff_forced_password_reset_enabled(cls):
+        """
+        Returns whether the configuration which forces password resets to occur has been turned on
+        """
+        return settings.FEATURES['ADVANCED_SECURITY'] and \
+            settings.ADVANCED_SECURITY_CONFIG.get(
+                'MIN_DAYS_FOR_STAFF_ACCOUNTS_PASSWORD_RESETS', None
+            )
+
+    @classmethod
+    def is_student_forced_password_reset_enabled(cls):
+        """
+        Returns whether the configuration which forces password resets to occur has been turned on
+        """
+        return settings.FEATURES['ADVANCED_SECURITY'] and \
+            settings.ADVANCED_SECURITY_CONFIG.get(
+                'MIN_DAYS_FOR_STUDENT_ACCOUNTS_PASSWORD_RESETS', None
+            )
+
+    @classmethod
+    def should_user_reset_password_now(cls, user):
+        """
+        Returns whether a password has 'expired' and should be reset. Note there are two different
+        expiry policies for staff and students
+        """
+        if not settings.FEATURES['ADVANCED_SECURITY']:
+            return False
+
+        days_before_password_reset = None
+        if user.is_staff:
+            if cls.is_staff_forced_password_reset_enabled():
+                days_before_password_reset = \
+                    settings.ADVANCED_SECURITY_CONFIG['MIN_DAYS_FOR_STAFF_ACCOUNTS_PASSWORD_RESETS']
+        elif cls.is_student_forced_password_reset_enabled():
+            days_before_password_reset = \
+                settings.ADVANCED_SECURITY_CONFIG['MIN_DAYS_FOR_STUDENT_ACCOUNTS_PASSWORD_RESETS']
+
+        if days_before_password_reset:
+            history = PasswordHistory.objects.filter(user=user).order_by('-time_set')
+            time_last_reset = None
+
+            if history:
+                # first element should be the last time we reset password
+                time_last_reset = history[0].time_set
+            else:
+                # no history, then let's take the date the user joined
+                time_last_reset = user.date_joined
+
+            now = timezone.now()
+
+            delta = now - time_last_reset
+
+            return delta.days >= days_before_password_reset
+
+        return False
+
+    @classmethod
+    def is_password_reset_too_soon(cls, user):
+        """
+        Verifies that the password is not getting reset too frequently
+        """
+        if not cls.is_password_reset_frequency_restricted():
+            return False
+
+        history = PasswordHistory.objects.filter(user=user).order_by('-time_set')
+
+        if not history:
+            return False
+
+        now = timezone.now()
+
+        delta = now - history[0].time_set
+
+        return delta.days < settings.ADVANCED_SECURITY_CONFIG['MIN_TIME_IN_DAYS_BETWEEN_ALLOWED_RESETS']
+
+    @classmethod
+    def is_allowable_password_reuse(cls, user, new_password):
+        """
+        Verifies that the password adheres to the reuse policies
+        """
+        if not settings.FEATURES['ADVANCED_SECURITY']:
+            return True
+
+        if user.is_staff and cls.is_staff_password_reuse_restricted():
+                min_diff_passwords_required = \
+                    settings.ADVANCED_SECURITY_CONFIG['MIN_DIFFERENT_STAFF_PASSWORDS_BEFORE_REUSE']
+        elif cls.is_student_password_reuse_restricted():
+            min_diff_passwords_required = \
+                settings.ADVANCED_SECURITY_CONFIG['MIN_DIFFERENT_STUDENT_PASSWORDS_BEFORE_REUSE']
+        else:
+            min_diff_passwords_required = 0
+
+        # just limit the result set to the number of different
+        # password we need
+        history = PasswordHistory.objects.filter(user=user).order_by('-time_set')[:min_diff_passwords_required]
+
+        for entry in history:
+
+            # be sure to re-use the same salt
+            # NOTE, how the salt is serialized in the password field is dependent on the algorithm
+            # in pbkdf2_sha256 [LMS] it's the 3rd element, in sha1 [unit tests] it's the 2nd element
+            hash_elements = entry.password.split('$')
+            algorithm = hash_elements[0]
+            if algorithm == 'pbkdf2_sha256':
+                hashed_password = make_password(new_password, hash_elements[2])
+            elif algorithm == 'sha1':
+                hashed_password = make_password(new_password, hash_elements[1])
+            else:
+                # This means we got something unexpected. We don't want to throw an exception, but
+                # log as an error and basically allow any password reuse
+                AUDIT_LOG.error('''
+                                Unknown password hashing algorithm "{0}" found in existing password
+                                hash, password reuse policy will not be enforced!!!
+                                '''.format(algorithm))
+                return True
+
+            if entry.password == hashed_password:
+                return False
+
+        return True
+
+
 class LoginFailures(models.Model):
     """
     This model will keep track of failed login attempts

common/djangoapps/student/tests/test_password_history.py

+# -*- coding: utf-8 -*-
+"""
+This test file will verify proper password history enforcement
+"""
+from django.test import TestCase
+from django.utils import timezone
+from mock import patch
+from student.tests.factories import UserFactory, AdminFactory
+
+from student.models import PasswordHistory
+from freezegun import freeze_time
+from datetime import timedelta
+
+from django.test.utils import override_settings
+
+
+@patch.dict("django.conf.settings.FEATURES", {'ADVANCED_SECURITY': True})
+class TestPasswordHistory(TestCase):
+    """
+    All the tests that assert proper behavior regarding password history
+    """
+
+    def _change_password(self, user, password):
+        """
+        Helper method to change password on user and record in the PasswordHistory
+        """
+        user.set_password(password)
+        user.save()
+        history = PasswordHistory()
+        history.create(user)
+
+    def _user_factory_with_history(self, is_staff=False, set_initial_history=True):
+        """
+        Helper method to generate either an Admin or a User
+        """
+        if is_staff:
+            user = AdminFactory()
+        else:
+            user = UserFactory()
+
+        user.date_joined = timezone.now()
+
+        if set_initial_history:
+            history = PasswordHistory()
+            history.create(user)
+
+        return user
+
+    @patch.dict("django.conf.settings.FEATURES", {'ADVANCED_SECURITY': False})
+    def test_disabled_feature(self):
+        """
+        Test that behavior is normal when this feature is not turned on
+        """
+        user = UserFactory()
+        staff = AdminFactory()
+
+        # if feature is disabled user can keep reusing same password
+        self.assertTrue(PasswordHistory.is_allowable_password_reuse(user, "test"))
+        self.assertTrue(PasswordHistory.is_allowable_password_reuse(staff, "test"))
+
+        self.assertFalse(PasswordHistory.should_user_reset_password_now(user))
+        self.assertFalse(PasswordHistory.should_user_reset_password_now(staff))
+
+    @patch.dict("django.conf.settings.ADVANCED_SECURITY_CONFIG", {'MIN_DIFFERENT_STAFF_PASSWORDS_BEFORE_REUSE': 2})
+    @patch.dict("django.conf.settings.ADVANCED_SECURITY_CONFIG", {'MIN_DIFFERENT_STUDENT_PASSWORDS_BEFORE_REUSE': 1})
+    def test_accounts_password_reuse(self):
+        """
+        Assert against the password reuse policy
+        """
+        user = self._user_factory_with_history()
+        staff = self._user_factory_with_history(is_staff=True)
+
+        # students need to user at least one different passwords before reuse
+        self.assertFalse(PasswordHistory.is_allowable_password_reuse(user, "test"))
+        self.assertTrue(PasswordHistory.is_allowable_password_reuse(user, "different"))
+        self._change_password(user, "different")
+
+        self.assertTrue(PasswordHistory.is_allowable_password_reuse(user, "test"))
+
+        # staff needs to use at least two different passwords before reuse
+        self.assertFalse(PasswordHistory.is_allowable_password_reuse(staff, "test"))
+        self.assertTrue(PasswordHistory.is_allowable_password_reuse(staff, "different"))
+        self._change_password(staff, "different")
+
+        self.assertFalse(PasswordHistory.is_allowable_password_reuse(staff, "test"))
+        self.assertFalse(PasswordHistory.is_allowable_password_reuse(staff, "different"))
+        self.assertTrue(PasswordHistory.is_allowable_password_reuse(staff, "third"))
+        self._change_password(staff, "third")
+
+        self.assertTrue(PasswordHistory.is_allowable_password_reuse(staff, "test"))
+
+    @override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.PBKDF2PasswordHasher'))
+    @patch.dict("django.conf.settings.ADVANCED_SECURITY_CONFIG", {'MIN_DIFFERENT_STAFF_PASSWORDS_BEFORE_REUSE': 2})
+    @patch.dict("django.conf.settings.ADVANCED_SECURITY_CONFIG", {'MIN_DIFFERENT_STUDENT_PASSWORDS_BEFORE_REUSE': 1})
+    def test_pbkdf2_sha256_password_reuse(self):
+        """
+        Assert against the password reuse policy but using the normal Django PBKDF2
+        """
+        user = self._user_factory_with_history()
+        staff = self._user_factory_with_history(is_staff=True)
+
+        # students need to user at least one different passwords before reuse
+        self.assertFalse(PasswordHistory.is_allowable_password_reuse(user, "test"))
+        self.assertTrue(PasswordHistory.is_allowable_password_reuse(user, "different"))
+        self._change_password(user, "different")
+
+        self.assertTrue(PasswordHistory.is_allowable_password_reuse(user, "test"))
+
+        # staff needs to use at least two different passwords before reuse
+        self.assertFalse(PasswordHistory.is_allowable_password_reuse(staff, "test"))
+        self.assertTrue(PasswordHistory.is_allowable_password_reuse(staff, "different"))
+        self._change_password(staff, "different")
+
+        self.assertFalse(PasswordHistory.is_allowable_password_reuse(staff, "test"))
+        self.assertFalse(PasswordHistory.is_allowable_password_reuse(staff, "different"))
+        self.assertTrue(PasswordHistory.is_allowable_password_reuse(staff, "third"))
+        self._change_password(staff, "third")
+
+        self.assertTrue(PasswordHistory.is_allowable_password_reuse(staff, "test"))
+
+    @patch.dict("django.conf.settings.ADVANCED_SECURITY_CONFIG", {'MIN_DAYS_FOR_STAFF_ACCOUNTS_PASSWORD_RESETS': 1})
+    @patch.dict("django.conf.settings.ADVANCED_SECURITY_CONFIG", {'MIN_DAYS_FOR_STUDENT_ACCOUNTS_PASSWORD_RESETS': 5})
+    def test_forced_password_change(self):
+        """
+        Assert when passwords must be reset
+        """
+        student = self._user_factory_with_history()
+        staff = self._user_factory_with_history(is_staff=True)
+        grandfathered_student = self._user_factory_with_history(set_initial_history=False)
+
+        self.assertFalse(PasswordHistory.should_user_reset_password_now(student))
+        self.assertFalse(PasswordHistory.should_user_reset_password_now(staff))
+        self.assertFalse(PasswordHistory.should_user_reset_password_now(grandfathered_student))
+
+        staff_reset_time = timezone.now() + timedelta(days=1)
+        with freeze_time(staff_reset_time):
+            self.assertFalse(PasswordHistory.should_user_reset_password_now(student))
+            self.assertFalse(PasswordHistory.should_user_reset_password_now(grandfathered_student))
+            self.assertTrue(PasswordHistory.should_user_reset_password_now(staff))
+
+            self._change_password(staff, 'Different')
+            self.assertFalse(PasswordHistory.should_user_reset_password_now(staff))
+
+        student_reset_time = timezone.now() + timedelta(days=5)
+
+        with freeze_time(student_reset_time):
+            self.assertTrue(PasswordHistory.should_user_reset_password_now(student))
+            self.assertTrue(PasswordHistory.should_user_reset_password_now(grandfathered_student))
+            self.assertTrue(PasswordHistory.should_user_reset_password_now(staff))
+
+            self._change_password(student, 'Different')
+            self.assertFalse(PasswordHistory.should_user_reset_password_now(student))
+
+            self._change_password(grandfathered_student, 'Different')
+            self.assertFalse(PasswordHistory.should_user_reset_password_now(grandfathered_student))
+
+            self._change_password(staff, 'Different')
+            self.assertFalse(PasswordHistory.should_user_reset_password_now(staff))
+
+    @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):
+        """
+        Assert that if we skip configuration, then user will never have to force reset password
+        """
+        student = self._user_factory_with_history()
+        staff = self._user_factory_with_history(is_staff=True)
+
+        # also create a user who doesn't have any history
+        grandfathered_student = UserFactory()
+        grandfathered_student.date_joined = timezone.now()
+
+        self.assertFalse(PasswordHistory.should_user_reset_password_now(student))
+        self.assertFalse(PasswordHistory.should_user_reset_password_now(staff))
+        self.assertFalse(PasswordHistory.should_user_reset_password_now(grandfathered_student))
+
+        staff_reset_time = timezone.now() + timedelta(days=100)
+        with freeze_time(staff_reset_time):
+            self.assertFalse(PasswordHistory.should_user_reset_password_now(student))
+            self.assertFalse(PasswordHistory.should_user_reset_password_now(grandfathered_student))
+            self.assertFalse(PasswordHistory.should_user_reset_password_now(staff))
+
+    @patch.dict("django.conf.settings.ADVANCED_SECURITY_CONFIG", {'MIN_TIME_IN_DAYS_BETWEEN_ALLOWED_RESETS': 1})
+    def test_too_frequent_password_resets(self):
+        """
+        Assert that a user should not be able to password reset too frequently
+        """
+        student = self._user_factory_with_history()
+        grandfathered_student = self._user_factory_with_history(set_initial_history=False)
+
+        self.assertTrue(PasswordHistory.is_password_reset_too_soon(student))
+        self.assertFalse(PasswordHistory.is_password_reset_too_soon(grandfathered_student))
+
+        staff_reset_time = timezone.now() + timedelta(days=100)
+        with freeze_time(staff_reset_time):
+            self.assertFalse(PasswordHistory.is_password_reset_too_soon(student))
+
+    @patch.dict("django.conf.settings.ADVANCED_SECURITY_CONFIG", {'MIN_TIME_IN_DAYS_BETWEEN_ALLOWED_RESETS': None})
+    def test_disabled_too_frequent_password_resets(self):
+        """
+        Verify properly default behavior when feature is disabled
+        """
+        student = self._user_factory_with_history()
+
+        self.assertFalse(PasswordHistory.is_password_reset_too_soon(student))

common/djangoapps/student/tests/test_password_policy.py

@@ -87,7 +87,7 @@ class TestPasswordPolicy(TestCase):
         )
 
     @patch.dict("django.conf.settings.PASSWORD_COMPLEXITY", {'LOWER': 3})
-    def test_password_not_enough_lowercase(self):
+    def test_password_enough_lowercase(self):
         self.url_params['password'] = 'ThisShouldPass'
         response = self.client.post(self.url, self.url_params)
         self.assertEqual(response.status_code, 200)

common/djangoapps/student/tests/test_reset_password.py

+"""
+Test the various password reset flows
+"""
+import json
+import re
+import unittest
+
+from django.core.cache import cache
+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
+from django.contrib.auth.tokens import default_token_generator
+from django.utils.http import int_to_base36
+
+from mock import Mock, patch
+from textwrap import dedent
+
+from student.views import password_reset, password_reset_confirm_wrapper
+from student.tests.factories import UserFactory
+from student.tests.test_email import mock_render_to_string
+
+
+class ResetPasswordTests(TestCase):
+    """ Tests that clicking reset password sends email, and doesn't activate the user
+    """
+    request_factory = RequestFactory()
+
+    def setUp(self):
+        self.user = UserFactory.create()
+        self.user.is_active = False
+        self.user.save()
+        self.token = default_token_generator.make_token(self.user)
+        self.uidb36 = int_to_base36(self.user.id)
+
+        self.user_bad_passwd = UserFactory.create()
+        self.user_bad_passwd.is_active = False
+        self.user_bad_passwd.password = UNUSABLE_PASSWORD
+        self.user_bad_passwd.save()
+
+    @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"""
+
+        bad_pwd_req = self.request_factory.post('/password_reset/', {'email': self.user_bad_passwd.email})
+        bad_pwd_resp = password_reset(bad_pwd_req)
+        # If they've got an unusable password, we return a successful response code
+        self.assertEquals(bad_pwd_resp.status_code, 200)
+        obj = json.loads(bad_pwd_resp.content)
+        self.assertEquals(obj, {
+            'success': True,
+            'value': "('registration/password_reset_done.html', [])",
+        })
+
+    @patch('student.views.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True))
+    def test_nonexist_email_password_reset(self):
+        """Now test the exception cases with of reset_password called with invalid email."""
+
+        bad_email_req = self.request_factory.post('/password_reset/', {'email': self.user.email + "makeItFail"})
+        bad_email_resp = password_reset(bad_email_req)
+        # Note: even if the email is bad, we return a successful response code
+        # This prevents someone potentially trying to "brute-force" find out which
+        # emails are and aren't registered with edX
+        self.assertEquals(bad_email_resp.status_code, 200)
+        obj = json.loads(bad_email_resp.content)
+        self.assertEquals(obj, {
+            'success': True,
+            'value': "('registration/password_reset_done.html', [])",
+        })
+
+    @patch('student.views.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True))
+    def test_password_reset_ratelimited(self):
+        """ Try (and fail) resetting password 30 times in a row on an non-existant email address """
+        cache.clear()
+
+        for i in xrange(30):
+            good_req = self.request_factory.post('/password_reset/', {
+                'email': 'thisdoesnotexist{0}@foo.com'.format(i)
+            })
+            good_resp = password_reset(good_req)
+            self.assertEquals(good_resp.status_code, 200)
+
+        # then the rate limiter should kick in and give a HttpForbidden response
+        bad_req = self.request_factory.post('/password_reset/', {'email': 'thisdoesnotexist@foo.com'})
+        bad_resp = password_reset(bad_req)
+        self.assertEquals(bad_resp.status_code, 403)
+
+        cache.clear()
+
+    @unittest.skipIf(
+        settings.FEATURES.get('DISABLE_RESET_EMAIL_TEST', False),
+        dedent("""
+            Skipping Test because CMS has not provided necessary templates for password reset.
+            If LMS tests print this message, that needs to be fixed.
+        """)
+    )
+    @patch('django.core.mail.send_mail')
+    @patch('student.views.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True))
+    def test_reset_password_email(self, send_email):
+        """Tests contents of reset password email, and that user is not active"""
+
+        good_req = self.request_factory.post('/password_reset/', {'email': self.user.email})
+        good_resp = password_reset(good_req)
+        self.assertEquals(good_resp.status_code, 200)
+        obj = json.loads(good_resp.content)
+        self.assertEquals(obj, {
+            'success': True,
+            'value': "('registration/password_reset_done.html', [])",
+        })
+
+        (subject, msg, from_addr, to_addrs) = send_email.call_args[0]
+        self.assertIn("Password reset", subject)
+        self.assertIn("You're receiving this e-mail because you requested a password reset", msg)
+        self.assertEquals(from_addr, settings.DEFAULT_FROM_EMAIL)
+        self.assertEquals(len(to_addrs), 1)
+        self.assertIn(self.user.email, to_addrs)
+
+        #test that the user is not active
+        self.user = User.objects.get(pk=self.user.pk)
+        self.assertFalse(self.user.is_active)
+        re.search(r'password_reset_confirm/(?P<uidb36>[0-9A-Za-z]+)-(?P<token>.+)/', msg).groupdict()
+
+    @patch('student.views.password_reset_confirm')
+    def test_reset_password_bad_token(self, reset_confirm):
+        """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['uidb36'], 'NO')
+        self.assertEquals(confirm_kwargs['token'], 'OP')
+        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):
+        """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['uidb36'], self.uidb36)
+        self.assertEquals(confirm_kwargs['token'], self.token)
+        self.user = User.objects.get(pk=self.user.pk)
+        self.assertTrue(self.user.is_active)
+
+    @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['uidb36'], self.uidb36)
+        self.assertEquals(confirm_kwargs['token'], self.token)
+        self.user = User.objects.get(pk=self.user.pk)
+        self.assertTrue(self.user.is_active)

common/djangoapps/student/tests/tests.py

@@ -5,21 +5,15 @@ when you run "manage.py test".
 Replace this with more appropriate tests for your application.
 """
 import logging
-import json
-import re
 import unittest
 from datetime import datetime, timedelta
 import pytz
 
-from django.core.cache import cache
 from django.conf import settings
 from django.test import TestCase
 from django.test.utils import override_settings
 from django.test.client import RequestFactory
 from django.contrib.auth.models import User, AnonymousUser
-from django.contrib.auth.hashers import UNUSABLE_PASSWORD
-from django.contrib.auth.tokens import default_token_generator
-from django.utils.http import int_to_base36
 from django.core.urlresolvers import reverse
 from django.http import HttpResponse
 
@@ -28,13 +22,11 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from courseware.tests.tests import TEST_DATA_MIXED_MODULESTORE
 
 from mock import Mock, patch, sentinel
-from textwrap import dedent
 
 from student.models import anonymous_id_for_user, user_by_anonymous_id, CourseEnrollment, unique_id_for_user
-from student.views import (process_survey_link, _cert_info, password_reset, password_reset_confirm_wrapper,
-                           change_enrollment, complete_course_mode_info, token, course_from_id)
+from student.views import (process_survey_link, _cert_info,
+                           change_enrollment, complete_course_mode_info, token)
 from student.tests.factories import UserFactory, CourseModeFactory
-from student.tests.test_email import mock_render_to_string
 
 import shoppingcart
 
@@ -44,127 +36,6 @@ COURSE_2 = 'edx/full/6.002_Spring_2012'
 log = logging.getLogger(__name__)
 
 
-class ResetPasswordTests(TestCase):
-    """ Tests that clicking reset password sends email, and doesn't activate the user
-    """
-    request_factory = RequestFactory()
-
-    def setUp(self):
-        self.user = UserFactory.create()
-        self.user.is_active = False
-        self.user.save()
-        self.token = default_token_generator.make_token(self.user)
-        self.uidb36 = int_to_base36(self.user.id)
-
-        self.user_bad_passwd = UserFactory.create()
-        self.user_bad_passwd.is_active = False
-        self.user_bad_passwd.password = UNUSABLE_PASSWORD
-        self.user_bad_passwd.save()
-
-    @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"""
-
-        bad_pwd_req = self.request_factory.post('/password_reset/', {'email': self.user_bad_passwd.email})
-        bad_pwd_resp = password_reset(bad_pwd_req)
-        # If they've got an unusable password, we return a successful response code
-        self.assertEquals(bad_pwd_resp.status_code, 200)
-        obj = json.loads(bad_pwd_resp.content)
-        self.assertEquals(obj, {
-            'success': True,
-            'value': "('registration/password_reset_done.html', [])",
-        })
-
-    @patch('student.views.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True))
-    def test_nonexist_email_password_reset(self):
-        """Now test the exception cases with of reset_password called with invalid email."""
-
-        bad_email_req = self.request_factory.post('/password_reset/', {'email': self.user.email+"makeItFail"})
-        bad_email_resp = password_reset(bad_email_req)
-        # Note: even if the email is bad, we return a successful response code
-        # This prevents someone potentially trying to "brute-force" find out which emails are and aren't registered with edX
-        self.assertEquals(bad_email_resp.status_code, 200)
-        obj = json.loads(bad_email_resp.content)
-        self.assertEquals(obj, {
-            'success': True,
-            'value': "('registration/password_reset_done.html', [])",
-        })
-
-    @patch('student.views.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True))
-    def test_password_reset_ratelimited(self):
-        """ Try (and fail) resetting password 30 times in a row on an non-existant email address """
-        cache.clear()
-
-        for i in xrange(30):
-            good_req = self.request_factory.post('/password_reset/', {'email': 'thisdoesnotexist@foo.com'})
-            good_resp = password_reset(good_req)
-            self.assertEquals(good_resp.status_code, 200)
-
-        # then the rate limiter should kick in and give a HttpForbidden response
-        bad_req = self.request_factory.post('/password_reset/', {'email': 'thisdoesnotexist@foo.com'})
-        bad_resp = password_reset(bad_req)
-        self.assertEquals(bad_resp.status_code, 403)
-
-        cache.clear()
-
-    @unittest.skipIf(
-        settings.FEATURES.get('DISABLE_RESET_EMAIL_TEST', False),
-        dedent("""
-            Skipping Test because CMS has not provided necessary templates for password reset.
-            If LMS tests print this message, that needs to be fixed.
-        """)
-    )
-    @patch('django.core.mail.send_mail')
-    @patch('student.views.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True))
-    def test_reset_password_email(self, send_email):
-        """Tests contents of reset password email, and that user is not active"""
-
-        good_req = self.request_factory.post('/password_reset/', {'email': self.user.email})
-        good_resp = password_reset(good_req)
-        self.assertEquals(good_resp.status_code, 200)
-        obj = json.loads(good_resp.content)
-        self.assertEquals(obj, {
-            'success': True,
-            'value': "('registration/password_reset_done.html', [])",
-        })
-
-        ((subject, msg, from_addr, to_addrs), sm_kwargs) = send_email.call_args
-        self.assertIn("Password reset", subject)
-        self.assertIn("You're receiving this e-mail because you requested a password reset", msg)
-        self.assertEquals(from_addr, settings.DEFAULT_FROM_EMAIL)
-        self.assertEquals(len(to_addrs), 1)
-        self.assertIn(self.user.email, to_addrs)
-
-        #test that the user is not active
-        self.user = User.objects.get(pk=self.user.pk)
-        self.assertFalse(self.user.is_active)
-        reset_match = re.search(r'password_reset_confirm/(?P<uidb36>[0-9A-Za-z]+)-(?P<token>.+)/', msg).groupdict()
-
-    @patch('student.views.password_reset_confirm')
-    def test_reset_password_bad_token(self, reset_confirm):
-        """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_args, confirm_kwargs) = reset_confirm.call_args
-        self.assertEquals(confirm_kwargs['uidb36'], 'NO')
-        self.assertEquals(confirm_kwargs['token'], 'OP')
-        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):
-        """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_args, confirm_kwargs) = reset_confirm.call_args
-        self.assertEquals(confirm_kwargs['uidb36'], self.uidb36)
-        self.assertEquals(confirm_kwargs['token'], self.token)
-        self.user = User.objects.get(pk=self.user.pk)
-        self.assertTrue(self.user.is_active)
-
-
 class CourseEndingTest(TestCase):
     """Test things related to course endings: certificates, surveys, etc"""
 

common/djangoapps/student/views.py

@@ -30,6 +30,8 @@ from django.utils.http import cookie_date, base36_to_int
 from django.utils.translation import ugettext as _, get_language
 from django.views.decorators.http import require_POST, require_GET
 
+from django.template.response import TemplateResponse
+
 from ratelimitbackend.exceptions import RateLimitException
 
 from edxmako.shortcuts import render_to_response, render_to_string
@@ -39,7 +41,7 @@ from student.models import (
     Registration, UserProfile, PendingNameChange,
     PendingEmailChange, CourseEnrollment, unique_id_for_user,
     CourseEnrollmentAllowed, UserStanding, LoginFailures,
-    create_comments_service_user
+    create_comments_service_user, PasswordHistory
 )
 from student.forms import PasswordResetFormNoActive
 from student.firebase_token_generator import create_token
@@ -731,6 +733,15 @@ def login_user(request, error=""):
                 "value": _('This account has been temporarily locked due to excessive login failures. Try again later.'),
             })  # TODO: this should be status code 429  # pylint: disable=fixme
 
+    # see if the user must reset his/her password due to any policy settings
+    if PasswordHistory.should_user_reset_password_now(user_found_by_email_lookup):
+        return JsonResponse({
+            "success": False,
+            "value": _('Your password has expired due to password policy on this account. You must '
+                       'reset your password before you can log in again. Please click the '
+                       'Forgot Password" link on this page to reset your password before logging in again.'),
+        })  # TODO: this should be status code 403  # pylint: disable=fixme
+
     # if the user doesn't exist, we want to set the username to an invalid
     # username so that authentication is guaranteed to fail and we can take
     # advantage of the ratelimited backend
@@ -955,6 +966,7 @@ def _do_create_account(post_vars):
                 is_active=False)
     user.set_password(post_vars['password'])
     registration = Registration()
+
     # TODO: Rearrange so that if part of the process fails, the whole process fails.
     # Right now, we can have e.g. no registration e-mail sent out and a zombie account
     try:
@@ -974,6 +986,11 @@ def _do_create_account(post_vars):
         else:
             raise
 
+    # add this account creation to password history
+    # NOTE, this will be a NOP unless the feature has been turned on in configuration
+    password_history_entry = PasswordHistory()
+    password_history_entry.create(user)
+
     registration.register(user)
 
     profile = UserProfile(user=user)
@@ -1403,12 +1420,71 @@ def password_reset_confirm_wrapper(
         user.save()
     except (ValueError, User.DoesNotExist):
         pass
-    # we also want to pass settings.PLATFORM_NAME in as extra_context
 
-    extra_context = {"platform_name": settings.PLATFORM_NAME}
-    return password_reset_confirm(
-        request, uidb36=uidb36, token=token, extra_context=extra_context
-    )
+    # tie in password strength enforcement as an optional level of
+    # security protection
+    err_msg = None
+
+    if request.method == 'POST':
+        password = request.POST['new_password1']
+        if settings.FEATURES.get('ENFORCE_PASSWORD_POLICY', False):
+            try:
+                validate_password_length(password)
+                validate_password_complexity(password)
+                validate_password_dictionary(password)
+            except ValidationError, 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:
+                num_distinct = settings.ADVANCED_SECURITY_CONFIG['MIN_DIFFERENT_STAFF_PASSWORDS_BEFORE_REUSE']
+            else:
+                num_distinct = settings.ADVANCED_SECURITY_CONFIG['MIN_DIFFERENT_STUDENT_PASSWORDS_BEFORE_REUSE']
+            err_msg = _("You are re-using a password that you have used recently. You must "
+                        "have {0} distinct password(s) before reusing a previous password.").format(num_distinct)
+
+        # also, check to see if passwords are getting reset too frequent
+        if PasswordHistory.is_password_reset_too_soon(user):
+            num_days = settings.ADVANCED_SECURITY_CONFIG['MIN_TIME_IN_DAYS_BETWEEN_ALLOWED_RESETS']
+            err_msg = _("You are resetting passwords too frequently. Due to security policies, "
+                        "{0} day(s) must elapse between password resets").format(num_days)
+
+    if err_msg:
+        # We have an password reset attempt which violates some security policy, use the
+        # existing Django template to communicate this back to the user
+        context = {
+            'validlink': True,
+            'form': None,
+            'title': _('Password reset unsuccessful'),
+            'err_msg': err_msg,
+        }
+        return TemplateResponse(request, 'registration/password_reset_confirm.html', context)
+    else:
+        # we also want to pass settings.PLATFORM_NAME in as extra_context
+        extra_context = {"platform_name": settings.PLATFORM_NAME}
+
+        if request.method == 'POST':
+            # remember what the old password hash is before we call down
+            old_password_hash = user.password
+
+            result = password_reset_confirm(
+                request, uidb36=uidb36, token=token, extra_context=extra_context
+            )
+
+            # get the updated user
+            updated_user = User.objects.get(id=uid_int)
+
+            # did the password hash change, if so record it in the PasswordHistory
+            if updated_user.password != old_password_hash:
+                entry = PasswordHistory()
+                entry.create(updated_user)
+
+            return result
+        else:
+            return password_reset_confirm(
+                request, uidb36=uidb36, token=token, extra_context=extra_context
+            )
 
 
 def reactivation_email_for_user(user):

lms/envs/aws.py

@@ -391,5 +391,9 @@ TIME_ZONE_DISPLAYED_FOR_DEADLINES = ENV_TOKENS.get("TIME_ZONE_DISPLAYED_FOR_DEAD
 ##### X-Frame-Options response header settings #####
 X_FRAME_OPTIONS = ENV_TOKENS.get('X_FRAME_OPTIONS', X_FRAME_OPTIONS)
 
+
 ##### Third-party auth options ################################################
 THIRD_PARTY_AUTH = AUTH_TOKENS.get('THIRD_PARTY_AUTH', THIRD_PARTY_AUTH)
+
+##### ADVANCED_SECURITY_CONFIG #####
+ADVANCED_SECURITY_CONFIG = ENV_TOKENS.get('ADVANCED_SECURITY_CONFIG', {})

lms/envs/common.py

@@ -240,6 +240,9 @@ FEATURES = {
 
     # Prevent concurrent logins per user
     'PREVENT_CONCURRENT_LOGINS': False,
+
+    # Turn off Advanced Security by default
+    'ADVANCED_SECURITY': False,
 }
 
 # Used for A/B testing
@@ -1528,3 +1531,7 @@ for app_name in OPTIONAL_APPS:
 # Stub for third_party_auth options.
 # See common/djangoapps/third_party_auth/settings.py for configuration details.
 THIRD_PARTY_AUTH = {}
+
+### ADVANCED_SECURITY_CONFIG
+# Empty by default
+ADVANCED_SECURITY_CONFIG = {}

lms/templates/registration/password_reset_confirm.html

@@ -86,11 +86,19 @@
           </h3>
         </div>
 
-        <div role="alert" class="status message submission-error">
+        {% if err_msg %}
+        <div role="alert" class="status message submission-error" style="display: block;">
+        {% else %}
+        <div role="alert" class="status message submission-error" style="display: none;">
+        {% endif %}
           <h3 class="message-title">{% trans "The following errors occurred while processing your registration: " %}</h3>
           <ul class="message-copy">
-            <li>{% trans "You must complete all fields." %}</li>
-            <li>{% trans "The two password fields didn't match." %}</li>
+            {% if err_msg %}
+              <li>{{err_msg}}</li>
+            {% else %}
+              <li>{% trans "You must complete all fields." %}</li>
+              <li>{% trans "The two password fields didn't match." %}</li>
+            {% endif %}
           </ul>
         </div>