From deb758c808aa81a6d370ab94a97ffd90ae1b7dde Mon Sep 17 00:00:00 2001 From: Dillon Dumesnil Date: Tue, 2 Oct 2018 14:21:07 -0400 Subject: [PATCH] Added in tests for the new password validation. Fixed old tests that relied on the old configuration values and old way of validating passwords. Also improved registration page by always showing error messages rather than hiding them on leaving the field. --- cms/envs/aws.py | 2 +- cms/envs/common.py | 26 +- cms/envs/production.py | 6 +- common/djangoapps/student/forms.py | 10 +- .../management/tests/test_create_user.py | 2 +- .../student/tests/test_password_policy.py | 204 +++++++------- .../student/tests/test_reset_password.py | 11 +- common/djangoapps/student/views/management.py | 6 +- .../util/password_policy_validators.py | 74 +++-- .../tests/test_password_policy_validators.py | 253 ++++++++++++++---- .../tests/studio/test_studio_course_team.py | 2 +- .../courseware/tests/test_password_history.py | 148 +--------- lms/envs/aws.py | 2 +- lms/envs/bok_choy.py | 4 - lms/envs/common.py | 25 +- lms/envs/devstack.py | 1 - lms/envs/production.py | 6 +- lms/envs/test.py | 2 - .../js/student_account/views/RegisterView.js | 4 +- .../djangoapps/password_policy/compliance.py | 4 +- .../password_policy/tests/test_compliance.py | 25 +- .../core/djangoapps/user_api/accounts/api.py | 5 +- .../user_api/accounts/tests/test_api.py | 4 +- .../user_api/accounts/tests/testutils.py | 8 +- openedx/core/djangoapps/user_api/api.py | 7 +- .../djangoapps/user_api/tests/test_views.py | 58 ++-- .../user_api/validation/tests/test_views.py | 20 +- .../djangoapps/user_authn/views/deprecated.py | 2 +- .../djangoapps/user_authn/views/register.py | 3 +- .../user_authn/views/tests/test_register.py | 27 +- 30 files changed, 482 insertions(+), 469 deletions(-) diff --git a/cms/envs/aws.py b/cms/envs/aws.py index 25d646ed1e..a7918cfd2e 100644 --- a/cms/envs/aws.py +++ b/cms/envs/aws.py @@ -447,7 +447,7 @@ MAX_FAILED_LOGIN_ATTEMPTS_ALLOWED = ENV_TOKENS.get("MAX_FAILED_LOGIN_ATTEMPTS_AL MAX_FAILED_LOGIN_ATTEMPTS_LOCKOUT_PERIOD_SECS = ENV_TOKENS.get("MAX_FAILED_LOGIN_ATTEMPTS_LOCKOUT_PERIOD_SECS", 15 * 60) #### PASSWORD POLICY SETTINGS ##### -AUTH_PASSWORD_VALIDATORS = ENV_TOKENS.get("AUTH_PASSWORD_VALIDATORS", []) +AUTH_PASSWORD_VALIDATORS = ENV_TOKENS.get("AUTH_PASSWORD_VALIDATORS", AUTH_PASSWORD_VALIDATORS) ### INACTIVITY SETTINGS #### SESSION_INACTIVITY_TIMEOUT_IN_SECONDS = AUTH_TOKENS.get("SESSION_INACTIVITY_TIMEOUT_IN_SECONDS") diff --git a/cms/envs/common.py b/cms/envs/common.py index 3c185494e1..d6b9f33d8e 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -200,9 +200,6 @@ FEATURES = { # an Open edX admin has added them to the course creator group. 'ENABLE_CREATOR_GROUP': True, - # whether to use password policy enforcement or not - 'ENFORCE_PASSWORD_POLICY': False, - # Turn off account locking if failed login attempts exceeds a limit 'ENABLE_MAX_FAILED_LOGIN_ATTEMPTS': False, @@ -1240,12 +1237,23 @@ EVENT_TRACKING_BACKENDS = { EVENT_TRACKING_PROCESSORS = [] #### PASSWORD POLICY SETTINGS ##### - -PASSWORD_MIN_LENGTH = None -PASSWORD_MAX_LENGTH = None -PASSWORD_COMPLEXITY = {} -PASSWORD_DICTIONARY_EDIT_DISTANCE_THRESHOLD = None -PASSWORD_DICTIONARY = [] +AUTH_PASSWORD_VALIDATORS = [ + { + "NAME": "django.contrib.auth.password_validation.UserAttributeSimilarityValidator", + }, + { + "NAME": "util.password_policy_validators.MinimumLengthValidator", + "OPTIONS": { + "min_length": 2 + } + }, + { + "NAME": "util.password_policy_validators.MaximumLengthValidator", + "OPTIONS": { + "max_length": 75 + } + }, +] ##### ACCOUNT LOCKOUT DEFAULT PARAMETERS ##### MAX_FAILED_LOGIN_ATTEMPTS_ALLOWED = 5 diff --git a/cms/envs/production.py b/cms/envs/production.py index 5a8842f9d2..65f6f8813a 100644 --- a/cms/envs/production.py +++ b/cms/envs/production.py @@ -446,11 +446,7 @@ MAX_FAILED_LOGIN_ATTEMPTS_ALLOWED = ENV_TOKENS.get("MAX_FAILED_LOGIN_ATTEMPTS_AL MAX_FAILED_LOGIN_ATTEMPTS_LOCKOUT_PERIOD_SECS = ENV_TOKENS.get("MAX_FAILED_LOGIN_ATTEMPTS_LOCKOUT_PERIOD_SECS", 15 * 60) #### PASSWORD POLICY SETTINGS ##### -PASSWORD_MIN_LENGTH = ENV_TOKENS.get("PASSWORD_MIN_LENGTH") -PASSWORD_MAX_LENGTH = ENV_TOKENS.get("PASSWORD_MAX_LENGTH") -PASSWORD_COMPLEXITY = ENV_TOKENS.get("PASSWORD_COMPLEXITY", {}) -PASSWORD_DICTIONARY_EDIT_DISTANCE_THRESHOLD = ENV_TOKENS.get("PASSWORD_DICTIONARY_EDIT_DISTANCE_THRESHOLD") -PASSWORD_DICTIONARY = ENV_TOKENS.get("PASSWORD_DICTIONARY", []) +AUTH_PASSWORD_VALIDATORS = ENV_TOKENS.get("AUTH_PASSWORD_VALIDATORS", AUTH_PASSWORD_VALIDATORS) ### INACTIVITY SETTINGS #### SESSION_INACTIVITY_TIMEOUT_IN_SECONDS = AUTH_TOKENS.get("SESSION_INACTIVITY_TIMEOUT_IN_SECONDS") diff --git a/common/djangoapps/student/forms.py b/common/djangoapps/student/forms.py index c43b10ca49..d3b3cdb5a8 100644 --- a/common/djangoapps/student/forms.py +++ b/common/djangoapps/student/forms.py @@ -27,7 +27,7 @@ from openedx.core.djangoapps.user_api import accounts as accounts_settings from openedx.core.djangoapps.user_api.preferences.api import get_user_preference from student.message_types import PasswordReset from student.models import CourseEnrollmentAllowed, email_exists_or_retired -from util.password_policy_validators import edX_validate_password +from util.password_policy_validators import validate_password def send_password_reset_email_for_user(user, request): @@ -225,14 +225,14 @@ class AccountCreationForm(forms.Form): data=None, extra_fields=None, extended_profile_fields=None, - enforce_password_policy=False, + do_third_party_auth=True, tos_required=True ): super(AccountCreationForm, self).__init__(data) extra_fields = extra_fields or {} self.extended_profile_fields = extended_profile_fields or {} - self.enforce_password_policy = enforce_password_policy + self.do_third_party_auth = do_third_party_auth if tos_required: self.fields["terms_of_service"] = TrueField( error_messages={"required": _("You must accept the terms of service.")} @@ -280,13 +280,13 @@ class AccountCreationForm(forms.Form): def clean_password(self): """Enforce password policies (if applicable)""" password = self.cleaned_data["password"] - if self.enforce_password_policy: + if not self.do_third_party_auth: # Creating a temporary user object to test password against username # This user should NOT be saved username = self.cleaned_data.get('username') email = self.cleaned_data.get('email') temp_user = User(username=username, email=email) if username else None - edX_validate_password(password, temp_user) + validate_password(password, temp_user) return password def clean_email(self): diff --git a/common/djangoapps/student/management/tests/test_create_user.py b/common/djangoapps/student/management/tests/test_create_user.py index e0a638b9a7..903d43faa7 100644 --- a/common/djangoapps/student/management/tests/test_create_user.py +++ b/common/djangoapps/student/management/tests/test_create_user.py @@ -22,7 +22,7 @@ class CreateUserMgmtTests(SharedModuleStoreTestCase): self.course = CourseFactory.create() self.user_model = get_user_model() self.default_email = 'testuser555@test.edx.org' - self.default_password = 'testuser@555@password' + self.default_password = 'b3TT3rPa$$w0rd!' # This is the default mode that the create_user commands gives a user enrollment self.default_course_mode = CourseMode.HONOR diff --git a/common/djangoapps/student/tests/test_password_policy.py b/common/djangoapps/student/tests/test_password_policy.py index 1b8dc1ce04..366fdf11ae 100644 --- a/common/djangoapps/student/tests/test_password_policy.py +++ b/common/djangoapps/student/tests/test_password_policy.py @@ -16,9 +16,9 @@ from mock import patch from openedx.core.djangoapps.external_auth.models import ExternalAuthMap from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory from openedx.core.djangoapps.user_authn.views.deprecated import create_account +from util.password_policy_validators import create_validator_config -@patch.dict("django.conf.settings.FEATURES", {'ENFORCE_PASSWORD_POLICY': True}) class TestPasswordPolicy(TestCase): """ Go through some password policy tests to make sure things are properly working @@ -35,7 +35,9 @@ class TestPasswordPolicy(TestCase): 'honor_code': 'true', } - @override_settings(PASSWORD_MIN_LENGTH=6) + @override_settings(AUTH_PASSWORD_VALIDATORS=[ + create_validator_config('util.password_policy_validators.MinimumLengthValidator', {'min_length': 6}) + ]) def test_password_length_too_short(self): self.url_params['password'] = 'aaa' response = self.client.post(self.url, self.url_params) @@ -43,10 +45,12 @@ class TestPasswordPolicy(TestCase): obj = json.loads(response.content) self.assertEqual( obj['value'], - "Enter a password with at least 6 characters.", + "This password is too short. It must contain at least 6 characters.", ) - @override_settings(PASSWORD_MIN_LENGTH=6) + @override_settings(AUTH_PASSWORD_VALIDATORS=[ + create_validator_config('util.password_policy_validators.MinimumLengthValidator', {'min_length': 6}) + ]) def test_password_length_long_enough(self): self.url_params['password'] = 'ThisIsALongerPassword' response = self.client.post(self.url, self.url_params) @@ -54,7 +58,9 @@ class TestPasswordPolicy(TestCase): obj = json.loads(response.content) self.assertTrue(obj['success']) - @override_settings(PASSWORD_MAX_LENGTH=12) + @override_settings(AUTH_PASSWORD_VALIDATORS=[ + create_validator_config('util.password_policy_validators.MaximumLengthValidator', {'max_length': 12}) + ]) def test_password_length_too_long(self): self.url_params['password'] = 'ThisPasswordIsWayTooLong' response = self.client.post(self.url, self.url_params) @@ -62,10 +68,12 @@ class TestPasswordPolicy(TestCase): obj = json.loads(response.content) self.assertEqual( obj['value'], - "Enter a password with at most 12 characters.", + "This password is too long. It must contain no more than 12 characters.", ) - @patch.dict("django.conf.settings.PASSWORD_COMPLEXITY", {'UPPER': 3}) + @override_settings(AUTH_PASSWORD_VALIDATORS=[ + create_validator_config('util.password_policy_validators.UppercaseValidator', {'min_upper': 3}) + ]) def test_password_not_enough_uppercase(self): self.url_params['password'] = 'thisshouldfail' response = self.client.post(self.url, self.url_params) @@ -73,10 +81,12 @@ class TestPasswordPolicy(TestCase): obj = json.loads(response.content) self.assertEqual( obj['value'], - "Enter a password with at least 3 uppercase letters.", + "This password must contain at least 3 uppercase letters.", ) - @patch.dict("django.conf.settings.PASSWORD_COMPLEXITY", {'UPPER': 3}) + @override_settings(AUTH_PASSWORD_VALIDATORS=[ + create_validator_config('util.password_policy_validators.UppercaseValidator', {'min_upper': 3}) + ]) def test_password_enough_uppercase(self): self.url_params['password'] = 'ThisShouldPass' response = self.client.post(self.url, self.url_params) @@ -84,7 +94,9 @@ class TestPasswordPolicy(TestCase): obj = json.loads(response.content) self.assertTrue(obj['success']) - @patch.dict("django.conf.settings.PASSWORD_COMPLEXITY", {'LOWER': 3}) + @override_settings(AUTH_PASSWORD_VALIDATORS=[ + create_validator_config('util.password_policy_validators.LowercaseValidator', {'min_lower': 3}) + ]) def test_password_not_enough_lowercase(self): self.url_params['password'] = 'THISSHOULDFAIL' response = self.client.post(self.url, self.url_params) @@ -92,10 +104,12 @@ class TestPasswordPolicy(TestCase): obj = json.loads(response.content) self.assertEqual( obj['value'], - "Enter a password with at least 3 lowercase letters.", + "This password must contain at least 3 lowercase letters.", ) - @patch.dict("django.conf.settings.PASSWORD_COMPLEXITY", {'LOWER': 3}) + @override_settings(AUTH_PASSWORD_VALIDATORS=[ + create_validator_config('util.password_policy_validators.LowercaseValidator', {'min_lower': 3}) + ]) def test_password_enough_lowercase(self): self.url_params['password'] = 'ThisShouldPass' response = self.client.post(self.url, self.url_params) @@ -103,26 +117,9 @@ class TestPasswordPolicy(TestCase): obj = json.loads(response.content) self.assertTrue(obj['success']) - @patch.dict("django.conf.settings.PASSWORD_COMPLEXITY", {'DIGITS': 3}) - def test_not_enough_digits(self): - self.url_params['password'] = 'thishasnodigits' - response = self.client.post(self.url, self.url_params) - self.assertEqual(response.status_code, 400) - obj = json.loads(response.content) - self.assertEqual( - obj['value'], - "Enter a password with at least 3 digits.", - ) - - @patch.dict("django.conf.settings.PASSWORD_COMPLEXITY", {'DIGITS': 3}) - def test_enough_digits(self): - self.url_params['password'] = 'Th1sSh0uldPa88' - response = self.client.post(self.url, self.url_params) - self.assertEqual(response.status_code, 200) - obj = json.loads(response.content) - self.assertTrue(obj['success']) - - @patch.dict("django.conf.settings.PASSWORD_COMPLEXITY", {'PUNCTUATION': 3}) + @override_settings(AUTH_PASSWORD_VALIDATORS=[ + create_validator_config('util.password_policy_validators.PunctuationValidator', {'min_punctuation': 3}) + ]) def test_not_enough_punctuations(self): self.url_params['password'] = 'thisshouldfail' response = self.client.post(self.url, self.url_params) @@ -130,10 +127,12 @@ class TestPasswordPolicy(TestCase): obj = json.loads(response.content) self.assertEqual( obj['value'], - "Enter a password with at least 3 punctuation marks.", + "This password must contain at least 3 punctuation marks.", ) - @patch.dict("django.conf.settings.PASSWORD_COMPLEXITY", {'PUNCTUATION': 3}) + @override_settings(AUTH_PASSWORD_VALIDATORS=[ + create_validator_config('util.password_policy_validators.PunctuationValidator', {'min_punctuation': 3}) + ]) def test_enough_punctuations(self): self.url_params['password'] = 'Th!sSh.uldPa$*' response = self.client.post(self.url, self.url_params) @@ -141,26 +140,9 @@ class TestPasswordPolicy(TestCase): obj = json.loads(response.content) self.assertTrue(obj['success']) - @patch.dict("django.conf.settings.PASSWORD_COMPLEXITY", {'WORDS': 3}) - def test_not_enough_words(self): - self.url_params['password'] = 'thisshouldfail' - response = self.client.post(self.url, self.url_params) - self.assertEqual(response.status_code, 400) - obj = json.loads(response.content) - self.assertEqual( - obj['value'], - "Enter a password with at least 3 words.", - ) - - @patch.dict("django.conf.settings.PASSWORD_COMPLEXITY", {'WORDS': 3}) - def test_enough_wordss(self): - self.url_params['password'] = u'this should pass' - response = self.client.post(self.url, self.url_params) - self.assertEqual(response.status_code, 200) - obj = json.loads(response.content) - self.assertTrue(obj['success']) - - @patch.dict("django.conf.settings.PASSWORD_COMPLEXITY", {'NUMERIC': 3}) + @override_settings(AUTH_PASSWORD_VALIDATORS=[ + create_validator_config('util.password_policy_validators.NumericValidator', {'min_numeric': 3}) + ]) def test_not_enough_numeric_characters(self): self.url_params['password'] = u'thishouldfail½2' response = self.client.post(self.url, self.url_params) @@ -168,18 +150,23 @@ class TestPasswordPolicy(TestCase): obj = json.loads(response.content) self.assertEqual( obj['value'], - "Enter a password with at least 3 numbers.", + "This password must contain at least 3 numbers.", ) - @patch.dict("django.conf.settings.PASSWORD_COMPLEXITY", {'NUMERIC': 3}) + @override_settings(AUTH_PASSWORD_VALIDATORS=[ + create_validator_config('util.password_policy_validators.NumericValidator', {'min_numeric': 3}) + ]) def test_enough_numeric_characters(self): - self.url_params['password'] = u'thisShouldPass½33' # This unicode 1/2 should count as a numeric value here + # This unicode 1/2 should count as a numeric value here + self.url_params['password'] = u'thisShouldPass½33' response = self.client.post(self.url, self.url_params) self.assertEqual(response.status_code, 200) obj = json.loads(response.content) self.assertTrue(obj['success']) - @patch.dict("django.conf.settings.PASSWORD_COMPLEXITY", {'ALPHABETIC': 3}) + @override_settings(AUTH_PASSWORD_VALIDATORS=[ + create_validator_config('util.password_policy_validators.AlphabeticValidator', {'min_alphabetic': 3}) + ]) def test_not_enough_alphabetic_characters(self): self.url_params['password'] = '123456ab' response = self.client.post(self.url, self.url_params) @@ -187,10 +174,12 @@ class TestPasswordPolicy(TestCase): obj = json.loads(response.content) self.assertEqual( obj['value'], - "Enter a password with at least 3 letters.", + "This password must contain at least 3 letters.", ) - @patch.dict("django.conf.settings.PASSWORD_COMPLEXITY", {'ALPHABETIC': 3}) + @override_settings(AUTH_PASSWORD_VALIDATORS=[ + create_validator_config('util.password_policy_validators.AlphabeticValidator', {'min_alphabetic': 3}) + ]) def test_enough_alphabetic_characters(self): self.url_params['password'] = u'𝒯𝓗Ï𝓼𝒫å𝓼𝓼𝔼𝓼' response = self.client.post(self.url, self.url_params) @@ -198,86 +187,65 @@ class TestPasswordPolicy(TestCase): obj = json.loads(response.content) self.assertTrue(obj['success']) - @patch.dict("django.conf.settings.PASSWORD_COMPLEXITY", { - 'PUNCTUATION': 3, - 'WORDS': 3, - 'DIGITS': 3, - 'LOWER': 3, - 'UPPER': 3, - }) + @override_settings(AUTH_PASSWORD_VALIDATORS=[ + create_validator_config('util.password_policy_validators.MinimumLengthValidator', {'min_length': 3}), + create_validator_config('util.password_policy_validators.UppercaseValidator', {'min_upper': 3}), + create_validator_config('util.password_policy_validators.NumericValidator', {'min_numeric': 3}), + create_validator_config('util.password_policy_validators.PunctuationValidator', {'min_punctuation': 3}), + ]) def test_multiple_errors_fail(self): self.url_params['password'] = 'thisshouldfail' response = self.client.post(self.url, self.url_params) self.assertEqual(response.status_code, 400) obj = json.loads(response.content) errstring = ( - "Enter a password with at least " - "3 uppercase letters & " - "3 digits & " - "3 punctuation marks & " - "3 words." + "This password must contain at least 3 uppercase letters. " + "This password must contain at least 3 numbers. " + "This password must contain at least 3 punctuation marks." ) self.assertEqual(obj['value'], errstring) - @patch.dict("django.conf.settings.PASSWORD_COMPLEXITY", { - 'PUNCTUATION': 3, - 'WORDS': 3, - 'DIGITS': 3, - 'LOWER': 3, - 'UPPER': 3, - }) + @override_settings(AUTH_PASSWORD_VALIDATORS=[ + create_validator_config('util.password_policy_validators.MinimumLengthValidator', {'min_length': 3}), + create_validator_config('util.password_policy_validators.UppercaseValidator', {'min_upper': 3}), + create_validator_config('util.password_policy_validators.LowercaseValidator', {'min_lower': 3}), + create_validator_config('util.password_policy_validators.NumericValidator', {'min_numeric': 3}), + create_validator_config('util.password_policy_validators.PunctuationValidator', {'min_punctuation': 3}), + ]) def test_multiple_errors_pass(self): - self.url_params['password'] = u'tH1s Sh0u!d P3#$' + self.url_params['password'] = u'tH1s Sh0u!d P3#$!' response = self.client.post(self.url, self.url_params) self.assertEqual(response.status_code, 200) obj = json.loads(response.content) self.assertTrue(obj['success']) - @override_settings(PASSWORD_DICTIONARY=['foo', 'bar']) - @override_settings(PASSWORD_DICTIONARY_EDIT_DISTANCE_THRESHOLD=1) - def test_dictionary_similarity_fail1(self): - self.url_params['password'] = 'foo' + @override_settings(AUTH_PASSWORD_VALIDATORS=[ + create_validator_config('django.contrib.auth.password_validation.CommonPasswordValidator') + ]) + def test_common_password_fail(self): + self.url_params['password'] = 'password' response = self.client.post(self.url, self.url_params) self.assertEqual(response.status_code, 400) obj = json.loads(response.content) self.assertEqual( obj['value'], - "Password is too similar to a dictionary word.", + "This password is too common.", ) - @override_settings(PASSWORD_DICTIONARY=['foo', 'bar']) - @override_settings(PASSWORD_DICTIONARY_EDIT_DISTANCE_THRESHOLD=1) - def test_dictionary_similarity_fail2(self): - self.url_params['password'] = 'bar' - response = self.client.post(self.url, self.url_params) - self.assertEqual(response.status_code, 400) - obj = json.loads(response.content) - self.assertEqual( - obj['value'], - "Password is too similar to a dictionary word.", - ) - - @override_settings(PASSWORD_DICTIONARY=['foo', 'bar']) - @override_settings(PASSWORD_DICTIONARY_EDIT_DISTANCE_THRESHOLD=1) - def test_dictionary_similarity_fail3(self): - self.url_params['password'] = 'fo0' - response = self.client.post(self.url, self.url_params) - self.assertEqual(response.status_code, 400) - obj = json.loads(response.content) - self.assertEqual( - obj['value'], - "Password is too similar to a dictionary word.", - ) - - @override_settings(PASSWORD_DICTIONARY=['foo', 'bar']) - @override_settings(PASSWORD_DICTIONARY_EDIT_DISTANCE_THRESHOLD=1) - def test_dictionary_similarity_pass(self): + @override_settings(AUTH_PASSWORD_VALIDATORS=[ + create_validator_config('django.contrib.auth.password_validation.CommonPasswordValidator') + ]) + def test_common_password_pass(self): self.url_params['password'] = 'this_is_ok' response = self.client.post(self.url, self.url_params) self.assertEqual(response.status_code, 200) obj = json.loads(response.content) self.assertTrue(obj['success']) + @override_settings(AUTH_PASSWORD_VALIDATORS=[ + create_validator_config('util.password_policy_validators.MinimumLengthValidator', {'min_length': 6}), + create_validator_config('util.password_policy_validators.MaximumLengthValidator', {'max_length': 75}), + ]) def test_with_unicode(self): self.url_params['password'] = u'四節比分和七年前' response = self.client.post(self.url, self.url_params) @@ -285,7 +253,9 @@ class TestPasswordPolicy(TestCase): obj = json.loads(response.content) self.assertTrue(obj['success']) - @override_settings(PASSWORD_MIN_LENGTH=6, SESSION_ENGINE='django.contrib.sessions.backends.cache') + @override_settings(AUTH_PASSWORD_VALIDATORS=[ + create_validator_config('util.password_policy_validators.MinimumLengthValidator', {'min_length': 6}) + ], SESSION_ENGINE='django.contrib.sessions.backends.cache') def test_ext_auth_password_length_too_short(self): """ Tests that even if password policy is enforced, ext_auth registrations aren't subject to it @@ -325,6 +295,9 @@ class TestUsernamePasswordNonmatch(TestCase): 'honor_code': 'true', } + @override_settings(AUTH_PASSWORD_VALIDATORS=[ + create_validator_config('django.contrib.auth.password_validation.UserAttributeSimilarityValidator') + ]) def test_with_username_password_match(self): self.url_params['username'] = "foobar" self.url_params['password'] = "foobar" @@ -333,9 +306,12 @@ class TestUsernamePasswordNonmatch(TestCase): obj = json.loads(response.content) self.assertEqual( obj['value'], - "Password cannot be the same as the username.", + "The password is too similar to the username.", ) + @override_settings(AUTH_PASSWORD_VALIDATORS=[ + create_validator_config('django.contrib.auth.password_validation.UserAttributeSimilarityValidator') + ]) def test_with_username_password_nonmatch(self): self.url_params['username'] = "foobar" self.url_params['password'] = "nonmatch" diff --git a/common/djangoapps/student/tests/test_reset_password.py b/common/djangoapps/student/tests/test_reset_password.py index 77ad5c0075..cb9c0ee075 100644 --- a/common/djangoapps/student/tests/test_reset_password.py +++ b/common/djangoapps/student/tests/test_reset_password.py @@ -29,6 +29,7 @@ from openedx.core.djangolib.testing.utils import CacheIsolationTestCase from student.tests.factories import UserFactory from student.tests.test_email import mock_render_to_string from student.views import SETTING_CHANGE_INITIATED, password_reset, password_reset_confirm_wrapper +from util.password_policy_validators import create_validator_config from util.testing import EventTestMixin from .test_configuration_overrides import fake_get_value @@ -350,16 +351,18 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): self.user.refresh_from_db() assert not self.user.is_active - @override_settings(PASSWORD_MIN_LENGTH=2) - @override_settings(PASSWORD_MAX_LENGTH=10) + @override_settings(AUTH_PASSWORD_VALIDATORS=[ + create_validator_config('util.password_policy_validators.MinimumLengthValidator', {'min_length': 2}), + create_validator_config('util.password_policy_validators.MaximumLengthValidator', {'max_length': 10}) + ]) @ddt.data( { 'password': '1', - 'error_message': 'Enter a password with at least 2 characters.', + 'error_message': 'This password is too short. It must contain at least 2 characters.', }, { 'password': '01234567891', - 'error_message': 'Enter a password with at most 10 characters.', + 'error_message': 'This password is too long. It must contain no more than 10 characters.', } ) def test_password_reset_with_invalid_length(self, password_dict): diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index 47ce54bf30..8652adceda 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -94,7 +94,7 @@ from student.text_me_the_app import TextMeTheAppFragmentView from util.bad_request_rate_limiter import BadRequestRateLimiter from util.db import outer_atomic from util.json_request import JsonResponse -from util.password_policy_validators import edX_validate_password +from util.password_policy_validators import validate_password log = logging.getLogger("edx.student") @@ -830,7 +830,7 @@ def password_reset_confirm_wrapper(request, uidb36=None, token=None): password = request.POST['new_password1'] try: - edX_validate_password(password, user=user) + validate_password(password, user=user) except ValidationError as err: # We have a password reset attempt which violates some security # policy, or any other validation. Use the existing Django template to communicate that @@ -839,7 +839,7 @@ def password_reset_confirm_wrapper(request, uidb36=None, token=None): 'validlink': True, 'form': None, 'title': _('Password reset unsuccessful'), - 'err_msg': err.message, + 'err_msg': ' '.join(err.messages), } context.update(platform_name) return TemplateResponse( diff --git a/common/djangoapps/util/password_policy_validators.py b/common/djangoapps/util/password_policy_validators.py index ded8d0040d..61dc5b089d 100644 --- a/common/djangoapps/util/password_policy_validators.py +++ b/common/djangoapps/util/password_policy_validators.py @@ -10,17 +10,45 @@ import unicodedata from django.conf import settings from django.contrib.auth.password_validation import ( get_default_password_validators, - validate_password, + validate_password as django_validate_password, MinimumLengthValidator as DjangoMinimumLengthValidator, ) from django.core.exceptions import ValidationError from django.utils.translation import ugettext as _, ungettext from six import text_type -from student.models import PasswordHistory - log = logging.getLogger(__name__) +# The following constant contains the assumption that the max password length will never exceed 5000 +# characters. The point of this restriction is to restrict the login page password field to prevent +# any sort of attacks involving sending massive passwords. +DEFAULT_MAX_PASSWORD_LENGTH = 5000 + + +def create_validator_config(name, options={}): + """ + This function is meant to be used for testing purposes to create validators + easily. It returns a validator config of the form: + { + "NAME": "util.password_policy_validators.SymbolValidator", + "OPTIONS": {"min_symbol": 1} + } + + Parameters: + name (str): the path name to the validator class to instantiate + options (dict): The dictionary of options to pass in to the validator. + These are used to initialize the validator with parameters. + If undefined, the default parameters will be used. + + Returns: + Dictionary containing the NAME and OPTIONS for the validator. These will + be used to instantiate an instance of the validator using Django. + """ + if options: + return {'NAME': name, 'OPTIONS': options} + + return {'NAME': name} + def password_validators_instruction_texts(): """ @@ -61,7 +89,7 @@ def password_validators_restrictions(): return complexity_restrictions -def edX_validate_password(password, user=None): +def validate_password(password, user=None): """ EdX's custom password validator for passwords. This function performs the following functions: @@ -89,7 +117,7 @@ def edX_validate_password(password, user=None): # no reason to get into weeds raise ValidationError([_('Invalid password.')]) - validate_password(password, user) + django_validate_password(password, user) def _validate_condition(password, fn, min_count): @@ -177,8 +205,8 @@ class AlphabeticValidator(object): return raise ValidationError( ungettext( - 'Your password must contain at least %(min_alphabetic)d letter.', - 'Your password must contain at least %(min_alphabetic)d letters.', + 'This password must contain at least %(min_alphabetic)d letter.', + 'This password must contain at least %(min_alphabetic)d letters.', self.min_alphabetic ), code='too_few_alphabetic_char', @@ -225,8 +253,8 @@ class NumericValidator(object): return raise ValidationError( ungettext( - 'Your password must contain at least %(min_numeric)d number.', - 'Your password must contain at least %(min_numeric)d numbers.', + 'This password must contain at least %(min_numeric)d number.', + 'This password must contain at least %(min_numeric)d numbers.', self.min_numeric ), code='too_few_numeric_char', @@ -273,8 +301,8 @@ class UppercaseValidator(object): return raise ValidationError( ungettext( - 'Your password must contain at least %(min_upper)d uppercase letter.', - 'Your password must contain at least %(min_upper)d uppercase letters.', + 'This password must contain at least %(min_upper)d uppercase letter.', + 'This password must contain at least %(min_upper)d uppercase letters.', self.min_upper ), code='too_few_uppercase_char', @@ -321,8 +349,8 @@ class LowercaseValidator(object): return raise ValidationError( ungettext( - 'Your password must contain at least %(min_lower)d lowercase letter.', - 'Your password must contain at least %(min_lower)d lowercase letters.', + 'This password must contain at least %(min_lower)d lowercase letter.', + 'This password must contain at least %(min_lower)d lowercase letters.', self.min_lower ), code='too_few_lowercase_char', @@ -355,11 +383,11 @@ class LowercaseValidator(object): class PunctuationValidator(object): """ - Validate whether the password contains at least min_punctuation punctuation characters + Validate whether the password contains at least min_punctuation punctuation marks as defined by unicode categories. Parameters: - min_punctuation (int): the minimum number of punctuation characters to require + min_punctuation (int): the minimum number of punctuation marks to require in the password. Must be >= 0. """ def __init__(self, min_punctuation=0): @@ -370,8 +398,8 @@ class PunctuationValidator(object): return raise ValidationError( ungettext( - 'Your password must contain at least %(min_punctuation)d punctuation character.', - 'Your password must contain at least %(min_punctuation)d punctuation characters.', + 'This password must contain at least %(min_punctuation)d punctuation mark.', + 'This password must contain at least %(min_punctuation)d punctuation marks.', self.min_punctuation ), code='too_few_punctuation_characters', @@ -380,16 +408,16 @@ class PunctuationValidator(object): def get_help_text(self): return ungettext( - "Your password must contain at least %(min_punctuation)d punctuation character.", - "Your password must contain at least %(min_punctuation)d punctuation characters.", + "Your password must contain at least %(min_punctuation)d punctuation mark.", + "Your password must contain at least %(min_punctuation)d punctuation marks.", self.min_punctuation ) % {'min_punctuation': self.min_punctuation} def get_instruction_text(self): if self.min_punctuation > 0: return ungettext( - '%(num)d punctuation character', - '%(num)d punctuation characters', + '%(num)d punctuation mark', + '%(num)d punctuation marks', self.min_punctuation ) % {'num': self.min_punctuation} else: @@ -418,8 +446,8 @@ class SymbolValidator(object): return raise ValidationError( ungettext( - 'Your password must contain at least %(min_symbol)d symbol.', - 'Your password must contain at least %(min_symbol)d symbols.', + 'This password must contain at least %(min_symbol)d symbol.', + 'This password must contain at least %(min_symbol)d symbols.', self.min_symbol ), code='too_few_symbols', diff --git a/common/djangoapps/util/tests/test_password_policy_validators.py b/common/djangoapps/util/tests/test_password_policy_validators.py index 97b636a358..d037d7557c 100644 --- a/common/djangoapps/util/tests/test_password_policy_validators.py +++ b/common/djangoapps/util/tests/test_password_policy_validators.py @@ -6,37 +6,44 @@ import unittest from ddt import data, ddt, unpack from django.conf import settings +from django.contrib.auth.models import User from django.core.exceptions import ValidationError from django.test.utils import override_settings from util.password_policy_validators import ( - password_instructions, password_min_length, validate_password, _validate_password_dictionary + create_validator_config, validate_password, password_validators_instruction_texts, ) @ddt class PasswordPolicyValidatorsTestCase(unittest.TestCase): - """ Tests for password validator utility functions """ + """ + Tests for password validator utility functions - @override_settings(PASSWORD_DICTIONARY_EDIT_DISTANCE_THRESHOLD=2) - @override_settings(PASSWORD_DICTIONARY=['testme']) - @mock.patch.dict(settings.FEATURES, {'ENFORCE_PASSWORD_POLICY': True}) - def test_validate_password_dictionary(self): - """ Tests dictionary checks """ - # Direct match - with self.assertRaises(ValidationError): - _validate_password_dictionary(u'testme') + The general framework I went with for testing the validators was to test: + 1) requiring a single check (also checks proper singular message) + 2) requiring multiple instances of the check (also checks proper plural message) + 3) successful check + """ - # Off by one - with self.assertRaises(ValidationError): - _validate_password_dictionary(u'estme') + def validation_errors_checker(self, password, msg, user=None): + """ + This helper function is used to check the proper error messages are + being displayed based on the password and validator. - # Off by two - with self.assertRaises(ValidationError): - _validate_password_dictionary(u'bestmet') - - # Off by three (should pass) - _validate_password_dictionary(u'bestem') + Parameters: + password (unicode): the password to validate on + user (django.contrib.auth.models.User): user object to use in validation. + This is an optional parameter unless the validator requires a + user object. + msg (str): The expected ValidationError message + """ + if msg is None: + validate_password(password, user) + else: + with self.assertRaises(ValidationError) as cm: + validate_password(password, user) + self.assertIn(msg, ' '.join(cm.exception.messages)) def test_unicode_password(self): """ Tests that validate_password enforces unicode """ @@ -46,45 +53,193 @@ class PasswordPolicyValidatorsTestCase(unittest.TestCase): # Sanity checks and demonstration of why this test is useful self.assertEqual(len(byte_str), 4) self.assertEqual(len(unicode_str), 1) - self.assertEqual(password_min_length(), 2) # Test length check - with self.assertRaises(ValidationError): - validate_password(byte_str) - validate_password(byte_str + byte_str) + self.validation_errors_checker(byte_str, 'This password is too short. It must contain at least 2 characters.') + self.validation_errors_checker(byte_str + byte_str, None) # Test badly encoded password - with self.assertRaises(ValidationError) as cm: - validate_password(b'\xff\xff') - self.assertEquals('Invalid password.', cm.exception.message) + self.validation_errors_checker(b'\xff\xff', 'Invalid password.') @data( - (u'', 'at least 2 characters & 2 letters & 1 number.'), - (u'a.', 'at least 2 letters & 1 number.'), - (u'a1', 'at least 2 letters.'), - (u'aa1', None), + ([create_validator_config('util.password_policy_validators.MinimumLengthValidator', {'min_length': 2})], + 'at least 2 characters.'), + + ([ + create_validator_config('util.password_policy_validators.MinimumLengthValidator', {'min_length': 2}), + create_validator_config('util.password_policy_validators.AlphabeticValidator', {'min_alphabetic': 2}), + ], 'characters, including 2 letters.'), + + ([ + create_validator_config('util.password_policy_validators.MinimumLengthValidator', {'min_length': 2}), + create_validator_config('util.password_policy_validators.AlphabeticValidator', {'min_alphabetic': 2}), + create_validator_config('util.password_policy_validators.NumericValidator', {'min_numeric': 1}), + ], 'characters, including 2 letters & 1 number.'), + + ([ + create_validator_config('util.password_policy_validators.MinimumLengthValidator', {'min_length': 2}), + create_validator_config('util.password_policy_validators.UppercaseValidator', {'min_upper': 3}), + create_validator_config('util.password_policy_validators.NumericValidator', {'min_numeric': 1}), + create_validator_config('util.password_policy_validators.SymbolValidator', {'min_symbol': 2}), + ], 'including 3 uppercase letters & 1 number & 2 symbols.'), ) @unpack - @override_settings(PASSWORD_COMPLEXITY={'ALPHABETIC': 2, 'NUMERIC': 1}) - @mock.patch.dict(settings.FEATURES, {'ENFORCE_PASSWORD_POLICY': True}) - def test_validation_errors(self, password, msg): - """ Tests validate_password error messages """ - if msg is None: - validate_password(password) - else: - with self.assertRaises(ValidationError) as cm: - validate_password(password) - self.assertIn(msg, cm.exception.message) + def test_password_instructions(self, config, msg): + """ Tests password instructions """ + with override_settings(AUTH_PASSWORD_VALIDATORS=config): + self.assertIn(msg, password_validators_instruction_texts()) @data( - ({}, 'at least 2 characters.'), - ({'ALPHABETIC': 2}, 'characters, including 2 letters.'), - ({'ALPHABETIC': 2, 'NUMERIC': 1}, 'characters, including 2 letters & 1 number.'), - ({'NON ASCII': 2, 'NUMERIC': 1, 'UPPER': 3}, 'including 3 uppercase letters & 1 number & 2 symbols.'), + (u'userna', u'username', 'test@example.com', 'The password is too similar to the username.'), + (u'password', u'username', 'password@example.com', 'The password is too similar to the email address.'), + (u'password', u'username', 'test@password.com', 'The password is too similar to the email address.'), + (u'password', u'username', 'test@example.com', None), ) @unpack - @mock.patch.dict(settings.FEATURES, {'ENFORCE_PASSWORD_POLICY': True}) - def test_password_instruction(self, config, msg): - """ Tests password_instruction """ - with override_settings(PASSWORD_COMPLEXITY=config): - self.assertIn(msg, password_instructions()) + @override_settings(AUTH_PASSWORD_VALIDATORS=[ + create_validator_config('django.contrib.auth.password_validation.UserAttributeSimilarityValidator') + ]) + def test_user_attribute_similarity_validation_errors(self, password, username, email, msg): + """ Tests validate_password error messages for the UserAttributeSimilarityValidator """ + user = User(username=username, email=email) + self.validation_errors_checker(password, msg, user) + + @data( + ([create_validator_config('util.password_policy_validators.MinimumLengthValidator', {'min_length': 1})], + u'', 'This password is too short. It must contain at least 1 character.'), + + ([create_validator_config('util.password_policy_validators.MinimumLengthValidator', {'min_length': 8})], + u'd', 'This password is too short. It must contain at least 8 characters.'), + + ([create_validator_config('util.password_policy_validators.MinimumLengthValidator', {'min_length': 8})], + u'longpassword', None), + ) + @unpack + def test_minimum_length_validation_errors(self, config, password, msg): + """ Tests validate_password error messages for the MinimumLengthValidator """ + with override_settings(AUTH_PASSWORD_VALIDATORS=config): + self.validation_errors_checker(password, msg) + + @data( + ([create_validator_config('util.password_policy_validators.MaximumLengthValidator', {'max_length': 1})], + u'longpassword', 'This password is too long. It must contain no more than 1 character.'), + + ([create_validator_config('util.password_policy_validators.MaximumLengthValidator', {'max_length': 10})], + u'longpassword', 'This password is too long. It must contain no more than 10 characters.'), + + ([create_validator_config('util.password_policy_validators.MaximumLengthValidator', {'max_length': 20})], + u'shortpassword', None), + ) + @unpack + def test_maximum_length_validation_errors(self, config, password, msg): + """ Tests validate_password error messages for the MaximumLengthValidator """ + with override_settings(AUTH_PASSWORD_VALIDATORS=config): + self.validation_errors_checker(password, msg) + + @data( + (u'password', 'This password is too common.'), + (u'good_password', None), + ) + @unpack + @override_settings(AUTH_PASSWORD_VALIDATORS=[ + create_validator_config('django.contrib.auth.password_validation.CommonPasswordValidator') + ]) + def test_common_password_validation_errors(self, password, msg): + """ Tests validate_password error messages for the CommonPasswordValidator """ + self.validation_errors_checker(password, msg) + + @data( + ([create_validator_config('util.password_policy_validators.AlphabeticValidator', {'min_alphabetic': 1})], + u'12345', 'This password must contain at least 1 letter.'), + + ([create_validator_config('util.password_policy_validators.AlphabeticValidator', {'min_alphabetic': 5})], + u'test123', 'This password must contain at least 5 letters.'), + + ([create_validator_config('util.password_policy_validators.AlphabeticValidator', {'min_alphabetic': 2})], + u'password', None), + ) + @unpack + def test_alphabetic_validation_errors(self, config, password, msg): + """ Tests validate_password error messages for the AlphabeticValidator """ + with override_settings(AUTH_PASSWORD_VALIDATORS=config): + self.validation_errors_checker(password, msg) + + @data( + ([create_validator_config('util.password_policy_validators.NumericValidator', {'min_numeric': 1})], + u'test', 'This password must contain at least 1 number.'), + + ([create_validator_config('util.password_policy_validators.NumericValidator', {'min_numeric': 4})], + u'test123', 'This password must contain at least 4 numbers.'), + + ([create_validator_config('util.password_policy_validators.NumericValidator', {'min_numeric': 2})], + u'password123', None), + ) + @unpack + def test_numeric_validation_errors(self, config, password, msg): + """ Tests validate_password error messages for the NumericValidator """ + with override_settings(AUTH_PASSWORD_VALIDATORS=config): + self.validation_errors_checker(password, msg) + + @data( + ([create_validator_config('util.password_policy_validators.UppercaseValidator', {'min_upper': 1})], + u'lowercase', 'This password must contain at least 1 uppercase letter.'), + + ([create_validator_config('util.password_policy_validators.UppercaseValidator', {'min_upper': 6})], + u'NOTenough', 'This password must contain at least 6 uppercase letters.'), + + ([create_validator_config('util.password_policy_validators.UppercaseValidator', {'min_upper': 1})], + u'camelCase', None), + ) + @unpack + def test_upper_case_validation_errors(self, config, password, msg): + """ Tests validate_password error messages for the UppercaseValidator """ + with override_settings(AUTH_PASSWORD_VALIDATORS=config): + self.validation_errors_checker(password, msg) + + @data( + ([create_validator_config('util.password_policy_validators.LowercaseValidator', {'min_lower': 1})], + u'UPPERCASE', 'This password must contain at least 1 lowercase letter.'), + + ([create_validator_config('util.password_policy_validators.LowercaseValidator', {'min_lower': 4})], + u'notENOUGH', 'This password must contain at least 4 lowercase letters.'), + + ([create_validator_config('util.password_policy_validators.LowercaseValidator', {'min_lower': 1})], + u'goodPassword', None), + ) + @unpack + def test_lower_case_validation_errors(self, config, password, msg): + """ Tests validate_password error messages for the LowercaseValidator """ + with override_settings(AUTH_PASSWORD_VALIDATORS=config): + self.validation_errors_checker(password, msg) + + @data( + ([create_validator_config('util.password_policy_validators.PunctuationValidator', {'min_punctuation': 1})], + u'no punctuation', 'This password must contain at least 1 punctuation mark.'), + + ([create_validator_config('util.password_policy_validators.PunctuationValidator', {'min_punctuation': 7})], + u'p@$$w0rd$!', 'This password must contain at least 7 punctuation marks.'), + + ([create_validator_config('util.password_policy_validators.PunctuationValidator', {'min_punctuation': 3})], + u'excl@m@t!on', None), + ) + @unpack + def test_punctuation_validation_errors(self, config, password, msg): + """ Tests validate_password error messages for the PunctuationValidator """ + with override_settings(AUTH_PASSWORD_VALIDATORS=config): + self.validation_errors_checker(password, msg) + + @data( + ([create_validator_config('util.password_policy_validators.SymbolValidator', {'min_symbol': 1})], + u'no symbol', 'This password must contain at least 1 symbol.'), + + ([create_validator_config('util.password_policy_validators.SymbolValidator', {'min_symbol': 3})], + u'☹️boo☹️', 'This password must contain at least 3 symbols.'), + + ([create_validator_config('util.password_policy_validators.SymbolValidator', {'min_symbol': 2})], + u'☪symbols!☹️', None), + ) + @unpack + def test_symbol_validation_errors(self, config, password, msg): + """ Tests validate_password error messages for the SymbolValidator """ + with override_settings(AUTH_PASSWORD_VALIDATORS=config): + self.validation_errors_checker(password, msg) diff --git a/common/test/acceptance/tests/studio/test_studio_course_team.py b/common/test/acceptance/tests/studio/test_studio_course_team.py index 09d02065b0..e4b1a56d22 100644 --- a/common/test/acceptance/tests/studio/test_studio_course_team.py +++ b/common/test/acceptance/tests/studio/test_studio_course_team.py @@ -16,7 +16,7 @@ class CourseTeamPageTest(StudioCourseTest): user = { 'username': username, 'email': username + "@example.com", - 'password': username + '123' + 'password': username + '123$%^' } AutoAuthPage( self.browser, no_login=True, diff --git a/lms/djangoapps/courseware/tests/test_password_history.py b/lms/djangoapps/courseware/tests/test_password_history.py index 5bf49b2c4b..bad1f5dd8b 100644 --- a/lms/djangoapps/courseware/tests/test_password_history.py +++ b/lms/djangoapps/courseware/tests/test_password_history.py @@ -17,6 +17,7 @@ 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}) @@ -161,148 +162,9 @@ class TestPasswordHistory(LoginEnrollmentTestCase): resp.content ) - @patch.dict("django.conf.settings.ADVANCED_SECURITY_CONFIG", {'MIN_DIFFERENT_STUDENT_PASSWORDS_BEFORE_REUSE': 1}) - def test_student_password_reset_reuse(self): - """ - Goes through the password reset flows to make sure the various password reuse policies are enforced - """ - student_email, _ = self._setup_user() - user = User.objects.get(email=student_email) - - err_msg = 'You are re\\\\u002Dusing a password that you have used recently. You must have 1 distinct password' - success_msg = 'Your Password Reset is Complete' - - 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.assertPasswordResetError(resp, err_msg) - - # now retry with a different password - resp = self.client.post('/password_reset_confirm/{0}-{1}/'.format(uidb36, token), { - 'new_password1': 'bar', - 'new_password2': 'bar' - }, follow=True) - - 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): - """ - Goes through the password reset flows to make sure the various password reuse policies are enforced - """ - staff_email, _ = self._setup_user(is_staff=True) - user = User.objects.get(email=staff_email) - - err_msg = 'You are re\\\\u002Dusing a password that you have used recently. You must have 2 distinct passwords' - success_msg = 'Your Password Reset is Complete' - - 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.assertPasswordResetError(resp, err_msg) - - # now use different one - user = User.objects.get(email=staff_email) - token = default_token_generator.make_token(user) - uidb36 = int_to_base36(user.id) - - resp = self.client.post('/password_reset_confirm/{0}-{1}/'.format(uidb36, token), { - 'new_password1': 'bar', - 'new_password2': 'bar', - }, follow=True) - - self.assertIn(success_msg, resp.content) - - # now try again with the first one - user = User.objects.get(email=staff_email) - token = default_token_generator.make_token(user) - uidb36 = int_to_base36(user.id) - - resp = self.client.post('/password_reset_confirm/{0}-{1}/'.format(uidb36, token), { - 'new_password1': 'foo', - 'new_password2': 'foo', - }, follow=True) - - self.assertPasswordResetError(resp, err_msg) - - # now use different one - user = User.objects.get(email=staff_email) - token = default_token_generator.make_token(user) - uidb36 = int_to_base36(user.id) - - resp = self.client.post('/password_reset_confirm/{0}-{1}/'.format(uidb36, token), { - 'new_password1': 'baz', - 'new_password2': 'baz', - }, follow=True) - - self.assertIn(success_msg, resp.content) - - # now we should be able to reuse the first one - user = User.objects.get(email=staff_email) - token = default_token_generator.make_token(user) - uidb36 = int_to_base36(user.id) - - resp = self.client.post('/password_reset_confirm/{0}-{1}/'.format(uidb36, token), { - 'new_password1': 'foo', - 'new_password2': 'foo', - }, follow=True) - - 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): - """ - Asserts the frequency limit on how often we can change passwords - """ - staff_email, _ = self._setup_user(is_staff=True) - - 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 - ) - - # pretend we're in the future - staff_reset_time = timezone.now() + timedelta(days=1) - with freeze_time(staff_reset_time): - 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.assertIn(success_msg, resp.content) - - @patch.dict("django.conf.settings.FEATURES", {'ENFORCE_PASSWORD_POLICY': True}) - @override_settings(PASSWORD_MIN_LENGTH=6) + @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 @@ -342,7 +204,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase): @ddt.data( ('foo', 'foobar', 'Error in resetting your password. Please try again.'), - ('', '', 'Enter a password with at least'), + ('', '', 'This password is too short. It must contain at least'), ) @ddt.unpack def test_password_reset_form_invalid(self, password1, password2, err_msg): diff --git a/lms/envs/aws.py b/lms/envs/aws.py index 4f56c9930d..37f68af3e9 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -652,7 +652,7 @@ MAX_FAILED_LOGIN_ATTEMPTS_ALLOWED = ENV_TOKENS.get("MAX_FAILED_LOGIN_ATTEMPTS_AL MAX_FAILED_LOGIN_ATTEMPTS_LOCKOUT_PERIOD_SECS = ENV_TOKENS.get("MAX_FAILED_LOGIN_ATTEMPTS_LOCKOUT_PERIOD_SECS", 15 * 60) #### PASSWORD POLICY SETTINGS ##### -AUTH_PASSWORD_VALIDATORS = ENV_TOKENS.get("AUTH_PASSWORD_VALIDATORS", []) +AUTH_PASSWORD_VALIDATORS = ENV_TOKENS.get("AUTH_PASSWORD_VALIDATORS", AUTH_PASSWORD_VALIDATORS) ### INACTIVITY SETTINGS #### SESSION_INACTIVITY_TIMEOUT_IN_SECONDS = AUTH_TOKENS.get("SESSION_INACTIVITY_TIMEOUT_IN_SECONDS") diff --git a/lms/envs/bok_choy.py b/lms/envs/bok_choy.py index b5882fdffe..74381db39f 100644 --- a/lms/envs/bok_choy.py +++ b/lms/envs/bok_choy.py @@ -173,7 +173,6 @@ YOUTUBE['TEXT_API']['url'] = "{0}:{1}/test_transcripts_youtube/".format(YOUTUBE_ ############################# SECURITY SETTINGS ################################ # Default to advanced security in common.py, so tests can reset here to use # a simpler security model -FEATURES['ENFORCE_PASSWORD_POLICY'] = False FEATURES['ENABLE_MAX_FAILED_LOGIN_ATTEMPTS'] = False FEATURES['SQUELCH_PII_IN_LOGS'] = False FEATURES['PREVENT_CONCURRENT_LOGINS'] = False @@ -183,9 +182,6 @@ FEATURES['ENABLE_MOBILE_REST_API'] = True # Show video bumper in LMS FEATURES['ENABLE_VIDEO_BUMPER'] = True # Show video bumper in LMS FEATURES['SHOW_BUMPER_PERIODICITY'] = 1 -PASSWORD_MIN_LENGTH = None -PASSWORD_COMPLEXITY = {} - # Enable courseware search for tests FEATURES['ENABLE_COURSEWARE_SEARCH'] = True diff --git a/lms/envs/common.py b/lms/envs/common.py index 9d44e68556..6c441558fb 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -202,9 +202,6 @@ FEATURES = { # Maximum number of rows to include in the csv file for downloading problem responses. 'MAX_PROBLEM_RESPONSES_COUNT': 5000, - # whether to use password policy enforcement or not - 'ENFORCE_PASSWORD_POLICY': True, - 'ENABLED_PAYMENT_REPORTS': [ "refund_report", "itemized_purchase_report", @@ -2594,11 +2591,23 @@ FINANCIAL_REPORTS = { POLICY_CHANGE_TASK_RATE_LIMIT = '300/h' #### PASSWORD POLICY SETTINGS ##### -PASSWORD_MIN_LENGTH = 8 -PASSWORD_MAX_LENGTH = None -PASSWORD_COMPLEXITY = {"UPPER": 1, "LOWER": 1, "DIGITS": 1} -PASSWORD_DICTIONARY_EDIT_DISTANCE_THRESHOLD = None -PASSWORD_DICTIONARY = [] +AUTH_PASSWORD_VALIDATORS = [ + { + "NAME": "django.contrib.auth.password_validation.UserAttributeSimilarityValidator", + }, + { + "NAME": "util.password_policy_validators.MinimumLengthValidator", + "OPTIONS": { + "min_length": 2 + } + }, + { + "NAME": "util.password_policy_validators.MaximumLengthValidator", + "OPTIONS": { + "max_length": 75 + } + }, +] ############################ ORA 2 ############################################ diff --git a/lms/envs/devstack.py b/lms/envs/devstack.py index 1f811dec70..c625c88c51 100644 --- a/lms/envs/devstack.py +++ b/lms/envs/devstack.py @@ -137,7 +137,6 @@ FEATURES['ENABLE_MOBILE_REST_API'] = True FEATURES['ENABLE_VIDEO_ABSTRACTION_LAYER_API'] = True ########################## SECURITY ####################### -FEATURES['ENFORCE_PASSWORD_POLICY'] = False FEATURES['ENABLE_MAX_FAILED_LOGIN_ATTEMPTS'] = False FEATURES['SQUELCH_PII_IN_LOGS'] = False FEATURES['PREVENT_CONCURRENT_LOGINS'] = False diff --git a/lms/envs/production.py b/lms/envs/production.py index 5929dacf88..6f229d23bd 100644 --- a/lms/envs/production.py +++ b/lms/envs/production.py @@ -648,11 +648,7 @@ MAX_FAILED_LOGIN_ATTEMPTS_ALLOWED = ENV_TOKENS.get("MAX_FAILED_LOGIN_ATTEMPTS_AL MAX_FAILED_LOGIN_ATTEMPTS_LOCKOUT_PERIOD_SECS = ENV_TOKENS.get("MAX_FAILED_LOGIN_ATTEMPTS_LOCKOUT_PERIOD_SECS", 15 * 60) #### PASSWORD POLICY SETTINGS ##### -PASSWORD_MIN_LENGTH = ENV_TOKENS.get("PASSWORD_MIN_LENGTH") -PASSWORD_MAX_LENGTH = ENV_TOKENS.get("PASSWORD_MAX_LENGTH") -PASSWORD_COMPLEXITY = ENV_TOKENS.get("PASSWORD_COMPLEXITY", {}) -PASSWORD_DICTIONARY_EDIT_DISTANCE_THRESHOLD = ENV_TOKENS.get("PASSWORD_DICTIONARY_EDIT_DISTANCE_THRESHOLD") -PASSWORD_DICTIONARY = ENV_TOKENS.get("PASSWORD_DICTIONARY", []) +AUTH_PASSWORD_VALIDATORS = ENV_TOKENS.get("AUTH_PASSWORD_VALIDATORS", AUTH_PASSWORD_VALIDATORS) ### INACTIVITY SETTINGS #### SESSION_INACTIVITY_TIMEOUT_IN_SECONDS = AUTH_TOKENS.get("SESSION_INACTIVITY_TIMEOUT_IN_SECONDS") diff --git a/lms/envs/test.py b/lms/envs/test.py index d2afa71876..08cfdd4e7e 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -223,8 +223,6 @@ FEATURES['ENABLE_MAX_FAILED_LOGIN_ATTEMPTS'] = False FEATURES['SQUELCH_PII_IN_LOGS'] = False FEATURES['PREVENT_CONCURRENT_LOGINS'] = False FEATURES['ADVANCED_SECURITY'] = False -PASSWORD_MIN_LENGTH = None -PASSWORD_COMPLEXITY = {} ######### Third-party auth ########## FEATURES['ENABLE_THIRD_PARTY_AUTH'] = True diff --git a/lms/static/js/student_account/views/RegisterView.js b/lms/static/js/student_account/views/RegisterView.js index 034f113e55..9daa4720c7 100644 --- a/lms/static/js/student_account/views/RegisterView.js +++ b/lms/static/js/student_account/views/RegisterView.js @@ -195,7 +195,9 @@ // Hide each input tip $(this).children().each(function() { - if (inputTipSelectors.indexOf($(this).attr('class')) >= 0) { + // This is a 1 instead of 0 so the error message for a field is not + // hidden on blur and only the help tip is hidden. + if (inputTipSelectors.indexOf($(this).attr('class')) >= 1) { $(this).addClass('hidden'); } }); diff --git a/openedx/core/djangoapps/password_policy/compliance.py b/openedx/core/djangoapps/password_policy/compliance.py index 53326b9957..0447060e1d 100644 --- a/openedx/core/djangoapps/password_policy/compliance.py +++ b/openedx/core/djangoapps/password_policy/compliance.py @@ -8,7 +8,7 @@ from django.conf import settings from django.utils.translation import ugettext as _ from util.date_utils import DEFAULT_SHORT_DATE_FORMAT, strftime_localized -from util.password_policy_validators import edX_validate_password +from util.password_policy_validators import validate_password class NonCompliantPasswordException(Exception): @@ -104,7 +104,7 @@ def _check_user_compliance(user, password): Returns a boolean indicating whether or not the user is compliant with password policy rules. """ try: - edX_validate_password(password, user=user) + validate_password(password, user=user) return True except Exception: # pylint: disable=broad-except # If anything goes wrong, we should assume the password is not compliant but we don't necessarily diff --git a/openedx/core/djangoapps/password_policy/tests/test_compliance.py b/openedx/core/djangoapps/password_policy/tests/test_compliance.py index 3a68c7c185..e05b371290 100644 --- a/openedx/core/djangoapps/password_policy/tests/test_compliance.py +++ b/openedx/core/djangoapps/password_policy/tests/test_compliance.py @@ -16,7 +16,7 @@ from openedx.core.djangoapps.password_policy.compliance import (NonCompliantPass should_enforce_compliance_on_login) from student.tests.factories import (CourseAccessRoleFactory, UserFactory) -from util.password_policy_validators import SecurityPolicyError, ValidationError, validate_password +from util.password_policy_validators import ValidationError date1 = parse_date('2018-01-01 00:00:00+00:00') @@ -93,36 +93,21 @@ class TestCompliance(TestCase): """ # Test that a user that passes validate_password returns True - with patch('openedx.core.djangoapps.password_policy.compliance.validate_password') as mock_validate_password: + with patch('openedx.core.djangoapps.password_policy.compliance.validate_password') as \ + mock_validate_password: user = UserFactory() # Mock validate_password to return True without checking the password mock_validate_password.return_value = True self.assertTrue(_check_user_compliance(user, None)) # Don't need a password here # Test that a user that does not pass validate_password returns False - with patch('openedx.core.djangoapps.password_policy.compliance.validate_password') as mock_validate_password: + with patch('openedx.core.djangoapps.password_policy.compliance.validate_password') as \ + mock_validate_password: user = UserFactory() # Mock validate_password to throw a ValidationError without checking the password mock_validate_password.side_effect = ValidationError('Some validation error') self.assertFalse(_check_user_compliance(user, None)) # Don't need a password here - @patch('student.models.PasswordHistory.is_allowable_password_reuse') - @override_settings(ADVANCED_SECURITY_CONFIG={'MIN_DIFFERENT_STUDENT_PASSWORDS_BEFORE_REUSE': 1}) - def test_ignore_reset_checks(self, mock_reuse): - """ - Test that we don't annoy user about compliance failures that only affect password resets - """ - user = UserFactory() - password = 'nope1234' - mock_reuse.return_value = False - - # Sanity check that normal validation would trip us up - with self.assertRaises(SecurityPolicyError): - validate_password(password, user=user) - - # Confirm that we don't trip on it - self.assertTrue(_check_user_compliance(user, password)) - @override_settings(PASSWORD_POLICY_COMPLIANCE_ROLLOUT_CONFIG={ 'STAFF_USER_COMPLIANCE_DEADLINE': date1, 'ELEVATED_PRIVILEGE_USER_COMPLIANCE_DEADLINE': date2, diff --git a/openedx/core/djangoapps/user_api/accounts/api.py b/openedx/core/djangoapps/user_api/accounts/api.py index 25b62aabb3..3f08e68e1f 100644 --- a/openedx/core/djangoapps/user_api/accounts/api.py +++ b/openedx/core/djangoapps/user_api/accounts/api.py @@ -18,7 +18,7 @@ from student.models import User, UserProfile, Registration, email_exists_or_reti from student import forms as student_forms from student import views as student_views from util.model_utils import emit_setting_changed_event -from util.password_policy_validators import edX_validate_password +from util.password_policy_validators import validate_password from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.user_api import errors, accounts, forms, helpers @@ -287,7 +287,6 @@ def create_account(username, password, email): * 3rd party auth * External auth (shibboleth) - * Complex password policies (ENFORCE_PASSWORD_POLICY) In addition, we assume that some functionality is handled at higher layers: @@ -665,7 +664,7 @@ def _validate_password(password, username=None, email=None): try: _validate_type(password, basestring, accounts.PASSWORD_BAD_TYPE_MSG) temp_user = User(username=username, email=email) if username else None - edX_validate_password(password, user=temp_user) + validate_password(password, user=temp_user) except errors.AccountDataBadType as invalid_password_err: raise errors.AccountPasswordInvalid(text_type(invalid_password_err)) except ValidationError as validation_err: diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_api.py b/openedx/core/djangoapps/user_api/accounts/tests/test_api.py index 5c81225196..bd70aaebb4 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_api.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_api.py @@ -387,9 +387,9 @@ class AccountCreationActivationAndPasswordChangeTest(TestCase): """ Test cases to cover the account initialization workflow """ - USERNAME = u'frank-underwood' + USERNAME = u'claire-underwood' PASSWORD = u'ṕáśśẃőŕd' - EMAIL = u'frank+underwood@example.com' + EMAIL = u'claire+underwood@example.com' IS_SECURE = False diff --git a/openedx/core/djangoapps/user_api/accounts/tests/testutils.py b/openedx/core/djangoapps/user_api/accounts/tests/testutils.py index 97e565719c..37d1c47efc 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/testutils.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/testutils.py @@ -7,7 +7,7 @@ from openedx.core.djangoapps.user_api.accounts import ( USERNAME_MIN_LENGTH, USERNAME_MAX_LENGTH, EMAIL_MAX_LENGTH, ) -from util.password_policy_validators import password_max_length, password_min_length +from util.password_policy_validators import DEFAULT_MAX_PASSWORD_LENGTH INVALID_NAMES = [ @@ -55,7 +55,7 @@ INVALID_PASSWORDS = [ None, u'', u'a', - u'a' * (password_max_length() + 1) + u'a' * (DEFAULT_MAX_PASSWORD_LENGTH + 1), ] INVALID_COUNTRIES = [ @@ -92,9 +92,7 @@ VALID_EMAILS = [ ] VALID_PASSWORDS = [ - u'password', # :) - u'a' * password_min_length(), - u'a' * password_max_length() + u'good_password_339', ] VALID_COUNTRIES = [ diff --git a/openedx/core/djangoapps/user_api/api.py b/openedx/core/djangoapps/user_api/api.py index 3a349409d5..dd7c6d8194 100644 --- a/openedx/core/djangoapps/user_api/api.py +++ b/openedx/core/djangoapps/user_api/api.py @@ -18,7 +18,7 @@ from openedx.features.enterprise_support.api import enterprise_customer_for_requ from student.forms import get_registration_extension_form from student.models import UserProfile from util.password_policy_validators import ( - password_validators_instruction_texts, password_validators_restrictions + password_validators_instruction_texts, password_validators_restrictions, DEFAULT_MAX_PASSWORD_LENGTH, ) @@ -118,10 +118,7 @@ def get_login_session_form(request): "password", label=password_label, field_type="password", - # The following restriction contains the assumption that the max password length will never exceed 5000 - # characters. The point of this restriction on the login page is to prevent any sort of attacks - # involving sending massive passwords. - restrictions={'max_length': 5000} + restrictions={'max_length': DEFAULT_MAX_PASSWORD_LENGTH} ) form_desc.add_field( diff --git a/openedx/core/djangoapps/user_api/tests/test_views.py b/openedx/core/djangoapps/user_api/tests/test_views.py index 37eda24941..56a1d2a571 100644 --- a/openedx/core/djangoapps/user_api/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/tests/test_views.py @@ -32,7 +32,10 @@ from third_party_auth.tests.testutil import simulate_running_pipeline, ThirdPart from third_party_auth.tests.utils import ( ThirdPartyOAuthTestMixin, ThirdPartyOAuthTestMixinFacebook, ThirdPartyOAuthTestMixinGoogle ) -from util.password_policy_validators import password_max_length, password_min_length +from util.password_policy_validators import ( + create_validator_config, password_validators_instruction_texts, password_validators_restrictions, + DEFAULT_MAX_PASSWORD_LENGTH, +) from .test_helpers import TestCaseForm from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory @@ -620,7 +623,7 @@ class LoginSessionViewTest(UserAPITestCase): "placeholder": "", "instructions": "", "restrictions": { - "max_length": password_max_length(), + "max_length": DEFAULT_MAX_PASSWORD_LENGTH, }, "errorMessages": {}, "supplementalText": "", @@ -1186,15 +1189,16 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): u"type": u"password", u"required": True, u"label": u"Password", - u"instructions": u'Your password must contain at least {} characters.'.format(password_min_length()), - u"restrictions": { - 'min_length': password_min_length(), - 'max_length': password_max_length(), - }, + u"instructions": password_validators_instruction_texts(), + u"restrictions": password_validators_restrictions(), } ) - @override_settings(PASSWORD_COMPLEXITY={'NON ASCII': 1, 'UPPER': 3}) + @override_settings(AUTH_PASSWORD_VALIDATORS=[ + create_validator_config('util.password_policy_validators.MinimumLengthValidator', {'min_length': 2}), + create_validator_config('util.password_policy_validators.UppercaseValidator', {'min_upper': 3}), + create_validator_config('util.password_policy_validators.SymbolValidator', {'min_symbol': 1}), + ]) def test_register_form_password_complexity(self): no_extra_fields_setting = {} @@ -1204,32 +1208,22 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): { u'name': u'password', u'label': u'Password', - u'instructions': u'Your password must contain at least {} characters.'.format(password_min_length()), - u'restrictions': { - 'min_length': password_min_length(), - 'max_length': password_max_length(), - }, + u"instructions": password_validators_instruction_texts(), + u"restrictions": password_validators_restrictions(), } ) - # Now with an enabled password policy - with mock.patch.dict(settings.FEATURES, {'ENFORCE_PASSWORD_POLICY': True}): - msg = u'Your password must contain at least {} characters, including '\ - u'3 uppercase letters & 1 symbol.'.format(password_min_length()) - self._assert_reg_field( - no_extra_fields_setting, - { - u'name': u'password', - u'label': u'Password', - u'instructions': msg, - u'restrictions': { - 'min_length': password_min_length(), - 'max_length': password_max_length(), - 'non_ascii': 1, - 'upper': 3, - }, - } - ) + msg = u'Your password must contain at least 2 characters, including '\ + u'3 uppercase letters & 1 symbol.' + self._assert_reg_field( + no_extra_fields_setting, + { + u'name': u'password', + u'label': u'Password', + u'instructions': msg, + u"restrictions": password_validators_restrictions(), + } + ) @override_settings(REGISTRATION_EXTENSION_FORM='openedx.core.djangoapps.user_api.tests.test_helpers.TestCaseForm') def test_extension_form_fields(self): @@ -2314,7 +2308,7 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): response_json, { u"username": [{u"user_message": USERNAME_BAD_LENGTH_MSG}], - u"password": [{u"user_message": u"A valid password is required"}], + u"password": [{u"user_message": u"This field is required."}], } ) diff --git a/openedx/core/djangoapps/user_api/validation/tests/test_views.py b/openedx/core/djangoapps/user_api/validation/tests/test_views.py index c9e02b8162..8046fa57a3 100644 --- a/openedx/core/djangoapps/user_api/validation/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/validation/tests/test_views.py @@ -16,7 +16,7 @@ from openedx.core.djangoapps.user_api import accounts from openedx.core.djangoapps.user_api.accounts.tests import testutils from openedx.core.lib.api import test_utils from openedx.core.djangoapps.user_api.validation.views import RegistrationValidationThrottle -from util.password_policy_validators import password_max_length, password_min_length +from util.password_policy_validators import DEFAULT_MAX_PASSWORD_LENGTH @ddt.ddt @@ -174,23 +174,29 @@ class RegistrationValidationViewTests(test_utils.ApiTestCase): ) def test_password_empty_validation_decision(self): - msg = u'Enter a password with at least {0} characters.'.format(password_min_length()) + # 2 is the default setting for minimum length found in lms/envs/common.py + # under AUTH_PASSWORD_VALIDATORS.MinimumLengthValidator + msg = u'This password is too short. It must contain at least 2 characters.' self.assertValidationDecision( {'password': ''}, {"password": msg} ) def test_password_bad_min_length_validation_decision(self): - password = 'p' * (password_min_length() - 1) - msg = u'Enter a password with at least {0} characters.'.format(password_min_length()) + password = 'p' + # 2 is the default setting for minimum length found in lms/envs/common.py + # under AUTH_PASSWORD_VALIDATORS.MinimumLengthValidator + msg = u'This password is too short. It must contain at least 2 characters.' self.assertValidationDecision( {'password': password}, {"password": msg} ) def test_password_bad_max_length_validation_decision(self): - password = 'p' * (password_max_length() + 1) - msg = u'Enter a password with at most {0} characters.'.format(password_max_length()) + password = 'p' * DEFAULT_MAX_PASSWORD_LENGTH + # 75 is the default setting for maximum length found in lms/envs/common.py + # under AUTH_PASSWORD_VALIDATORS.MaximumLengthValidator + msg = u'This password is too long. It must contain no more than 75 characters.' self.assertValidationDecision( {'password': password}, {"password": msg} @@ -199,7 +205,7 @@ class RegistrationValidationViewTests(test_utils.ApiTestCase): def test_password_equals_username_validation_decision(self): self.assertValidationDecision( {"username": "somephrase", "password": "somephrase"}, - {"username": "", "password": u"Password cannot be the same as the username."} + {"username": "", "password": u"The password is too similar to the username."} ) @override_settings( diff --git a/openedx/core/djangoapps/user_authn/views/deprecated.py b/openedx/core/djangoapps/user_authn/views/deprecated.py index 79bc813a78..4d2f496384 100644 --- a/openedx/core/djangoapps/user_authn/views/deprecated.py +++ b/openedx/core/djangoapps/user_authn/views/deprecated.py @@ -142,7 +142,7 @@ def create_account(request, post_override=None): { "success": False, "field": field, - "value": error_list[0], + "value": ' '.join(error_list), }, status=400 ) diff --git a/openedx/core/djangoapps/user_authn/views/register.py b/openedx/core/djangoapps/user_authn/views/register.py index 951cf9120b..66009b957b 100644 --- a/openedx/core/djangoapps/user_authn/views/register.py +++ b/openedx/core/djangoapps/user_authn/views/register.py @@ -137,7 +137,6 @@ def create_account_with_params(request, params): do_external_auth, eamap = pre_account_creation_external_auth(request, params) extended_profile_fields = configuration_helpers.get_value('extended_profile_fields', []) - enforce_password_policy = not do_external_auth # Can't have terms of service for certain SHIB users, like at Stanford registration_fields = getattr(settings, 'REGISTRATION_EXTRA_FIELDS', {}) tos_required = ( @@ -154,7 +153,7 @@ def create_account_with_params(request, params): data=params, extra_fields=extra_fields, extended_profile_fields=extended_profile_fields, - enforce_password_policy=enforce_password_policy, + do_third_party_auth=do_external_auth, tos_required=tos_required, ) custom_form = get_registration_extension_form(data=params) diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_register.py b/openedx/core/djangoapps/user_authn/views/tests/test_register.py index 7e4e166ad1..17b5ff16e9 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_register.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_register.py @@ -637,10 +637,14 @@ class TestCreateAccountValidation(TestCase): del params["email"] assert_email_error("A properly formatted e-mail is required") - # Empty, too short - for email in ["", "a"]: - params["email"] = email - assert_email_error("A properly formatted e-mail is required") + # Empty + params["email"] = "" + assert_email_error("A properly formatted e-mail is required") + + #too short + params["email"] = "a" + assert_email_error("A properly formatted e-mail is required " + "Ensure this value has at least 3 characters (it has 1).") # Too long params["email"] = '{email}@example.com'.format( @@ -703,18 +707,21 @@ class TestCreateAccountValidation(TestCase): # Missing del params["password"] - assert_password_error("A valid password is required") + assert_password_error("This field is required.") - # Empty, too short - for password in ["", "a"]: - params["password"] = password - assert_password_error("A valid password is required") + # Empty + params["password"] = "" + assert_password_error("This field is required.") + + # Too short + params["password"] = "a" + assert_password_error("This password is too short. It must contain at least 2 characters.") # Password policy is tested elsewhere # Matching username params["username"] = params["password"] = "test_username_and_password" - assert_password_error("Password cannot be the same as the username.") + assert_password_error("The password is too similar to the username.") def test_name(self): params = dict(self.minimal_params)