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.
This commit is contained in:
Dillon Dumesnil
2018-10-02 14:21:07 -04:00
parent 4fa27f98dc
commit deb758c808
30 changed files with 482 additions and 469 deletions

View File

@@ -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

View File

@@ -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,

View File

@@ -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:

View File

@@ -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

View File

@@ -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 = [

View File

@@ -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(

View File

@@ -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."}],
}
)

View File

@@ -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(

View File

@@ -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
)

View File

@@ -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)

View File

@@ -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)