UserCreationForm doesn't allow empty password even if password fields are specified as "not required"

[buffgecko12]

There was a change made to UserCreationForm (django.contrib.auth.forms) in Django 5.1 which seems to require a password to be provided even if the password1 / password2 fields are explicitly marked as required = False. It looks like the validate_passwords() method is still being called even if the fields are marked as required = False.

Please see this issue for the details:

​https://stackoverflow.com/questions/79387857/django-5-1-usercreationform-wont-allow-empty-passwords

---

I tried responding to your comment but the webpage won't let me submit my reply -- it says my submission is probably spam. Here's my response to your comment:

I would love to work on it, but I don't want to hold up the release. Please go ahead.

[Natalia Bidart]

Hello buffgecko12, thank you for your report. I think you are correct, this is a bug introduced in e626716c28b6286f8cf0f8174077f3d2244f3eb3 that should be addressed. Would you like to work on a PR?

[buffgecko12]

Hi Natalia!! This is exciting, I have never modified the Django code before.

Do I just clone the Django Git repository, create / push my own branch to it, and submit a pull request?

​https://github.com/django/django.git

What is the general procedure?

Chris

[Antoliny]

I think these documents will be helpful for you.
​Writing your first contribution for Django
​Submitting contributions

[buffgecko12]

Thank you. Is there a timeline for when this bug needs to be fixed and a PR submitted? Just to gauge if I am able to take this on or not.

[Antoliny]

Replying to buffgecko12:

Thank you. Is there a timeline for when this bug needs to be fixed and a PR submitted? Just to gauge if I am able to take this on or not.

I'm not sure about the deadline. However, since this bug is a release blocker, I assume it might be a priority. (This is just my guess.)

[Antoliny]

I looked into this issue.
I think this issue occurs because when usable_password key is not present in cleaned_data, the usable_password variable is set to True, causing the validation to be performed.

django/contrib/auth/forms.py

@@ -200 +200 @@
         **kwargs,
     ):
         usable_password = (
-            self.cleaned_data.pop(usable_password_field_name, None) != "false"
+            (self.cleaned_data.get("password2", "") != "")
+            or (self.cleaned_data.pop(usable_password_field_name, "false") != "false")
         )
         self.cleaned_data["set_usable_password"] = usable_password

I modified the default value as above and adjusted the usable_password assignment value based on the presence of password2. I'm not sure if this is the best solution, but I hope it helps you.

[Antoliny]

This issue is related to passwords, I believe it should be designed as defensively as possible.
Unless there is a clear intention from the user (such as having usable_password set to None in the author's form), usable_password should remain True. This aspect is not covered in the above changes.

if hasattr(self, "usable_password") and self.usable_password == None:
    ...

I think there might be another condition needed as well.

[Gregory Mariani]

test in test_forms.UserCreationFormTest could be something like that:

    def test_empty_passwords_with_required_false(self):
        class CustomUserCreationForm(UserCreationForm):
            password1 = CharField(required=False)
            password2 = CharField(required=False)

            class Meta(UserCreationForm.Meta):
                model = ExtensionUser

        form_data = {
            "username": "testuser",
            # 'password1' & 'password2' are not given
        }
        form = CustomUserCreationForm(data=form_data)
        self.assertTrue(form.is_valid())
        self.assertNotIn("password1", form.errors)
        self.assertNotIn("password2", form.errors)

[Natalia Bidart]

Replying to buffgecko12:

Thank you. Is there a timeline for when this bug needs to be fixed and a PR submitted? Just to gauge if I am able to take this on or not.

As Antoliny says, this is a release blocker so we should have a solution ready by next week, when we have planned the next patch release for Django 5.1.

It's completely fine if you don't have time, me or someone else would grab this since it's a priority to get solved. Let me know so I start working on this!

[Antoliny]

If it's okay, can I work on this ticket?
Nessita, if you want to work on it, please feel free to do so. I'd be happy to follow your lead!

[Natalia Bidart]

Replying to Antoliny:

If it's okay, can I work on this ticket?
Nessita, if you want to work on it, please feel free to do so. I'd be happy to follow your lead!

I'm taking a look to have an idea of the fix but you I would love if you work on this. Otherwise I could use another pair of eyes for what I'll propose. Right now I'm inclined to change the required=False in create_usable_password_field to be required=True and then tweak the checks in AdminPasswordChangeForm and AdminUserCreationForm. With that, which would be an "isolated" refactoring, there would be a second commit considering the required in password validation.

What do you think?

[Antoliny]

Replying to Natalia Bidart:

Replying to Antoliny:

If it's okay, can I work on this ticket?
Nessita, if you want to work on it, please feel free to do so. I'd be happy to follow your lead!

I'm taking a look to have an idea of the fix but you I would love if you work on this. Otherwise I could use another pair of eyes for what I'll propose. Right now I'm inclined to change the required=False in create_usable_password_field to be required=True and then tweak the checks in AdminPasswordChangeForm and AdminUserCreationForm. With that, which would be an "isolated" refactoring, there would be a second commit considering the required in password validation.

What do you think?

I think it's a good approach. By changing required to True, the user will be able to clearly express their intent regarding the unusable_password.

[Natalia Bidart]

@buffgecko12, could you please confirm if the proposed solution in ​this PR solves your issue?

[buffgecko12]

Yes, from a visual inspection it looks good.

How do I test the fix in my application? Do I just copy the new django/contrib/auth/forms.py file into my django directory?

I already have code in my application to work around the bug, so I'd need a little time to update the code and test as well.

[Natalia Bidart]

Replying to buffgecko12:

Yes, from a visual inspection it looks good.

How do I test the fix in my application? Do I just copy the new django/contrib/auth/forms.py file into my django directory?

You can use a virtual env with Django installed via pip install -e, see further docs in ​this link.

[GitHub <noreply@…>]

In d15454a:

Fixed #36140 -- Allowed BaseUserCreationForm to define non required password fields.

Regression in e626716c28b6286f8cf0f8174077f3d2244f3eb3.

Thanks buffgecko12 for the report and Sarah Boyce for the review.

[Natalia <124304+nessita@…>]

In affad13:

[5.2.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.

[Natalia <124304+nessita@…>]

In 8552eef:

[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.

[GitHub <noreply@…>]

In 328d54f:

[5.1.x] Refs #36140 -- Added missing import in django/contrib/auth/forms.py.

Follow up to 8552eef95e400d5bad3261b28ad2500b57070d57.