Skip to content
Browse files

Merge pull request #9545 from senny/9535_secure_password_blank

`has_secure_password` is not invalid when assigning empty Strings
  • Loading branch information...
2 parents 7039fdd + 8c1687b commit cf09ac380e7d786d0d688983fb2080dc693f65a1 @rafaelfranca rafaelfranca committed Mar 7, 2013
View
25 activemodel/CHANGELOG.md
@@ -1,5 +1,30 @@
## 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.
+
+ Example:
+
+ class Blog
+ def title=(new_title)
+ @title = new_title.downcase
+ end
+
+ # previously this would override the setter above.
+ validates_confirmation_of :title
+ end
+
+ *Yves Senn*
## Rails 4.0.0.beta1 (February 25, 2013) ##
View
10 activemodel/lib/active_model/secure_password.rb
@@ -48,15 +48,15 @@ def has_secure_password(options = {})
attr_reader :password
+ include InstanceMethodsOnActivation
+
if options.fetch(:validations, true)
validates_confirmation_of :password
validates_presence_of :password, :on => :create
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 @@ def password=(unencrypted_password)
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
View
6 activemodel/lib/active_model/validations/confirmation.rb
@@ -10,9 +10,13 @@ def validate_each(record, attribute, value)
end
def setup(klass)
- klass.send(:attr_accessor, *attributes.map do |attribute|
+ klass.send(:attr_reader, *attributes.map do |attribute|
:"#{attribute}_confirmation" unless klass.method_defined?(:"#{attribute}_confirmation")
end.compact)
+
+ klass.send(:attr_writer, *attributes.map do |attribute|
+ :"#{attribute}_confirmation" unless klass.method_defined?(:"#{attribute}_confirmation=")
+ end.compact)
end
end
View
6 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
View
31 activemodel/test/cases/validations/confirmation_validation_test.rb
@@ -71,4 +71,35 @@ def test_title_confirmation_with_i18n_attribute
I18n.backend = @old_backend
end
+ test "does not override confirmation reader if present" do
+ klass = Class.new do
+ include ActiveModel::Validations
+
+ def title_confirmation
+ "expected title"
+ end
+
+ validates_confirmation_of :title
+ end
+
+ assert_equal "expected title", klass.new.title_confirmation,
+ "confirmation validation should not override the reader"
+ end
+
+ test "does not override confirmation writer if present" do
+ klass = Class.new do
+ include ActiveModel::Validations
+
+ def title_confirmation=(value)
+ @title_confirmation = "expected title"
+ end
+
+ validates_confirmation_of :title
+ end
+
+ model = klass.new
+ model.title_confirmation = "new title"
+ assert_equal "expected title", model.title_confirmation,
+ "confirmation validation should not override the writer"
+ end
end

0 comments on commit cf09ac3

Please sign in to comment.
Something went wrong with that request. Please try again.