summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authornessita <124304+nessita@users.noreply.github.com>2025-02-01 22:49:07 -0300
committerNatalia <124304+nessita@users.noreply.github.com>2025-02-01 22:50:26 -0300
commitaffad13d0c56184e2089cd7e8ecd80dd4217f6c4 (patch)
treedc9496afe0bfa2efb0fb9557756dd658e4ed780b
parente939cffa504837d90cf2958306f57649ee6a8323 (diff)
[5.2.x] Fixed #36140 -- Allowed BaseUserCreationForm to define non required password fields.
Regression in e626716c28b6286f8cf0f8174077f3d2244f3eb3. Thanks buffgecko12 for the report and Sarah Boyce for the review. Backport of d15454a6e84a595ffc8dc1b926282f484f782a8f from main.
-rw-r--r--django/contrib/auth/forms.py53
-rw-r--r--docs/releases/5.1.6.txt4
-rw-r--r--tests/auth_tests/test_forms.py68
3 files changed, 103 insertions, 22 deletions
diff --git a/django/contrib/auth/forms.py b/django/contrib/auth/forms.py
index cd177fa5b6..cd02887027 100644
--- a/django/contrib/auth/forms.py
+++ b/django/contrib/auth/forms.py
@@ -109,14 +109,14 @@ class SetPasswordMixin:
def create_password_fields(label1=_("Password"), label2=_("Password confirmation")):
password1 = forms.CharField(
label=label1,
- required=False,
+ required=True,
strip=False,
widget=forms.PasswordInput(attrs={"autocomplete": "new-password"}),
help_text=password_validation.password_validators_help_text_html(),
)
password2 = forms.CharField(
label=label2,
- required=False,
+ required=True,
widget=forms.PasswordInput(attrs={"autocomplete": "new-password"}),
strip=False,
help_text=_("Enter the same password as before, for verification."),
@@ -132,20 +132,6 @@ class SetPasswordMixin:
password1 = self.cleaned_data.get(password1_field_name)
password2 = self.cleaned_data.get(password2_field_name)
- 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 and password2_field_name not in self.errors:
- error = ValidationError(
- self.fields[password2_field_name].error_messages["required"],
- code="required",
- )
- self.add_error(password2_field_name, error)
-
if password1 and password2 and password1 != password2:
error = ValidationError(
self.error_messages["password_mismatch"],
@@ -193,19 +179,39 @@ class SetUnusablePasswordMixin:
help_text=help_text,
)
+ @sensitive_variables("password1", "password2")
def validate_passwords(
self,
- *args,
+ password1_field_name="password1",
+ password2_field_name="password2",
usable_password_field_name="usable_password",
- **kwargs,
):
usable_password = (
self.cleaned_data.pop(usable_password_field_name, None) != "false"
)
self.cleaned_data["set_usable_password"] = usable_password
- if usable_password:
- super().validate_passwords(*args, **kwargs)
+ if not usable_password:
+ return
+
+ password1 = self.cleaned_data.get(password1_field_name)
+ password2 = self.cleaned_data.get(password2_field_name)
+
+ 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 and password2_field_name not in self.errors:
+ error = ValidationError(
+ self.fields[password2_field_name].error_messages["required"],
+ code="required",
+ )
+ self.add_error(password2_field_name, error)
+
+ super().validate_passwords(password1_field_name, password2_field_name)
def validate_password_for_user(self, user, **kwargs):
if self.cleaned_data["set_usable_password"]:
@@ -575,6 +581,8 @@ class AdminPasswordChangeForm(SetUnusablePasswordMixin, SetPasswordMixin, forms.
super().__init__(*args, **kwargs)
self.fields["password1"].widget.attrs["autofocus"] = True
if self.user.has_usable_password():
+ self.fields["password1"].required = False
+ self.fields["password2"].required = False
self.fields["usable_password"] = (
SetUnusablePasswordMixin.create_usable_password_field(
self.usable_password_help_text
@@ -601,3 +609,8 @@ class AdminPasswordChangeForm(SetUnusablePasswordMixin, SetPasswordMixin, forms.
class AdminUserCreationForm(SetUnusablePasswordMixin, UserCreationForm):
usable_password = SetUnusablePasswordMixin.create_usable_password_field()
+
+ def __init__(self, *args, **kwargs):
+ super().__init__(*args, **kwargs)
+ self.fields["password1"].required = False
+ self.fields["password2"].required = False
diff --git a/docs/releases/5.1.6.txt b/docs/releases/5.1.6.txt
index 3b23192033..10fd9aca8b 100644
--- a/docs/releases/5.1.6.txt
+++ b/docs/releases/5.1.6.txt
@@ -12,3 +12,7 @@ Bugfixes
* Fixed a regression in Django 5.1.5 that caused ``validate_ipv6_address()``
and ``validate_ipv46_address()`` to crash when handling non-string values
(:ticket:`36098`).
+
+* Fixed a regression in Django 5.1 where password fields, despite being set to
+ ``required=False``, were still treated as required in forms derived from
+ :class:`~django.contrib.auth.forms.BaseUserCreationForm` (:ticket:`36140`).
diff --git a/tests/auth_tests/test_forms.py b/tests/auth_tests/test_forms.py
index 5d81d8f7fd..4740cb6200 100644
--- a/tests/auth_tests/test_forms.py
+++ b/tests/auth_tests/test_forms.py
@@ -1,8 +1,10 @@
import datetime
import re
+import sys
import urllib.parse
from unittest import mock
+from django import forms
from django.contrib.auth.forms import (
AdminPasswordChangeForm,
AdminUserCreationForm,
@@ -13,6 +15,7 @@ from django.contrib.auth.forms import (
ReadOnlyPasswordHashField,
ReadOnlyPasswordHashWidget,
SetPasswordForm,
+ SetPasswordMixin,
UserChangeForm,
UserCreationForm,
UsernameField,
@@ -24,13 +27,14 @@ from django.contrib.sites.models import Site
from django.core import mail
from django.core.exceptions import ValidationError
from django.core.mail import EmailMultiAlternatives
-from django.forms import forms
from django.forms.fields import CharField, Field, IntegerField
-from django.test import SimpleTestCase, TestCase, override_settings
+from django.test import RequestFactory, SimpleTestCase, TestCase, override_settings
from django.urls import reverse
from django.utils import translation
from django.utils.text import capfirst
from django.utils.translation import gettext as _
+from django.views.debug import technical_500_response
+from django.views.decorators.debug import sensitive_variables
from .models.custom_user import (
CustomUser,
@@ -412,6 +416,19 @@ class CustomUserCreationFormTest(TestDataMixin, TestCase):
user = form.save(commit=True)
self.assertSequenceEqual(user.orgs.all(), [organization])
+ def test_custom_form_with_non_required_password(self):
+ class CustomUserCreationForm(BaseUserCreationForm):
+ password1 = forms.CharField(required=False)
+ password2 = forms.CharField(required=False)
+ another_field = forms.CharField(required=True)
+
+ data = {
+ "username": "testclientnew",
+ "another_field": "Content",
+ }
+ form = CustomUserCreationForm(data)
+ self.assertIs(form.is_valid(), True, form.errors)
+
class UserCreationFormTest(BaseUserCreationFormTest):
@@ -1671,3 +1688,50 @@ class AdminUserCreationFormTest(BaseUserCreationFormTest):
u = form.save()
self.assertEqual(u.username, data["username"])
self.assertFalse(u.has_usable_password())
+
+
+class SensitiveVariablesTest(TestDataMixin, TestCase):
+ @sensitive_variables("data")
+ def test_passwords_marked_as_sensitive_in_admin_forms(self):
+ data = {
+ "password1": "passwordsensitive",
+ "password2": "sensitivepassword",
+ "usable_password": "true",
+ }
+ forms = [
+ AdminUserCreationForm({**data, "username": "newusername"}),
+ AdminPasswordChangeForm(self.u1, data),
+ ]
+
+ password1_fragment = """
+ <td>password1</td>
+ <td class="code"><pre>&#x27;********************&#x27;</pre></td>
+ """
+ password2_fragment = """
+ <td>password2</td>
+ <td class="code"><pre>&#x27;********************&#x27;</pre></td>
+ """
+ error = ValueError("Forced error")
+ for form in forms:
+ with self.subTest(form=form):
+ with mock.patch.object(
+ SetPasswordMixin, "validate_passwords", side_effect=error
+ ):
+ try:
+ form.is_valid()
+ except ValueError:
+ exc_info = sys.exc_info()
+ else:
+ self.fail("Form validation should have failed.")
+
+ response = technical_500_response(RequestFactory().get("/"), *exc_info)
+
+ self.assertNotContains(response, "sensitivepassword", status_code=500)
+ self.assertNotContains(response, "passwordsensitive", status_code=500)
+ self.assertContains(response, str(error), status_code=500)
+ self.assertContains(
+ response, password1_fragment, html=True, status_code=500
+ )
+ self.assertContains(
+ response, password2_fragment, html=True, status_code=500
+ )