diff options
| -rw-r--r-- | django/contrib/auth/admin.py | 4 | ||||
| -rw-r--r-- | django/contrib/auth/forms.py | 98 | ||||
| -rw-r--r-- | docs/releases/5.1.1.txt | 6 | ||||
| -rw-r--r-- | docs/releases/5.1.txt | 10 | ||||
| -rw-r--r-- | docs/topics/auth/default.txt | 32 | ||||
| -rw-r--r-- | tests/auth_tests/test_forms.py | 110 |
6 files changed, 173 insertions, 87 deletions
diff --git a/django/contrib/auth/admin.py b/django/contrib/auth/admin.py index 90a53a142c..00d7e79811 100644 --- a/django/contrib/auth/admin.py +++ b/django/contrib/auth/admin.py @@ -5,8 +5,8 @@ from django.contrib.admin.utils import unquote from django.contrib.auth import update_session_auth_hash from django.contrib.auth.forms import ( AdminPasswordChangeForm, + AdminUserCreationForm, UserChangeForm, - UserCreationForm, ) from django.contrib.auth.models import Group, User from django.core.exceptions import PermissionDenied @@ -71,7 +71,7 @@ class UserAdmin(admin.ModelAdmin): ), ) form = UserChangeForm - add_form = UserCreationForm + add_form = AdminUserCreationForm change_password_form = AdminPasswordChangeForm list_display = ("username", "email", "first_name", "last_name", "is_staff") list_filter = ("is_staff", "is_superuser", "is_active", "groups") diff --git a/django/contrib/auth/forms.py b/django/contrib/auth/forms.py index 31e96ff91c..a11668944a 100644 --- a/django/contrib/auth/forms.py +++ b/django/contrib/auth/forms.py @@ -96,18 +96,11 @@ class UsernameField(forms.CharField): class SetPasswordMixin: """ Form mixin that validates and sets a password for a user. - - This mixin also support setting an unusable password for a user. """ error_messages = { "password_mismatch": _("The two password fields didn’t match."), } - usable_password_help_text = _( - "Whether the user will be able to authenticate using a password or not. " - "If disabled, they may still be able to authenticate using other backends, " - "such as Single Sign-On or LDAP." - ) @staticmethod def create_password_fields(label1=_("Password"), label2=_("Password confirmation")): @@ -127,33 +120,14 @@ class SetPasswordMixin: ) return password1, password2 - @staticmethod - def create_usable_password_field(help_text=usable_password_help_text): - return forms.ChoiceField( - label=_("Password-based authentication"), - required=False, - initial="true", - choices={"true": _("Enabled"), "false": _("Disabled")}, - widget=forms.RadioSelect(attrs={"class": "radiolist inline"}), - help_text=help_text, - ) - def validate_passwords( self, password1_field_name="password1", password2_field_name="password2", - usable_password_field_name="usable_password", ): - usable_password = ( - self.cleaned_data.pop(usable_password_field_name, None) != "false" - ) - self.cleaned_data["set_usable_password"] = usable_password password1 = self.cleaned_data.get(password1_field_name) password2 = self.cleaned_data.get(password2_field_name) - if not usable_password: - return self.cleaned_data - if not password1 and password1_field_name not in self.errors: error = ValidationError( self.fields[password1_field_name].error_messages["required"], @@ -177,19 +151,68 @@ class SetPasswordMixin: def validate_password_for_user(self, user, password_field_name="password2"): password = self.cleaned_data.get(password_field_name) - if password and self.cleaned_data["set_usable_password"]: + if password: try: password_validation.validate_password(password, user) except ValidationError as error: self.add_error(password_field_name, error) def set_password_and_save(self, user, password_field_name="password1", commit=True): + user.set_password(self.cleaned_data[password_field_name]) + if commit: + user.save() + return user + + +class SetUnusablePasswordMixin: + """ + Form mixin that allows setting an unusable password for a user. + + This mixin should be used in combination with `SetPasswordMixin`. + """ + + usable_password_help_text = _( + "Whether the user will be able to authenticate using a password or not. " + "If disabled, they may still be able to authenticate using other backends, " + "such as Single Sign-On or LDAP." + ) + + @staticmethod + def create_usable_password_field(help_text=usable_password_help_text): + return forms.ChoiceField( + label=_("Password-based authentication"), + required=False, + initial="true", + choices={"true": _("Enabled"), "false": _("Disabled")}, + widget=forms.RadioSelect(attrs={"class": "radiolist inline"}), + help_text=help_text, + ) + + def validate_passwords( + self, + *args, + 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) + + def validate_password_for_user(self, user, **kwargs): if self.cleaned_data["set_usable_password"]: - user.set_password(self.cleaned_data[password_field_name]) + super().validate_password_for_user(user, **kwargs) + + def set_password_and_save(self, user, commit=True, **kwargs): + if self.cleaned_data["set_usable_password"]: + user = super().set_password_and_save(user, **kwargs, commit=commit) else: user.set_unusable_password() - if commit: - user.save() + if commit: + user.save() return user @@ -197,10 +220,12 @@ class BaseUserCreationForm(SetPasswordMixin, forms.ModelForm): """ A form that creates a user, with no privileges, from the given username and password. + + This is the documented base class for customizing the user creation form. + It should be kept mostly unchanged to ensure consistency and compatibility. """ password1, password2 = SetPasswordMixin.create_password_fields() - usable_password = SetPasswordMixin.create_usable_password_field() class Meta: model = User @@ -520,13 +545,13 @@ class PasswordChangeForm(SetPasswordForm): return old_password -class AdminPasswordChangeForm(SetPasswordMixin, forms.Form): +class AdminPasswordChangeForm(SetUnusablePasswordMixin, SetPasswordMixin, forms.Form): """ A form used to change the password of a user in the admin interface. """ required_css_class = "required" - usable_password_help_text = SetPasswordMixin.usable_password_help_text + ( + usable_password_help_text = SetUnusablePasswordMixin.usable_password_help_text + ( '<ul id="id_unusable_warning" class="messagelist"><li class="warning">' "If disabled, the current password for this user will be lost.</li></ul>" ) @@ -538,7 +563,7 @@ class AdminPasswordChangeForm(SetPasswordMixin, forms.Form): self.fields["password1"].widget.attrs["autofocus"] = True if self.user.has_usable_password(): self.fields["usable_password"] = ( - SetPasswordMixin.create_usable_password_field( + SetUnusablePasswordMixin.create_usable_password_field( self.usable_password_help_text ) ) @@ -558,3 +583,8 @@ class AdminPasswordChangeForm(SetPasswordMixin, forms.Form): if "set_usable_password" in data or "password1" in data and "password2" in data: return ["password"] return [] + + +class AdminUserCreationForm(SetUnusablePasswordMixin, UserCreationForm): + + usable_password = SetUnusablePasswordMixin.create_usable_password_field() diff --git a/docs/releases/5.1.1.txt b/docs/releases/5.1.1.txt index 25c0b4c297..417584b58e 100644 --- a/docs/releases/5.1.1.txt +++ b/docs/releases/5.1.1.txt @@ -12,3 +12,9 @@ Bugfixes * Fixed a regression in Django 5.1 that caused a crash of ``Window()`` when passing an empty sequence to the ``order_by`` parameter, and a crash of ``Prefetch()`` for a sliced queryset without ordering (:ticket:`35665`). + +* Fixed a regression in Django 5.1 where a new ``usable_password`` field was + included in :class:`~django.contrib.auth.forms.BaseUserCreationForm` (and + children). A new :class:`~django.contrib.auth.forms.AdminUserCreationForm` + including this field was added, isolating the feature to the admin where it + was intended (:ticket:`35678`). diff --git a/docs/releases/5.1.txt b/docs/releases/5.1.txt index f47fa8bd3f..bc868fddda 100644 --- a/docs/releases/5.1.txt +++ b/docs/releases/5.1.txt @@ -118,11 +118,11 @@ Minor features * The default ``parallelism`` of the ``ScryptPasswordHasher`` is increased from 1 to 5, to follow OWASP recommendations. -* :class:`~django.contrib.auth.forms.BaseUserCreationForm` and - :class:`~django.contrib.auth.forms.AdminPasswordChangeForm` now support - disabling password-based authentication by setting an unusable password on - form save. This is now available in the admin when visiting the user creation - and password change pages. +* The new :class:`~django.contrib.auth.forms.AdminUserCreationForm` and + the existing :class:`~django.contrib.auth.forms.AdminPasswordChangeForm` now + support disabling password-based authentication by setting an unusable + password on form save. This is now available in the admin when visiting the + user creation and password change pages. * :func:`~.django.contrib.auth.decorators.login_required`, :func:`~.django.contrib.auth.decorators.permission_required`, and diff --git a/docs/topics/auth/default.txt b/docs/topics/auth/default.txt index 10401717cf..d184b979fe 100644 --- a/docs/topics/auth/default.txt +++ b/docs/topics/auth/default.txt @@ -1665,6 +1665,23 @@ provides several built-in forms located in :mod:`django.contrib.auth.forms`: Option to disable (or reenable) password-based authentication was added. +.. class:: AdminUserCreationForm + + .. versionadded:: 5.1.1 + + A form used in the admin interface to create a new user. Inherits from + :class:`UserCreationForm`. + + It includes an additional ``usable_password`` field, enabled by default. If + ``usable_password`` is enabled, it verifies that ``password1`` and + ``password2`` are non empty and match, validates the password using + :func:`~django.contrib.auth.password_validation.validate_password`, and + sets the user's password using + :meth:`~django.contrib.auth.models.User.set_password()`. + If ``usable_password`` is disabled, no password validation is done, and + password-based authentication is disabled for the user by calling + :meth:`~django.contrib.auth.models.User.set_unusable_password()`. + .. class:: AuthenticationForm A form for logging a user in. @@ -1755,21 +1772,12 @@ provides several built-in forms located in :mod:`django.contrib.auth.forms`: A :class:`~django.forms.ModelForm` for creating a new user. This is the recommended base class if you need to customize the user creation form. - It has four fields: ``username`` (from the user model), ``password1``, - ``password2``, and ``usable_password`` (the latter is enabled by default). - If ``usable_password`` is enabled, it verifies that ``password1`` and - ``password2`` are non empty and match, validates the password using + It has three fields: ``username`` (from the user model), ``password1``, + and ``password2``. It verifies that ``password1`` and ``password2`` match, + validates the password using :func:`~django.contrib.auth.password_validation.validate_password`, and sets the user's password using :meth:`~django.contrib.auth.models.User.set_password()`. - If ``usable_password`` is disabled, no password validation is done, and - password-based authentication is disabled for the user by calling - :meth:`~django.contrib.auth.models.User.set_unusable_password()`. - - .. versionchanged:: 5.1 - - Option to create users with disabled password-based authentication was - added. .. class:: UserCreationForm diff --git a/tests/auth_tests/test_forms.py b/tests/auth_tests/test_forms.py index 22b0aa6718..13f86b6b77 100644 --- a/tests/auth_tests/test_forms.py +++ b/tests/auth_tests/test_forms.py @@ -5,6 +5,7 @@ from unittest import mock from django.contrib.auth.forms import ( AdminPasswordChangeForm, + AdminUserCreationForm, AuthenticationForm, BaseUserCreationForm, PasswordChangeForm, @@ -79,6 +80,12 @@ class BaseUserCreationFormTest(TestDataMixin, TestCase): form_class = BaseUserCreationForm + def test_form_fields(self): + form = self.form_class() + self.assertEqual( + list(form.fields.keys()), ["username", "password1", "password2"] + ) + def test_user_already_exists(self): data = { "username": "testclient", @@ -239,16 +246,6 @@ class BaseUserCreationFormTest(TestDataMixin, TestCase): form["password2"].errors, ) - # passwords are not validated if `usable_password` is unset - data = { - "username": "othertestclient", - "password1": "othertestclient", - "password2": "othertestclient", - "usable_password": "false", - } - form = BaseUserCreationForm(data) - self.assertIs(form.is_valid(), True, form.errors) - def test_password_whitespace_not_stripped(self): data = { "username": "testuser", @@ -330,19 +327,6 @@ class BaseUserCreationFormTest(TestDataMixin, TestCase): ["The password is too similar to the first name."], ) - # passwords are not validated if `usable_password` is unset - form = CustomUserCreationForm( - { - "username": "testuser", - "password1": "testpassword", - "password2": "testpassword", - "first_name": "testpassword", - "last_name": "lastname", - "usable_password": "false", - } - ) - self.assertIs(form.is_valid(), True, form.errors) - def test_username_field_autocapitalize_none(self): form = self.form_class() self.assertEqual( @@ -362,17 +346,6 @@ class BaseUserCreationFormTest(TestDataMixin, TestCase): form.fields[field_name].widget.attrs["autocomplete"], autocomplete ) - def test_unusable_password(self): - data = { - "username": "new-user-which-does-not-exist", - "usable_password": "false", - } - form = BaseUserCreationForm(data) - self.assertIs(form.is_valid(), True, form.errors) - u = form.save() - self.assertEqual(u.username, data["username"]) - self.assertFalse(u.has_usable_password()) - class CustomUserCreationFormTest(TestDataMixin, TestCase): @@ -1602,3 +1575,72 @@ class AdminPasswordChangeFormTest(TestDataMixin, TestCase): self.assertIs(form.is_valid(), True) # Valid despite password empty/mismatch. user = form.save(commit=True) self.assertIs(user.has_usable_password(), False) + + +class AdminUserCreationFormTest(BaseUserCreationFormTest): + + form_class = AdminUserCreationForm + + def test_form_fields(self): + form = self.form_class() + self.assertEqual( + list(form.fields.keys()), + ["username", "password1", "password2", "usable_password"], + ) + + @override_settings( + AUTH_PASSWORD_VALIDATORS=[ + { + "NAME": ( + "django.contrib.auth.password_validation." + "UserAttributeSimilarityValidator" + ) + }, + { + "NAME": ( + "django.contrib.auth.password_validation.MinimumLengthValidator" + ), + "OPTIONS": { + "min_length": 12, + }, + }, + ] + ) + def test_no_password_validation_if_unusable_password_set(self): + data = { + "username": "otherclient", + "password1": "otherclient", + "password2": "otherclient", + "usable_password": "false", + } + form = self.form_class(data) + # Passwords are not validated if `usable_password` is unset. + self.assertIs(form.is_valid(), True, form.errors) + + class CustomUserCreationForm(self.form_class): + class Meta(self.form_class.Meta): + model = User + fields = ("username", "email", "first_name", "last_name") + + form = CustomUserCreationForm( + { + "username": "testuser", + "password1": "testpassword", + "password2": "testpassword", + "first_name": "testpassword", + "last_name": "lastname", + "usable_password": "false", + } + ) + self.assertIs(form.is_valid(), True, form.errors) + + def test_unusable_password(self): + data = { + "username": "new-user-which-does-not-exist", + "usable_password": "false", + } + form = self.form_class(data) + self.assertIs(form.is_valid(), True, form.errors) + u = form.save() + self.assertEqual(u.username, data["username"]) + self.assertFalse(u.has_usable_password()) |