From 4f365720d1b87b7a8fe7d8d77c157b63d853e334 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Mon, 26 Jun 2023 13:25:38 +0200 Subject: [PATCH] Fix Active Record encryption not picking up encryption settings with eager-loading (#48577) This deals with a problem where, when eager-loading is enabled, Active Record fails to pick up settings affecting encryption schemes. The solution here is to resolve encryption schemes lazily, when the attribute type is used. Follow-up from https://github.com/rails/rails/pull/48530 Closes https://github.com/rails/rails/issues/48204#issuecomment-1607127334 --- .../encryption/encryptable_record.rb | 18 +++++++++--------- .../encryption/encryptable_record_test.rb | 15 +++++++++++++++ .../test/cases/encryption/scheme_test.rb | 4 ++-- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/activerecord/lib/active_record/encryption/encryptable_record.rb b/activerecord/lib/active_record/encryption/encryptable_record.rb index e1dc373001bf0..9af5c0feab556 100644 --- a/activerecord/lib/active_record/encryption/encryptable_record.rb +++ b/activerecord/lib/active_record/encryption/encryptable_record.rb @@ -44,11 +44,9 @@ module EncryptableRecord # encryption is used, they will be used to generate additional ciphertexts to check in the queries. def encrypts(*names, key_provider: nil, key: nil, deterministic: false, downcase: false, ignore_case: false, previous: [], **context_properties) self.encrypted_attributes ||= Set.new # not using :default because the instance would be shared across classes - scheme = scheme_for key_provider: key_provider, key: key, deterministic: deterministic, downcase: downcase, \ - ignore_case: ignore_case, previous: previous, **context_properties names.each do |name| - encrypt_attribute name, scheme + encrypt_attribute name, key_provider: key_provider, key: key, deterministic: deterministic, downcase: downcase, ignore_case: ignore_case, previous: previous, **context_properties end end @@ -67,9 +65,9 @@ def source_attribute_from_preserved_attribute(attribute_name) private def scheme_for(key_provider: nil, key: nil, deterministic: false, downcase: false, ignore_case: false, previous: [], **context_properties) ActiveRecord::Encryption::Scheme.new(key_provider: key_provider, key: key, deterministic: deterministic, - downcase: downcase, ignore_case: ignore_case, **context_properties).tap do |scheme| + downcase: downcase, ignore_case: ignore_case, **context_properties).tap do |scheme| scheme.previous_schemes = global_previous_schemes_for(scheme) + - Array.wrap(previous).collect { |scheme_config| ActiveRecord::Encryption::Scheme.new(**scheme_config) } + Array.wrap(previous).collect { |scheme_config| ActiveRecord::Encryption::Scheme.new(**scheme_config) } end end @@ -79,15 +77,17 @@ def global_previous_schemes_for(scheme) end end - def encrypt_attribute(name, attribute_scheme) + def encrypt_attribute(name, key_provider: nil, key: nil, deterministic: false, downcase: false, ignore_case: false, previous: [], **context_properties) encrypted_attributes << name.to_sym attribute name do |cast_type| - ActiveRecord::Encryption::EncryptedAttributeType.new(scheme: attribute_scheme, cast_type: cast_type, - default: columns_hash[name.to_s]&.default) + scheme = scheme_for key_provider: key_provider, key: key, deterministic: deterministic, downcase: downcase, \ + ignore_case: ignore_case, previous: previous, **context_properties + ActiveRecord::Encryption::EncryptedAttributeType.new(scheme: scheme, cast_type: cast_type, + default: columns_hash[name.to_s]&.default) end - preserve_original_encrypted(name) if attribute_scheme.ignore_case? + preserve_original_encrypted(name) if ignore_case ActiveRecord::Encryption.encrypted_attribute_was_declared(self, name) end diff --git a/activerecord/test/cases/encryption/encryptable_record_test.rb b/activerecord/test/cases/encryption/encryptable_record_test.rb index 26dd0266c7d35..92d1a4eb91894 100644 --- a/activerecord/test/cases/encryption/encryptable_record_test.rb +++ b/activerecord/test/cases/encryption/encryptable_record_test.rb @@ -332,6 +332,21 @@ def name assert_equal "Post 1", encrypted_post_class_sha_256.last.title end + test "encryption schemes are resolved when used, not when declared" do + OtherEncryptedPost = Class.new(Post) do + self.table_name = "posts" + encrypts :title + end + + ActiveRecord::Encryption.configure \ + primary_key: "the primary key", + deterministic_key: "the deterministic key", + key_derivation_salt: "the salt", + support_sha1_for_non_deterministic_encryption: true + + assert OtherEncryptedPost.type_for_attribute(:title).scheme.previous_schemes.one? + end + private def build_derived_key_provider_with(hash_digest_class) ActiveRecord::Encryption.with_encryption_context(key_generator: ActiveRecord::Encryption::KeyGenerator.new(hash_digest_class: hash_digest_class)) do diff --git a/activerecord/test/cases/encryption/scheme_test.rb b/activerecord/test/cases/encryption/scheme_test.rb index 55e2e3d1f1016..d587f52fa4d36 100644 --- a/activerecord/test/cases/encryption/scheme_test.rb +++ b/activerecord/test/cases/encryption/scheme_test.rb @@ -4,7 +4,7 @@ require "models/book" class ActiveRecord::Encryption::SchemeTest < ActiveRecord::EncryptionTestCase - test "validates config options when declaring encrypted attributes" do + test "validates config options when using encrypted attributes" do assert_invalid_declaration deterministic: false, ignore_case: true assert_invalid_declaration key: "1234", key_provider: ActiveRecord::Encryption::DerivedSecretKeyProvider.new("my secret") @@ -37,6 +37,6 @@ def declare_and_use_class(**options) def declare_encrypts_with(options) Class.new(Book) do encrypts :name, **options - end + end.type_for_attribute(:name) end end