Merge pull request rails#44540 from matthewd/encryption-configure-onl…

…y-keyprovider

Don't require encryption credentials when using a custom key provider

activerecord/lib/active_record/encryption/config.rb

@@ -21,6 +21,14 @@ def previous=(previous_schemes_properties)
         end
       end
 
+      %w(key_derivation_salt primary_key deterministic_key).each do |key|
+        silence_redefinition_of_method key
+        define_method(key) do
+          instance_variable_get(:"@#{key}").presence or
+            raise Errors::Configuration, "Missing Active Record encryption credential: active_record_encryption.#{key}"
+        end
+      end
+
       private
         def set_defaults
           self.store_key_references = false

activerecord/lib/active_record/encryption/configurable.rb

@@ -17,13 +17,11 @@ module Configurable
           delegate name, to: :context
         end
 
-        def configure(primary_key:, deterministic_key:, key_derivation_salt:, **properties) # :nodoc:
+        def configure(primary_key: nil, deterministic_key: nil, key_derivation_salt: nil, **properties) # :nodoc:
           config.primary_key = primary_key
           config.deterministic_key = deterministic_key
           config.key_derivation_salt = key_derivation_salt
 
-          context.key_provider = ActiveRecord::Encryption::DerivedSecretKeyProvider.new(primary_key)
-
           properties.each do |name, value|
             [:context, :config].each do |configurable_object_name|
               configurable_object = ActiveRecord::Encryption.send(configurable_object_name)

activerecord/lib/active_record/encryption/context.rb

@@ -12,16 +12,19 @@ module Encryption
     class Context
       PROPERTIES = %i[ key_provider key_generator cipher message_serializer encryptor frozen_encryption ]
 
-      PROPERTIES.each do |name|
-        attr_accessor name
-      end
+      attr_accessor(*PROPERTIES)
 
       def initialize
         set_defaults
       end
 
       alias frozen_encryption? frozen_encryption
 
+      silence_redefinition_of_method :key_provider
+      def key_provider
+        @key_provider ||= build_default_key_provider
+      end
+
       private
         def set_defaults
           self.frozen_encryption = false
@@ -30,6 +33,10 @@ def set_defaults
           self.encryptor = ActiveRecord::Encryption::Encryptor.new
           self.message_serializer = ActiveRecord::Encryption::MessageSerializer.new
         end
+
+        def build_default_key_provider
+          ActiveRecord::Encryption::DerivedSecretKeyProvider.new(ActiveRecord::Encryption.config.primary_key)
+        end
     end
   end
 end

activerecord/lib/active_record/encryption/key_generator.rb

@@ -30,10 +30,14 @@ def generate_random_hex_key(length: key_length)
       #
       # The generated key will be salted with the value of +ActiveRecord::Encryption.key_derivation_salt+
       def derive_key_from(password, length: key_length)
-        ActiveSupport::KeyGenerator.new(password).generate_key(ActiveRecord::Encryption.config.key_derivation_salt, length)
+        ActiveSupport::KeyGenerator.new(password).generate_key(key_derivation_salt, length)
       end
 
       private
+        def key_derivation_salt
+          @key_derivation_salt ||= ActiveRecord::Encryption.config.key_derivation_salt
+        end
+
         def key_length
           @key_length ||= ActiveRecord::Encryption.cipher.key_length
         end

activerecord/lib/active_record/encryption/scheme.rb

@@ -45,10 +45,7 @@ def fixed?
       end
 
       def key_provider
-        @key_provider ||= begin
-          validate_keys!
-          @key_provider_param || build_key_provider
-        end
+        @key_provider ||= @key_provider_param || build_key_provider || default_key_provider
       end
 
       def merge(other_scheme)
@@ -74,26 +71,17 @@ def validate_config!
           raise Errors::Configuration, "key_provider: and key: can't be used simultaneously" if @key_provider_param && @key
         end
 
-        def validate_keys!
-          validate_credential :key_derivation_salt
-          validate_credential :primary_key, "needs to be configured to use non-deterministic encryption" unless @deterministic
-          validate_credential :deterministic_key, "needs to be configured to use deterministic encryption" if @deterministic
-        end
-
-        def validate_credential(key, error_message = "is not configured")
-          unless ActiveRecord::Encryption.config.public_send(key).present?
-            raise Errors::Configuration, "#{key} #{error_message}. Please configure it via credential "\
-              "active_record_encryption.#{key} or by setting config.active_record.encryption.#{key}"
-          end
-        end
-
         def build_key_provider
           return DerivedSecretKeyProvider.new(@key) if @key.present?
 
-          if @deterministic && (deterministic_key = ActiveRecord::Encryption.config.deterministic_key)
-            DeterministicKeyProvider.new(deterministic_key)
+          if @deterministic
+            DeterministicKeyProvider.new(ActiveRecord::Encryption.config.deterministic_key)
           end
         end
+
+        def default_key_provider
+          ActiveRecord::Encryption.key_provider
+        end
     end
   end
 end

activerecord/test/cases/encryption/config_test.rb

@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+require "cases/encryption/helper"
+
+class ActiveRecord::Encryption::ConfigTest < ActiveRecord::EncryptionTestCase
+  setup do
+    @config = ActiveRecord::Encryption::Config.new
+  end
+
+  test "required keys will raise a config error when accessed but not set" do
+    @config.primary_key = nil
+    assert_raises ActiveRecord::Encryption::Errors::Configuration do
+      @config.primary_key
+    end
+
+    @config.primary_key = "some key"
+    assert_nothing_raised do
+      @config.primary_key
+    end
+  end
+end

activerecord/test/cases/encryption/encryptable_record_test.rb

