Hi,
I've found some inconsistent behavior in how password validation via the
`UserAttributeSimilarityValidator` is handled by `contrib/auth`. I did not
find any mention of this in Trac or on the mailing list, but apologies if this
has already been discussed.
AFAICT: When creating a user, the password is only checked in similarity to the
`username` field. When changing a password, the password is checked in
similarity to `username`, `first_name`, `last_name`, and `email`.
This seems undesirable - it is possible to set a password at creation time that
might be rejected if set during a password change.
In short:
When `SetPasswordForm` calls password validation, it has all of the fields on
the `User` instance, but when `UserCreationForm` does so, it manually adds the
one field being checked.
In depth:
In `django/contrib/auth/forms.py`, the `SetPasswordForm` and `UserCreationForm`
both call `password_validation.validate_password()` in the `clean_password2()`
method. In both instances, the forms pass `password2` and `self.instance`. In
the `UserCreationForm`, the `ModelForm` hasn't yet created the `User` instance.
`UserCreationForm` manually adds `username` to the empty `User` instance on
line 105 to allow for user attribute validation.
Rather than check the validity of the passwords in the `password2` clean
method, it seems like it would be better to wait until the `User` instance has
been created. We can use the `ModelForm` `_post_clean()` hook to achieve this.
def _post_clean(self):
super()._post_clean() # creates self.instance
password = self.cleaned_data.get("password1")
if password:
try:
password_validation.validate_password(password, self.instance)
except ValidationError as error:
self.add_error("password1", error)
This keeps the two forms' behaviors consistent. Note that I've opted to use
`password1` instead of `password2` because it feels more logical to show the
problem above both passwords, rather than between the two fields.
Is there a reason *not* to make this change? Are there specific reasons for why
we are only checking the `username` field during creation?
Feedback appreciated. If others approve of this, I will open a PR.
Andrew
http://jambonsw.com
http://django-unleashed.com
--
You received this message because you are subscribed to the Google Groups
"Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-developers/878428A0-4F59-403A-A611-DD9208A605C9%40andrewsforge.com.
For more options, visit https://groups.google.com/d/optout.
