Skip to content

Commit 8552eef

Browse files
committed
[5.1.x] Fixed #36140 -- Allowed BaseUserCreationForm to define non required password fields.
Regression in e626716. Thanks buffgecko12 for the report and Sarah Boyce for the review. Backport of d15454a from main.
1 parent 76b4fb7 commit 8552eef

File tree

3 files changed

+103
-22
lines changed

3 files changed

+103
-22
lines changed

django/contrib/auth/forms.py

Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -108,14 +108,14 @@ class SetPasswordMixin:
108108
def create_password_fields(label1=_("Password"), label2=_("Password confirmation")):
109109
password1 = forms.CharField(
110110
label=label1,
111-
required=False,
111+
required=True,
112112
strip=False,
113113
widget=forms.PasswordInput(attrs={"autocomplete": "new-password"}),
114114
help_text=password_validation.password_validators_help_text_html(),
115115
)
116116
password2 = forms.CharField(
117117
label=label2,
118-
required=False,
118+
required=True,
119119
widget=forms.PasswordInput(attrs={"autocomplete": "new-password"}),
120120
strip=False,
121121
help_text=_("Enter the same password as before, for verification."),
@@ -130,20 +130,6 @@ def validate_passwords(
130130
password1 = self.cleaned_data.get(password1_field_name)
131131
password2 = self.cleaned_data.get(password2_field_name)
132132

133-
if not password1 and password1_field_name not in self.errors:
134-
error = ValidationError(
135-
self.fields[password1_field_name].error_messages["required"],
136-
code="required",
137-
)
138-
self.add_error(password1_field_name, error)
139-
140-
if not password2 and password2_field_name not in self.errors:
141-
error = ValidationError(
142-
self.fields[password2_field_name].error_messages["required"],
143-
code="required",
144-
)
145-
self.add_error(password2_field_name, error)
146-
147133
if password1 and password2 and password1 != password2:
148134
error = ValidationError(
149135
self.error_messages["password_mismatch"],
@@ -190,19 +176,39 @@ def create_usable_password_field(help_text=usable_password_help_text):
190176
help_text=help_text,
191177
)
192178

179+
@sensitive_variables("password1", "password2")
193180
def validate_passwords(
194181
self,
195-
*args,
182+
password1_field_name="password1",
183+
password2_field_name="password2",
196184
usable_password_field_name="usable_password",
197-
**kwargs,
198185
):
199186
usable_password = (
200187
self.cleaned_data.pop(usable_password_field_name, None) != "false"
201188
)
202189
self.cleaned_data["set_usable_password"] = usable_password
203190

204-
if usable_password:
205-
super().validate_passwords(*args, **kwargs)
191+
if not usable_password:
192+
return
193+
194+
password1 = self.cleaned_data.get(password1_field_name)
195+
password2 = self.cleaned_data.get(password2_field_name)
196+
197+
if not password1 and password1_field_name not in self.errors:
198+
error = ValidationError(
199+
self.fields[password1_field_name].error_messages["required"],
200+
code="required",
201+
)
202+
self.add_error(password1_field_name, error)
203+
204+
if not password2 and password2_field_name not in self.errors:
205+
error = ValidationError(
206+
self.fields[password2_field_name].error_messages["required"],
207+
code="required",
208+
)
209+
self.add_error(password2_field_name, error)
210+
211+
super().validate_passwords(password1_field_name, password2_field_name)
206212

207213
def validate_password_for_user(self, user, **kwargs):
208214
if self.cleaned_data["set_usable_password"]:
@@ -569,6 +575,8 @@ def __init__(self, user, *args, **kwargs):
569575
super().__init__(*args, **kwargs)
570576
self.fields["password1"].widget.attrs["autofocus"] = True
571577
if self.user.has_usable_password():
578+
self.fields["password1"].required = False
579+
self.fields["password2"].required = False
572580
self.fields["usable_password"] = (
573581
SetUnusablePasswordMixin.create_usable_password_field(
574582
self.usable_password_help_text
@@ -595,3 +603,8 @@ def changed_data(self):
595603
class AdminUserCreationForm(SetUnusablePasswordMixin, UserCreationForm):
596604

597605
usable_password = SetUnusablePasswordMixin.create_usable_password_field()
606+
607+
def __init__(self, *args, **kwargs):
608+
super().__init__(*args, **kwargs)
609+
self.fields["password1"].required = False
610+
self.fields["password2"].required = False

docs/releases/5.1.6.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,7 @@ Bugfixes
1212
* Fixed a regression in Django 5.1.5 that caused ``validate_ipv6_address()``
1313
and ``validate_ipv46_address()`` to crash when handling non-string values
1414
(:ticket:`36098`).
15+
16+
* Fixed a regression in Django 5.1 where password fields, despite being set to
17+
``required=False``, were still treated as required in forms derived from
18+
:class:`~django.contrib.auth.forms.BaseUserCreationForm` (:ticket:`36140`).

tests/auth_tests/test_forms.py

Lines changed: 66 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import datetime
22
import re
3+
import sys
34
import urllib.parse
45
from unittest import mock
56

7+
from django import forms
68
from django.contrib.auth.forms import (
79
AdminPasswordChangeForm,
810
AdminUserCreationForm,
@@ -13,6 +15,7 @@
1315
ReadOnlyPasswordHashField,
1416
ReadOnlyPasswordHashWidget,
1517
SetPasswordForm,
18+
SetPasswordMixin,
1619
UserChangeForm,
1720
UserCreationForm,
1821
UsernameField,
@@ -24,13 +27,14 @@
2427
from django.core import mail
2528
from django.core.exceptions import ValidationError
2629
from django.core.mail import EmailMultiAlternatives
27-
from django.forms import forms
2830
from django.forms.fields import CharField, Field, IntegerField
29-
from django.test import SimpleTestCase, TestCase, override_settings
31+
from django.test import RequestFactory, SimpleTestCase, TestCase, override_settings
3032
from django.urls import reverse
3133
from django.utils import translation
3234
from django.utils.text import capfirst
3335
from django.utils.translation import gettext as _
36+
from django.views.debug import technical_500_response
37+
from django.views.decorators.debug import sensitive_variables
3438

3539
from .models.custom_user import (
3640
CustomUser,
@@ -412,6 +416,19 @@ class Meta(BaseUserCreationForm.Meta):
412416
user = form.save(commit=True)
413417
self.assertSequenceEqual(user.orgs.all(), [organization])
414418

419+
def test_custom_form_with_non_required_password(self):
420+
class CustomUserCreationForm(BaseUserCreationForm):
421+
password1 = forms.CharField(required=False)
422+
password2 = forms.CharField(required=False)
423+
another_field = forms.CharField(required=True)
424+
425+
data = {
426+
"username": "testclientnew",
427+
"another_field": "Content",
428+
}
429+
form = CustomUserCreationForm(data)
430+
self.assertIs(form.is_valid(), True, form.errors)
431+
415432

416433
class UserCreationFormTest(BaseUserCreationFormTest):
417434

@@ -1665,3 +1682,50 @@ def test_unusable_password(self):
16651682
u = form.save()
16661683
self.assertEqual(u.username, data["username"])
16671684
self.assertFalse(u.has_usable_password())
1685+
1686+
1687+
class SensitiveVariablesTest(TestDataMixin, TestCase):
1688+
@sensitive_variables("data")
1689+
def test_passwords_marked_as_sensitive_in_admin_forms(self):
1690+
data = {
1691+
"password1": "passwordsensitive",
1692+
"password2": "sensitivepassword",
1693+
"usable_password": "true",
1694+
}
1695+
forms = [
1696+
AdminUserCreationForm({**data, "username": "newusername"}),
1697+
AdminPasswordChangeForm(self.u1, data),
1698+
]
1699+
1700+
password1_fragment = """
1701+
<td>password1</td>
1702+
<td class="code"><pre>&#x27;********************&#x27;</pre></td>
1703+
"""
1704+
password2_fragment = """
1705+
<td>password2</td>
1706+
<td class="code"><pre>&#x27;********************&#x27;</pre></td>
1707+
"""
1708+
error = ValueError("Forced error")
1709+
for form in forms:
1710+
with self.subTest(form=form):
1711+
with mock.patch.object(
1712+
SetPasswordMixin, "validate_passwords", side_effect=error
1713+
):
1714+
try:
1715+
form.is_valid()
1716+
except ValueError:
1717+
exc_info = sys.exc_info()
1718+
else:
1719+
self.fail("Form validation should have failed.")
1720+
1721+
response = technical_500_response(RequestFactory().get("/"), *exc_info)
1722+
1723+
self.assertNotContains(response, "sensitivepassword", status_code=500)
1724+
self.assertNotContains(response, "passwordsensitive", status_code=500)
1725+
self.assertContains(response, str(error), status_code=500)
1726+
self.assertContains(
1727+
response, password1_fragment, html=True, status_code=500
1728+
)
1729+
self.assertContains(
1730+
response, password2_fragment, html=True, status_code=500
1731+
)

0 commit comments

Comments
 (0)