#101 ✓moved_to_github
José Valim

is validates_length_of :password_confirmation needed?

Reported by José Valim | May 2nd, 2009 @ 09:41 AM

Ben,

Is validates_length_of :password_confirmation needed?

If the password is too short and the password confirmation does match, it will fail because the password is too short. We don't need to say that both password and password confirmation are too short.

I do worry because it will show an unnecessary error message, so my users shouldn't be worried with it.

Comments and changes to this ticket

  • Ben Johnson

    Ben Johnson May 4th, 2009 @ 12:15 AM

    • State changed from “new” to “open”

    Good point, I think this was left over from authlogic 2.0. I was initially validating the length of the confirmation instead of the length of the password and I guess I forgot to remove it. Let me run some tests with it out and make sure it doesn't break anything.

  • Ben Johnson

    Ben Johnson May 4th, 2009 @ 12:27 AM

    • State changed from “open” to “resolved”

    Ok, so I removed that, ran some tests and I get this problem:

    u = User.new
    u.valid?
    => false
    u.errors.on(:password)
    => nil
    u.errors.on(:password_confirmation)
    => nil
    

    By removing that, a confirmation is not required. Also the rails documentation states:

    NOTE: This check is performed only if password_confirmation is not nil, and by default only on save. To require confirmation, make sure to add a presence check for the confirmation attribute

    Hope that helps. I agree, it is a little confusing, but I'm sure there is a good reason they did it this way.

  • José Valim

    José Valim May 4th, 2009 @ 02:31 AM

    Exactly, I'm aware of this.

    They don't require the presence of the confirmation, because they don't want to write it on tests or when creating a record in the console (it's just an interface thing).

    If you remove the validates_length_of from password_confirmation, but the field is present on the views, it will always send a blank value, and then it's considered as required.

    If the user fill in just the password and the password confirmation is left blank, a doesn't match message will be shown.

    By removing it, someone could take advantage of this behavior when faking post requests, but if the person is faking post requests, it's not the presence of the confirmation field that is going to stop him. :)

  • Ben Johnson

    Ben Johnson May 11th, 2009 @ 05:04 PM

    • State changed from “resolved” to “open”

    Good point, let me work up some more tests and I'll see if I can get this to make sense.

  • Ben Johnson

    Ben Johnson August 7th, 2009 @ 06:20 PM

    • State changed from “open” to “moved_to_github”

    [state:"moved_to_github" bulk edit command]

  • wichit44214

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

Object based authentication solution that handles all of the non sense for you. It's as easy as ActiveRecord is with a database.

People watching this ticket

Pages