Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Improve password complexity messaging

Browse Source 
Send password form instructions that include password complexity and
also send error messages back that include all parts of the
complexity instead of single parts at a time.

And clean up phrasing to be more consistent.
This commit is contained in:
Michael Terry
2018-03-20 09:58:02 -04:00
committed by Michael Terry
parent 795dda47d8
commit a576d682ff
10 changed files with 235 additions and 79 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
common/djangoapps/student/tests/test_create_account.py
View File

@@ -711,7 +711,7 @@ class TestCreateAccountValidation(TestCase):
# Matching username
params["username"] = params["password"] = "test_username_and_password"
assert_password_error("Password cannot be the same as the username")
assert_password_error("Password cannot be the same as the username.")
def test_name(self):
params = dict(self.minimal_params)

37
common/djangoapps/student/tests/test_password_policy.py
View File

@@ -43,7 +43,7 @@ class TestPasswordPolicy(TestCase):
obj = json.loads(response.content)
self.assertEqual(
obj['value'],
"Password: Invalid Length (must be 6 characters or more)",
"Enter a password with at least 6 characters.",
)
@override_settings(PASSWORD_MIN_LENGTH=6)
@@ -62,7 +62,7 @@ class TestPasswordPolicy(TestCase):
obj = json.loads(response.content)
self.assertEqual(
obj['value'],
"Password: Invalid Length (must be 12 characters or fewer)",
"Enter a password with at most 12 characters.",
)
@patch.dict("django.conf.settings.PASSWORD_COMPLEXITY", {'UPPER': 3})
@@ -73,7 +73,7 @@ class TestPasswordPolicy(TestCase):
obj = json.loads(response.content)
self.assertEqual(
obj['value'],
"Password: Must be more complex (must contain 3 or more uppercase characters)",
"Enter a password with at least 3 uppercase letters.",
)
@patch.dict("django.conf.settings.PASSWORD_COMPLEXITY", {'UPPER': 3})
@@ -92,7 +92,7 @@ class TestPasswordPolicy(TestCase):
obj = json.loads(response.content)
self.assertEqual(
obj['value'],
"Password: Must be more complex (must contain 3 or more lowercase characters)",
"Enter a password with at least 3 lowercase letters.",
)
@patch.dict("django.conf.settings.PASSWORD_COMPLEXITY", {'LOWER': 3})
@@ -111,7 +111,7 @@ class TestPasswordPolicy(TestCase):
obj = json.loads(response.content)
self.assertEqual(
obj['value'],
"Password: Must be more complex (must contain 3 or more digits)",
"Enter a password with at least 3 digits.",
)
@patch.dict("django.conf.settings.PASSWORD_COMPLEXITY", {'DIGITS': 3})
@@ -130,7 +130,7 @@ class TestPasswordPolicy(TestCase):
obj = json.loads(response.content)
self.assertEqual(
obj['value'],
"Password: Must be more complex (must contain 3 or more punctuation characters)",
"Enter a password with at least 3 punctuation marks.",
)
@patch.dict("django.conf.settings.PASSWORD_COMPLEXITY", {'PUNCTUATION': 3})
@@ -149,7 +149,7 @@ class TestPasswordPolicy(TestCase):
obj = json.loads(response.content)
self.assertEqual(
obj['value'],
"Password: Must be more complex (must contain 3 or more unique words)",
"Enter a password with at least 3 words.",
)
@patch.dict("django.conf.settings.PASSWORD_COMPLEXITY", {'WORDS': 3})
@@ -168,7 +168,7 @@ class TestPasswordPolicy(TestCase):
obj = json.loads(response.content)
self.assertEqual(
obj['value'],
"Password: Must be more complex (must contain 3 or more numbers)",
"Enter a password with at least 3 numbers.",
)
@patch.dict("django.conf.settings.PASSWORD_COMPLEXITY", {'NUMERIC': 3})
@@ -187,7 +187,7 @@ class TestPasswordPolicy(TestCase):
obj = json.loads(response.content)
self.assertEqual(
obj['value'],
"Password: Must be more complex (must contain 3 or more letters)",
"Enter a password with at least 3 letters.",
)
@patch.dict("django.conf.settings.PASSWORD_COMPLEXITY", {'ALPHABETIC': 3})
@@ -211,12 +211,11 @@ class TestPasswordPolicy(TestCase):
self.assertEqual(response.status_code, 400)
obj = json.loads(response.content)
errstring = (
"Password: Must be more complex ("
"must contain 3 or more uppercase characters, "
"must contain 3 or more digits, "
"must contain 3 or more punctuation characters, "
"must contain 3 or more unique words"
")"
"Enter a password with at least "
"3 uppercase letters & "
"3 digits & "
"3 punctuation marks & "
"3 words."
)
self.assertEqual(obj['value'], errstring)
@@ -243,7 +242,7 @@ class TestPasswordPolicy(TestCase):
obj = json.loads(response.content)
self.assertEqual(
obj['value'],
"Password: Too similar to a restricted dictionary word.",
"Password is too similar to a dictionary word.",
)
@override_settings(PASSWORD_DICTIONARY=['foo', 'bar'])
@@ -255,7 +254,7 @@ class TestPasswordPolicy(TestCase):
obj = json.loads(response.content)
self.assertEqual(
obj['value'],
"Password: Too similar to a restricted dictionary word.",
"Password is too similar to a dictionary word.",
)
@override_settings(PASSWORD_DICTIONARY=['foo', 'bar'])
@@ -267,7 +266,7 @@ class TestPasswordPolicy(TestCase):
obj = json.loads(response.content)
self.assertEqual(
obj['value'],
"Password: Too similar to a restricted dictionary word.",
"Password is too similar to a dictionary word.",
)
@override_settings(PASSWORD_DICTIONARY=['foo', 'bar'])
@@ -334,7 +333,7 @@ class TestUsernamePasswordNonmatch(TestCase):
obj = json.loads(response.content)
self.assertEqual(
obj['value'],
"Password cannot be the same as the username",
"Password cannot be the same as the username.",
)
def test_with_username_password_nonmatch(self):