@@ -30,7 +30,7 @@ class ActiveRecord::Encryption::EncryptableRecordTest < ActiveRecord::Encryption
 
     post = EncryptedPost.create!(title: "The Starfleet is here!", body: "take cover!")
     post.reload.tags_count # accessing regular attributes works
-    assert_invalid_key_cant_read_attribute(post, :title)
+    assert_invalid_key_cant_read_attribute(post, :body)
   end
 
   test "ignores nil values" do

activerecord/test/cases/encryption/helper.rb

@@ -17,7 +17,7 @@ def assert_encrypted_attribute(model, attribute_name, expected_value)
     end
 
     def assert_invalid_key_cant_read_attribute(model, attribute_name)
-      if model.type_for_attribute(attribute_name).key_provider.present?
+      if model.type_for_attribute(attribute_name).key_provider.respond_to?(:keys=)
         assert_invalid_key_cant_read_attribute_with_custom_key_provider(model, attribute_name)
       else
         assert_invalid_key_cant_read_attribute_with_default_key_provider(model, attribute_name)
@@ -91,11 +91,14 @@ def assert_invalid_key_cant_read_attribute_with_custom_key_provider(model, attri
 
         model.reload
 
-        attribute_type.key_provider.key = ActiveRecord::Encryption::Key.derive_from "other custom attribute secret"
+        original_keys = attribute_type.key_provider.keys
+        attribute_type.key_provider.keys = [ ActiveRecord::Encryption::Key.derive_from("other custom attribute secret") ]
 
         assert_raises ActiveRecord::Encryption::Errors::Decryption do
           model.public_send(attribute_name)
         end
+      ensure
+        attribute_type.key_provider.keys = original_keys
       end
   end
 
@@ -141,22 +144,29 @@ def assert_slower_by_at_most(threshold_factor, baseline:, baseline_label: BASELI
 
 class ActiveRecord::EncryptionTestCase < ActiveRecord::TestCase
   include ActiveRecord::Encryption::EncryptionHelpers, ActiveRecord::Encryption::PerformanceHelpers
-  # , PerformanceHelpers
 
-  ENCRYPTION_ATTRIBUTES_TO_RESET = %i[ primary_key deterministic_key key_derivation_salt store_key_references
-    key_derivation_salt support_unencrypted_data encrypt_fixtures forced_encoding_for_deterministic_encryption ]
+  ENCRYPTION_PROPERTIES_TO_RESET = {
+    config: %i[ primary_key deterministic_key key_derivation_salt store_key_references
+      key_derivation_salt support_unencrypted_data encrypt_fixtures
+      forced_encoding_for_deterministic_encryption ],
+    context: %i[ key_provider ]
+  }
 
   setup do
-    ENCRYPTION_ATTRIBUTES_TO_RESET.each do |property|
-      instance_variable_set "@_original_#{property}", ActiveRecord::Encryption.config.public_send(property)
+    ENCRYPTION_PROPERTIES_TO_RESET.each do |key, properties|
+      properties.each do |property|
+        instance_variable_set "@_original_encryption_#{key}_#{property}", ActiveRecord::Encryption.public_send(key).public_send(property)
+      end
     end
     ActiveRecord::Encryption.config.previous_schemes.clear
     ActiveRecord::Encryption.encrypted_attribute_declaration_listeners&.clear
   end
 
   teardown do
-    ENCRYPTION_ATTRIBUTES_TO_RESET.each do |property|
-      ActiveRecord::Encryption.config.public_send("#{property}=", instance_variable_get("@_original_#{property}"))
+    ENCRYPTION_PROPERTIES_TO_RESET.each do |key, properties|
+      properties.each do |property|
+        ActiveRecord::Encryption.public_send(key).public_send("#{property}=", instance_variable_get("@_original_encryption_#{key}_#{property}"))
+      end
     end
   end
 end

activerecord/test/cases/encryption/scheme_test.rb

@@ -13,38 +13,6 @@ class ActiveRecord::Encryption::SchemeTest < ActiveRecord::EncryptionTestCase
     assert_valid_declaration key_provider: ActiveRecord::Encryption::DerivedSecretKeyProvider.new("my secret")
   end
 
-  test "validates primary_key is set for non deterministic encryption" do
-    ActiveRecord::Encryption.config.primary_key = nil
-
-    assert_raise ActiveRecord::Encryption::Errors::Configuration do
-      declare_and_use_class
-    end
-
-    assert_nothing_raised do
-      declare_and_use_class deterministic: true
-    end
-  end
-
-  test "validates deterministic_key is set for non deterministic encryption" do
-    ActiveRecord::Encryption.config.deterministic_key = nil
-
-    assert_raise ActiveRecord::Encryption::Errors::Configuration do
-      declare_and_use_class deterministic: true
-    end
-
-    assert_nothing_raised do
-      declare_and_use_class
-    end
-  end
-
-  test "validates key_derivation_salt is set" do
-    ActiveRecord::Encryption.config.key_derivation_salt = nil
-
-    assert_raise ActiveRecord::Encryption::Errors::Configuration do
-      declare_and_use_class
-    end
-  end
-
   private
     def assert_invalid_declaration(**options)
       assert_raises ActiveRecord::Encryption::Errors::Configuration do

activerecord/test/models/post_encrypted.rb

@@ -7,7 +7,7 @@ class EncryptedPost < Post
 
   # We want to modify the key for testing purposes
   class MutableDerivedSecretKeyProvider < ActiveRecord::Encryption::DerivedSecretKeyProvider
-    attr_accessor :key
+    attr_accessor :keys
   end
 
   encrypts :title