[Django] #36703: Undocumented change of SetPasswordForm in django 5.1 release notes

[Django]

#36703: Undocumented change of SetPasswordForm in django 5.1 release notes
-------------------------------------+-------------------------------------
Reporter: Laurent Bergeron | Type:
| Uncategorized
Status: new | Component:
| Documentation
Version: 5.1 | Severity: Normal
Keywords: Authentication, | Triage Stage:
Forms | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Since version 5.1 of django, django.contrib.auth.forms.SetPasswordForm
looks like this:


{{{
class SetPasswordForm(SetPasswordMixin, forms.Form):
"""
A form that lets a user set their password without entering the old
password
"""

new_password1, new_password2 =
SetPasswordMixin.create_password_fields(
label1=_("New password"), label2=_("New password confirmation")
)

def __init__(self, user, *args, **kwargs):
self.user = user
super().__init__(*args, **kwargs)

def clean(self):
self.validate_passwords("new_password1", "new_password2")
self.validate_password_for_user(self.user, "new_password2")
return super().clean()

def save(self, commit=True):
return self.set_password_and_save(self.user, "new_password1",
commit=commit)
}}}

Before version 5.1 though, it looked like this:

{{{
class SetPasswordForm(forms.Form):
"""
A form that lets a user set their password without entering the old
password
"""

error_messages = {
"password_mismatch": _("The two password fields didn’t match."),
}
new_password1 = forms.CharField(
label=_("New password"),
widget=forms.PasswordInput(attrs={"autocomplete": "new-
password"}),
strip=False,
help_text=password_validation.password_validators_help_text_html(),
)
new_password2 = forms.CharField(
label=_("New password confirmation"),
strip=False,
widget=forms.PasswordInput(attrs={"autocomplete": "new-
password"}),
)

def __init__(self, user, *args, **kwargs):
self.user = user
super().__init__(*args, **kwargs)

def clean_new_password2(self):
password1 = self.cleaned_data.get("new_password1")
password2 = self.cleaned_data.get("new_password2")
if password1 and password2 and password1 != password2:
raise ValidationError(
self.error_messages["password_mismatch"],
code="password_mismatch",
)
password_validation.validate_password(password2, self.user)
return password2

def save(self, commit=True):
password = self.cleaned_data["new_password1"]
self.user.set_password(password)
if commit:
self.user.save()
return self.user
}}}

I can't see this change described anywhere in the 5.1 release note
[https://docs.djangoproject.com/en/5.2/releases/5.1/]

Do I have a blind spot and the change is in fact described in the patch
note ? If it is not, should it be or is it too small of a change to be
part of the release notes?

Personally, it caused some of my tests to fail when I upgraded from 4.2 to
5.2. I had some logic to modify the error messages and it broke because of
the change.
--
Ticket URL: <https://code.djangoproject.com/ticket/36703>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36703: Undocumented change of SetPasswordForm in django 5.1 release notes
-------------------------------------+-------------------------------------

Reporter: Laurent Bergeron | Owner: (none)

Type: Uncategorized | Status: new
Component: Documentation | Version: 5.1

Severity: Normal | Resolution:

Keywords: Authentication, | Triage Stage:
Forms | Unreviewed

Has patch: 0 | Needs documentation: 1

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Laurent Bergeron):

* needs_docs: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/36703#comment:1>

[Django]

#36703: Undocumented change of SetPasswordForm in django 5.1 release notes
-------------------------------------+-------------------------------------
Reporter: Laurent Bergeron | Owner: (none)
Type: Uncategorized | Status: new
Component: Documentation | Version: 5.1
Severity: Normal | Resolution:
Keywords: Authentication, | Triage Stage:
Forms | Unreviewed
Has patch: 0 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by ontowhee):

Hello! It looks like the changes were from #34429, and the release note
you linked mentions `The new AdminUserCreationForm and the existing
AdminPasswordChangeForm now support disabling password-based
authentication by setting an unusable password on form save.`

> Personally, it caused some of my tests to fail when I upgraded from 4.2
to 5.2. I had some logic to modify the error messages and it broke because
of the change.

Can you provide reproduction steps?
--
Ticket URL: <https://code.djangoproject.com/ticket/36703#comment:2>

[Django]

#36703: Undocumented change of SetPasswordForm in django 5.1 release notes
-------------------------------------+-------------------------------------
Reporter: Laurent Bergeron | Owner: (none)
Type: Uncategorized | Status: new
Component: Documentation | Version: 5.1
Severity: Normal | Resolution:
Keywords: Authentication, | Triage Stage:
Forms | Unreviewed
Has patch: 0 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Laurent Bergeron):

Replying to [comment:2 ontowhee]:

> Hello! It looks like the changes were from #34429, and the release note
you linked mentions `The new AdminUserCreationForm and the existing
AdminPasswordChangeForm now support disabling password-based
authentication by setting an unusable password on form save.`

Yeah that seems to be it.

> Can you provide reproduction steps?

I created a custom password change form which had a clean method that
looked like this:

{{{
def clean(self):
if "new_password2" in self.errors:
new_password2_errors = self.errors["new_password2"].as_data()
for error in new_password2_errors:
if error.code in self.error_messages:
custom_error_message = self.error_messages[error.code]
error.message = custom_error_message
return super().clean()
}}}
And there is a dictionary in the form class that contains custom error
messages (self.error_messages) too.

But then I updated Django and this method couldn't change any error
messages because the errors on new_password2 (and other field I believe)
are now raised in super().clean(). The method worked before I updated
though as the errors were raised before my clean method.

To solve the issue I edited my method so it looks like this:

{{{
def clean(self):
super().clean()
if "new_password2" in self.errors:
new_password2_errors = self.errors["new_password2"].as_data()
for error in new_password2_errors:
if error.code in self.error_messages:
custom_error_message = self.error_messages[error.code]
error.message = custom_error_message
return self.cleaned_data
}}}

Now that super().clean() is executed before my error messages replacement
code, the latter works as expected. To be clear I didn't open this issue
to seek for help on how to solve this issue. I wanted to make sure this
change was detailed in the release notes.
--
Ticket URL: <https://code.djangoproject.com/ticket/36703#comment:3>

[Django]

#36703: Undocumented change of SetPasswordForm in django 5.1 release notes
-------------------------------------+-------------------------------------
Reporter: Laurent Bergeron | Owner: (none)

Type: Bug | Status: closed
Component: contrib.auth | Version: 5.1
Severity: Normal | Resolution: invalid

Keywords: Authentication, | Triage Stage:
Forms | Unreviewed

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Natalia Bidart):

* component: Documentation => contrib.auth
* easy: 1 => 0
* needs_docs: 1 => 0
* resolution: => invalid
* status: new => closed
* type: Uncategorized => Bug

Comment:

Replying to [comment:3 Laurent Bergeron]:

> Replying to [comment:2 ontowhee]:
> > Hello! It looks like the changes were from #34429, and the release
note you linked mentions `The new AdminUserCreationForm and the existing
AdminPasswordChangeForm now support disabling password-based
authentication by setting an unusable password on form save.`
>
> Yeah that seems to be it.

Hello Laurent! Thank you for taking the time to create the report. And
thank you Lilian for the triage.

> Now that super().clean() is executed before my error messages
replacement code, the latter works as expected. To be clear I didn't open
this issue to seek for help on how to solve this issue. I wanted to make
sure this change was detailed in the release notes.

The release notes don't (and can't) include every behavioral detail of
internal changes. In this case, the documented pattern is to call
`super()` first when overriding `clean()`, as described here:
https://docs.djangoproject.com/en/5.2/ref/forms/validation/#cleaning-and-
validating-fields-that-depend-on-each-other. Since the current behavior
aligns with the docs, I think there's nothing to change here, so I'm
closing this accordingly.
--
Ticket URL: <https://code.djangoproject.com/ticket/36703#comment:4>