17289 enforce minimum password strength (#17299)

* 17289 add password validation

* 17289 add password validation

* 17289 fix tests

* 17289 fix tests

* Update netbox/utilities/password_validation.py

Co-authored-by: Jeremy Stretch <jstretch@netboxlabs.com>

* Update netbox/utilities/password_validation.py

Co-authored-by: Jeremy Stretch <jstretch@netboxlabs.com>

* Update netbox/utilities/password_validation.py

Co-authored-by: Jeremy Stretch <jstretch@netboxlabs.com>

* 17289 update tests

* 17289 remove common password check

* 17289 fix user create

* 17289 revert _post_clean

---------

Co-authored-by: Jeremy Stretch <jstretch@netboxlabs.com>
This commit is contained in:
Arthur Hanson 2024-08-30 14:23:01 -07:00 committed by GitHub
parent b4dd57f3c7
commit 00874ac9e7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 94 additions and 22 deletions

View File

@ -21,7 +21,6 @@ from netbox.plugins import PluginConfig
from utilities.release import load_release_data from utilities.release import load_release_data
from utilities.string import trailing_slash from utilities.string import trailing_slash
# #
# Environment setup # Environment setup
# #
@ -63,7 +62,17 @@ for parameter in ('ALLOWED_HOSTS', 'DATABASE', 'SECRET_KEY', 'REDIS'):
ADMINS = getattr(configuration, 'ADMINS', []) ADMINS = getattr(configuration, 'ADMINS', [])
ALLOW_TOKEN_RETRIEVAL = getattr(configuration, 'ALLOW_TOKEN_RETRIEVAL', True) ALLOW_TOKEN_RETRIEVAL = getattr(configuration, 'ALLOW_TOKEN_RETRIEVAL', True)
ALLOWED_HOSTS = getattr(configuration, 'ALLOWED_HOSTS') # Required 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', '')) BASE_PATH = trailing_slash(getattr(configuration, 'BASE_PATH', ''))
CHANGELOG_SKIP_EMPTY_CHANGES = getattr(configuration, 'CHANGELOG_SKIP_EMPTY_CHANGES', True) CHANGELOG_SKIP_EMPTY_CHANGES = getattr(configuration, 'CHANGELOG_SKIP_EMPTY_CHANGES', True)
CENSUS_REPORTING_ENABLED = getattr(configuration, 'CENSUS_REPORTING_ENABLED', True) CENSUS_REPORTING_ENABLED = getattr(configuration, 'CENSUS_REPORTING_ENABLED', True)

View File

@ -38,26 +38,26 @@ class UserTest(APIViewTestCases.APIViewTestCase):
permissions[2].object_types.add(ObjectType.objects.get_by_natural_key('dcim', 'rack')) permissions[2].object_types.add(ObjectType.objects.get_by_natural_key('dcim', 'rack'))
users = ( users = (
User(username='User1', password='password1'), User(username='User1', password='FooBarFooBar1'),
User(username='User2', password='password2'), User(username='User2', password='FooBarFooBar2'),
User(username='User3', password='password3'), User(username='User3', password='FooBarFooBar3'),
) )
User.objects.bulk_create(users) User.objects.bulk_create(users)
cls.create_data = [ cls.create_data = [
{ {
'username': 'User4', 'username': 'User4',
'password': 'password4', 'password': 'FooBarFooBar4',
'permissions': [permissions[0].pk], 'permissions': [permissions[0].pk],
}, },
{ {
'username': 'User5', 'username': 'User5',
'password': 'password5', 'password': 'FooBarFooBar5',
'permissions': [permissions[1].pk], 'permissions': [permissions[1].pk],
}, },
{ {
'username': 'User6', 'username': 'User6',
'password': 'password6', 'password': 'FooBarFooBar6',
'permissions': [permissions[2].pk], 'permissions': [permissions[2].pk],
}, },
] ]
@ -77,12 +77,12 @@ class UserTest(APIViewTestCases.APIViewTestCase):
user_credentials = { user_credentials = {
'username': 'newuser', 'username': 'newuser',
'password': 'abc123', 'password': 'abc123FOO',
} }
user = User.objects.create_user(**user_credentials) user = User.objects.create_user(**user_credentials)
data = { data = {
'password': 'newpassword' 'password': 'FooBarFooBar1'
} }
url = reverse('users-api:user-detail', kwargs={'pk': user.id}) url = reverse('users-api:user-detail', kwargs={'pk': user.id})
response = self.client.patch(url, data, format='json', **self.header) response = self.client.patch(url, data, format='json', **self.header)
@ -102,7 +102,7 @@ class UserTest(APIViewTestCases.APIViewTestCase):
data = { data = {
'username': 'new_user', 'username': 'new_user',
'password': 'foo', 'password': 'f1A',
} }
url = reverse('users-api:user-list') url = reverse('users-api:user-list')
@ -111,10 +111,30 @@ class UserTest(APIViewTestCases.APIViewTestCase):
self.assertEqual(response.status_code, 400) self.assertEqual(response.status_code, 400)
# Password long enough # Password long enough
data['password'] = 'foobar123' data['password'] = 'FooBar123'
response = self.client.post(url, data, format='json', **self.header) response = self.client.post(url, data, format='json', **self.header)
self.assertEqual(response.status_code, 201) 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)
class GroupTest(APIViewTestCases.APIViewTestCase): class GroupTest(APIViewTestCases.APIViewTestCase):
model = Group model = Group

View File

@ -38,8 +38,8 @@ class UserTestCase(
'first_name': 'firstx', 'first_name': 'firstx',
'last_name': 'lastx', 'last_name': 'lastx',
'email': 'userx@foo.com', 'email': 'userx@foo.com',
'password': 'pass1xxx', 'password': 'pass1xxxABCD',
'confirm_password': 'pass1xxx', 'confirm_password': 'pass1xxxABCD',
} }
cls.csv_data = ( cls.csv_data = (
@ -60,10 +60,6 @@ class UserTestCase(
'last_name': 'newlastname', '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): def test_password_validation_enforced(self):
""" """
Test that any configured password validation rules (AUTH_PASSWORD_VALIDATORS) are enforced. Test that any configured password validation rules (AUTH_PASSWORD_VALIDATORS) are enforced.
@ -71,8 +67,8 @@ class UserTestCase(
self.add_permissions('users.add_user') self.add_permissions('users.add_user')
data = { data = {
'username': 'new_user', 'username': 'new_user',
'password': 'foo', 'password': 'F1a',
'confirm_password': 'foo', 'confirm_password': 'F1a',
} }
# Password too short # Password too short
@ -84,10 +80,30 @@ class UserTestCase(
self.assertHttpStatus(response, 200) self.assertHttpStatus(response, 200)
# Password long enough # Password long enough
data['password'] = 'foobar123' data['password'] = 'fooBarFoo123'
data['confirm_password'] = 'foobar123' data['confirm_password'] = 'fooBarFoo123'
self.assertHttpStatus(self.client.post(**request), 302) 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)
class GroupTestCase( class GroupTestCase(
ViewTestCases.GetObjectViewTestCase, ViewTestCases.GetObjectViewTestCase,

View File

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