has_secure_password allows saving of a new password even when password_confirmation is nil #4146

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants

clizzin commented Dec 23, 2011

Here's the setup:
The schema is a User model with fields first_name:string, last_name:string, email:string, password_digest:string.
The User model has_secure_password. It also specifies attr_accessible :password, :password_confirmation.

I can then do the following at the console:

1.9.2p290 :069 > u = User.create! :first_name => 'Foo', :last_name => 'Bar', :email => 'foobar@foobar.com', :password => 'foobar', :password_confirmation => 'foobar'
   (0.2ms)  SELECT 1 FROM "users" WHERE LOWER("users"."email") = LOWER('foobar@foobar.com') LIMIT 1
Binary data inserted for `string` type on column `password_digest`
  SQL (0.8ms)  INSERT INTO "users" ("created_at", "email", "first_name", "last_name", "password_digest", "updated_at") VALUES (?, ?, ?, ?, ?, ?)  [["created_at", Fri, 23 Dec 2011 12:04:24 UTC +00:00], ["email", "foobar@foobar.com"], ["first_name", "Foo"], ["last_name", "Bar"], ["password_digest", "$2a$10$3DiPzn9jPi.7hHMc82NsmOnh.I88F0jufbtxR4zyllWh.MGH97O2y"], ["updated_at", Fri, 23 Dec 2011 12:04:24 UTC +00:00]]
 => #<User id: 4, created_at: "2011-12-23 12:04:24", updated_at: "2011-12-23 12:04:24", first_name: "Foo", last_name: "Bar", email: "foobar@foobar.com", password_digest: "$2a$10$3DiPzn9jPi.7hHMc82NsmOnh.I88F0jufbtxR4zyllWh..."> 

1.9.2p290 :070 > u = User.find(4)
  User Load (0.3ms)  SELECT "users".* FROM "users" WHERE "users"."id" = ? LIMIT 1  [["id", 4]]
 => #<User id: 4, created_at: "2011-12-23 12:04:24", updated_at: "2011-12-23 12:04:24", first_name: "Foo", last_name: "Bar", email: "foobar@foobar.com", password_digest: "$2a$10$3DiPzn9jPi.7hHMc82NsmOnh.I88F0jufbtxR4zyllWh..."> 

1.9.2p290 :071 > u.password = 'foofoo'
 => "foofoo" 

1.9.2p290 :072 > u.password
 => "foofoo" 

1.9.2p290 :073 > u.password_confirmation
 => nil 

1.9.2p290 :074 > u.password_digest_changed?
 => true 

1.9.2p290 :075 > u.save!
   (0.2ms)  SELECT 1 FROM "users" WHERE (LOWER("users"."email") = LOWER('foobar@foobar.com') AND "users"."id" != 4) LIMIT 1
   (0.4ms)  UPDATE "users" SET "password_digest" = '$2a$10$qjkWkaGYgRH9EogPsj/3AOM3/NiuQrAd79V4THCzqyWxxBhsFTVVS', "updated_at" = '2011-12-23 12:04:44.189911' WHERE "users"."id" = 4
 => true

This seems like a bug to me. has_secure_password should, as I understand it, validate the presence of both password and password_confirmation before saving a new value of password_digest. Even if the presence of password_confirmation is not explicitly validated, the presence of a password is validated and the matching of password and password_confirmation is validated, so allowing a non-nil password with a nil password_confirmation seems like a bug.

Let me know if I'm thinking about this the wrong way.

clizzin commented Dec 23, 2011

I am proud to introduce the DirtyUser to the Rails test suite.

Contributor

josevalim commented Jan 5, 2012

That's how validates confirmation works in Rails. It works with nil so APIs don't need to send a password, nor in console.

http://blog.plataformatec.com.br/2009/08/do-not-burden-your-users-with-validations/

@josevalim josevalim closed this Jan 5, 2012

clizzin commented Jan 5, 2012

Ahhh okay. Thanks for the explanation!

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment