hashicorp / vault-rails

A Rails plugin for easily integrating Vault secrets
Mozilla Public License 2.0
339 stars 53 forks source link

In Rails versions > 5.0, calling save in an after_create hook means that vault_attributes are not persisted #111

Open radditude opened 4 years ago

radditude commented 4 years ago

This is a pretty fun bit of confusing behavior!

Let's say you have a model that looks like this:

class Foo
  include Vault::EncryptedModel

  vault_attribute :secret_thing

  after_create :set_unrelated_column

  def set_unrelated_column
    unrelated_column = true
    save!
  end
end

This is obviously a contrived example, but let's run with it since there are all sorts of reasons someone might want to set an attribute after create.

Since the after_create runs before vault-rails's after_save, you will experience the following weird behavior:

foo = Foo.create(secret_thing: "secret")
> true

foo.reload
> true

foo.secret_thing
> nil

foo.secret_thing_encrypted
> nil

foo.update(secret_thing: "secret")
> true

foo.reload
> true

foo.secret_thing
> "secret"

foo.secret_thing_encrypted
> "[encrypted string]"

A repro (note all the failing non-Rails-5.0 tests)

Basically, when vault-rails checks to see if the previous save included a change to a vault attribute, it assumes that the most recent save call is the one that triggered the callback. However, in this case, the most recent save call is in the after_create hook, and didn't include any changes to vault attributes.

This is probably why the Rails guides include some very strongly cautionary words about updating attributes in callbacks:

Care should be taken in callbacks to avoid updating attributes. For example, avoid running update(attribute: "value") and similar code during callbacks. This can alter the state of the model and may result in unexpected side effects during commit. Instead, you should try to assign values in the before_create or earlier callbacks.


I'm not actually sure of the best way to resolve this; one path would be to change vault-rails's after_save to a before_save and let the upcoming save take care of actually persisting the updated encrypted column.

However, there's still the possibility of confusing behavior in that scenario if anyone is modifying vault attributes in a before_update or before_create - since vault-rails's before_save would run first, any changes made to vault attributes in before_update or before_create wouldn't be reflected in the persisted encrypted column.

In the meantime, I hope this issue will help anyone else trying to diagnose a similar problem!