[Fixed]-Why is checking if two passwords match in Django so complicated?

by

31👍

✅

Here’s what I would do:

This is the only clean method you need to define to make sure 2 passwords are correct and that the username is valid.

Use the clean_fieldname method so that you don’t need to do more work to validate the username.

def clean_password2(self):
    password1 = self.cleaned_data.get('password1')
    password2 = self.cleaned_data.get('password2')

    if not password2:
        raise forms.ValidationError("You must confirm your password")
    if password1 != password2:
        raise forms.ValidationError("Your passwords do not match")
    return password2

You’re absolutely right, you dont need to validate the username being unique, because the ModelForm knows it needs to be unique.

The problem with your code is that you are overriding the clean() method, which means the ModelForm isn’t doing its ‘real’ clean().

To get the default validation, call super(MyForm, self).clean() or better yet don’t override clean at all and only specify clean_password2.

👤Yuji 'Tomita' Tomita

5👍

Firstly, are you seriously complaining about four lines of boiler-plate code? If it really bothers you, create a PasswordForm class that contains that clean logic and sublass it for your own forms as necessary.

Secondly, you don’t have to validate unique constraints manually. As you say, the ModelForm does it for you.

Edit after comment

This ‘weird syntax’ is because checking that two password fields match is a different flow than the normal scheme of things. For a start, you’re checking in the main clean method rather than the field-specific clean_myfield. If it had been the latter, you just raise an exception and Django does indeed remove the field data.

So no, this isn’t 7 lines on every form – see my note about subclassing – and it certainly isn’t 7 lines times multiple fields, because you don’t want to do this for any other type of field.

👤Daniel Roseman

5👍

http://k0001.wordpress.com/2007/11/15/dual-password-field-with-django/

Edit: found out the way the admin form deals with the problem: http://code.djangoproject.com/svn/django/trunk/django/contrib/auth/forms.py

class AdminPasswordChangeForm(forms.Form):
    """
    A form used to change the password of a user in the admin interface.
    """
    password1 = forms.CharField(label=_("Password"), widget=forms.PasswordInput)
    password2 = forms.CharField(label=_("Password (again)"), widget=forms.PasswordInput)

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

    def clean_password2(self):
        password1 = self.cleaned_data.get('password1')
        password2 = self.cleaned_data.get('password2')
        if password1 and password2:
            if password1 != password2:
                raise forms.ValidationError(_("The two password fields didn't match."))
        return password2

    def save(self, commit=True):
        """
        Saves the new password.
        """
        self.user.set_password(self.cleaned_data["password1"])
        if commit:
            self.user.save()
        return self.user
👤StefanNch

1👍

You might want to add an else: part for the first if. Currently the function returns the data without setting any errors even if one of the passwords are absent – is that the intended behavior?

else:
    self._errors["password"] = self.error_class(['One or both of the passwords not found'])

if "password1" in data and "password2" in data: This makes sure that both passwords are present. Without this line, you’ll get an error in the next line where you read data[password1] and data[password2] if either of them are not present.

Next three lines compare the passwords and sets the appropriate error message – that’s required, isn’t it?

As they say, make things as simple as possible, not anymore.

👤Amarghosh

Leave a comment