Skip to content

Commit

Permalink
Remove secure password code from uniqueness matcher
Browse files Browse the repository at this point in the history
The uniqueness matcher creates an existing version of the model under
test if one doesn't already exist (so that it can compare this existing
version with an unpersisted version which it modifies in various ways as
it steps through all of the various tests). Back in 2014, it would
create this existing version by simply making a new instance of the
model, setting the attribute under test to some value, and then saving
the instance. However, this step would fail if there were other
attributes on the model that were necessary to save the record.
Specifically, if the model had `has_secure_password` on it and
`password_digest` was not set, then [the record would fail on
save][issue-371], because of a `before_create` that got triggered by
`validates_uniqueness_of` to serve as a check. To fix this, then, it was
necessary to [set `password_digest` to some meaningful value][pr-426]
before saving the record.

Since then, [`has_secure_password` has been updated in
Rails][uniq-validation] so that instead of checking to see that
`password_digest` is set in a `before_create`, it runs a confirmation
validation on `password` (but only if `password` is set). So this step
won't fail anymore (unless `password` is somehow present on the model).
What this means is that we don't need to concern ourselves with password
attributes or digestible attributes in general, so we can remove the old
2014 code from the uniqueness matcher entirely.

[issue-371]: #371
[pr-426]: #426
[uniq-validation]: rails/rails@8ca5923
  • Loading branch information
mcmire committed Jun 1, 2019
1 parent 5bf4a53 commit 9f0def1
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -538,21 +538,12 @@ def find_existing_record

def create_existing_record
@given_record.tap do |existing_record|
ensure_secure_password_set(existing_record)
existing_record.save(validate: false)
end
rescue ::ActiveRecord::StatementInvalid => error
raise ExistingRecordInvalid.create(underlying_exception: error)
end

def ensure_secure_password_set(instance)
Shoulda::Matchers::RailsShim.digestible_attributes_in(instance).
each do |attribute|
instance.send("#{attribute}=", 'password')
instance.send("#{attribute}_confirmation=", 'password')
end
end

def update_existing_record!(value)
if existing_value_read != value
set_attribute_on_existing_record!(@attribute, value)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1053,23 +1053,6 @@ def configure_validation_matcher(matcher)
expect(record).to validate_uniqueness.allow_nil
end
end

if active_record_supports_has_secure_password?
context 'when the model is declared with has_secure_password' do
it 'accepts' do
model = define_model_validating_uniqueness(
validation_options: { allow_nil: true },
additional_attributes: [{ name: :password_digest, type: :string }]
) do |m|
m.has_secure_password
end

record = build_record_from(model, attribute_name => nil)

expect(record).to validate_uniqueness.allow_nil
end
end
end
end

context 'when the validation is not declared with allow_nil' do
Expand Down

0 comments on commit 9f0def1

Please sign in to comment.