Merge pull request #73 from FundingCircle/move-type-from-proxy-to-vau…

…lt-attribute

Move type from vault attribute proxy to vault attribute

CHANGELOG.md

@@ -1,5 +1,13 @@
 # Vault Rails Changelog
 
+## v0.6.13 (April 12, 2019)
+
+IMPROVEMENTS
+- Add default `StringSerializer`, which will be used when no other serializer is defined
+
+BREAKING CHANGES
+- Move the type option from vault_attribute_proxy to vault_attribute
+
 ## v0.6.12 (March 14, 2019)
 
 NEW FEATURES

gemfiles/rails_4.2.gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: ..
   specs:
-    fc-vault-rails (0.6.12)
+    fc-vault-rails (0.6.13)
       activerecord (>= 4.2, < 5.0)
       vault (~> 0.7)
 

lib/vault/attribute_proxy.rb

@@ -36,23 +36,7 @@ def vault_attribute_proxy(non_encrypted_attribute, encrypted_attribute, options=
         define_method("#{non_encrypted_attribute}=") do |value|
           super(value) unless options[:encrypted_attribute_only]
 
-          # Manual casting is necessary. Because if encrypted_attribute_only, we may not call super
-          # and cannot rely on ActiveRecord to do the casting for us.
-          type_constant_name = options.fetch(:type, :string).to_s.camelize
-
-          type = ActiveRecord::Type.const_get(type_constant_name).new
-
-          cast_value = if type.respond_to? :type_cast_from_user
-                         # ActiveRecord 4.2
-                         type.type_cast_from_user(value)
-                       elsif type.respond_to? :serialize
-                         # ActiveRecord 5.0
-                         type.serialize(value)
-                       else
-                         value
-                       end
-
-          send("#{encrypted_attribute}=", cast_value)
+          send("#{encrypted_attribute}=", value)
         end
       end
     end

lib/vault/encrypted_model.rb

@@ -62,6 +62,10 @@ def vault_attribute(attribute, options = {})
           serializer.define_singleton_method(:decode, &options[:decode])
         end
 
+        unless serializer
+          serializer = Vault::Rails::Serializers::StringSerializer
+        end
+
         # Getter
         define_method("#{attribute}") do
           if instance_variable_defined?("@#{attribute}")
@@ -73,11 +77,12 @@ def vault_attribute(attribute, options = {})
 
         # Setter
         define_method("#{attribute}=") do |value|
+          cast_value = cast_value_to_type(options, value)
           # We always set it as changed without comparing with the current value
           # because we allow our held values to be mutated, so we need to assume
           # that if you call attr=, you want it send back regardless.
           attribute_will_change!("#{attribute}")
-          instance_variable_set("@#{attribute}", value)
+          instance_variable_set("@#{attribute}", cast_value)
         end
 
         # Checker
@@ -181,8 +186,7 @@ def encrypt_value(attribute, value)
         serializer = options[:serializer]
         convergent = options[:convergent]
 
-        # Apply the serializer to the value, if one exists
-        plaintext = serializer ? serializer.encode(value) : value
+        plaintext = serializer.encode(value)
 
         Vault::Rails.encrypt(path, key, plaintext, Vault.client, convergent)
       end
@@ -268,6 +272,17 @@ def __vault_load_attribute!(attribute, options)
         instance_variable_set("@#{attribute}", plaintext)
       end
 
+      def cast_value_to_type(options, value)
+        type_constant_name = options.fetch(:type, :value).to_s.camelize
+        type = ActiveRecord::Type.const_get(type_constant_name).new
+
+        if type.respond_to?(:type_cast_from_user)
+          type.type_cast_from_user(value)
+        else
+          value
+        end
+      end
+
       # Encrypt all the attributes using Vault and set the encrypted values back
       # on this model.
       # @return [true]

lib/vault/rails.rb

@@ -16,6 +16,7 @@
 require_relative 'rails/serializers/time_serializer'
 require_relative 'rails/serializers/date_time_serializer'
 require_relative 'rails/serializers/ipaddr_serializer'
+require_relative 'rails/serializers/string_serializer'
 require_relative 'rails/version'
 
 module Vault
@@ -31,6 +32,7 @@ module Rails
       time:     Vault::Rails::Serializers::TimeSerializer,
       datetime: Vault::Rails::Serializers::DateTimeSerializer,
       ipaddr:   Vault::Rails::Serializers::IPAddrSerializer,
+      string:   Vault::Rails::Serializers::StringSerializer
     }.freeze
 
     # The warning string to print when running in development mode.

