mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Don't require encryption credentials when using a custom key provider
Instead of pre-validating the configuration, we now check for the required values when they're used. Co-authored-by: Alex Ghiculescu <alex@tanda.co> Co-authored-by: Jorge Manrubia <jorge.manrubia@gmail.com> Co-authored-by: John Hawthorn <john@hawthorn.email>
This commit is contained in:
parent
53f703b3e8
commit
0b2772e675
10 changed files with 73 additions and 69 deletions
|
@ -21,6 +21,14 @@ module ActiveRecord
|
|||
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
|
||||
|
|
|
@ -17,13 +17,11 @@ module ActiveRecord
|
|||
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)
|
||||
|
|
|
@ -12,9 +12,7 @@ module ActiveRecord
|
|||
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
|
||||
|
@ -22,6 +20,11 @@ module ActiveRecord
|
|||
|
||||
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 @@ module ActiveRecord
|
|||
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
|
||||
|
|
|
@ -30,10 +30,14 @@ module ActiveRecord
|
|||
#
|
||||
# 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
|
||||
|
|
|
@ -45,10 +45,7 @@ module ActiveRecord
|
|||
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 @@ module ActiveRecord
|
|||
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
|
||||
|
|
21
activerecord/test/cases/encryption/config_test.rb
Normal file
21
activerecord/test/cases/encryption/config_test.rb
Normal file
|
@ -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
|
|
@ -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
|
||||
|
|
|
@ -17,7 +17,7 @@ module ActiveRecord::Encryption
|
|||
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 @@ module ActiveRecord::Encryption
|
|||
|
||||
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 @@ end
|
|||
|
||||
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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
Loading…
Reference in a new issue