2

I have the following model for storing previously used hashed passwords:

class PasswordHistory(models.Model):
    user = models.ForeignKey(User, on_delete=models.CASCADE)
    password = models.CharField(max_length=128, unique=True)
    created_at = models.DateTimeField(auto_now_add=True)
    updated_at = models.DateTimeField(auto_now = True)

In the change password form I want check if the new password the user is changing to has not been used the past 5 times.

Here is my form validation:

class ProfileForm(forms.ModelForm):
    password1 = forms.CharField(widget=forms.PasswordInput(), required=False)
    password2 = forms.CharField(widget=forms.PasswordInput(), required=False)

    class Meta:
        model = Employee

        
    user_id = None
    
    def __init__(self, *args, **kwargs):
        self.user_id = kwargs.pop('user_id', None)
        super(ProfileForm, self).__init__(*args, **kwargs)
        
    def clean_password2(self):
        password1 = self.cleaned_data['password1']
        password2 = self.cleaned_data['password2']
        if password1 != password2:
            raise forms.ValidationError('Passwords do not match.')
        

        user = User.objects.get(pk=self.user_id)
        hashed_password = make_password(password1)
        password_histories = PasswordHistory.objects.filter(
        user=user,
        password_hashed_password
    )
    if password_histories.exists():
        raise forms.ValidationError('That password has already been used')        
    return password2

The problem is that the passwords are different every time, even when I attempt the same plain text password over and over again. Therefore:

if password_histories.exists():

Never returns true.

How can I compare past passwords if they are always different due to salt? Thanks

Atma
  • 29,141
  • 56
  • 198
  • 299

1 Answers1

4

The .set_password function indeed does not return anything, it simply sets the password. Like you say however, the hashing is based on a (random) salt, and thus the hash will be different each time. Therefore you should use the .check_password(…) function [Django-doc], to verify if it somehow matches a hashed variant:

from django.contrib.auth.hashers import check_password

class ProfileForm(forms.ModelForm):
    password1 = forms.CharField(widget=forms.PasswordInput(), required=False)
    password2 = forms.CharField(widget=forms.PasswordInput(), required=False)

    class Meta:
        model = Employee
    
    def __init__(self, *args, **kwargs):
        self.user_id = kwargs.pop('user_id', None)
        super(ProfileForm, self).__init__(*args, **kwargs)
        
    def clean_password2(self):
        password1 = self.cleaned_data['password1']
        password2 = self.cleaned_data['password2']
        if password1 != password2:
            raise forms.ValidationError('Passwords do not match.')
        user = User.objects.get(pk=self.user_id)
        password_histories = PasswordHistory.objects.filter(
            user=user
        )
        for pw in password_histories:
            if check_password(password2, pw.password):
                raise forms.ValidationError('That password has already been used')
        return password2

So if we found a hashed password that matches the given raw password, we can return the password. If by the end of the for loop, we did not find any such password, we can return password2, otherwise we raise an errro.

Willem Van Onsem
  • 443,496
  • 30
  • 428
  • 555
  • The make_password prints out a completely different hash for the same plain text password. – Atma Aug 22 '20 at 18:02
  • @Atma: well that is rather strange, since the `User.set_password` calls this behind the curtains: https://github.com/django/django/blob/master/django/contrib/auth/base_user.py#L98 So unless you altered the hasher, etc. something fishy is going on. Note that you shoudl be careful not to *double hash* the password (as in hashing a hashed version already). That is why calling `.set_password` during cleaning is *not* a good idea. Since then you will often call it again in the `save` method or in the view, and thus store the hash *of the hash*. – Willem Van Onsem Aug 22 '20 at 18:05
  • This is the case even in the django admin. I can set the password over and over again to the same plain text password and it will be different each time because of salt. – Atma Aug 22 '20 at 18:07
  • @Atma: ah yes, you should indeed verify the hash, not hash the password. – Willem Van Onsem Aug 22 '20 at 18:15
  • I like it a lot – Atma Aug 22 '20 at 18:16
  • 1
    People helping people, this is powerful stuff – Atma Aug 22 '20 at 18:27