diff --git a/cms/envs/aws.py b/cms/envs/aws.py index f5954abd41..14dae321bb 100644 --- a/cms/envs/aws.py +++ b/cms/envs/aws.py @@ -455,9 +455,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', {}) - ################ ADVANCED COMPONENT/PROBLEM TYPES ############### ADVANCED_PROBLEM_TYPES = ENV_TOKENS.get('ADVANCED_PROBLEM_TYPES', ADVANCED_PROBLEM_TYPES) diff --git a/cms/envs/common.py b/cms/envs/common.py index 73c9bde775..e4be757e81 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -223,9 +223,6 @@ FEATURES = { # Prevent concurrent logins per user 'PREVENT_CONCURRENT_LOGINS': False, - # Turn off Advanced Security by default - 'ADVANCED_SECURITY': False, - # Turn off Video Upload Pipeline through Studio, by default 'ENABLE_VIDEO_UPLOAD_PIPELINE': False, @@ -1317,10 +1314,6 @@ for app_name, insert_before in OPTIONAL_APPS: INSTALLED_APPS.append(app_name) -### ADVANCED_SECURITY_CONFIG -# Empty by default -ADVANCED_SECURITY_CONFIG = {} - ### External auth usage -- prefixes for ENROLLMENT_DOMAIN SHIBBOLETH_DOMAIN_PREFIX = 'shib:' OPENID_DOMAIN_PREFIX = 'openid:' diff --git a/cms/envs/production.py b/cms/envs/production.py index 8d4fa755d6..2fc776a3b7 100644 --- a/cms/envs/production.py +++ b/cms/envs/production.py @@ -458,9 +458,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', {}) - ################ ADVANCED COMPONENT/PROBLEM TYPES ############### ADVANCED_PROBLEM_TYPES = ENV_TOKENS.get('ADVANCED_PROBLEM_TYPES', ADVANCED_PROBLEM_TYPES) diff --git a/common/djangoapps/student/helpers.py b/common/djangoapps/student/helpers.py index aa4d6c796e..ee7c75b5be 100644 --- a/common/djangoapps/student/helpers.py +++ b/common/djangoapps/student/helpers.py @@ -39,7 +39,6 @@ from openedx.core.djangoapps.theming.helpers import get_themes from openedx.core.djangoapps.user_authn.utils import is_safe_login_or_logout_redirect from student.models import ( LinkedInAddToProfileConfiguration, - PasswordHistory, Registration, UserAttribute, UserProfile, @@ -645,11 +644,6 @@ def do_create_account(form, custom_form=None): 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_fields = [ diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 2d84e1bade..3b99c90b66 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -792,204 +792,12 @@ EVENT_NAME_ENROLLMENT_MODE_CHANGED = 'edx.course.enrollment.mode_changed' 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) + This model is deprecated, no longer used, and slated for removal. """ user = models.ForeignKey(User, on_delete=models.CASCADE) 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 - """ - if not settings.FEATURES['ADVANCED_SECURITY']: - return False - min_diff_pw = settings.ADVANCED_SECURITY_CONFIG.get( - 'MIN_DIFFERENT_STUDENT_PASSWORDS_BEFORE_REUSE', 0 - ) - return min_diff_pw > 0 - - @classmethod - def is_staff_password_reuse_restricted(cls): - """ - Returns whether the configuration which limits password reuse has been turned on - """ - if not settings.FEATURES['ADVANCED_SECURITY']: - return False - min_diff_pw = settings.ADVANCED_SECURITY_CONFIG.get( - 'MIN_DIFFERENT_STAFF_PASSWORDS_BEFORE_REUSE', 0 - ) - return min_diff_pw > 0 - - @classmethod - def is_password_reset_frequency_restricted(cls): - """ - Returns whether the configuration which limits the password reset frequency has been turned on - """ - if not settings.FEATURES['ADVANCED_SECURITY']: - return False - min_days_between_reset = settings.ADVANCED_SECURITY_CONFIG.get( - 'MIN_TIME_IN_DAYS_BETWEEN_ALLOWED_RESETS' - ) - return min_days_between_reset - - @classmethod - def is_staff_forced_password_reset_enabled(cls): - """ - Returns whether the configuration which forces password resets to occur has been turned on - """ - if not settings.FEATURES['ADVANCED_SECURITY']: - return False - min_days_between_reset = settings.ADVANCED_SECURITY_CONFIG.get( - 'MIN_DAYS_FOR_STAFF_ACCOUNTS_PASSWORD_RESETS' - ) - return min_days_between_reset - - @classmethod - def is_student_forced_password_reset_enabled(cls): - """ - Returns whether the configuration which forces password resets to occur has been turned on - """ - if not settings.FEATURES['ADVANCED_SECURITY']: - return False - min_days_pw_reset = settings.ADVANCED_SECURITY_CONFIG.get( - 'MIN_DAYS_FOR_STUDENT_ACCOUNTS_PASSWORD_RESETS' - ) - return min_days_pw_reset - - @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 - """ - assert user - - 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 - """ - assert user - - 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 - - @classmethod - def retire_user(cls, user_id): - """ - Updates the password in all rows corresponding to a user - to an empty string as part of removing PII for user retirement. - """ - return cls.objects.filter(user_id=user_id).update(password="") - class LoginFailures(models.Model): """ diff --git a/common/djangoapps/student/tests/test_events.py b/common/djangoapps/student/tests/test_events.py index 7b66b985b4..55c79dc00b 100644 --- a/common/djangoapps/student/tests/test_events.py +++ b/common/djangoapps/student/tests/test_events.py @@ -130,7 +130,7 @@ class TestUserEvents(UserSettingsEventTestMixin, TestCase): """ Verify that we don't emit events for related fields. """ - self.user.passwordhistory_set.create(password='new_password') + self.user.loginfailures_set.create() self.user.save() self.assert_no_events_were_emitted() diff --git a/common/djangoapps/student/tests/test_password_history.py b/common/djangoapps/student/tests/test_password_history.py deleted file mode 100644 index 26be7bd093..0000000000 --- a/common/djangoapps/student/tests/test_password_history.py +++ /dev/null @@ -1,228 +0,0 @@ -# -*- coding: utf-8 -*- -""" -This test file will verify proper password history enforcement -""" -from datetime import timedelta - -from django.test import TestCase -from django.test.utils import override_settings -from django.utils import timezone -from freezegun import freeze_time -from mock import patch - -from student.models import PasswordHistory -from student.tests.factories import AdminFactory, UserFactory - - -@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)) - - # We need some policy in place to create a history. It doesn't matter what it is. - @patch.dict("django.conf.settings.ADVANCED_SECURITY_CONFIG", {'MIN_DAYS_FOR_STUDENT_ACCOUNTS_PASSWORD_RESETS': 5}) - def test_retirement(self): - """ - Verify that the user's password history contains no actual - passwords after retirement is called. - """ - user = self._user_factory_with_history() - - # create multiple rows in the password history table - self._change_password(user, "different") - self._change_password(user, "differentagain") - # ensure the rows were actually created and stored the passwords - self.assertTrue(PasswordHistory.objects.filter(user_id=user.id).exists()) - for row in PasswordHistory.objects.filter(user_id=user.id): - self.assertFalse(row.password == "") - - # retire the user and ensure that the rows are still present, but with no passwords - PasswordHistory.retire_user(user.id) - self.assertTrue(PasswordHistory.objects.filter(user_id=user.id).exists()) - for row in PasswordHistory.objects.filter(user_id=user.id): - self.assertEqual(row.password, "") diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index 9ffeae26ea..5a639b9a07 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -79,7 +79,6 @@ from student.message_types import EmailChange, PasswordReset from student.models import ( AccountRecovery, CourseEnrollment, - PasswordHistory, PendingEmailChange, Registration, RegistrationCookieConfiguration, @@ -935,11 +934,6 @@ def password_reset_confirm_wrapper(request, uidb36=None, token=None): # 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) - else: response = password_reset_confirm( request, uidb64=uidb64, token=token, extra_context=platform_name diff --git a/lms/djangoapps/courseware/tests/test_password_history.py b/lms/djangoapps/courseware/tests/test_password_history.py deleted file mode 100644 index bad1f5dd8b..0000000000 --- a/lms/djangoapps/courseware/tests/test_password_history.py +++ /dev/null @@ -1,227 +0,0 @@ -""" -This file will test through the LMS some of the PasswordHistory features -""" -import json -from datetime import timedelta -from uuid import uuid4 - -import ddt -from django.contrib.auth.models import User -from django.contrib.auth.tokens import default_token_generator -from django.urls import reverse -from django.test.utils import override_settings -from django.utils import timezone -from django.utils.http import int_to_base36 -from freezegun import freeze_time -from mock import patch - -from courseware.tests.helpers import LoginEnrollmentTestCase -from student.models import PasswordHistory -from util.password_policy_validators import create_validator_config - - -@patch.dict("django.conf.settings.FEATURES", {'ADVANCED_SECURITY': True}) -@ddt.ddt -class TestPasswordHistory(LoginEnrollmentTestCase): - """ - Go through some of the PasswordHistory use cases - """ - shard = 1 - - def _login(self, email, password, should_succeed=True, err_msg_check=None): - """ - Override the base implementation so we can do appropriate asserts - """ - resp = self.client.post(reverse('login'), {'email': email, 'password': password}) - data = json.loads(resp.content) - - self.assertEqual(resp.status_code, 200) - if should_succeed: - self.assertTrue(data['success']) - else: - self.assertFalse(data['success']) - if err_msg_check: - self.assertIn(err_msg_check, data['value']) - - def _setup_user(self, is_staff=False, password=None): - """ - Override the base implementation to randomize the email - """ - email = 'foo_{0}@test.com'.format(uuid4().hex[:8]) - password = password if password else 'foo' - username = 'test_{0}'.format(uuid4().hex[:8]) - self.create_account(username, email, password) - self.activate_user(email) - - # manually twiddle the is_staff bit, if needed - if is_staff: - user = User.objects.get(email=email) - user.is_staff = True - user.save() - - return email, password - - def _update_password(self, email, new_password): - """ - Helper method to reset a password - """ - user = User.objects.get(email=email) - user.set_password(new_password) - user.save() - history = PasswordHistory() - history.create(user) - - def assertPasswordResetError(self, response, error_message, valid_link=True): - """ - 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.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}) - @patch.dict("django.conf.settings.ADVANCED_SECURITY_CONFIG", {'MIN_DAYS_FOR_STUDENT_ACCOUNTS_PASSWORD_RESETS': None}) - def test_no_forced_password_change(self): - """ - Makes sure default behavior is correct when we don't have this turned on - """ - - email, password = self._setup_user() - self._login(email, password) - - email, password = self._setup_user(is_staff=True) - self._login(email, password) - - @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): - """ - Make sure password are viewed as expired in LMS after the policy time has elapsed - """ - - student_email, student_password = self._setup_user() - staff_email, staff_password = self._setup_user(is_staff=True) - - self._login(student_email, student_password) - self._login(staff_email, staff_password) - - staff_reset_time = timezone.now() + timedelta(days=1) - with freeze_time(staff_reset_time): - self._login(student_email, student_password) - - # staff should fail because password expired - self._login(staff_email, staff_password, should_succeed=False, - err_msg_check="Your password has expired due to password policy on this account") - - # if we reset the password, we should be able to log in - self._update_password(staff_email, "updated") - self._login(staff_email, "updated") - - student_reset_time = timezone.now() + timedelta(days=5) - with freeze_time(student_reset_time): - # Both staff and student logins should fail because user must - # reset the password - - self._login(student_email, student_password, should_succeed=False, - err_msg_check="Your password has expired due to password policy on this account") - self._update_password(student_email, "updated") - self._login(student_email, "updated") - - self._login(staff_email, staff_password, should_succeed=False, - err_msg_check="Your password has expired due to password policy on this account") - self._update_password(staff_email, "updated2") - self._login(staff_email, "updated2") - - def test_allow_all_password_reuse(self): - """ - Tests that password_reset flows work as expected if reuse config is missing, meaning - passwords can always be reused - """ - student_email, _ = self._setup_user() - user = User.objects.get(email=student_email) - - err_msg = 'You are re-using a password that you have used recently.' - - 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': 'foo', - 'new_password2': 'foo' - }, follow=True) - - self.assertNotIn( - err_msg, - resp.content - ) - - @override_settings(AUTH_PASSWORD_VALIDATORS=[ - create_validator_config('util.password_policy_validators.MinimumLengthValidator', {'min_length': 6}) - ]) - def test_password_policy_on_password_reset(self): - """ - This makes sure the proper asserts on password policy also works on password reset - """ - staff_email, _ = self._setup_user(is_staff=True, password='foofoo') - - success_msg = 'Your Password Reset is Complete' - - # try to reset password, it should fail - user = User.objects.get(email=staff_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': 'foo', - 'new_password2': 'foo', - }, follow=True) - - self.assertNotIn( - success_msg, - resp.content - ) - - # try to reset password with a long enough password - user = User.objects.get(email=staff_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': 'foofoo', - 'new_password2': 'foofoo', - }, follow=True) - - self.assertIn(success_msg, resp.content) - - @ddt.data( - ('foo', 'foobar', 'Error in resetting your password. Please try again.'), - ('', '', 'This password is too short. It must contain at least'), - ) - @ddt.unpack - def test_password_reset_form_invalid(self, password1, password2, err_msg): - """ - Tests that password reset fail when providing bad passwords and error message is displayed - to the user. - """ - user_email, _ = self._setup_user() - - # 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) diff --git a/lms/djangoapps/courseware/tests/test_password_reset.py b/lms/djangoapps/courseware/tests/test_password_reset.py new file mode 100644 index 0000000000..7a32e930bb --- /dev/null +++ b/lms/djangoapps/courseware/tests/test_password_reset.py @@ -0,0 +1,117 @@ +""" +This file will test through the LMS some of the password reset features +""" +from uuid import uuid4 + +import ddt +from django.contrib.auth.models import User +from django.contrib.auth.tokens import default_token_generator +from django.test.utils import override_settings +from django.utils.http import int_to_base36 + +from courseware.tests.helpers import LoginEnrollmentTestCase +from util.password_policy_validators import create_validator_config + + +@ddt.ddt +class TestPasswordReset(LoginEnrollmentTestCase): + """ + Go through some of the password reset use cases + """ + shard = 1 + + def _setup_user(self, is_staff=False, password=None): + """ + Override the base implementation to randomize the email + """ + email = 'foo_{0}@test.com'.format(uuid4().hex[:8]) + password = password if password else 'foo' + username = 'test_{0}'.format(uuid4().hex[:8]) + self.create_account(username, email, password) + self.activate_user(email) + + # manually twiddle the is_staff bit, if needed + if is_staff: + user = User.objects.get(email=email) + user.is_staff = True + user.save() + + return email, password + + def assertPasswordResetError(self, response, error_message, valid_link=True): + """ + 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.assertEqual(response.status_code, 200) + self.assertEqual(response.context_data['validlink'], valid_link) + self.assertIn(error_message, response.content) + + @override_settings(AUTH_PASSWORD_VALIDATORS=[ + create_validator_config('util.password_policy_validators.MinimumLengthValidator', {'min_length': 6}) + ]) + def test_password_policy_on_password_reset(self): + """ + This makes sure the proper asserts on password policy also works on password reset + """ + staff_email, _ = self._setup_user(is_staff=True, password='foofoo') + + success_msg = 'Your Password Reset is Complete' + + # try to reset password, it should fail + user = User.objects.get(email=staff_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': 'foo', + 'new_password2': 'foo', + }, follow=True) + + self.assertNotIn( + success_msg, + resp.content + ) + + # try to reset password with a long enough password + user = User.objects.get(email=staff_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': 'foofoo', + 'new_password2': 'foofoo', + }, follow=True) + + self.assertIn(success_msg, resp.content) + + @ddt.data( + ('foo', 'foobar', 'Error in resetting your password. Please try again.'), + ('', '', 'This password is too short. It must contain at least'), + ) + @ddt.unpack + def test_password_reset_form_invalid(self, password1, password2, err_msg): + """ + Tests that password reset fail when providing bad passwords and error message is displayed + to the user. + """ + user_email, _ = self._setup_user() + + # 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) diff --git a/lms/envs/aws.py b/lms/envs/aws.py index 98285a4eba..439597f85e 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -733,9 +733,6 @@ if FEATURES.get('ENABLE_OAUTH2_PROVIDER'): TPA_PROVIDER_BURST_THROTTLE = ENV_TOKENS.get('TPA_PROVIDER_BURST_THROTTLE', TPA_PROVIDER_BURST_THROTTLE) TPA_PROVIDER_SUSTAINED_THROTTLE = ENV_TOKENS.get('TPA_PROVIDER_SUSTAINED_THROTTLE', TPA_PROVIDER_SUSTAINED_THROTTLE) -##### ADVANCED_SECURITY_CONFIG ##### -ADVANCED_SECURITY_CONFIG = ENV_TOKENS.get('ADVANCED_SECURITY_CONFIG', {}) - ##### GOOGLE ANALYTICS IDS ##### GOOGLE_ANALYTICS_ACCOUNT = AUTH_TOKENS.get('GOOGLE_ANALYTICS_ACCOUNT') GOOGLE_ANALYTICS_TRACKING_ID = AUTH_TOKENS.get('GOOGLE_ANALYTICS_TRACKING_ID') diff --git a/lms/envs/bok_choy.py b/lms/envs/bok_choy.py index e87c4ee056..70f0b8080a 100644 --- a/lms/envs/bok_choy.py +++ b/lms/envs/bok_choy.py @@ -181,7 +181,6 @@ YOUTUBE['TEXT_API']['url'] = "{0}:{1}/test_transcripts_youtube/".format(YOUTUBE_ FEATURES['ENABLE_MAX_FAILED_LOGIN_ATTEMPTS'] = False FEATURES['SQUELCH_PII_IN_LOGS'] = False FEATURES['PREVENT_CONCURRENT_LOGINS'] = False -FEATURES['ADVANCED_SECURITY'] = False FEATURES['ENABLE_MOBILE_REST_API'] = True # Show video bumper in LMS FEATURES['ENABLE_VIDEO_BUMPER'] = True # Show video bumper in LMS diff --git a/lms/envs/common.py b/lms/envs/common.py index a68d48b993..23c28252a9 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -238,9 +238,6 @@ FEATURES = { # Prevent concurrent logins per user 'PREVENT_CONCURRENT_LOGINS': True, - # Turn on Advanced Security by default - 'ADVANCED_SECURITY': True, - # When a logged in user goes to the homepage ('/') should the user be # redirected to the dashboard - this is default Open edX behavior. Set to # False to not redirect the user @@ -2919,10 +2916,6 @@ for app_name, insert_before in OPTIONAL_APPS: except (IndexError, ValueError): INSTALLED_APPS.append(app_name) -### ADVANCED_SECURITY_CONFIG -# Empty by default -ADVANCED_SECURITY_CONFIG = {} - ### External auth usage -- prefixes for ENROLLMENT_DOMAIN SHIBBOLETH_DOMAIN_PREFIX = 'shib:' OPENID_DOMAIN_PREFIX = 'openid:' diff --git a/lms/envs/devstack.py b/lms/envs/devstack.py index 8b436ac3f7..49af7ea8c7 100644 --- a/lms/envs/devstack.py +++ b/lms/envs/devstack.py @@ -142,7 +142,6 @@ FEATURES['ENABLE_VIDEO_ABSTRACTION_LAYER_API'] = True FEATURES['ENABLE_MAX_FAILED_LOGIN_ATTEMPTS'] = False FEATURES['SQUELCH_PII_IN_LOGS'] = False FEATURES['PREVENT_CONCURRENT_LOGINS'] = False -FEATURES['ADVANCED_SECURITY'] = False ########################### Milestones ################################# FEATURES['MILESTONES_APP'] = True diff --git a/lms/envs/production.py b/lms/envs/production.py index 7b24c7e377..a98925a535 100644 --- a/lms/envs/production.py +++ b/lms/envs/production.py @@ -729,9 +729,6 @@ if FEATURES.get('ENABLE_OAUTH2_PROVIDER'): OAUTH_ID_TOKEN_EXPIRATION = ENV_TOKENS.get('OAUTH_ID_TOKEN_EXPIRATION', OAUTH_ID_TOKEN_EXPIRATION) OAUTH_DELETE_EXPIRED = ENV_TOKENS.get('OAUTH_DELETE_EXPIRED', OAUTH_DELETE_EXPIRED) -##### ADVANCED_SECURITY_CONFIG ##### -ADVANCED_SECURITY_CONFIG = ENV_TOKENS.get('ADVANCED_SECURITY_CONFIG', {}) - ##### GOOGLE ANALYTICS IDS ##### GOOGLE_ANALYTICS_ACCOUNT = AUTH_TOKENS.get('GOOGLE_ANALYTICS_ACCOUNT') GOOGLE_ANALYTICS_TRACKING_ID = AUTH_TOKENS.get('GOOGLE_ANALYTICS_TRACKING_ID') diff --git a/lms/envs/test.py b/lms/envs/test.py index 86be9082bc..d5f094f6eb 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -228,7 +228,6 @@ FEATURES['ENFORCE_PASSWORD_POLICY'] = False FEATURES['ENABLE_MAX_FAILED_LOGIN_ATTEMPTS'] = False FEATURES['SQUELCH_PII_IN_LOGS'] = False FEATURES['PREVENT_CONCURRENT_LOGINS'] = False -FEATURES['ADVANCED_SECURITY'] = False ######### Third-party auth ########## FEATURES['ENABLE_THIRD_PARTY_AUTH'] = True diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py index ae0c1822a8..2aa14cbb6f 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py @@ -57,7 +57,6 @@ from student.models import ( CourseEnrollment, CourseEnrollmentAllowed, ManualEnrollmentAudit, - PasswordHistory, PendingEmailChange, PendingNameChange, Registration, @@ -1533,7 +1532,6 @@ class TestLMSAccountRetirementPost(RetirementTestCase, ModuleStoreTestCase): # other setup PendingNameChange.objects.create(user=self.test_user, new_name=self.pii_standin, rationale=self.pii_standin) - PasswordHistory.objects.create(user=self.test_user, password=self.pii_standin) # setup for doing POST from test client self.headers = build_jwt_headers(self.test_superuser) @@ -1563,7 +1561,6 @@ class TestLMSAccountRetirementPost(RetirementTestCase, ModuleStoreTestCase): self.assertEqual(RevisionPluginRevision.objects.get(user=self.test_user).ip_address, None) self.assertEqual(ArticleRevision.objects.get(user=self.test_user).ip_address, None) self.assertFalse(PendingNameChange.objects.filter(user=self.test_user).exists()) - self.assertEqual(PasswordHistory.objects.get(user=self.test_user).password, '') self.assertEqual( ManualEnrollmentAudit.objects.get( diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index 1ed32c376e..b743c377f9 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -47,7 +47,6 @@ from openedx.core.lib.api.parsers import MergePatchParser from student.models import ( CourseEnrollment, ManualEnrollmentAudit, - PasswordHistory, PendingNameChange, CourseEnrollmentAllowed, LoginFailures, @@ -825,7 +824,6 @@ class LMSAccountRetirementView(ViewSet): RevisionPluginRevision.retire_user(retirement.user) ArticleRevision.retire_user(retirement.user) PendingNameChange.delete_by_user_value(retirement.user, field='user') - PasswordHistory.retire_user(retirement.user.id) ManualEnrollmentAudit.retire_manual_enrollments(retirement.user, retirement.retired_email) CreditRequest.retire_user(retirement) diff --git a/openedx/core/djangoapps/user_authn/views/login.py b/openedx/core/djangoapps/user_authn/views/login.py index 7798c7e8bf..92b955fb14 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/openedx/core/djangoapps/user_authn/views/login.py @@ -26,10 +26,7 @@ from openedx.core.djangoapps.password_policy import compliance as password_polic from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.util.user_messages import PageLevelMessages from openedx.core.djangolib.markup import HTML, Text -from student.models import ( - LoginFailures, - PasswordHistory, -) +from student.models import LoginFailures from student.views import send_reactivation_email_for_user from student.forms import send_password_reset_email_for_user from track import segment @@ -134,16 +131,6 @@ def _check_excessive_login_attempts(user): 'to excessive login failures. Try again later.')) -def _check_forced_password_reset(user): - """ - See if the user must reset his/her password due to any policy settings - """ - if user and PasswordHistory.should_user_reset_password_now(user): - raise AuthFailedError(_('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.')) - - def _enforce_password_policy_compliance(request, user): try: password_policy_compliance.enforce_compliance_on_login(user, request.POST.get('password')) @@ -359,7 +346,6 @@ def login_user(request): _check_shib_redirect(email_user) _check_excessive_login_attempts(email_user) - _check_forced_password_reset(email_user) possibly_authenticated_user = email_user diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_login.py b/openedx/core/djangoapps/user_authn/views/tests/test_login.py index 0e69f90f2a..23ffa1a90a 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_login.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_login.py @@ -110,8 +110,7 @@ class LoginTest(CacheIsolationTestCase): value='Email or password is incorrect') self._assert_audit_log(mock_audit_log, 'warning', [u'Login failed', u'Unknown user email', nonexistent_email]) - @patch.dict("django.conf.settings.FEATURES", {'ADVANCED_SECURITY': True}) - def test_login_fail_incorrect_email_with_advanced_security(self): + def test_login_fail_incorrect_email(self): nonexistent_email = u'not_a_user@edx.org' response, mock_audit_log = self._login_response( nonexistent_email,