lib/vault/rails/serializers/string_serializer.rb

@@ -0,0 +1,17 @@
+module Vault
+  module Rails
+    module Serializers
+      module StringSerializer
+        module_function
+
+        def encode(value)
+          value.blank? ? value : value.to_s
+        end
+
+        def decode(value)
+          value
+        end
+      end
+    end
+  end
+end

lib/vault/rails/version.rb

@@ -1,5 +1,5 @@
 module Vault
   module Rails
-    VERSION = "0.6.12"
+    VERSION = "0.6.13"
   end
 end

spec/dummy/app/models/lazy_person.rb

@@ -10,8 +10,8 @@ class LazyPerson < ActiveRecord::Base
 
   vault_attribute :ssn
 
-  vault_attribute :date_of_birth_plaintext
-  vault_attribute_proxy :date_of_birth, :date_of_birth_plaintext, type: :date
+  vault_attribute :date_of_birth_plaintext, type: :date
+  vault_attribute_proxy :date_of_birth, :date_of_birth_plaintext
 
   vault_attribute :credit_card,
     encrypted_column: :cc_encrypted,

spec/dummy/app/models/person.rb

@@ -4,8 +4,8 @@ class Person < ActiveRecord::Base
   include Vault::EncryptedModel
   include Vault::AttributeProxy
 
-  vault_attribute :date_of_birth_plaintext
-  vault_attribute_proxy :date_of_birth, :date_of_birth_plaintext, type: :date
+  vault_attribute :date_of_birth_plaintext, type: :date
+  vault_attribute_proxy :date_of_birth, :date_of_birth_plaintext
 
   vault_attribute :county_plaintext, encrypted_column: :county_encrypted
   vault_attribute_proxy :county, :county_plaintext
@@ -37,7 +37,6 @@ class Person < ActiveRecord::Base
   vault_attribute :driving_licence_number, convergent: true
   validates :driving_licence_number, vault_uniqueness: true, allow_nil: true
 
-  vault_attribute :ip_address, convergent: true, serialize: :ipaddr
+  vault_attribute :ip_address, convergent: true, serialize: :ipaddr, type: 'IPAddr'
   validates :ip_address, vault_uniqueness: true, allow_nil: true
 end
-

spec/integration/rails_spec.rb

@@ -423,6 +423,18 @@
     end
   end
 
+  context 'when called with type other than string' do
+    it 'casts the value to the correct type' do
+      person = Person.new
+      date_string = '2000-10-10'
+      date = Date.parse(date_string)
+
+      person.date_of_birth_plaintext = date_string
+
+      expect(person.date_of_birth_plaintext).to eq date
+    end
+  end
+
   context 'with errors' do
     it 'raises the appropriate exception' do
       expect {

spec/unit/rails/serializers/string_serializer_spec.rb

@@ -0,0 +1,17 @@
+require 'spec_helper'
+
+describe Vault::Rails::Serializers::StringSerializer do
+  context 'blank values' do
+    it 'encodes blank values without changing them' do
+      expect(subject.encode(nil)).to eq nil
+    end
+  end
+
+  it 'encodes values to strings' do
+    expect(subject.encode({a: 3})).to eq "{:a=>3}"
+  end
+
+  it 'decodes the value by simply returing it' do
+    expect(subject.decode('foo')).to eq 'foo'
+  end
+end