From 8c1687bbf8dd518d64fc7180b33c1cb57f29a69a Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Mon, 4 Mar 2013 18:56:05 +0100 Subject: [PATCH] `has_secure_password` is not invalid when assigning empty Strings. Closes #9535. With 692b3b6 the `password=` setter does no longer set blank passwords. This triggered validation errors when assigning empty Strings to `password` and `password_confirmation`. This patch only sets the confirmation if it is not `blank?`. --- activemodel/CHANGELOG.md | 10 ++++++++++ activemodel/lib/active_model/secure_password.rb | 10 ++++++++-- activemodel/test/cases/secure_password_test.rb | 6 ++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index e518334d5d..8c54ec3d45 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -1,5 +1,15 @@ ## Rails 4.0.0 (unreleased) ## +* `has_secure_password` does not fail the confirmation validation + when assigning empty String to `password` and `password_confirmation`. + + Example: + + # given User has_secure_password. + @user.password = "" + @user.password_confirmation = "" + @user.valid?(:update) # used to be false + * `validates_confirmation_of` does not override writer methods for the confirmation attribute if no reader is defined. diff --git a/activemodel/lib/active_model/secure_password.rb b/activemodel/lib/active_model/secure_password.rb index 6644b60609..9324a1ad0a 100644 --- a/activemodel/lib/active_model/secure_password.rb +++ b/activemodel/lib/active_model/secure_password.rb @@ -48,6 +48,8 @@ module ActiveModel attr_reader :password + include InstanceMethodsOnActivation + if options.fetch(:validations, true) validates_confirmation_of :password validates_presence_of :password, :on => :create @@ -55,8 +57,6 @@ module ActiveModel before_create { raise "Password digest missing on new record" if password_digest.blank? } end - include InstanceMethodsOnActivation - if respond_to?(:attributes_protected_by_default) def self.attributes_protected_by_default #:nodoc: super + ['password_digest'] @@ -99,6 +99,12 @@ module ActiveModel self.password_digest = BCrypt::Password.create(unencrypted_password, cost: cost) end end + + def password_confirmation=(unencrypted_password) + unless unencrypted_password.blank? + @password_confirmation = unencrypted_password + end + end end end end diff --git a/activemodel/test/cases/secure_password_test.rb b/activemodel/test/cases/secure_password_test.rb index 7783bb25d5..02cd3b8a93 100644 --- a/activemodel/test/cases/secure_password_test.rb +++ b/activemodel/test/cases/secure_password_test.rb @@ -88,4 +88,10 @@ class SecurePasswordTest < ActiveModel::TestCase @user.password = "secret" assert_equal BCrypt::Engine::MIN_COST, @user.password_digest.cost end + + test "blank password_confirmation does not result in a confirmation error" do + @user.password = "" + @user.password_confirmation = "" + assert @user.valid?(:update), "user should be valid" + end end