Skip to content

Fixes #16964: Ensure configured password validators are enforced #16990

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion netbox/users/api/serializers_/users.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from django.contrib.auth import get_user_model
from django.contrib.auth import get_user_model, password_validation
from drf_spectacular.types import OpenApiTypes
from drf_spectacular.utils import extend_schema_field
from rest_framework import serializers
Expand Down Expand Up @@ -61,6 +61,14 @@ class Meta:
'password': {'write_only': True}
}

def validate(self, data):

# Enforce password validation rules (if configured)
if not self.nested and data.get('password'):
password_validation.validate_password(data['password'], self.instance)

return super().validate(data)

def create(self, validated_data):
"""
Extract the password from validated data and set it separately to ensure proper hash generation.
Expand Down
6 changes: 5 additions & 1 deletion netbox/users/forms/model_forms.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from django import forms
from django.conf import settings
from django.contrib.auth import get_user_model
from django.contrib.auth import get_user_model, password_validation
from django.contrib.postgres.forms import SimpleArrayField
from django.core.exceptions import FieldError
from django.utils.safestring import mark_safe
Expand Down Expand Up @@ -227,6 +227,10 @@ def clean(self):
if self.cleaned_data['password'] and self.cleaned_data['password'] != self.cleaned_data['confirm_password']:
raise forms.ValidationError(_("Passwords do not match! Please check your input and try again."))

# Enforce password validation rules (if configured)
if self.cleaned_data['password']:
password_validation.validate_password(self.cleaned_data['password'], self.instance)


class GroupForm(forms.ModelForm):
users = DynamicModelMultipleChoiceField(
Expand Down
26 changes: 26 additions & 0 deletions netbox/users/tests/test_api.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from django.contrib.auth import get_user_model
from django.test import override_settings
from django.urls import reverse

from core.models import ObjectType
Expand Down Expand Up @@ -93,6 +94,31 @@ def test_that_password_is_changed(self):
user.refresh_from_db()
self.assertTrue(user.check_password(data['password']))

@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',
}
url = reverse('users-api:user-list')

# Password too short
response = self.client.post(url, data, format='json', **self.header)
self.assertEqual(response.status_code, 400)

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


class GroupTest(APIViewTestCases.APIViewTestCase):
model = Group
Expand Down
32 changes: 31 additions & 1 deletion netbox/users/tests/test_views.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from django.test import override_settings

from core.models import ObjectType
from users.models import *
from utilities.testing import ViewTestCases, create_test_user
from utilities.testing import ViewTestCases, create_test_user, extract_form_failures


class UserTestCase(
Expand Down Expand Up @@ -58,6 +60,34 @@ 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 too short
request = {
'path': self._get_url('add'),
'data': data,
}
response = self.client.post(**request)
self.assertHttpStatus(response, 200)

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


class GroupTestCase(
ViewTestCases.GetObjectViewTestCase,
Expand Down