mirror of
https://github.com/thoughtbot/shoulda-matchers.git
synced 2022-11-09 12:01:38 -05:00
Validating presence of a secure password may raise an error
If you have a model that declares `has_secure_password` and you also have a presence validation on the password, and you write a test against this validation using an instance of your model where the password is already set, then your test will fail. This is because has_secure_password (at least on Rails 4) defines #password= such that if it is given nil, then the password will not be overwritten with nil. This interferes with how our validate_presence_of matcher works. Unfortunately there is not a great way to get around this (using \#write_attribute won't work, either). So in this case we raise a helpful error message that instructs the user to use an empty record against `validates_presence_of`.
This commit is contained in:
parent
bc110f73b9
commit
6173a160ac
6 changed files with 110 additions and 3 deletions
7
NEWS.md
7
NEWS.md
|
@ -37,6 +37,13 @@
|
||||||
`greater_than`, `greater_than_or_equal_to`, `less_than`, `less_than_or_equal_
|
`greater_than`, `greater_than_or_equal_to`, `less_than`, `less_than_or_equal_
|
||||||
to` or `equal_to` is not appropriate.
|
to` or `equal_to` is not appropriate.
|
||||||
|
|
||||||
|
* Change `validate_presence_of` under Rails 4 so that if you are using it with a
|
||||||
|
user whose model `has_secure_password` and whose password is set to a value,
|
||||||
|
you will be instructed to use a user whose password is blank instead. The
|
||||||
|
reason for this change is due to the fact that Rails 4's version of
|
||||||
|
`has_secure_password` defines #password= such that `nil` will be ignored,
|
||||||
|
which interferes with how `validate_presence_of` works.
|
||||||
|
|
||||||
# v 2.5.0
|
# v 2.5.0
|
||||||
|
|
||||||
* Fix Rails/Test::Unit integration to ensure that the test case classes we are
|
* Fix Rails/Test::Unit integration to ensure that the test case classes we are
|
||||||
|
|
|
@ -68,7 +68,7 @@ module Shoulda # :nodoc:
|
||||||
|
|
||||||
values_to_match.none? do |value|
|
values_to_match.none? do |value|
|
||||||
self.value = value
|
self.value = value
|
||||||
instance.__send__("#{attribute_to_set}=", value)
|
set_and_double_check_attribute!(attribute_to_set, value)
|
||||||
errors_match?
|
errors_match?
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
@ -93,6 +93,24 @@ module Shoulda # :nodoc:
|
||||||
:instance, :attribute_to_set, :attribute_to_check_message_against,
|
:instance, :attribute_to_set, :attribute_to_check_message_against,
|
||||||
:context, :value, :matched_error
|
:context, :value, :matched_error
|
||||||
|
|
||||||
|
def set_and_double_check_attribute!(attribute_name, value)
|
||||||
|
instance.__send__("#{attribute_name}=", value)
|
||||||
|
|
||||||
|
if value.nil?
|
||||||
|
ensure_attribute_was_cleared!(attribute_name)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def ensure_attribute_was_cleared!(attribute_name)
|
||||||
|
if instance.respond_to?(attribute_name)
|
||||||
|
actual_value = instance.__send__(attribute_name)
|
||||||
|
|
||||||
|
if !actual_value.nil?
|
||||||
|
raise Shoulda::Matchers::ActiveModel::CouldNotClearAttribute.create(actual_value)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
def errors_match?
|
def errors_match?
|
||||||
has_messages? && errors_for_attribute_match?
|
has_messages? && errors_for_attribute_match?
|
||||||
end
|
end
|
||||||
|
|
|
@ -2,7 +2,46 @@ module Shoulda # :nodoc:
|
||||||
module Matchers
|
module Matchers
|
||||||
module ActiveModel # :nodoc:
|
module ActiveModel # :nodoc:
|
||||||
class CouldNotDetermineValueOutsideOfArray < RuntimeError; end
|
class CouldNotDetermineValueOutsideOfArray < RuntimeError; end
|
||||||
|
|
||||||
class NonNullableBooleanError < Shoulda::Matchers::Error; end
|
class NonNullableBooleanError < Shoulda::Matchers::Error; end
|
||||||
|
|
||||||
|
class CouldNotClearAttribute < Shoulda::Matchers::Error
|
||||||
|
def self.create(actual_value)
|
||||||
|
super(actual_value: actual_value)
|
||||||
|
end
|
||||||
|
|
||||||
|
attr_accessor :actual_value
|
||||||
|
|
||||||
|
def message
|
||||||
|
"Expected value to be nil, but was #{actual_value.inspect}."
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
class CouldNotSetPasswordError < Shoulda::Matchers::Error
|
||||||
|
def self.create(model)
|
||||||
|
super(model: model)
|
||||||
|
end
|
||||||
|
|
||||||
|
attr_accessor :model
|
||||||
|
|
||||||
|
def message
|
||||||
|
<<-EOT.strip
|
||||||
|
The validation failed because your #{model_name} model declares `has_secure_password`, and
|
||||||
|
`validate_presence_of` was called on a #{record_name} which has `password` already set to a value.
|
||||||
|
Please use a #{record_name} with an empty `password` instead.
|
||||||
|
EOT
|
||||||
|
end
|
||||||
|
|
||||||
|
private
|
||||||
|
|
||||||
|
def model_name
|
||||||
|
model.name
|
||||||
|
end
|
||||||
|
|
||||||
|
def record_name
|
||||||
|
model_name.humanize.downcase
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -20,7 +20,6 @@ module Shoulda # :nodoc:
|
||||||
end
|
end
|
||||||
|
|
||||||
class ValidatePresenceOfMatcher < ValidationMatcher # :nodoc:
|
class ValidatePresenceOfMatcher < ValidationMatcher # :nodoc:
|
||||||
|
|
||||||
def with_message(message)
|
def with_message(message)
|
||||||
@expected_message = message if message
|
@expected_message = message if message
|
||||||
self
|
self
|
||||||
|
@ -30,6 +29,12 @@ module Shoulda # :nodoc:
|
||||||
super(subject)
|
super(subject)
|
||||||
@expected_message ||= :blank
|
@expected_message ||= :blank
|
||||||
disallows_value_of(blank_value, @expected_message)
|
disallows_value_of(blank_value, @expected_message)
|
||||||
|
rescue Shoulda::Matchers::ActiveModel::CouldNotClearAttribute => error
|
||||||
|
if @attribute == :password
|
||||||
|
raise Shoulda::Matchers::ActiveModel::CouldNotSetPasswordError.create(subject.class)
|
||||||
|
else
|
||||||
|
raise error
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def description
|
def description
|
||||||
|
|
|
@ -1,5 +1,24 @@
|
||||||
module Shoulda
|
module Shoulda
|
||||||
module Matchers
|
module Matchers
|
||||||
class Error < StandardError; end
|
class Error < StandardError
|
||||||
|
def self.create(attributes)
|
||||||
|
allocate.tap do |error|
|
||||||
|
attributes.each do |name, value|
|
||||||
|
error.__send__("#{name}=", value)
|
||||||
|
end
|
||||||
|
|
||||||
|
error.__send__(:initialize)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def initialize(*args)
|
||||||
|
super
|
||||||
|
@message = message
|
||||||
|
end
|
||||||
|
|
||||||
|
def message
|
||||||
|
""
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -144,6 +144,25 @@ describe Shoulda::Matchers::ActiveModel::ValidatePresenceOfMatcher do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
if rails_4_x?
|
||||||
|
context 'against a pre-set password in a model that has_secure_password' do
|
||||||
|
it 'raises an error to instruct the user' do
|
||||||
|
user_class = define_model :user, password_digest: :string do
|
||||||
|
has_secure_password
|
||||||
|
validates_presence_of :password
|
||||||
|
end
|
||||||
|
|
||||||
|
user = user_class.new
|
||||||
|
user.password = 'something'
|
||||||
|
|
||||||
|
error_class = Shoulda::Matchers::ActiveModel::CouldNotSetPasswordError
|
||||||
|
expect {
|
||||||
|
expect(user).to validate_presence_of(:password)
|
||||||
|
}.to raise_error(error_class)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
def matcher
|
def matcher
|
||||||
validate_presence_of(:attr)
|
validate_presence_of(:attr)
|
||||||
end
|
end
|
||||||
|
|
Loading…
Add table
Reference in a new issue