From 8552eef95e400d5bad3261b28ad2500b57070d57 Mon Sep 17 00:00:00 2001
From: nessita <124304+nessita@users.noreply.github.com>
Date: Sat, 1 Feb 2025 22:49:07 -0300
Subject: [PATCH] [5.1.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.
---
django/contrib/auth/forms.py | 53 ++++++++++++++++----------
docs/releases/5.1.6.txt | 4 ++
tests/auth_tests/test_forms.py | 68 +++++++++++++++++++++++++++++++++-
3 files changed, 103 insertions(+), 22 deletions(-)
diff --git a/django/contrib/auth/forms.py b/django/contrib/auth/forms.py
index edf672a6e5..3664673a5a 100644
--- a/django/contrib/auth/forms.py
+++ b/django/contrib/auth/forms.py
@@ -108,14 +108,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."),
@@ -130,20 +130,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"],
@@ -190,19 +176,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"]:
@@ -569,6 +575,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
@@ -595,3 +603,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 9463f07d57..c4a33bbd62 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):
@@ -1665,3 +1682,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 = """
+ | password1 |
+ '********************' |
+ """
+ password2_fragment = """
+ password2 |
+ '********************' |
+ """
+ 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
+ )