Skip to content

Commit

Permalink
Adding basic validation for local passwords (#13789)
Browse files Browse the repository at this point in the history
* Adding basic validation for local passwords

* Adding edit screen

* Fixing tests
  • Loading branch information
john-westcott-iv authored Apr 13, 2023
1 parent 12a4c30 commit fba4e06
Show file tree
Hide file tree
Showing 7 changed files with 884 additions and 430 deletions.
18 changes: 18 additions & 0 deletions awx/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -995,6 +995,24 @@ def validate_password(self, value):
django_validate_password(value)
if not self.instance and value in (None, ''):
raise serializers.ValidationError(_('Password required for new User.'))

# Check if a password is too long
password_max_length = User._meta.get_field('password').max_length
if len(value) > password_max_length:
raise serializers.ValidationError(_('Password max length is {}'.format(password_max_length)))
if getattr(settings, 'LOCAL_PASSWORD_MIN_LENGTH', 0) and len(value) < getattr(settings, 'LOCAL_PASSWORD_MIN_LENGTH'):
raise serializers.ValidationError(_('Password must be at least {} characters long.'.format(getattr(settings, 'LOCAL_PASSWORD_MIN_LENGTH'))))
if getattr(settings, 'LOCAL_PASSWORD_MIN_DIGITS', 0) and sum(c.isdigit() for c in value) < getattr(settings, 'LOCAL_PASSWORD_MIN_DIGITS'):
raise serializers.ValidationError(_('Password must contain at least {} digits.'.format(getattr(settings, 'LOCAL_PASSWORD_MIN_DIGITS'))))
if getattr(settings, 'LOCAL_PASSWORD_MIN_UPPER', 0) and sum(c.isupper() for c in value) < getattr(settings, 'LOCAL_PASSWORD_MIN_UPPER'):
raise serializers.ValidationError(
_('Password must contain at least {} uppercase characters.'.format(getattr(settings, 'LOCAL_PASSWORD_MIN_UPPER')))
)
if getattr(settings, 'LOCAL_PASSWORD_MIN_SPECIAL', 0) and sum(not c.isalnum() for c in value) < getattr(settings, 'LOCAL_PASSWORD_MIN_SPECIAL'):
raise serializers.ValidationError(
_('Password must contain at least {} special characters.'.format(getattr(settings, 'LOCAL_PASSWORD_MIN_SPECIAL')))
)

return value

def _update_password(self, obj, new_password):
Expand Down
75 changes: 75 additions & 0 deletions awx/main/tests/functional/api/test_serializers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import pytest

from django.test.utils import override_settings

from rest_framework.serializers import ValidationError

from awx.api.serializers import UserSerializer
from django.contrib.auth.models import User


@pytest.mark.parametrize(
"password,min_length,min_digits,min_upper,min_special,expect_error",
[
# Test length
("a", 1, 0, 0, 0, False),
("a", 2, 0, 0, 0, True),
("aa", 2, 0, 0, 0, False),
("aaabcDEF123$%^", 2, 0, 0, 0, False),
# Test digits
("a", 0, 1, 0, 0, True),
("1", 0, 1, 0, 0, False),
("1", 0, 2, 0, 0, True),
("12", 0, 2, 0, 0, False),
("12abcDEF123$%^", 0, 2, 0, 0, False),
# Test upper
("a", 0, 0, 1, 0, True),
("A", 0, 0, 1, 0, False),
("A", 0, 0, 2, 0, True),
("AB", 0, 0, 2, 0, False),
("ABabcDEF123$%^", 0, 0, 2, 0, False),
# Test special
("a", 0, 0, 0, 1, True),
("!", 0, 0, 0, 1, False),
("!", 0, 0, 0, 2, True),
("!@", 0, 0, 0, 2, False),
("!@abcDEF123$%^", 0, 0, 0, 2, False),
],
)
@pytest.mark.django_db
def test_validate_password_rules(password, min_length, min_digits, min_upper, min_special, expect_error):
user_serializer = UserSerializer()

# First test password with no params, this should always pass
try:
user_serializer.validate_password(password)
except ValidationError:
assert False, f"Password {password} should not have validation issue if no params are used"

with override_settings(
LOCAL_PASSWORD_MIN_LENGTH=min_length, LOCAL_PASSWORD_MIN_DIGITS=min_digits, LOCAL_PASSWORD_MIN_UPPER=min_upper, LOCAL_PASSWORD_MIN_SPECIAL=min_special
):
if expect_error:
with pytest.raises(ValidationError):
user_serializer.validate_password(password)
else:
try:
user_serializer.validate_password(password)
except ValidationError:
assert False, "validate_password raised an unexpected exception"


@pytest.mark.django_db
def test_validate_password_too_long():
password_max_length = User._meta.get_field('password').max_length
password = "x" * password_max_length

user_serializer = UserSerializer()
try:
user_serializer.validate_password(password)
except ValidationError:
assert False, f"Password {password} should not have validation"

password = f"{password}x"
with pytest.raises(ValidationError):
user_serializer.validate_password(password)
44 changes: 44 additions & 0 deletions awx/sso/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -1603,6 +1603,50 @@ def get_saml_entity_id():
],
)

