Fixed #36140 -- Allowed BaseUserCreationForm to define non required password fields.

Regression in e626716c28b6286f8cf0f8174077f3d2244f3eb3.

Thanks buffgecko12 for the report and Sarah Boyce for the review.

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

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`).

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
+                )