25

I have the following form. How can I check the password from the user again, before the user can change his email address finally? Even if the user is logged in, I just want to be sure that it is really the user. Just a security thing.

How do I do it with .check_password()?

'EmailChangeForm' object has no attribute 'user'

/home/craphunter/workspace/project/trunk/project/auth/user/email_change/forms.py in clean_password, line 43
from django import forms
from django.db.models.loading import cache
from django.utils.translation import ugettext_lazy as _
from django.contrib.auth.models import User


class EmailChangeForm(forms.Form):
    
    email = forms.EmailField(label='New E-mail', max_length=75)
    password = forms.CharField(widget=forms.PasswordInput)

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

    def clean_password(self):
        valid = self.user.check_password(self.cleaned_data['password'])
        if not valid:
            raise forms.ValidationError("Password Incorrect")
        return valid
    
    def __init__(self, username=None, *args, **kwargs):
        """Constructor.
        
        **Mandatory arguments**
        
        ``username``
            The username of the user that requested the email change.
        
        """
        self.username = username
        super(EmailChangeForm, self).__init__(*args, **kwargs)
    
    def clean_email(self):
        """Checks whether the new email address differs from the user's current
        email address.
        
        """
        email = self.cleaned_data.get('email')
        
        User = cache.get_model('auth', 'User')
        user = User.objects.get(username__exact=self.username)
        
        # Check if the new email address differs from the current email address.
        if user.email == email:
            raise forms.ValidationError('New email address cannot be the same \
                as your current email address')
        
        return email
craphunter
  • 820
  • 2
  • 15
  • 27

3 Answers3

30

I would refactor your code to look something like this:

View:

@login_required
def view(request, extra_context=None, ...):

    form = EmailChangeForm(user=request.user, data=request.POST or None)

    if request.POST and form.is_valid():
        send_email_change_request(request.user,
                                  form.cleaned_data['email'],
                                  https=request.is_secure())
        return redirect(success_url)
    ...

Password validation goes to form:

class EmailChangeForm(Form):
    email = ...
    old_password = CharField(..., widget=Password())

    def __init__(self, user, data=None):
        self.user = user
        super(EmailChangeForm, self).__init__(data=data)

    def clean_old_password(self):
        password = self.cleaned_data.get('password', None)
        if not self.user.check_password(password):
            raise ValidationError('Invalid password')

Extract logic from view:

 def send_email_change_request(user, new_email, https=True):

    site = cache.get_model('sites', 'Site')

    email = new_email
    verification_key = generate_key(user, email)

    current_site = Site.objects.get_current()
    site_name = current_site.name
    domain = current_site.domain

    protocol = 'https' if https else 'http'

    # First clean all email change requests made by this user
    qs = EmailChangeRequest.objects.filter(user=request.user)
    qs.delete()

    # Create an email change request
    change_request = EmailChangeRequest(
       user = request.user,
       verification_key = verification_key,
       email = email
    )
    change_request.save()

    # Prepare context
    c = {
        'email': email,
        'site_domain': 'dev.tolisto.de',
        'site_name': 'tolisto',
        'user': self.user,
        'verification_key': verification_key,
        'protocol': protocol,
    }
    c.update(extra_context)
    context = Context(c)

    # Send success email
    subject = "Subject" # I don't think that using template for 
                        # subject is good idea
    message = render_to_string(email_message_template_name, context_instance=context)

    send_mail(subject, message, None, [email])

Don't put complicated things inside views (such as rendering and sending email).

Fabio
  • 3,015
  • 2
  • 29
  • 49
Ski
  • 14,197
  • 3
  • 54
  • 64
12

I feel like you answered your own question : )

The docs on the check_password method are here: https://docs.djangoproject.com/en/dev/topics/auth/customizing/#django.contrib.auth.models.AbstractBaseUser.check_password

success = user.check_password(request.POST['submitted_password'])
if success: 
   # do your email changing magic
else:
   return http.HttpResponse("Your password is incorrect") 
   # or more appropriately your template with errors

Since you're already passing in request.user into your form constructor (looks like you've overriden __init__ for your own reasons) you could put all of your logic in the form without any trouble.

class MyForm(forms.Form):
     # ...
     password = forms.CharField(widget=forms.PasswordInput)
     
     def __init__(self, user, *args, **kwargs):
          super(MyForm, self).__init__(*args, **kwargs)
          self.user = user
          
     def clean_password(self):
         valid = self.user.check_password(self.cleaned_data['password'])
         if not valid:
             raise forms.ValidationError("Password Incorrect")
         return valid

update after seeing your forms

OK. The main problem is that __init__ has been defined twice, making the first statement useless. Second problem I see is that we'd be doing multiple queries for user when we really don't have to.

We've strayed from your original question quite a bit, but hopefully this is a learning experience.

I've changed only a few things:

  • Removed the extra __init__ definition
  • Changed __init__ to accept a User instance instead of a text username
  • Removed the query for User.objects.get(username=username) since we're passing in a user object.

Just remember to pass the form constructor user=request.user instead of username=request.user.username

class EmailChangeForm(forms.Form):
    email = forms.EmailField(label='New E-mail', max_length=75)
    password = forms.CharField(widget=forms.PasswordInput)
    
    def __init__(self, user=None, *args, **kwargs):
        self.user = user
        super(EmailChangeForm, self).__init__(*args, **kwargs)
        
    def clean_password(self):
        valid = self.user.check_password(self.cleaned_data['password'])
        if not valid:
            raise forms.ValidationError("Password Incorrect")
        
    def clean_email(self):
        email = self.cleaned_data.get('email')
        
        # no need to query a user object if we're passing it in anyways.
        user = self.user 
        
        # Check if the new email address differs from the current email address.
        if user.email == email:
            raise forms.ValidationError('New email address cannot be the same \
                as your current email address')
                
        return email

