password validation triggered even I update non-password attribute

[Ronald Fischer]

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.

--
Posted via http://www.ruby-forum.com/.

[Eric Saupe]

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.

[Ronald Fischer]

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

[Matt Jones]

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

[Jason Fleetwood-Boldt]

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

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-ta...@googlegroups.com.
To post to this group, send email to rubyonra...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/rubyonrails-talk/53fb439d-0a42-482c-b77e-957174e14c0b%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Ronald Fischer]

Matt Jones wrote in post #1154197:

> On Saturday, 2 August 2014 11:41:46 UTC-4, Ruby-Forum.com User wrote:

>> > 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. :-(

> 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 Fischer]

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!

[Jason Fleetwood-Boldt]

On Aug 5, 2014, at 2:44 AM, Ronald Fischer <li...@ruby-forum.com> wrote:

> 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.

[Jason Fleetwood-Boldt]

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

-- 
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-ta...@googlegroups.com.
To post to this group, send email to rubyonra...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/rubyonrails-talk/7685EEB7-3D84-4296-9178-9AB0AA3672FB%40datatravels.com.