4
common/djangoapps/student/tests/test_reset_password.py
View File

@@ -299,11 +299,11 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase):
@ddt.data(
{
'password': '1',
'error_message': 'Password: Invalid Length (must be 2 characters or more)',
'error_message': 'Enter a password with at least 2 characters.',
},
{
'password': '01234567891',
'error_message': 'Password: Invalid Length (must be 10 characters or fewer)'
'error_message': 'Enter a password with at most 10 characters.',
}
)
def test_password_reset_with_invalid_length(self, password_dict):

186
common/djangoapps/util/password_policy_validators.py
View File

@@ -7,6 +7,7 @@ authored by dstufft (https://github.com/dstufft)
"""
from __future__ import division
import logging
import string
import unicodedata
@@ -20,6 +21,21 @@ from six import text_type
from student.models import PasswordHistory
log = logging.getLogger(__name__)
# In description order
_allowed_password_complexity = [
'ALPHABETIC',
'UPPER',
'LOWER',
'NUMERIC',
'DIGITS',
'PUNCTUATION',
'NON ASCII',
'WORDS',
]
class SecurityPolicyError(ValidationError):
pass
@@ -47,6 +63,92 @@ def password_max_length():
return max_length
def password_complexity():
"""
:return: A dict of complexity requirements from settings
"""
complexity = {}
if settings.FEATURES.get('ENFORCE_PASSWORD_POLICY', False):
complexity = getattr(settings, 'PASSWORD_COMPLEXITY', {})
valid_complexity = {x: y for x, y in complexity.iteritems() if x in _allowed_password_complexity}
if not password_complexity.logged:
invalid = frozenset(complexity.keys()) - frozenset(valid_complexity.keys())
for key in invalid:
log.warning('Unrecognized %s value in PASSWORD_COMPLEXITY setting.', key)
password_complexity.logged = True
return valid_complexity
# Declare static variable for the function above, which helps avoid issuing multiple log warnings.
# We don't instead keep a cached version of the complexity rules, because that might trip up unit tests.
password_complexity.logged = False
def _password_complexity_descriptions(which=None):
"""
which: A list of which complexities to describe, None if you want the configured ones
:return: A list of complexity descriptions
"""
descs = []
complexity = password_complexity()
if which is None:
which = complexity.keys()
for key in _allowed_password_complexity: # we iterate over allowed keys so that we get the order right
value = complexity.get(key, 0) if key in which else 0
if not value:
continue
if key == 'ALPHABETIC':
# Translators: This appears in a list of password requirements
descs.append(ungettext('{num} letter', '{num} letters', value).format(num=value))
elif key == 'UPPER':
# Translators: This appears in a list of password requirements
descs.append(ungettext('{num} uppercase letter', '{num} uppercase letters', value).format(num=value))
elif key == 'LOWER':
# Translators: This appears in a list of password requirements
descs.append(ungettext('{num} lowercase letter', '{num} lowercase letters', value).format(num=value))
elif key == 'DIGITS':
# Translators: This appears in a list of password requirements
descs.append(ungettext('{num} digit', '{num} digits', value).format(num=value))
elif key == 'NUMERIC':
# Translators: This appears in a list of password requirements
descs.append(ungettext('{num} number', '{num} numbers', value).format(num=value))
elif key == 'PUNCTUATION':
# Translators: This appears in a list of password requirements
descs.append(ungettext('{num} punctuation mark', '{num} punctuation marks', value).format(num=value))
elif key == 'NON ASCII': # note that our definition of non-ascii is non-letter, non-digit, non-punctuation
# Translators: This appears in a list of password requirements
descs.append(ungettext('{num} symbol', '{num} symbols', value).format(num=value))
elif key == 'WORDS':
# Translators: This appears in a list of password requirements
descs.append(ungettext('{num} word', '{num} words', value).format(num=value))
else:
raise Exception('Unexpected complexity value {}'.format(key))
return descs
def password_instructions():
"""
:return: A string suitable for display to the user to tell them what password to enter
"""
min_length = password_min_length()
reqs = _password_complexity_descriptions()
if not reqs:
return ungettext('Your password must contain at least {num} character.',
'Your password must contain at least {num} characters.',
min_length).format(num=min_length)
else:
return ungettext('Your password must contain at least {num} character, including {requirements}.',
'Your password must contain at least {num} characters, including {requirements}.',
min_length).format(num=min_length, requirements=' & '.join(reqs))
def validate_password(password, user=None, username=None):
"""
Checks user-provided password against our current site policy.
@@ -61,19 +163,22 @@ def validate_password(password, user=None, username=None):
username = username or (user and user.username)
if user:
validate_password_security(password, user)
_validate_password_security(password, user)
validate_password_length(password)
_validate_password_dictionary(password)
_validate_password_against_username(password, username)
if settings.FEATURES.get('ENFORCE_PASSWORD_POLICY', False):
validate_password_complexity(password)
validate_password_dictionary(password)
# Some messages are composable, so we'll add them together here
errors = [_validate_password_length(password)]
errors += _validate_password_complexity(password)
errors = filter(None, errors)
if username:
validate_password_against_username(password, username)
if errors:
msg = _('Enter a password with at least {requirements}.').format(requirements=' & '.join(errors))
raise ValidationError(msg)
def validate_password_security(password, user):
def _validate_password_security(password, user):
"""
Check password reuse and similar operational security policy considerations.
"""
@@ -103,33 +208,36 @@ def validate_password_security(password, user):
).format(num=num_days))
def validate_password_length(value):
def _validate_password_length(value):
"""
Validator that enforces minimum length of a password
"""
message = _("Password: Invalid Length ({0})")
code = "length"
min_length = password_min_length()
max_length = password_max_length()
if min_length and len(value) < min_length:
raise ValidationError(message.format(_("must be {0} characters or more").format(min_length)), code=code)
# This is an error that can be composed with other requirements, so just return a fragment
# Translators: This appears in a list of password requirements
return ungettext(
"{num} character",
"{num} characters",
min_length
).format(num=min_length)
elif max_length and len(value) > max_length:
raise ValidationError(message.format(_("must be {0} characters or fewer").format(max_length)), code=code)
raise ValidationError(ungettext(
"Enter a password with at most {num} character.",
"Enter a password with at most {num} characters.",
max_length
).format(num=max_length))
def validate_password_complexity(value):
def _validate_password_complexity(value):
"""
Validator that enforces minimum complexity
"""
message = _("Password: Must be more complex ({0})")
code = "complexity"
complexities = getattr(settings, "PASSWORD_COMPLEXITY", None)
if complexities is None:
return
complexities = password_complexity()
if not complexities:
return []
# Sets are here intentionally
uppercase, lowercase, digits, non_ascii, punctuation = set(), set(), set(), set(), set()
@@ -156,37 +264,45 @@ def validate_password_complexity(value):
errors = []
if len(uppercase) < complexities.get("UPPER", 0):
errors.append(_("must contain {0} or more uppercase characters").format(complexities["UPPER"]))
errors.append('UPPER')
if len(lowercase) < complexities.get("LOWER", 0):
errors.append(_("must contain {0} or more lowercase characters").format(complexities["LOWER"]))
errors.append('LOWER')
if len(digits) < complexities.get("DIGITS", 0):
errors.append(_("must contain {0} or more digits").format(complexities["DIGITS"]))
errors.append('DIGITS')
if len(punctuation) < complexities.get("PUNCTUATION", 0):
errors.append(_("must contain {0} or more punctuation characters").format(complexities["PUNCTUATION"]))
errors.append('PUNCTUATION')
if len(non_ascii) < complexities.get("NON ASCII", 0):
errors.append(_("must contain {0} or more non ascii characters").format(complexities["NON ASCII"]))
errors.append('NON ASCII')
if len(words) < complexities.get("WORDS", 0):
errors.append(_("must contain {0} or more unique words").format(complexities["WORDS"]))
errors.append('WORDS')
if len(numeric) < complexities.get("NUMERIC", 0):
errors.append(_("must contain {0} or more numbers").format(complexities["NUMERIC"]))
errors.append('NUMERIC')
if len(alphabetic) < complexities.get("ALPHABETIC", 0):
errors.append(_("must contain {0} or more letters").format(complexities["ALPHABETIC"]))
errors.append('ALPHABETIC')
if errors:
raise ValidationError(message.format(u', '.join(errors)), code=code)
return _password_complexity_descriptions(errors)
else:
return []
def validate_password_against_username(password, username):
def _validate_password_against_username(password, username):
if not username:
return
if password == username:
# Translators: This message is shown to users who enter a password matching
# the username they enter(ed).
raise ValidationError(_(u"Password cannot be the same as the username"))
raise ValidationError(_(u"Password cannot be the same as the username."))
def validate_password_dictionary(value):
def _validate_password_dictionary(value):
"""
Insures that the password is not too similar to a defined set of dictionary words
"""
if not settings.FEATURES.get('ENFORCE_PASSWORD_POLICY', False):
return
password_max_edit_distance = getattr(settings, "PASSWORD_DICTIONARY_EDIT_DISTANCE_THRESHOLD", None)
password_dictionary = getattr(settings, "PASSWORD_DICTIONARY", None)
@@ -194,5 +310,5 @@ def validate_password_dictionary(value):
for word in password_dictionary:
edit_distance = distance(text_type(value), text_type(word))
if edit_distance <= password_max_edit_distance:
raise ValidationError(_("Password: Too similar to a restricted dictionary word."),
raise ValidationError(_("Password is too similar to a dictionary word."),
code="dictionary_word")

46
common/djangoapps/util/tests/test_password_policy_validators.py
View File

@@ -1,31 +1,67 @@
"""Tests for util.password_policy_validators module."""
import mock
import unittest
from ddt import data, ddt, unpack
from django.conf import settings
from django.core.exceptions import ValidationError
from django.test.utils import override_settings
from util.password_policy_validators import validate_password_dictionary
from util.password_policy_validators import password_instructions, validate_password, _validate_password_dictionary
@ddt
class PasswordPolicyValidatorsTestCase(unittest.TestCase):
""" 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('testme')
_validate_password_dictionary('testme')
# Off by one
with self.assertRaises(ValidationError):
validate_password_dictionary('estme')
_validate_password_dictionary('estme')
# Off by two
with self.assertRaises(ValidationError):
validate_password_dictionary('bestmet')
_validate_password_dictionary('bestmet')
# Off by three (should pass)
validate_password_dictionary('bestem')
_validate_password_dictionary('bestem')
@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),
)
@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)
@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.'),
)
@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())

2
lms/djangoapps/courseware/tests/test_password_history.py
View File

@@ -343,7 +343,7 @@ class TestPasswordHistory(LoginEnrollmentTestCase):
@ddt.data(
('foo', 'foobar', 'Error in resetting your password. Please try again.'),
('', '', 'Password: Invalid Length'),
('', '', 'Enter a password with at least'),
)
@ddt.unpack
def test_password_reset_form_invalid(self, password1, password2, err_msg):

22
openedx/core/djangoapps/user_api/api.py
View File

@@ -15,7 +15,9 @@ from openedx.core.djangoapps.user_api.helpers import FormDescription
from openedx.features.enterprise_support.api import enterprise_customer_for_request
from student.forms import get_registration_extension_form
from student.models import UserProfile
from util.password_policy_validators import password_max_length, password_min_length
from util.password_policy_validators import (
password_complexity, password_instructions, password_max_length, password_min_length
)
def get_password_reset_form():
@@ -415,23 +417,21 @@ class RegistrationFormFactory(object):
# meant to hold the user's password.
password_label = _(u"Password")
restrictions = {}
if settings.FEATURES.get('ENFORCE_PASSWORD_POLICY', False):
complexities = getattr(settings, 'PASSWORD_COMPLEXITY', {})
for key, value in complexities.iteritems():
api_key = key.lower().replace(' ', '_')
restrictions[api_key] = value
restrictions.update({
restrictions = {
"min_length": password_min_length(),
"max_length": password_max_length(),
})
}
complexities = password_complexity()
for key, value in complexities.iteritems():
api_key = key.lower().replace(' ', '_')
restrictions[api_key] = value
form_desc.add_field(
"password",
label=password_label,
field_type="password",
instructions=password_instructions(),
restrictions=restrictions,
required=required
)

5
openedx/core/djangoapps/user_api/tests/test_views.py
View File

@@ -1055,6 +1055,7 @@ 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(),
@@ -1072,6 +1073,7 @@ 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(),
@@ -1081,11 +1083,14 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase):
# 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(),

8
openedx/core/djangoapps/user_api/validation/tests/test_views.py
View File

@@ -172,7 +172,7 @@ class RegistrationValidationViewTests(test_utils.ApiTestCase):
)
def test_password_empty_validation_decision(self):
msg = u'Password: Invalid Length (must be {0} characters or more)'.format(password_min_length())
msg = u'Enter a password with at least {0} characters.'.format(password_min_length())
self.assertValidationDecision(
{'password': ''},
{"password": msg}
@@ -180,7 +180,7 @@ class RegistrationValidationViewTests(test_utils.ApiTestCase):
def test_password_bad_min_length_validation_decision(self):
password = 'p' * (password_min_length() - 1)
msg = u'Password: Invalid Length (must be {0} characters or more)'.format(password_min_length())
msg = u'Enter a password with at least {0} characters.'.format(password_min_length())
self.assertValidationDecision(
{'password': password},
{"password": msg}
@@ -188,7 +188,7 @@ class RegistrationValidationViewTests(test_utils.ApiTestCase):
def test_password_bad_max_length_validation_decision(self):
password = 'p' * (password_max_length() + 1)
msg = u'Password: Invalid Length (must be {0} characters or fewer)'.format(password_max_length())
msg = u'Enter a password with at most {0} characters.'.format(password_max_length())
self.assertValidationDecision(
{'password': password},
{"password": msg}
@@ -197,5 +197,5 @@ 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"Password cannot be the same as the username."}
)

2
openedx/core/djangoapps/user_api/validation/views.py
View File

@@ -57,7 +57,7 @@ class RegistrationValidationView(APIView):
>>> {
>>> "validation_decisions": {
>>> "username": "",
>>> "password": "Password cannot be the same as the username"
>>> "password": "Password cannot be the same as the username."
>>> }
>>> }