Finally since we're talking about good practice here, I'd recommend following through with Skirmantas suggestions about moving your current view code to a form method so you can simply call myform.send_confirmation_email.

Sounds like a good exercise!

phoenix
  • 7,988
  • 6
  • 39
  • 45
Yuji 'Tomita' Tomita
  • 115,817
  • 29
  • 282
  • 245
  • Thanks dude! Yes, I am a newbee in django...sometimes the solution is so simple. – craphunter Jan 27 '11 at 22:54
  • No problem man! Yes, I love django and python. Oh, you know what? To streamline, it occurs to me you should just add a password field to your form since you're already passing in the user object. Just define `password = forms.CharField(widget=forms.PasswordInput})` and override `clean_password` to do the password checking. That way you can stay in the forms framework for redisplaying the error form. – Yuji 'Tomita' Tomita Jan 27 '11 at 23:03
  • What do you mean with clean_password? Is there an typing error in password })? – craphunter Jan 27 '11 at 23:16
  • Ohh, my young grasshopper, you must learn the ways of form cleaning: http://docs.djangoproject.com/en/dev/ref/forms/validation/#cleaning-a-specific-field-attribute It's a piece of what makes django forms amazing. In your forms definition, there are certain methods called automagically, one being clean_fieldname for every field you have defined in your form. You can do per field validation in these methods, by either raising forms.validationerror OR returning the valid value. Updated answer with example! – Yuji 'Tomita' Tomita Jan 27 '11 at 23:21
  • If I do it like your way to put it in the form.py. I can type any password I want. What is wrong? Thanks for the doc link! I am a very young grasshopper! – craphunter Jan 27 '11 at 23:26
  • Can you show your updated form / view / template? If you've added a field called password in your form, are using {{ myform.password }} in your template, have defined clean_password AND raised `forms.ValidationError` if the password doesn't match, your call to `form.is_valid()` would fail. – Yuji 'Tomita' Tomita Jan 27 '11 at 23:37
  • I have updated it. See above. Error message and the html code. Where should I define cleand_password? Anywhere else than in forms.py? – craphunter Jan 27 '11 at 23:47
  • Ah, you should have told me the error message. It says `self.user` is not defined. You have to look in your `def __init__` statement and set `self.user` to `user` so `clean_password` has access to the active `user` object. I wrote some sample code above. (You can post your current `__init__` override if you want me to look) – Yuji 'Tomita' Tomita Jan 27 '11 at 23:51
  • Yes, that would be very nice! I have updated my forms.py. See above! TIA! – craphunter Jan 28 '11 at 00:04
  • Ouch I opened up a can of worms. Ok, looks like you've defined `__init__` already (if you define it twice, only bottom most one will be used), which is why `self.user` doesn't exist. Because you're already passing in the `user` object, let's get rid of that query in `clean_email`. Posting new code – Yuji 'Tomita' Tomita Jan 28 '11 at 00:25
  • I got it! Thanks again! See my post! – craphunter Jan 28 '11 at 00:26
  • This is even better than my solution! Really thanks again. I learned a lot! You know, I guess my main problem is, that I don't actually understand the __init__.py As I mentinoed. I am a very young grasshopper! I need more literature! THANKS! – craphunter Jan 28 '11 at 00:35
  • 2
    Awesome! I'm glad you're learning new stuff. init.py? `__init__.py` tells python that the folder is python related. The `__init__` method on a python object is called when it is first created. http://docs.python.org/reference/datamodel.html#object.__init__ It's the code that is triggered when we initialize the class (`init`). form = MyForm() <-- calls __init__. The `super()` makes sure that the parent class's __init__ is called as well. Ah found a SO post: http://stackoverflow.com/questions/625083/python-init-and-self-what-do-they-do – Yuji 'Tomita' Tomita Jan 28 '11 at 00:46
1

thanks again to Yuji. It works when I don't have in my first def __init__ the variable user. I also added in def clean_password the first 2 lines from the def clean_email

from django import forms
from django.db.models.loading import cache
from django.utils.translation import ugettext_lazy as _
from django.contrib.auth.models import User


class EmailChangeForm(forms.Form):

    email = forms.EmailField(label='New E-mail', max_length=75)
    password = forms.CharField(widget=forms.PasswordInput)

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

    def clean_password(self):
        User = cache.get_model('auth', 'User')
        user = User.objects.get(username__exact=self.username)
        valid = user.check_password(self.cleaned_data['password'])
        if not valid:
            raise forms.ValidationError("Password Incorrect")
        return valid

    def __init__(self, username=None, *args, **kwargs):
        """Constructor.

        **Mandatory arguments**

        ``username``
            The username of the user that requested the email change.

        """
        self.username = username
        super(EmailChangeForm, self).__init__(*args, **kwargs)

    def clean_email(self):
        """Checks whether the new email address differs from the user's current
        email address.

        """
        email = self.cleaned_data.get('email')

        User = cache.get_model('auth', 'User')
        user = User.objects.get(username__exact=self.username)

        # Check if the new email address differs from the current email address.
        if user.email == email:
            raise forms.ValidationError('New email address cannot be the same \
                as your current email address')

        return email
craphunter
  • 820
  • 2
  • 15
  • 27
  • Congrats that this is working! However take a look at my updated answer since you're doing 2 queries for User when it could be None! Could be a good exercise :) – Yuji 'Tomita' Tomita Jan 28 '11 at 00:33
  • Thanks again to Portland. I did your advise and it is working! I go to bed! In Germany it is 2 o'clock in the morning! Have a nice evening! – craphunter Jan 28 '11 at 00:46