register(
'LOCAL_PASSWORD_MIN_LENGTH',
field_class=fields.IntegerField,
min_value=0,
default=0,
label=_('Minimum number of characters in local password'),
help_text=_('Minimum number of characters required in a local password. 0 means no minimum'),
category=_('Authentication'),
category_slug='authentication',
)

register(
'LOCAL_PASSWORD_MIN_DIGITS',
field_class=fields.IntegerField,
min_value=0,
default=0,
label=_('Minimum number of digit characters in local password'),
help_text=_('Minimum number of digit characters required in a local password. 0 means no minimum'),
category=_('Authentication'),
category_slug='authentication',
)

register(
'LOCAL_PASSWORD_MIN_UPPER',
field_class=fields.IntegerField,
min_value=0,
default=0,
label=_('Minimum number of uppercase characters in local password'),
help_text=_('Minimum number of uppercase characters required in a local password. 0 means no minimum'),
category=_('Authentication'),
category_slug='authentication',
)

register(
'LOCAL_PASSWORD_MIN_SPECIAL',
field_class=fields.IntegerField,
min_value=0,
default=0,
label=_('Minimum number of special characters in local password'),
help_text=_('Minimum number of special characters required in a local password. 0 means no minimum'),
category=_('Authentication'),
category_slug='authentication',
)


def tacacs_validate(serializer, attrs):
if not serializer.instance or not hasattr(serializer.instance, 'TACACSPLUS_HOST') or not hasattr(serializer.instance, 'TACACSPLUS_SECRET'):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,11 @@ function MiscAuthenticationEdit() {
'SOCIAL_AUTH_ORGANIZATION_MAP',
'SOCIAL_AUTH_TEAM_MAP',
'SOCIAL_AUTH_USER_FIELDS',
'SOCIAL_AUTH_USERNAME_IS_FULL_EMAIL'
'SOCIAL_AUTH_USERNAME_IS_FULL_EMAIL',
'LOCAL_PASSWORD_MIN_LENGTH',
'LOCAL_PASSWORD_MIN_DIGITS',
'LOCAL_PASSWORD_MIN_UPPER',
'LOCAL_PASSWORD_MIN_SPECIAL'
);

const authenticationData = {
Expand Down Expand Up @@ -247,6 +251,30 @@ function MiscAuthenticationEdit() {
name="SOCIAL_AUTH_USERNAME_IS_FULL_EMAIL"
config={authentication.SOCIAL_AUTH_USERNAME_IS_FULL_EMAIL}
/>
<InputField
name="LOCAL_PASSWORD_MIN_LENGTH"
config={authentication.LOCAL_PASSWORD_MIN_LENGTH}
type="number"
isRequired
/>
<InputField
name="LOCAL_PASSWORD_MIN_DIGITS"
config={authentication.LOCAL_PASSWORD_MIN_DIGITS}
type="number"
isRequired
/>
<InputField
name="LOCAL_PASSWORD_MIN_UPPER"
config={authentication.LOCAL_PASSWORD_MIN_UPPER}
type="number"
isRequired
/>
<InputField
name="LOCAL_PASSWORD_MIN_SPECIAL"
config={authentication.LOCAL_PASSWORD_MIN_SPECIAL}
type="number"
isRequired
/>
{submitError && <FormSubmitError error={submitError} />}
{revertError && <FormSubmitError error={revertError} />}
</FormColumnLayout>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ const authenticationData = {
SOCIAL_AUTH_TEAM_MAP: null,
SOCIAL_AUTH_USER_FIELDS: null,
SOCIAL_AUTH_USERNAME_IS_FULL_EMAIL: false,
LOCAL_PASSWORD_MIN_LENGTH: 0,
LOCAL_PASSWORD_MIN_DIGITS: 0,
LOCAL_PASSWORD_MIN_UPPER: 0,
LOCAL_PASSWORD_MIN_SPECIAL: 0,
};

describe('<MiscAuthenticationEdit />', () => {
Expand Down
Loading

0 comments on commit fba4e06

Please sign in to comment.