Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

17289 enforce minimum password strength #17299

Merged
merged 11 commits into from
Aug 30, 2024
13 changes: 11 additions & 2 deletions netbox/netbox/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
from utilities.release import load_release_data
from utilities.string import trailing_slash


#
# Environment setup
#
Expand Down Expand Up @@ -63,7 +62,17 @@
ADMINS = getattr(configuration, 'ADMINS', [])
ALLOW_TOKEN_RETRIEVAL = getattr(configuration, 'ALLOW_TOKEN_RETRIEVAL', True)
ALLOWED_HOSTS = getattr(configuration, 'ALLOWED_HOSTS') # Required
AUTH_PASSWORD_VALIDATORS = getattr(configuration, 'AUTH_PASSWORD_VALIDATORS', [])
AUTH_PASSWORD_VALIDATORS = getattr(configuration, 'AUTH_PASSWORD_VALIDATORS', [
{
"NAME": "django.contrib.auth.password_validation.MinimumLengthValidator",
"OPTIONS": {
"min_length": 12,
},
},
{
"NAME": "utilities.password_validation.AlphanumericPasswordValidator",
},
])
BASE_PATH = trailing_slash(getattr(configuration, 'BASE_PATH', ''))
CHANGELOG_SKIP_EMPTY_CHANGES = getattr(configuration, 'CHANGELOG_SKIP_EMPTY_CHANGES', True)
CENSUS_REPORTING_ENABLED = getattr(configuration, 'CENSUS_REPORTING_ENABLED', True)
Expand Down
40 changes: 30 additions & 10 deletions netbox/users/tests/test_api.py
arthanson marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -38,26 +38,26 @@ def setUpTestData(cls):
permissions[2].object_types.add(ObjectType.objects.get_by_natural_key('dcim', 'rack'))

users = (
User(username='User1', password='password1'),
User(username='User2', password='password2'),
User(username='User3', password='password3'),
User(username='User1', password='FooBarFooBar1'),
User(username='User2', password='FooBarFooBar2'),
User(username='User3', password='FooBarFooBar3'),
)
User.objects.bulk_create(users)

cls.create_data = [
{
'username': 'User4',
'password': 'password4',
'password': 'FooBarFooBar4',
'permissions': [permissions[0].pk],
},
{
'username': 'User5',
'password': 'password5',
'password': 'FooBarFooBar5',
'permissions': [permissions[1].pk],
},
{
'username': 'User6',
'password': 'password6',
'password': 'FooBarFooBar6',
'permissions': [permissions[2].pk],
},
]
Expand All @@ -77,12 +77,12 @@ def test_that_password_is_changed(self):

user_credentials = {
'username': 'newuser',
'password': 'abc123',
'password': 'abc123FOO',
}
user = User.objects.create_user(**user_credentials)

data = {
'password': 'newpassword'
'password': 'FooBarFooBar1'
}
url = reverse('users-api:user-detail', kwargs={'pk': user.id})
response = self.client.patch(url, data, format='json', **self.header)
Expand All @@ -102,7 +102,7 @@ def test_password_validation_enforced(self):

data = {
'username': 'new_user',
'password': 'foo',
'password': 'f1A',
}
url = reverse('users-api:user-list')

Expand All @@ -111,10 +111,30 @@ def test_password_validation_enforced(self):
self.assertEqual(response.status_code, 400)

# Password long enough
data['password'] = 'foobar123'
data['password'] = 'FooBar123'
response = self.client.post(url, data, format='json', **self.header)
self.assertEqual(response.status_code, 201)

# Password no number
data['password'] = 'foobarFoo'
response = self.client.post(url, data, format='json', **self.header)
self.assertEqual(response.status_code, 400)

# Password no letter
data['password'] = '123456789012'
response = self.client.post(url, data, format='json', **self.header)
self.assertEqual(response.status_code, 400)

# Password no uppercase
data['password'] = 'foobarfoo1'
response = self.client.post(url, data, format='json', **self.header)
self.assertEqual(response.status_code, 400)

# Password no lowercase
data['password'] = 'FOOBARFOO1'
response = self.client.post(url, data, format='json', **self.header)
self.assertEqual(response.status_code, 400)
arthanson marked this conversation as resolved.
Show resolved Hide resolved


class GroupTest(APIViewTestCases.APIViewTestCase):
model = Group
Expand Down
36 changes: 26 additions & 10 deletions netbox/users/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ def setUpTestData(cls):
'first_name': 'firstx',
'last_name': 'lastx',
'email': '[email protected]',
'password': 'pass1xxx',
'confirm_password': 'pass1xxx',
'password': 'pass1xxxABCD',
'confirm_password': 'pass1xxxABCD',
}

cls.csv_data = (
Expand All @@ -60,19 +60,15 @@ def setUpTestData(cls):
'last_name': 'newlastname',
}

@override_settings(AUTH_PASSWORD_VALIDATORS=[{
'NAME': 'django.contrib.auth.password_validation.MinimumLengthValidator',
'OPTIONS': {'min_length': 8}
}])
def test_password_validation_enforced(self):
"""
Test that any configured password validation rules (AUTH_PASSWORD_VALIDATORS) are enforced.
"""
self.add_permissions('users.add_user')
data = {
'username': 'new_user',
'password': 'foo',
'confirm_password': 'foo',
'password': 'F1a',
'confirm_password': 'F1a',
}

# Password too short
Expand All @@ -84,10 +80,30 @@ def test_password_validation_enforced(self):
self.assertHttpStatus(response, 200)

# Password long enough
data['password'] = 'foobar123'
data['confirm_password'] = 'foobar123'
data['password'] = 'fooBarFoo123'
data['confirm_password'] = 'fooBarFoo123'
self.assertHttpStatus(self.client.post(**request), 302)

# Password no number
data['password'] = 'FooBarFooBar'
data['confirm_password'] = 'FooBarFooBar'
self.assertHttpStatus(self.client.post(**request), 200)

# Password no letter
data['password'] = '123456789123'
data['confirm_password'] = '123456789123'
self.assertHttpStatus(self.client.post(**request), 200)

# Password no uppercase
data['password'] = 'foobar123abc'
data['confirm_password'] = 'foobar123abc'
self.assertHttpStatus(self.client.post(**request), 200)

# Password no lowercase
data['password'] = 'FOOBAR123ABC'
data['confirm_password'] = 'FOOBAR123ABC'
self.assertHttpStatus(self.client.post(**request), 200)
arthanson marked this conversation as resolved.
Show resolved Hide resolved


class GroupTestCase(
ViewTestCases.GetObjectViewTestCase,
Expand Down
27 changes: 27 additions & 0 deletions netbox/utilities/password_validation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
from django.core.exceptions import ValidationError
from django.utils.translation import gettext as _


class AlphanumericPasswordValidator:
"""
Validate that the password has at least one numeral, one uppercase letter and one lowercase letter.
"""

def validate(self, password, user=None):
if not any(char.isdigit() for char in password):
raise ValidationError(
_("Password must have at least one numeral."),
)

if not any(char.isupper() for char in password):
raise ValidationError(
_("Password must have at least one uppercase letter."),
)

if not any(char.islower() for char in password):
raise ValidationError(
_("Password must have at least one lowercase letter."),
)

def get_help_text(self):
return _("Your password must contain at least one numeral, one uppercase letter and one lowercase letter.")