ankane / lockbox

Modern encryption for Ruby and Rails
MIT License
1.44k stars 68 forks source link

Idea: Don't return virtual attributes in attribute methods unless the ciphertext attribute is present #186

Open 4ndypanda opened 8 months ago

4ndypanda commented 8 months ago

I was also annoyed by this issue since I constantly run into ActiveModel::MissingAttributeError when doing stuff like

# User class defined in test/support/active_record.rb
User.create!(email: "test@example.org")
user = User.select('id').last

# Normally this is done with some metaprogramming, just showing an example
# that raises ActiveModel::MissingAttributeError
user.email if user.has_attribute?("email")

What are your thoughts on overriding #init_with_attributes in ActiveRecord::Core? Before ever setting the @attributes instance variable for the model, we could filter and check if the ciphertext attribute is present

# From ActiveRecord::Core
def init_with_attribute(attributes, new_record = false)
  # filter out lockbox attributes that don't have ciphertext attribute loaded
  attributes = attributes.reject do |attribute, _value|
    self.lockbox_attributes[attribute.to_sym].present? &&
    !attributes.key?(
      self.lockbox_attributes[attribute.to_sym][:encrypted_attribute],
    )
  end
  super(attributes, new_record)
end

I was trying to see where else to do it. The caller here takes the attributes from the database result set and merges it with the self.class._default_attributes that includes the virtual attributes.

# Given a class, an attributes hash, +instantiate_instance_of+ returns a
# new instance of the class. Accepts only keys as strings.
def instantiate_instance_of(klass, attributes, column_types = {}, &block)
  attributes = klass.attributes_builder.build_from_database(attributes, column_types)
  klass.allocate.init_with_attributes(attributes, &block)
end

Overriding attributes_builder.build_from_database (source) is another option.

What are your thoughts?

4ndypanda commented 8 months ago

A tricky thing is if people do want virtual attributes with a ciphertext virtual attribute (not from an ActiveRecord column)

class User < ActiveRecord::Base
  # This is never saved to the database, idk why one
  # would want this though.
  attribute :foo_ciphertext
  has_encrypted :foo
end

This is one case where it probably should still show up in the attribute methods attributes, attribute_names, has_attribute?.

ankane commented 8 months ago

Hey @4ndypanda, will take a look at this as part of 2.0 since it's a breaking change.