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)