Fixed #35477 -- Corrected 'required' errors in auth password set/change forms.

The auth forms using SetPasswordMixin were incorrectly including the
'This field is required.' error when additional validations (e.g.,
overriding `clean_password1`) were performed and failed.
This fix ensures accurate error reporting for password fields.

Co-authored-by: Natalia <124304+nessita@users.noreply.github.com>
This commit is contained in:
Fabian Braun 2024-05-28 08:15:12 +02:00 committed by nessita
parent 0f694ce2eb
commit 339977d444
2 changed files with 71 additions and 2 deletions

View File

@ -154,14 +154,14 @@ class SetPasswordMixin:
if not usable_password:
return self.cleaned_data
if not password1:
if not password1 and password1_field_name not in self.errors:
error = ValidationError(
self.fields[password1_field_name].error_messages["required"],
code="required",
)
self.add_error(password1_field_name, error)
if not password2:
if not password2 and password2_field_name not in self.errors:
error = ValidationError(
self.fields[password2_field_name].error_messages["required"],
code="required",

View File

@ -60,6 +60,21 @@ class TestDataMixin:
)
class ExtraValidationFormMixin:
def __init__(self, *args, failing_fields=None, **kwargs):
super().__init__(*args, **kwargs)
self.failing_fields = failing_fields or {}
def failing_helper(self, field_name):
if field_name in self.failing_fields:
errors = [
ValidationError(error, code="invalid")
for error in self.failing_fields[field_name]
]
raise ValidationError(errors)
return self.cleaned_data[field_name]
class BaseUserCreationFormTest(TestDataMixin, TestCase):
def test_user_already_exists(self):
data = {
@ -324,6 +339,22 @@ class BaseUserCreationFormTest(TestDataMixin, TestCase):
"</li></ul>",
)
def test_password_extra_validations(self):
class ExtraValidationForm(ExtraValidationFormMixin, BaseUserCreationForm):
def clean_password1(self):
return self.failing_helper("password1")
def clean_password2(self):
return self.failing_helper("password2")
data = {"username": "extra", "password1": "abc", "password2": "abc"}
for fields in (["password1"], ["password2"], ["password1", "password2"]):
with self.subTest(fields=fields):
errors = {field: [f"Extra validation for {field}."] for field in fields}
form = ExtraValidationForm(data, failing_fields=errors)
self.assertIs(form.is_valid(), False)
self.assertDictEqual(form.errors, errors)
@override_settings(
AUTH_PASSWORD_VALIDATORS=[
{
@ -865,6 +896,27 @@ class SetPasswordFormTest(TestDataMixin, TestCase):
form.fields[field_name].widget.attrs["autocomplete"], autocomplete
)
def test_password_extra_validations(self):
class ExtraValidationForm(ExtraValidationFormMixin, SetPasswordForm):
def clean_new_password1(self):
return self.failing_helper("new_password1")
def clean_new_password2(self):
return self.failing_helper("new_password2")
user = User.objects.get(username="testclient")
data = {"new_password1": "abc", "new_password2": "abc"}
for fields in (
["new_password1"],
["new_password2"],
["new_password1", "new_password2"],
):
with self.subTest(fields=fields):
errors = {field: [f"Extra validation for {field}."] for field in fields}
form = ExtraValidationForm(user, data, failing_fields=errors)
self.assertIs(form.is_valid(), False)
self.assertDictEqual(form.errors, errors)
class PasswordChangeFormTest(TestDataMixin, TestCase):
def test_incorrect_password(self):
@ -1456,6 +1508,23 @@ class AdminPasswordChangeFormTest(TestDataMixin, TestCase):
self.assertEqual(form.cleaned_data["password2"], data["password2"])
self.assertEqual(form.changed_data, ["password"])
def test_password_extra_validations(self):
class ExtraValidationForm(ExtraValidationFormMixin, AdminPasswordChangeForm):
def clean_password1(self):
return self.failing_helper("password1")
def clean_password2(self):
return self.failing_helper("password2")
user = User.objects.get(username="testclient")
data = {"username": "extra", "password1": "abc", "password2": "abc"}
for fields in (["password1"], ["password2"], ["password1", "password2"]):
with self.subTest(fields=fields):
errors = {field: [f"Extra validation for {field}."] for field in fields}
form = ExtraValidationForm(user, data, failing_fields=errors)
self.assertIs(form.is_valid(), False)
self.assertDictEqual(form.errors, errors)
def test_non_matching_passwords(self):
user = User.objects.get(username="testclient")
data = {"password1": "password1", "password2": "password2"}