Merge branch 'master' into version-table-with-uuid

CHANGELOG.md

@@ -11,12 +11,20 @@ recommendations of [keepachangelog.com](http://keepachangelog.com/).
 
 ### Added
 
-- None
+- [#1416](https://github.com/paper-trail-gem/paper_trail/pull/1416) - Adds a
+  model-configurable option `synchronize_version_creation_timestamp` which, if
+  set to false, opts out of synchronizing timestamps between `Version.created_at`
+  and the record's `updated_at`.
 
 ### Fixed
 
 - Version table will use uuid as primary key type if uuid flag is set
 
+- [#1422](https://github.com/paper-trail-gem/paper_trail/pull/1422) - Fix the
+  issue that unencrypted plaintext values are versioned with ActiveRecord
+  encryption (since Rails 7) when using JSON serialization on PostgreSQL json
+  columns.
+
 ## 14.0.0 (2022-11-26)
 
 ### Breaking Changes

lib/paper_trail.rb

@@ -114,6 +114,10 @@ def config
     def version
       VERSION::STRING
     end
+
+    def active_record_gte_7_0?
+      @active_record_gte_7_0 ||= ::ActiveRecord.gem_version >= ::Gem::Version.new("7.0.0")
+    end
   end
 end
 

lib/paper_trail/attribute_serializers/cast_attribute_serializer.rb

@@ -32,7 +32,7 @@ def deserialize(attr, val)
         if defined_enums[attr] && val.is_a?(::String)
           # Because PT 4 used to save the string version of enums to `object_changes`
           val
-        elsif rails_gte_7_0? && val.is_a?(ActiveRecord::Type::Time::Value)
+        elsif PaperTrail.active_record_gte_7_0? && val.is_a?(ActiveRecord::Type::Time::Value)
           # Because Rails 7 time attribute throws a delegation error when you deserialize
           # it with the factory.
           # See ActiveRecord::Type::Time::Value crashes when loaded from YAML on rails 7.0
@@ -43,10 +43,6 @@ def deserialize(attr, val)
         end
       end
 
-      def rails_gte_7_0?
-        ::ActiveRecord.gem_version >= ::Gem::Version.new("7.0.0")
-      end
-
       def serialize(attr, val)
         AttributeSerializerFactory.for(@klass, attr).serialize(val)
       end

lib/paper_trail/attribute_serializers/object_attribute.rb

@@ -8,6 +8,12 @@ module AttributeSerializers
     class ObjectAttribute
       def initialize(model_class)
         @model_class = model_class
+
+        # ActiveRecord since 7.0 has a built-in encryption mechanism
+        @encrypted_attributes =
+          if PaperTrail.active_record_gte_7_0?
+            @model_class.encrypted_attributes&.map(&:to_s)
+          end
       end
 
       def serialize(attributes)
@@ -23,14 +29,18 @@ def deserialize(attributes)
       # Modifies `attributes` in place.
       # TODO: Return a new hash instead.
       def alter(attributes, serialization_method)
-        # Don't serialize before values before inserting into columns of type
+        # Don't serialize non-encrypted before values before inserting into columns of type
         # `JSON` on `PostgreSQL` databases.
-        return attributes if object_col_is_json?
+        attributes_to_serialize =
+          object_col_is_json? ? attributes.slice(*@encrypted_attributes) : attributes
+        return attributes if attributes_to_serialize.blank?
 
         serializer = CastAttributeSerializer.new(@model_class)
-        attributes.each do |key, value|
+        attributes_to_serialize.each do |key, value|
           attributes[key] = serializer.send(serialization_method, key, value)
         end
+
+        attributes
       end
 
       def object_col_is_json?

lib/paper_trail/attribute_serializers/object_changes_attribute.rb

@@ -8,6 +8,12 @@ module AttributeSerializers
     class ObjectChangesAttribute
       def initialize(item_class)
         @item_class = item_class
+
+        # ActiveRecord since 7.0 has a built-in encryption mechanism
+        @encrypted_attributes =
+          if PaperTrail.active_record_gte_7_0?
+            @item_class.encrypted_attributes&.map(&:to_s)
+          end
       end
 
       def serialize(changes)
@@ -23,17 +29,21 @@ def deserialize(changes)
       # Modifies `changes` in place.
       # TODO: Return a new hash instead.
       def alter(changes, serialization_method)
-        # Don't serialize before values before inserting into columns of type
+        # Don't serialize non-encrypted before values before inserting into columns of type
         # `JSON` on `PostgreSQL` databases.
-        return changes if object_changes_col_is_json?
+        changes_to_serialize =
+          object_changes_col_is_json? ? changes.slice(*@encrypted_attributes) : changes.clone
+        return changes if changes_to_serialize.blank?
 
         serializer = CastAttributeSerializer.new(@item_class)
-        changes.clone.each do |key, change|
+        changes_to_serialize.each do |key, change|
           # `change` is an Array with two elements, representing before and after.
           changes[key] = Array(change).map do |value|
             serializer.send(serialization_method, key, value)
           end
         end
+
+        changes
       end
 
       def object_changes_col_is_json?

lib/paper_trail/has_paper_trail.rb

@@ -53,6 +53,10 @@ module ClassMethods
       #   - A Hash - options passed to `has_many`, plus `name:` and `scope:`.
       # - :version - The name to use for the method which returns the version
       #   the instance was reified from. Default is `:version`.
+      # - :synchronize_version_creation_timestamp - By default, paper trail
+      #   sets the `created_at` field for a new Version equal to the `updated_at`
+      #   column of the model being updated. If you instead want `created_at` to
+      #   populate with the current timestamp, set this option to `false`.
       #
       # Plugins like the experimental `paper_trail-association_tracking` gem
       # may accept additional options.

lib/paper_trail/record_trail.rb

@@ -226,7 +226,8 @@ def build_version_on_update(force:, in_after_callback:, is_touch:)
       # unnatural to tamper with creation timestamps in this way. But, this
       # feature has existed for a long time, almost a decade now, and some users
       # may rely on it now.
-      if @record.respond_to?(:updated_at)
+      if @record.respond_to?(:updated_at) &&
+          @record.paper_trail_options[:synchronize_version_creation_timestamp] != false
         data[:created_at] = @record.updated_at
       end
 

spec/dummy_app/app/models/fruit.rb

@@ -5,4 +5,8 @@ class Fruit < ApplicationRecord
   if ENV["DB"] == "postgres"
     has_paper_trail versions: { class_name: "JsonVersion" }
   end
+
+  if PaperTrail.active_record_gte_7_0?
+    encrypts :supplier
+  end
 end

spec/dummy_app/app/models/gizmo.rb

@@ -0,0 +1,5 @@
+# frozen_string_literal: true
+
+class Gizmo < ApplicationRecord
+  has_paper_trail synchronize_version_creation_timestamp: false
+end

spec/dummy_app/app/models/vegetable.rb

@@ -5,4 +5,8 @@ class Vegetable < ApplicationRecord
   has_paper_trail versions: {
     class_name: ENV["DB"] == "postgres" ? "JsonbVersion" : "PaperTrail::Version"
   }, on: %i[create update]
+
+  if PaperTrail.active_record_gte_7_0?
+    encrypts :supplier
+  end
 end

spec/dummy_app/db/migrate/20110208155312_set_up_test_tables.rb

@@ -48,6 +48,11 @@ def up
       t.timestamps null: true, limit: 6
     end
 
+    create_table :gizmos, force: true do |t|
+      t.string :name
+      t.timestamps null: true, limit: 6
+    end
+
     create_table :widgets, force: true do |t|
       t.string    :name
       t.text      :a_text
@@ -285,6 +290,7 @@ def up
       t.string :color
       t.integer :mass
       t.string :name
+      t.text :supplier
     end
 
     create_table :boolits, force: true do |t|
@@ -369,6 +375,7 @@ def up
       t.string :color
       t.integer :mass
       t.string :name
+      t.text :supplier
     end
   end
 

spec/models/gizmo_spec.rb

@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+require "spec_helper"
+require "support/performance_helpers"
+
+RSpec.describe Gizmo, type: :model, versioning: true do
+  context "with a persisted record" do
+    it "does not use the gizmo `updated_at` as the version's `created_at`" do
+      gizmo = described_class.create(name: "Fred", created_at: Time.current - 1.day)
+      gizmo.name = "Allen"
+      gizmo.save(touch: false)
+      expect(gizmo.versions.last.created_at).not_to(eq(gizmo.updated_at))
+    end
+  end
+end

spec/models/version_spec.rb

@@ -2,6 +2,7 @@
 
 require "spec_helper"
 require "support/shared_examples/queries"
+require "support/shared_examples/active_record_encryption"
 
 module PaperTrail
   ::RSpec.describe Version, type: :model do
@@ -121,6 +122,8 @@ module PaperTrail
           ::Fruit, # uses JsonVersion
           :mass
         )
+
+        include_examples("active_record_encryption", ::Fruit)
       end
 
       context "with jsonb columns", versioning: true do
@@ -130,6 +133,8 @@ module PaperTrail
           ::Vegetable, # uses JsonbVersion
           :mass
         )
+
+        include_examples("active_record_encryption", ::Vegetable)
       end
     end
   end

spec/spec_helper.rb

@@ -7,7 +7,7 @@
 SimpleCov.start do
   add_filter %w[Appraisals Gemfile Rakefile doc gemfiles spec]
 end
-SimpleCov.minimum_coverage(ENV["DB"] == "postgres" ? 96.8 : 92.4)
+SimpleCov.minimum_coverage(ENV["DB"] == "postgres" ? 96.79 : 92.4)
 
 require "byebug"
 require_relative "support/pt_arel_helpers"

spec/support/shared_examples/active_record_encryption.rb

@@ -0,0 +1,58 @@
+# frozen_string_literal: true
+
+RSpec.shared_examples "active_record_encryption" do |model|
+  if PaperTrail.active_record_gte_7_0?
+    context "when ActiveRecord Encryption is enabled", versioning: true do
+      let(:record) { model.create(supplier: "ABC", name: "Tomato") }
+
+      before do
+        ActiveRecord::Encryption.configure(
+          primary_key: "test",
+          deterministic_key: "test",
+          key_derivation_salt: "test"
+        )
+      end
+
+      it "is versioned with encrypted values" do
+        original_supplier, original_name = record.values_at(:supplier, :name)
+
+        # supplier is encrypted, name is not
+        record.update!(supplier: "XYZ", name: "Avocado")
+
+        expect(record.versions.count).to be 2
+        expect(record.versions.pluck(:event)).to include("create", "update")
+
+        # versioned encrypted value should be something like
+        # "{\"p\":\"zDQU\",\"h\":{\"iv\":\"h2OADmJT3DfK1EZc\",\"at\":\"Urcd0mGSwyu9rGT1vrE5cg==\"}}"
+
+        # check paper trail object
+        object = record.versions.last.object
+        expect(object.to_s).not_to include("XYZ")
+        versioned_supplier, versioned_name = object.values_at("supplier", "name")
+        # encrypted column should be versioned with encrypted value
+        expect(versioned_supplier).not_to eq(original_supplier)
+        # non-encrypted column should be versioned with the original value
+        expect(versioned_name).to eq(original_name)
+        parsed_versioned_supplier = JSON.parse(versioned_supplier)
+        expect(parsed_versioned_supplier)
+          .to match(hash_including("p", "h" => hash_including("iv", "at")))
+
+        # check paper trail object_changes
+        object_changes = record.versions.last.object_changes
+        expect(object_changes.to_s).not_to include("XYZ")
+        supplier_changes, name_changes = object_changes.values_at("supplier", "name")
+        expect(supplier_changes).not_to eq([original_supplier, "XYZ"])
+        expect(name_changes).to eq([original_name, "Avocado"])
+        supplier_changes.each do |supplier|
+          parsed_supplier = JSON.parse(supplier)
+          expect(parsed_supplier).to match(hash_including("p", "h" => hash_including("iv", "at")))
+        end
+      end
+
+      it "reifies encrypted values to decrypted values" do
+        record.update!(supplier: "XYZ", name: "Avocado")
+        expect(record.versions.last.reify.supplier).to eq "ABC"
+      end
+    end
+  end
+end