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 <[email protected]> * Update netbox/utilities/password_validation.py Co-authored-by: Jeremy Stretch <[email protected]> * Update netbox/utilities/password_validation.py Co-authored-by: Jeremy Stretch <[email protected]> * 17289 update tests * 17289 remove common password check * 17289 fix user create * 17289 revert _post_clean --------- Co-authored-by: Jeremy Stretch <[email protected]>
netbox-community · Aug 30, 2024 · 00874ac · 00874ac
1 parent b4dd57f
commit 00874ac
Show file tree

Hide file tree

Showing 4 changed files with 94 additions and 22 deletions.
diff --git a/netbox/netbox/settings.py b/netbox/netbox/settings.py
@@ -21,7 +21,6 @@
 from utilities.release import load_release_data
 from utilities.string import trailing_slash
 
-
 #
 # Environment setup
 #
@@ -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)

diff --git a/netbox/users/tests/test_api.py b/netbox/users/tests/test_api.py
@@ -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],
             },
         ]
@@ -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)
@@ -102,7 +102,7 @@ def test_password_validation_enforced(self):
 
         data = {
             'username': 'new_user',
-            'password': 'foo',
+            'password': 'f1A',
         }
         url = reverse('users-api:user-list')
 
@@ -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)
+
 
 class GroupTest(APIViewTestCases.APIViewTestCase):
     model = Group

diff --git a/netbox/users/tests/test_views.py b/netbox/users/tests/test_views.py
@@ -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 = (
@@ -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
@@ -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)
+
 
 class GroupTestCase(
     ViewTestCases.GetObjectViewTestCase,

diff --git a/netbox/utilities/password_validation.py b/netbox/utilities/password_validation.py
@@ -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.")