password validation triggered even I update non-password attribute

I have a User model:

  create_table "users", force: true do |t|     t.string "name"     t.string "email"     t.datetime "created_at"     t.datetime "updated_at"     t.string "password_digest"   end   add_index "users", ["name"], name: "index_users_on_name"

Then, I have the following validation:

class User < ActiveRecord::Base   has_secure_password   validates :password, length: {minimum: 4} end

Then, I update in my controller the name and email of a certain user:

   ...     logger.debug('++++++++ new_values after: '+new_values.inspect)     if @user.update_attributes(new_values)       logger.debug('+++++++++++++ SUCCESS')       ...    end

(new_values is of type ActionController::Parameters)

My log shows

  ++++++++ new_values after: {"name"=>"1111", "email"=>"1111"}

However, the update_attributes fails, and the error message says:

  Password is too short (minimum is 4 characters)

What I don't understand is, that I don't supply a new password. Why, then, is password validation triggered here? And how should I implement this?

Background for this question:

This code is executed only, when a user wants to edit his profile data (which, for the time being, consist only of name and email). It is ensured that only a logged in user can execute this code, and only for his own data. Therefore, I don't require to enter the password. He had entered it anyway when logging in.

I had this same problem a while ago. The issue comes in the update_attributes since it wants to update all of the attributes that are being passed to it, which is including an empty password. To fix it you’ll need to do two things. First change the validates to only validate if a password is being passed.

validates :password, length: { minimum: 6 }, :if => :password

``

Second, remove the parameters for password if the are blank on your update method.

if params[:password].blank? params.delete(:password) end

``

That should get you where you want to go.

Eric Saupe wrote in post #1154001:

First change the validates to only validate if a password is being passed.

validates :password, length: { minimum: 6 }, :if => :password

Second, remove the parameters for password if the are blank on your update method.

if params[:password].blank?   params.delete(:password) end

The latter I only did, because in this case it would obviously not work, but I was not aware of the trick for the validates function!

I think I'll have to do this for all 'validates' in my application, because I often have the case that I will update only some of the attributes. I wonder *why* validates looks at attributes which are not part of the update. Is there a use case where this makes sense?

Ronald

This is usually the desired behavior, because you want to ensure that the whole record is valid before saving.

:password is a outlier here, since it isn’t persisted to the database. In your case, I might add something like:

validates :password, length: { minimum: 6 }, on: :create

Since (unless users can change their passwords) you only need to check it when creating a new record.

–Matt Jones

I generally avoid code like that because it creates OOO dependancies (but in a small app might work fine).

In fact you’ve stumbled onto one of the really smelly parts of Rails, IMHO.

What I usually do in cases like these (in fact I’m working on something right at this moment) is that I enforce User objects to have a state (“new”, “onboarding”, “registered”, etc) and then enforce the validations only in certain states.

validates :password, if: lambda { self.registration_state == “registered” }

This is actually tedious to do, but writing code this way creates a better long-term implementation because it enforces rules in a more maintainable way. If you don’t do this and you start down the path of callbacks & validation headaches and those are always nasty (which, from the sound of things, you are probably already in.)

Also, read this book:

http://www.amazon.com/Lean-Architecture-Agile-Software-Development/dp/0470684208

Matt Jones wrote in post #1154197:

> method. because I often have the case that I will update only some of the attributes. I wonder *why* validates looks at attributes which are not part of the update. Is there a use case where this makes sense?

This is usually the desired behavior, because you want to ensure that the *whole* record is valid before saving.

Well, I am reading the record from the database, and only update some fields, so I thought the remaining fields should be treated as consistent.

:password is a outlier here, since it isn't persisted to the database.

Maybe this is the reason why the problem shows up here. I thought that the record already has the password_digest, so it would be fine. :frowning:

In your case, I might add something like:

validates :password, length: { minimum: 6 }, on: :create

Since (unless users can change their passwords) you only need to check it when creating a new record.

That's exactly the problem: The user should be able to change the password, but if he only changes one of the other fields, he should not need to supply the password (since he is already logged in anyway). I implemented this by providing an entry form with empty password (and empty password confirmation). From the response, I remove both password fields from the params hash, if both are empty, and I also remove the user id, if it has not changed (because there is an index on it, and I don't want to trigger unnecessary index update operations). After this, I pass the params to update_attributes.

When it comes to updating profile information, I had expected that this would be the "normal" way to do it. Am I wrong in this respect?

Ronald

Jason Fb wrote in post #1154202:

validates :password, if: lambda { self.registration_state == "registered" }

This is an interesting idea. In my case, user attributes change after creation only if the user updates his profile, or when he clicks on the "forgot my password" link and I have to create a new password for him.

I could therefore create a flag password_validation_required, which is true by default, but can be switched off on demand (because my application knows the cases, where validation is not necessary).

The question is: Would you consider a good idea to automatically switch on password validation inside the lambda expression? I.e.

lambda {   t=self.password_validation_required   self.password_validation_required=true   t }

or can this potentially lead to problems later on?

Finally, a syntax question: You are writing

  lambda { .... }

I would have written

  -> { ... }

Are both notations exactly the same?

Also, read this book:

Thank you for the recommendation!

Ronald

lambda { t=self.password_validation_required self.password_validation_required=true t }

or can this potentially lead to problems later on?

I would avoid that. Without looking at your whole code I can't really say the right way to do that. However, I recommend you take a look at devise. It's a little top-heavy but it's worth it once you get the hang of it. Also, if you don't want to actually use it, read the source code and see how they do it with password validation.

Finally, a syntax question: You are writing

lambda { .... }

I would have written

-> { ... }

You're right -- they are equivalent in Ruby 2.0 (and to make matters worse I actually switched between Ruby 1 and Ruby 2 syntax in the same line of code).

Probably best to stick with the shorthand if you're using Ruby 2.

I actually implemented my code this way, it uses devise (see devise docs), but adds these to the model:

validate :password_not_blank

def password_not_blank if !self.password.nil? && self.password.blank? errors.add(:password, “can not be blank”) end end

this is completely counter-intuitive to read, although was the only way I could get around the chicken-egg problems created by the fact that password= and password_confirmation= are “settable” attributes on my model but the actual field used by devise is encrypted_password (this part comes from the devise gem).

If I just do validates :password, then it hiccups when you go to save a record anywhere in the app except upon creation (that is, anywhere where password= hasn’t been called on the instance)

Basically it is saying if the password is NOT nil (we aren’t trying to set it in the action), then also make sure it is not actually empty string.

This is a common problem I’ve seen before. Not to beat a dead horse, but does anyone have any other patterns to this kind of a problem?

Thanks,

Jason