Skip to content

Commit

Permalink
Fixed #35477 -- Corrected 'required' errors in auth password set/chan…
Browse files Browse the repository at this point in the history
…ge 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 <[email protected]>
  • Loading branch information
fsbraun and nessita committed May 30, 2024
1 parent 0f694ce commit 339977d
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 2 deletions.
4 changes: 2 additions & 2 deletions django/contrib/auth/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,14 +154,14 @@ def validate_passwords(
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",
Expand Down
69 changes: 69 additions & 0 deletions tests/auth_tests/test_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,21 @@ def setUpTestData(cls):
)


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 = {
Expand Down Expand Up @@ -324,6 +339,22 @@ def test_password_help_text(self):
"</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=[
{
Expand Down Expand Up @@ -865,6 +896,27 @@ def test_html_autocomplete_attributes(self):
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):
Expand Down Expand Up @@ -1456,6 +1508,23 @@ def test_password_whitespace_not_stripped(self):
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"}
Expand Down

0 comments on commit 339977d

Please sign in to comment.