mirror of
				https://github.com/django/django.git
				synced 2025-10-31 01:25:32 +00:00 
			
		
		
		
	[5.2.x] Fixed #36140 -- Allowed BaseUserCreationForm to define non required password fields.
Regression ine626716c28. Thanks buffgecko12 for the report and Sarah Boyce for the review. Backport ofd15454a6e8from main.
This commit is contained in:
		| @@ -109,14 +109,14 @@ class SetPasswordMixin: | |||||||
|     def create_password_fields(label1=_("Password"), label2=_("Password confirmation")): |     def create_password_fields(label1=_("Password"), label2=_("Password confirmation")): | ||||||
|         password1 = forms.CharField( |         password1 = forms.CharField( | ||||||
|             label=label1, |             label=label1, | ||||||
|             required=False, |             required=True, | ||||||
|             strip=False, |             strip=False, | ||||||
|             widget=forms.PasswordInput(attrs={"autocomplete": "new-password"}), |             widget=forms.PasswordInput(attrs={"autocomplete": "new-password"}), | ||||||
|             help_text=password_validation.password_validators_help_text_html(), |             help_text=password_validation.password_validators_help_text_html(), | ||||||
|         ) |         ) | ||||||
|         password2 = forms.CharField( |         password2 = forms.CharField( | ||||||
|             label=label2, |             label=label2, | ||||||
|             required=False, |             required=True, | ||||||
|             widget=forms.PasswordInput(attrs={"autocomplete": "new-password"}), |             widget=forms.PasswordInput(attrs={"autocomplete": "new-password"}), | ||||||
|             strip=False, |             strip=False, | ||||||
|             help_text=_("Enter the same password as before, for verification."), |             help_text=_("Enter the same password as before, for verification."), | ||||||
| @@ -132,20 +132,6 @@ class SetPasswordMixin: | |||||||
|         password1 = self.cleaned_data.get(password1_field_name) |         password1 = self.cleaned_data.get(password1_field_name) | ||||||
|         password2 = self.cleaned_data.get(password2_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: |         if password1 and password2 and password1 != password2: | ||||||
|             error = ValidationError( |             error = ValidationError( | ||||||
|                 self.error_messages["password_mismatch"], |                 self.error_messages["password_mismatch"], | ||||||
| @@ -193,19 +179,39 @@ class SetUnusablePasswordMixin: | |||||||
|             help_text=help_text, |             help_text=help_text, | ||||||
|         ) |         ) | ||||||
|  |  | ||||||
|  |     @sensitive_variables("password1", "password2") | ||||||
|     def validate_passwords( |     def validate_passwords( | ||||||
|         self, |         self, | ||||||
|         *args, |         password1_field_name="password1", | ||||||
|  |         password2_field_name="password2", | ||||||
|         usable_password_field_name="usable_password", |         usable_password_field_name="usable_password", | ||||||
|         **kwargs, |  | ||||||
|     ): |     ): | ||||||
|         usable_password = ( |         usable_password = ( | ||||||
|             self.cleaned_data.pop(usable_password_field_name, None) != "false" |             self.cleaned_data.pop(usable_password_field_name, None) != "false" | ||||||
|         ) |         ) | ||||||
|         self.cleaned_data["set_usable_password"] = usable_password |         self.cleaned_data["set_usable_password"] = usable_password | ||||||
|  |  | ||||||
|         if usable_password: |         if not usable_password: | ||||||
|             super().validate_passwords(*args, **kwargs) |             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): |     def validate_password_for_user(self, user, **kwargs): | ||||||
|         if self.cleaned_data["set_usable_password"]: |         if self.cleaned_data["set_usable_password"]: | ||||||
| @@ -575,6 +581,8 @@ class AdminPasswordChangeForm(SetUnusablePasswordMixin, SetPasswordMixin, forms. | |||||||
|         super().__init__(*args, **kwargs) |         super().__init__(*args, **kwargs) | ||||||
|         self.fields["password1"].widget.attrs["autofocus"] = True |         self.fields["password1"].widget.attrs["autofocus"] = True | ||||||
|         if self.user.has_usable_password(): |         if self.user.has_usable_password(): | ||||||
|  |             self.fields["password1"].required = False | ||||||
|  |             self.fields["password2"].required = False | ||||||
|             self.fields["usable_password"] = ( |             self.fields["usable_password"] = ( | ||||||
|                 SetUnusablePasswordMixin.create_usable_password_field( |                 SetUnusablePasswordMixin.create_usable_password_field( | ||||||
|                     self.usable_password_help_text |                     self.usable_password_help_text | ||||||
| @@ -601,3 +609,8 @@ class AdminPasswordChangeForm(SetUnusablePasswordMixin, SetPasswordMixin, forms. | |||||||
| class AdminUserCreationForm(SetUnusablePasswordMixin, UserCreationForm): | class AdminUserCreationForm(SetUnusablePasswordMixin, UserCreationForm): | ||||||
|  |  | ||||||
|     usable_password = SetUnusablePasswordMixin.create_usable_password_field() |     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 | ||||||
|   | |||||||
| @@ -12,3 +12,7 @@ Bugfixes | |||||||
| * Fixed a regression in Django 5.1.5 that caused ``validate_ipv6_address()`` | * Fixed a regression in Django 5.1.5 that caused ``validate_ipv6_address()`` | ||||||
|   and ``validate_ipv46_address()`` to crash when handling non-string values |   and ``validate_ipv46_address()`` to crash when handling non-string values | ||||||
|   (:ticket:`36098`). |   (: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`). | ||||||
|   | |||||||
| @@ -1,8 +1,10 @@ | |||||||
| import datetime | import datetime | ||||||
| import re | import re | ||||||
|  | import sys | ||||||
| import urllib.parse | import urllib.parse | ||||||
| from unittest import mock | from unittest import mock | ||||||
|  |  | ||||||
|  | from django import forms | ||||||
| from django.contrib.auth.forms import ( | from django.contrib.auth.forms import ( | ||||||
|     AdminPasswordChangeForm, |     AdminPasswordChangeForm, | ||||||
|     AdminUserCreationForm, |     AdminUserCreationForm, | ||||||
| @@ -13,6 +15,7 @@ from django.contrib.auth.forms import ( | |||||||
|     ReadOnlyPasswordHashField, |     ReadOnlyPasswordHashField, | ||||||
|     ReadOnlyPasswordHashWidget, |     ReadOnlyPasswordHashWidget, | ||||||
|     SetPasswordForm, |     SetPasswordForm, | ||||||
|  |     SetPasswordMixin, | ||||||
|     UserChangeForm, |     UserChangeForm, | ||||||
|     UserCreationForm, |     UserCreationForm, | ||||||
|     UsernameField, |     UsernameField, | ||||||
| @@ -24,13 +27,14 @@ from django.contrib.sites.models import Site | |||||||
| from django.core import mail | from django.core import mail | ||||||
| from django.core.exceptions import ValidationError | from django.core.exceptions import ValidationError | ||||||
| from django.core.mail import EmailMultiAlternatives | from django.core.mail import EmailMultiAlternatives | ||||||
| from django.forms import forms |  | ||||||
| from django.forms.fields import CharField, Field, IntegerField | 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.urls import reverse | ||||||
| from django.utils import translation | from django.utils import translation | ||||||
| from django.utils.text import capfirst | from django.utils.text import capfirst | ||||||
| from django.utils.translation import gettext as _ | 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 ( | from .models.custom_user import ( | ||||||
|     CustomUser, |     CustomUser, | ||||||
| @@ -412,6 +416,19 @@ class CustomUserCreationFormTest(TestDataMixin, TestCase): | |||||||
|         user = form.save(commit=True) |         user = form.save(commit=True) | ||||||
|         self.assertSequenceEqual(user.orgs.all(), [organization]) |         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): | class UserCreationFormTest(BaseUserCreationFormTest): | ||||||
|  |  | ||||||
| @@ -1671,3 +1688,50 @@ class AdminUserCreationFormTest(BaseUserCreationFormTest): | |||||||
|         u = form.save() |         u = form.save() | ||||||
|         self.assertEqual(u.username, data["username"]) |         self.assertEqual(u.username, data["username"]) | ||||||
|         self.assertFalse(u.has_usable_password()) |         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>'********************'</pre></td> | ||||||
|  |          """ | ||||||
|  |         password2_fragment = """ | ||||||
|  |          <td>password2</td> | ||||||
|  |          <td class="code"><pre>'********************'</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 | ||||||
|  |                 ) | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user