Encrypt attribute values manually without needing save callback

[brandonc]

Description

I'd like vault-rails to set the *_encrypted value attribute without calling save. This way, it can be used with gems like activerecord-import that create many records at once.

Implementation Notes

Splits __vault_persist_attribute!, which is normally called on each attribute after_save, into two methods. The new method, __vault_write_encrypted_attribute!, is also called on each attribute when vault_encrypt_attributes! is called on the model

p = Person.new(ssn: "123-45-6789").vault_encrypt_attributes!
p.ssn_encrypted
"vault:dev:flu/yp9oeYYFgjcZH2hVBA=="

[chrisarcand]

I'm a little uneasy at changing where in the lifecycle the write happens - at least without an explicit opt-in for the change in behavior. Ideally, the writes can happen after save even in an import scenario.

I'd suggest looking into one of these paths:

1. Import, but with callbacks. Either a custom solution here in this gem to allow for it, or: Have you seen this? 👉 https://github.com/instacart/activerecord-import_with_callbacks/blob/main/lib/active_record/import_with_callbacks.rb

Sure, it may be a little slower than your current solution, but I'd trade a little bit of speed for more correctness (allowing for bulk inserts, but callbacks are still called after the database transaction).

2. Encrypt on set, but explicitly opt-in.

Flavor A: Add a flag to opt-in to this new behavior:

class Person < ActiveRecord::Base
  include Vault::EncryptedModel
  vault_attribute :ssn, encrypt_on_set: true  # opt-in for activerecord-import
end

Flavor B: Add an explicit encrypt method to this API

# Then, at the caller, you can explicitly write the encrypted values ahead of the import cycle:
people = 1000.times.map { |i| 
  Person.new(ssn: "123-45-#{i}").tap(&:vault_encrypt_attributes!)
}
Person.import(people)

# This is essentially just publicizing your new `__vault_write_encrypted_attribute!` interface

Any of these paths might yield leaving the default behavior as-is while still allowing for bulk import scenarios.

[brandonc]

@chrisarcand Thanks for the ideas to leave the existing behavior intact. I was hesitant to expand the public interface at first but I think it is the right move to add an explicit encrypt method the the model.

I felt the config options like encrypt_on_set weren't suitable because the import use case shouldn't be dependent on the model definition.

[chrisarcand]

Looks reasonable to me! Quick question...

[chrisarcand]

What's this for?

[brandonc]

All rails versions before 7.1 have a direct dependency on concurrent-ruby (We use an older version for testing), but concurrent-ruby versions 1.3.5+ removed the dependency on Logger. When using Rails 7.0- and concurrent-ruby 1.3.5+, you get uninitialized constant ActiveSupport::LoggerThreadSafeLevel::Logger (NameError) when starting tests-- so I just pinned the version to 1.3.4 instead of upgrading the dummy rails installation.

[brandonc]

Added a clarifying comment to the Gemfile. This is not a gem dependency, just a developer one

[chrisarcand]

Nice, got it!

[chrisarcand]

Maybe add a p.persisted? to demo that it isn't saved.

[brandonc]

👍

[chrisarcand]

LGTM!