diff --git a/.travis.yml b/.travis.yml index bbc447ba..29f27020 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,6 +6,9 @@ sudo: false gemfile: - Gemfile - gemfiles/rails_4.2.gemfile + - gemfiles/rails_5.0.gemfile + - gemfiles/rails_5.1.gemfile + - gemfiles/rails_5.2.gemfile before_install: - wget -O vault.zip -q https://releases.hashicorp.com/vault/${VAULT_VERSION}/vault_${VAULT_VERSION}_linux_amd64.zip diff --git a/Appraisals b/Appraisals index 72063733..e91a0573 100644 --- a/Appraisals +++ b/Appraisals @@ -1,3 +1,12 @@ appraise "rails-4.2" do gem "rails", "~> 4.2.8" end +appraise "rails-5.0" do + gem "rails", "~> 5.0.0" +end +appraise "rails-5.1" do + gem "rails", "~> 5.1.0" +end +appraise "rails-5.2" do + gem "rails", "~> 5.2.0" +end diff --git a/CHANGELOG.md b/CHANGELOG.md index 31ebd26e..5f772833 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Vault Rails Changelog +## Unreleased + +BUG FIXES +- Actually persist encrypted attributes when using + `vault_persist_before_save!` in rails 5.2 + ## v0.7.0 (October 24, 2018) NOTABLE CHANGES diff --git a/gemfiles/rails_5.0.gemfile b/gemfiles/rails_5.0.gemfile new file mode 100644 index 00000000..10f52e7a --- /dev/null +++ b/gemfiles/rails_5.0.gemfile @@ -0,0 +1,7 @@ +# This file was generated by Appraisal + +source "https://rubygems.org" + +gem "rails", "~> 5.0.0" + +gemspec path: "../" diff --git a/gemfiles/rails_5.0.gemfile.lock b/gemfiles/rails_5.0.gemfile.lock new file mode 100644 index 00000000..86b5c9b1 --- /dev/null +++ b/gemfiles/rails_5.0.gemfile.lock @@ -0,0 +1,155 @@ +PATH + remote: .. + specs: + fc-vault-rails (0.7.0) + activerecord (>= 4.2.8, < 6.0) + vault (~> 0.7) + +GEM + remote: https://rubygems.org/ + specs: + actioncable (5.0.7) + actionpack (= 5.0.7) + nio4r (>= 1.2, < 3.0) + websocket-driver (~> 0.6.1) + actionmailer (5.0.7) + actionpack (= 5.0.7) + actionview (= 5.0.7) + activejob (= 5.0.7) + mail (~> 2.5, >= 2.5.4) + rails-dom-testing (~> 2.0) + actionpack (5.0.7) + actionview (= 5.0.7) + activesupport (= 5.0.7) + rack (~> 2.0) + rack-test (~> 0.6.3) + rails-dom-testing (~> 2.0) + rails-html-sanitizer (~> 1.0, >= 1.0.2) + actionview (5.0.7) + activesupport (= 5.0.7) + builder (~> 3.1) + erubis (~> 2.7.0) + rails-dom-testing (~> 2.0) + rails-html-sanitizer (~> 1.0, >= 1.0.3) + activejob (5.0.7) + activesupport (= 5.0.7) + globalid (>= 0.3.6) + activemodel (5.0.7) + activesupport (= 5.0.7) + activerecord (5.0.7) + activemodel (= 5.0.7) + activesupport (= 5.0.7) + arel (~> 7.0) + activesupport (5.0.7) + concurrent-ruby (~> 1.0, >= 1.0.2) + i18n (>= 0.7, < 2) + minitest (~> 5.1) + tzinfo (~> 1.1) + appraisal (2.2.0) + bundler + rake + thor (>= 0.14.0) + arel (7.1.4) + aws-sigv4 (1.0.3) + builder (3.2.3) + byebug (10.0.2) + coderay (1.1.2) + concurrent-ruby (1.1.3) + crass (1.0.4) + diff-lcs (1.3) + erubis (2.7.0) + globalid (0.4.1) + activesupport (>= 4.2.0) + i18n (1.1.1) + concurrent-ruby (~> 1.0) + loofah (2.2.3) + crass (~> 1.0.2) + nokogiri (>= 1.5.9) + mail (2.7.1) + mini_mime (>= 0.1.1) + method_source (0.9.2) + mini_mime (1.0.1) + mini_portile2 (2.3.0) + minitest (5.11.3) + nio4r (2.3.1) + nokogiri (1.8.5) + mini_portile2 (~> 2.3.0) + pry (0.12.1) + coderay (~> 1.1.0) + method_source (~> 0.9.0) + rack (2.0.6) + rack-test (0.6.3) + rack (>= 1.0) + rails (5.0.7) + actioncable (= 5.0.7) + actionmailer (= 5.0.7) + actionpack (= 5.0.7) + actionview (= 5.0.7) + activejob (= 5.0.7) + activemodel (= 5.0.7) + activerecord (= 5.0.7) + activesupport (= 5.0.7) + bundler (>= 1.3.0) + railties (= 5.0.7) + sprockets-rails (>= 2.0.0) + rails-dom-testing (2.0.3) + activesupport (>= 4.2.0) + nokogiri (>= 1.6) + rails-html-sanitizer (1.0.4) + loofah (~> 2.2, >= 2.2.2) + railties (5.0.7) + actionpack (= 5.0.7) + activesupport (= 5.0.7) + method_source + rake (>= 0.8.7) + thor (>= 0.18.1, < 2.0) + rake (10.5.0) + rspec (3.8.0) + rspec-core (~> 3.8.0) + rspec-expectations (~> 3.8.0) + rspec-mocks (~> 3.8.0) + rspec-core (3.8.0) + rspec-support (~> 3.8.0) + rspec-expectations (3.8.2) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.8.0) + rspec-mocks (3.8.0) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.8.0) + rspec-support (3.8.0) + sprockets (3.7.2) + concurrent-ruby (~> 1.0) + rack (> 1, < 3) + sprockets-rails (3.2.1) + actionpack (>= 4.0) + activesupport (>= 4.0) + sprockets (>= 3.0.0) + sqlite3 (1.3.13) + thor (0.20.3) + thread_safe (0.3.6) + tzinfo (1.2.5) + thread_safe (~> 0.1) + vault (0.12.0) + aws-sigv4 + websocket-driver (0.6.5) + websocket-extensions (>= 0.1.0) + websocket-extensions (0.1.3) + wwtd (1.3.0) + +PLATFORMS + ruby + +DEPENDENCIES + appraisal (~> 2.1) + bundler + byebug + fc-vault-rails! + pry + rails (~> 5.0.0) + rake (~> 10.0) + rspec (~> 3.2) + sqlite3 + wwtd + +BUNDLED WITH + 1.16.6 diff --git a/gemfiles/rails_5.1.gemfile b/gemfiles/rails_5.1.gemfile new file mode 100644 index 00000000..6100e830 --- /dev/null +++ b/gemfiles/rails_5.1.gemfile @@ -0,0 +1,7 @@ +# This file was generated by Appraisal + +source "https://rubygems.org" + +gem "rails", "~> 5.1.0" + +gemspec path: "../" diff --git a/gemfiles/rails_5.1.gemfile.lock b/gemfiles/rails_5.1.gemfile.lock new file mode 100644 index 00000000..b1e8c9af --- /dev/null +++ b/gemfiles/rails_5.1.gemfile.lock @@ -0,0 +1,155 @@ +PATH + remote: .. + specs: + fc-vault-rails (0.7.0) + activerecord (>= 4.2.8, < 6.0) + vault (~> 0.7) + +GEM + remote: https://rubygems.org/ + specs: + actioncable (5.1.6) + actionpack (= 5.1.6) + nio4r (~> 2.0) + websocket-driver (~> 0.6.1) + actionmailer (5.1.6) + actionpack (= 5.1.6) + actionview (= 5.1.6) + activejob (= 5.1.6) + mail (~> 2.5, >= 2.5.4) + rails-dom-testing (~> 2.0) + actionpack (5.1.6) + actionview (= 5.1.6) + activesupport (= 5.1.6) + rack (~> 2.0) + rack-test (>= 0.6.3) + rails-dom-testing (~> 2.0) + rails-html-sanitizer (~> 1.0, >= 1.0.2) + actionview (5.1.6) + activesupport (= 5.1.6) + builder (~> 3.1) + erubi (~> 1.4) + rails-dom-testing (~> 2.0) + rails-html-sanitizer (~> 1.0, >= 1.0.3) + activejob (5.1.6) + activesupport (= 5.1.6) + globalid (>= 0.3.6) + activemodel (5.1.6) + activesupport (= 5.1.6) + activerecord (5.1.6) + activemodel (= 5.1.6) + activesupport (= 5.1.6) + arel (~> 8.0) + activesupport (5.1.6) + concurrent-ruby (~> 1.0, >= 1.0.2) + i18n (>= 0.7, < 2) + minitest (~> 5.1) + tzinfo (~> 1.1) + appraisal (2.2.0) + bundler + rake + thor (>= 0.14.0) + arel (8.0.0) + aws-sigv4 (1.0.3) + builder (3.2.3) + byebug (10.0.2) + coderay (1.1.2) + concurrent-ruby (1.1.3) + crass (1.0.4) + diff-lcs (1.3) + erubi (1.7.1) + globalid (0.4.1) + activesupport (>= 4.2.0) + i18n (1.1.1) + concurrent-ruby (~> 1.0) + loofah (2.2.3) + crass (~> 1.0.2) + nokogiri (>= 1.5.9) + mail (2.7.1) + mini_mime (>= 0.1.1) + method_source (0.9.2) + mini_mime (1.0.1) + mini_portile2 (2.3.0) + minitest (5.11.3) + nio4r (2.3.1) + nokogiri (1.8.5) + mini_portile2 (~> 2.3.0) + pry (0.12.1) + coderay (~> 1.1.0) + method_source (~> 0.9.0) + rack (2.0.6) + rack-test (1.1.0) + rack (>= 1.0, < 3) + rails (5.1.6) + actioncable (= 5.1.6) + actionmailer (= 5.1.6) + actionpack (= 5.1.6) + actionview (= 5.1.6) + activejob (= 5.1.6) + activemodel (= 5.1.6) + activerecord (= 5.1.6) + activesupport (= 5.1.6) + bundler (>= 1.3.0) + railties (= 5.1.6) + sprockets-rails (>= 2.0.0) + rails-dom-testing (2.0.3) + activesupport (>= 4.2.0) + nokogiri (>= 1.6) + rails-html-sanitizer (1.0.4) + loofah (~> 2.2, >= 2.2.2) + railties (5.1.6) + actionpack (= 5.1.6) + activesupport (= 5.1.6) + method_source + rake (>= 0.8.7) + thor (>= 0.18.1, < 2.0) + rake (10.5.0) + rspec (3.8.0) + rspec-core (~> 3.8.0) + rspec-expectations (~> 3.8.0) + rspec-mocks (~> 3.8.0) + rspec-core (3.8.0) + rspec-support (~> 3.8.0) + rspec-expectations (3.8.2) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.8.0) + rspec-mocks (3.8.0) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.8.0) + rspec-support (3.8.0) + sprockets (3.7.2) + concurrent-ruby (~> 1.0) + rack (> 1, < 3) + sprockets-rails (3.2.1) + actionpack (>= 4.0) + activesupport (>= 4.0) + sprockets (>= 3.0.0) + sqlite3 (1.3.13) + thor (0.20.3) + thread_safe (0.3.6) + tzinfo (1.2.5) + thread_safe (~> 0.1) + vault (0.12.0) + aws-sigv4 + websocket-driver (0.6.5) + websocket-extensions (>= 0.1.0) + websocket-extensions (0.1.3) + wwtd (1.3.0) + +PLATFORMS + ruby + +DEPENDENCIES + appraisal (~> 2.1) + bundler + byebug + fc-vault-rails! + pry + rails (~> 5.1.0) + rake (~> 10.0) + rspec (~> 3.2) + sqlite3 + wwtd + +BUNDLED WITH + 1.16.6 diff --git a/gemfiles/rails_5.2.gemfile b/gemfiles/rails_5.2.gemfile new file mode 100644 index 00000000..5a706dcb --- /dev/null +++ b/gemfiles/rails_5.2.gemfile @@ -0,0 +1,7 @@ +# This file was generated by Appraisal + +source "https://rubygems.org" + +gem "rails", "~> 5.2.0" + +gemspec path: "../" diff --git a/gemfiles/rails_5.2.gemfile.lock b/gemfiles/rails_5.2.gemfile.lock new file mode 100644 index 00000000..86a01d6c --- /dev/null +++ b/gemfiles/rails_5.2.gemfile.lock @@ -0,0 +1,163 @@ +PATH + remote: .. + specs: + fc-vault-rails (0.7.0) + activerecord (>= 4.2.8, < 6.0) + vault (~> 0.7) + +GEM + remote: https://rubygems.org/ + specs: + actioncable (5.2.1) + actionpack (= 5.2.1) + nio4r (~> 2.0) + websocket-driver (>= 0.6.1) + actionmailer (5.2.1) + actionpack (= 5.2.1) + actionview (= 5.2.1) + activejob (= 5.2.1) + mail (~> 2.5, >= 2.5.4) + rails-dom-testing (~> 2.0) + actionpack (5.2.1) + actionview (= 5.2.1) + activesupport (= 5.2.1) + rack (~> 2.0) + rack-test (>= 0.6.3) + rails-dom-testing (~> 2.0) + rails-html-sanitizer (~> 1.0, >= 1.0.2) + actionview (5.2.1) + activesupport (= 5.2.1) + builder (~> 3.1) + erubi (~> 1.4) + rails-dom-testing (~> 2.0) + rails-html-sanitizer (~> 1.0, >= 1.0.3) + activejob (5.2.1) + activesupport (= 5.2.1) + globalid (>= 0.3.6) + activemodel (5.2.1) + activesupport (= 5.2.1) + activerecord (5.2.1) + activemodel (= 5.2.1) + activesupport (= 5.2.1) + arel (>= 9.0) + activestorage (5.2.1) + actionpack (= 5.2.1) + activerecord (= 5.2.1) + marcel (~> 0.3.1) + activesupport (5.2.1) + concurrent-ruby (~> 1.0, >= 1.0.2) + i18n (>= 0.7, < 2) + minitest (~> 5.1) + tzinfo (~> 1.1) + appraisal (2.2.0) + bundler + rake + thor (>= 0.14.0) + arel (9.0.0) + aws-sigv4 (1.0.3) + builder (3.2.3) + byebug (10.0.2) + coderay (1.1.2) + concurrent-ruby (1.1.3) + crass (1.0.4) + diff-lcs (1.3) + erubi (1.7.1) + globalid (0.4.1) + activesupport (>= 4.2.0) + i18n (1.1.1) + concurrent-ruby (~> 1.0) + loofah (2.2.3) + crass (~> 1.0.2) + nokogiri (>= 1.5.9) + mail (2.7.1) + mini_mime (>= 0.1.1) + marcel (0.3.3) + mimemagic (~> 0.3.2) + method_source (0.9.2) + mimemagic (0.3.2) + mini_mime (1.0.1) + mini_portile2 (2.3.0) + minitest (5.11.3) + nio4r (2.3.1) + nokogiri (1.8.5) + mini_portile2 (~> 2.3.0) + pry (0.12.1) + coderay (~> 1.1.0) + method_source (~> 0.9.0) + rack (2.0.6) + rack-test (1.1.0) + rack (>= 1.0, < 3) + rails (5.2.1) + actioncable (= 5.2.1) + actionmailer (= 5.2.1) + actionpack (= 5.2.1) + actionview (= 5.2.1) + activejob (= 5.2.1) + activemodel (= 5.2.1) + activerecord (= 5.2.1) + activestorage (= 5.2.1) + activesupport (= 5.2.1) + bundler (>= 1.3.0) + railties (= 5.2.1) + sprockets-rails (>= 2.0.0) + rails-dom-testing (2.0.3) + activesupport (>= 4.2.0) + nokogiri (>= 1.6) + rails-html-sanitizer (1.0.4) + loofah (~> 2.2, >= 2.2.2) + railties (5.2.1) + actionpack (= 5.2.1) + activesupport (= 5.2.1) + method_source + rake (>= 0.8.7) + thor (>= 0.19.0, < 2.0) + rake (10.5.0) + rspec (3.8.0) + rspec-core (~> 3.8.0) + rspec-expectations (~> 3.8.0) + rspec-mocks (~> 3.8.0) + rspec-core (3.8.0) + rspec-support (~> 3.8.0) + rspec-expectations (3.8.2) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.8.0) + rspec-mocks (3.8.0) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.8.0) + rspec-support (3.8.0) + sprockets (3.7.2) + concurrent-ruby (~> 1.0) + rack (> 1, < 3) + sprockets-rails (3.2.1) + actionpack (>= 4.0) + activesupport (>= 4.0) + sprockets (>= 3.0.0) + sqlite3 (1.3.13) + thor (0.20.3) + thread_safe (0.3.6) + tzinfo (1.2.5) + thread_safe (~> 0.1) + vault (0.12.0) + aws-sigv4 + websocket-driver (0.7.0) + websocket-extensions (>= 0.1.0) + websocket-extensions (0.1.3) + wwtd (1.3.0) + +PLATFORMS + ruby + +DEPENDENCIES + appraisal (~> 2.1) + bundler + byebug + fc-vault-rails! + pry + rails (~> 5.2.0) + rake (~> 10.0) + rspec (~> 3.2) + sqlite3 + wwtd + +BUNDLED WITH + 1.16.6 diff --git a/lib/vault/encrypted_model.rb b/lib/vault/encrypted_model.rb index aea98b9f..a3743cdb 100644 --- a/lib/vault/encrypted_model.rb +++ b/lib/vault/encrypted_model.rb @@ -228,7 +228,7 @@ def __vault_load_attribute!(attribute, options) # on this model. # @return [true] def __vault_persist_attributes! - changes = __vault_encrypt_attributes! + changes = __vault_encrypt_attributes!(in_after_save: true) # If there are any changes to the model, update them all at once, # skipping any callbacks and validation. This is okay, because we are @@ -238,11 +238,11 @@ def __vault_persist_attributes! true end - def __vault_encrypt_attributes! + def __vault_encrypt_attributes!(in_after_save: false) changes = {} self.class.__vault_attributes.each do |attribute, options| - if c = self.__vault_encrypt_attribute!(attribute, options) + if c = self.__vault_encrypt_attribute!(attribute, options, in_after_save: in_after_save) changes.merge!(c) end end @@ -252,11 +252,11 @@ def __vault_encrypt_attributes! # Encrypt a single attribute using Vault and persist back onto the # encrypted attribute value. - def __vault_encrypt_attribute!(attribute, options) + def __vault_encrypt_attribute!(attribute, options, in_after_save: false) # Only persist changed attributes to minimize requests - this helps # minimize the number of requests to Vault. - if ActiveRecord.version >= Gem::Version.new('5.2.0') + if in_after_save && ActiveRecord.version >= Gem::Version.new('5.2.0') # ActiveRecord 5.2 changes the behaviour of `changed` in `after_*` callbacks # https://www.ombulabs.com/blog/rails/upgrades/active-record-5-1-api-changes.html # https://api.rubyonrails.org/classes/ActiveRecord/AttributeMethods/Dirty.html#method-i-saved_change_to_attribute diff --git a/spec/unit/encrypted_model_spec.rb b/spec/unit/encrypted_model_spec.rb index 86412957..78ac780f 100644 --- a/spec/unit/encrypted_model_spec.rb +++ b/spec/unit/encrypted_model_spec.rb @@ -91,8 +91,11 @@ describe '#vault_persist_before_save!' do context "when not used" do + # Person hasn't had `vault_persist_before_save!` called on it + let(:model_class) { Person } + it "the model has an after_save callback" do - save_callbacks = Person._save_callbacks.select do |cb| + save_callbacks = model_class._save_callbacks.select do |cb| cb.filter == :__vault_persist_attributes! end @@ -104,17 +107,29 @@ expect(persist_callback.kind).to eq :after end - it 'calls the correnct callback' do - eager_person = Person.new(ssn: '123-45-6789') - expect(eager_person).to receive(:__vault_persist_attributes!) + it 'calls the correct callback' do + record = model_class.new(ssn: '123-45-6789') + expect(record).to receive(:__vault_persist_attributes!) + + record.save + end + + it 'encrypts the attribute if it has been saved' do + record = model_class.new(ssn: '123-45-6789') + expect(Vault::Rails).to receive(:encrypt).with('transit', 'dummy_people_ssn', anything, anything, anything).and_call_original + + record.save - eager_person.save + expect(record.ssn_encrypted).not_to be_nil end end context "when used" do + # EagerPerson has had `vault_persist_before_save!` called on it + let(:model_class) { EagerPerson } + it "the model does not have an after_save callback" do - save_callbacks = EagerPerson._save_callbacks.select do |cb| + save_callbacks = model_class._save_callbacks.select do |cb| cb.filter == :__vault_persist_attributes! end @@ -122,7 +137,7 @@ end it "the model has a before_save callback" do - save_callbacks = EagerPerson._save_callbacks.select do |cb| + save_callbacks = model_class._save_callbacks.select do |cb| cb.filter == :__vault_encrypt_attributes! end @@ -135,11 +150,20 @@ end it 'calls the correct callback' do - eager_person = EagerPerson.new(ssn: '123-45-6789') - expect(eager_person).not_to receive(:__vault_persist_attributes!) - expect(eager_person).to receive(:__vault_encrypt_attributes!) + record = model_class.new(ssn: '123-45-6789') + expect(record).not_to receive(:__vault_persist_attributes!) + expect(record).to receive(:__vault_encrypt_attributes!) + + record.save + end + + it 'encrypts the attribute if it has been saved' do + record = model_class.new(ssn: '123-45-6789') + expect(Vault::Rails).to receive(:encrypt).with('transit', 'dummy_people_ssn',anything,anything,anything).and_call_original + + record.save - eager_person.save + expect(record.ssn_encrypted).not_to be_nil end end end