mirrors/django
mirrors/django
0
0
Fork 0
mirror of https://github.com/django/django.git synced 2024-11-21 19:09:18 +01:00
Code Issues Releases Wiki Activity

Fixed #35678 -- Removed "usable_password" field from BaseUserCreationForm.

Browse Source 
Refs #34429: Following the implementation allowing the setting of
unusable passwords via the admin site, the `BaseUserCreationForm` and
`UserCreationForm` were extended to include a new field for choosing
whether password-based authentication for the new user should be enabled
or disabled at creation time.
Given that these forms are designed to be extended when implementing
custom user models, this branch ensures that this new field is moved to
a new, admin-dedicated, user creation form `AdminUserCreationForm`.

Regression in e626716c28.

Thanks Simon Willison for the report, Fabian Braun and Sarah Boyce for
the review.
This commit is contained in:
Natalia 2024-08-15 10:27:24 -03:00 committed by nessita
parent b60fd8722f
commit 0ebed5fa95
6 changed files with 174 additions and 88 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
django/contrib/auth/admin.py
View File

@ -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")

100
django/contrib/auth/forms.py
View File

@ -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,30 +151,81 @@ 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):
if self.cleaned_data["set_usable_password"]:
user.set_password(self.cleaned_data[password_field_name])
else:
user.set_unusable_password()
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"]:
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()
return user
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()

6
docs/releases/5.1.1.txt
View File

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

10
docs/releases/5.1.txt
View File

@ -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

32
docs/topics/auth/default.txt
View File

@ -1645,6 +1645,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.
@ -1735,21 +1752,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

110
tests/auth_tests/test_forms.py
View File

@ -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())