diff options
29 files changed, 297 insertions, 89 deletions
diff --git a/actionpack/lib/action_dispatch/http/content_security_policy.rb b/actionpack/lib/action_dispatch/http/content_security_policy.rb index 35041fd072..855be5ce2e 100644 --- a/actionpack/lib/action_dispatch/http/content_security_policy.rb +++ b/actionpack/lib/action_dispatch/http/content_security_policy.rb @@ -132,7 +132,7 @@ module ActionDispatch #:nodoc: worker_src: "worker-src" }.freeze - NONCE_DIRECTIVES = %w[script-src].freeze + NONCE_DIRECTIVES = %w[script-src style-src].freeze private_constant :MAPPINGS, :DIRECTIVES, :NONCE_DIRECTIVES diff --git a/actionpack/test/dispatch/content_security_policy_test.rb b/actionpack/test/dispatch/content_security_policy_test.rb index 4f9a4ff2bd..13ad22b5c5 100644 --- a/actionpack/test/dispatch/content_security_policy_test.rb +++ b/actionpack/test/dispatch/content_security_policy_test.rb @@ -339,6 +339,11 @@ class ContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationTest p.script_src :self end + content_security_policy only: :style_src do |p| + p.default_src false + p.style_src :self + end + content_security_policy(false, only: :no_policy) content_security_policy_report_only only: :report_only @@ -363,6 +368,10 @@ class ContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationTest head :ok end + def style_src + head :ok + end + def no_policy head :ok end @@ -381,6 +390,7 @@ class ContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationTest get "/conditional", to: "policy#conditional" get "/report-only", to: "policy#report_only" get "/script-src", to: "policy#script_src" + get "/style-src", to: "policy#style_src" get "/no-policy", to: "policy#no_policy" end end @@ -441,6 +451,11 @@ class ContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationTest assert_policy "script-src 'self' 'nonce-iyhD0Yc0W+c='" end + def test_adds_nonce_to_style_src_content_security_policy + get "/style-src" + assert_policy "style-src 'self' 'nonce-iyhD0Yc0W+c='" + end + def test_generates_no_content_security_policy get "/no-policy" diff --git a/activejob/lib/active_job/exceptions.rb b/activejob/lib/active_job/exceptions.rb index d8384c81b6..bb25afbca6 100644 --- a/activejob/lib/active_job/exceptions.rb +++ b/activejob/lib/active_job/exceptions.rb @@ -46,18 +46,15 @@ module ActiveJob # end def retry_on(*exceptions, wait: 3.seconds, attempts: 5, queue: nil, priority: nil) rescue_from(*exceptions) do |error| - payload = { - job: self, - adapter: self.class.queue_adapter, - error: error, - wait: wait - } - if executions < attempts - ActiveSupport::Notifications.instrument("enqueue_retry.active_job", payload) do - retry_job wait: determine_delay(wait), queue: queue, priority: priority - end + retry_job wait: determine_delay(wait), queue: queue, priority: priority, error: error else + payload = { + job: self, + adapter: self.class.queue_adapter, + error: error + } + if block_given? ActiveSupport::Notifications.instrument("retry_stopped.active_job", payload) do yield self, error @@ -127,7 +124,16 @@ module ActiveJob # end # end def retry_job(options = {}) - enqueue options + payload = { + job: self, + adapter: self.class.queue_adapter, + error: options[:error], + wait: options[:wait] + } + + ActiveSupport::Notifications.instrument("enqueue_retry.active_job", payload) do + enqueue options + end end private diff --git a/activejob/lib/active_job/logging.rb b/activejob/lib/active_job/logging.rb index 96a3e6bf48..0abee4ed98 100644 --- a/activejob/lib/active_job/logging.rb +++ b/activejob/lib/active_job/logging.rb @@ -94,7 +94,7 @@ module ActiveJob wait = event.payload[:wait] error do - "Retrying #{job.class} in #{wait} seconds, due to a #{ex.class}. The original exception was #{ex.cause.inspect}." + "Retrying #{job.class} in #{wait.inspect} seconds, due to a #{ex&.class.inspect}. The original exception was #{ex&.cause.inspect}." end end diff --git a/activejob/test/cases/logging_test.rb b/activejob/test/cases/logging_test.rb index 2e8d2ef7c0..b5bf40c83b 100644 --- a/activejob/test/cases/logging_test.rb +++ b/activejob/test/cases/logging_test.rb @@ -173,6 +173,11 @@ class LoggingTest < ActiveSupport::TestCase end end + def test_enqueue_retry_logging_on_retry_job + perform_enqueued_jobs { RescueJob.perform_later "david" } + assert_match(/Retrying RescueJob in nil seconds, due to a nil\. The original exception was nil\./, @logger.messages) + end + def test_retry_stopped_logging perform_enqueued_jobs do RetryJob.perform_later "CustomCatchError", 6 diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 477bc1b54a..cfc5647969 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,13 @@ +* Don't update counter cache unless the record is actually saved. + + Fixes #31493, #33113, #33117. + + *Ryuta Kamizono* + +* Deprecate `ActiveRecord::Result#to_hash` in favor of `ActiveRecord::Result#to_a`. + + *Gannon McGibbon*, *Kevin Cheng* + * SQLite3 adapter supports expression indexes. ``` diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb index 3d4ad1dd5b..544aec5e8b 100644 --- a/activerecord/lib/active_record/associations/belongs_to_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_association.rb @@ -50,11 +50,8 @@ module ActiveRecord def replace(record) if record raise_on_type_mismatch!(record) - update_counters_on_replace(record) set_inverse_instance(record) @updated = true - else - decrement_counters end replace_keys(record) @@ -80,19 +77,6 @@ module ActiveRecord reflection.counter_cache_column && owner.persisted? end - def update_counters_on_replace(record) - if require_counter_update? && different_target?(record) - owner.instance_variable_set :@_after_replace_counter_called, true - record.increment!(reflection.counter_cache_column, touch: reflection.options[:touch]) - decrement_counters - end - end - - # Checks whether record is different to the current target, without loading it - def different_target?(record) - record._read_attribute(primary_key(record)) != owner._read_attribute(reflection.foreign_key) - end - def replace_keys(record) owner[reflection.foreign_key] = record ? record._read_attribute(primary_key(record)) : nil end diff --git a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb index 3fd2fb5f67..9ae452e7a1 100644 --- a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb @@ -19,10 +19,6 @@ module ActiveRecord owner[reflection.foreign_type] = record ? record.class.polymorphic_name : nil end - def different_target?(record) - super || record.class != klass - end - def inverse_reflection_for(record) reflection.polymorphic_inverse_of(record.class) end diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index 0166ed98ca..da4cc343eb 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -34,9 +34,7 @@ module ActiveRecord::Associations::Builder # :nodoc: foreign_key = reflection.foreign_key cache_column = reflection.counter_cache_column - if @_after_replace_counter_called ||= false - @_after_replace_counter_called = false - elsif association(reflection.name).target_changed? + if association(reflection.name).target_changed? if reflection.polymorphic? model = attribute_in_database(reflection.foreign_type).try(:constantize) model_was = attribute_before_last_save(reflection.foreign_type).try(:constantize) diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index d32fd5ea09..baa0a29afd 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -576,7 +576,7 @@ module ActiveRecord column end else - basic_structure.to_hash + basic_structure.to_a end end diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb index 694ff85fa1..9b985e049b 100644 --- a/activerecord/lib/active_record/model_schema.rb +++ b/activerecord/lib/active_record/model_schema.rb @@ -375,7 +375,7 @@ module ActiveRecord # default values when instantiating the Active Record object for this table. def column_defaults load_schema - @column_defaults ||= _default_attributes.to_hash + @column_defaults ||= _default_attributes.deep_dup.to_hash end def _default_attributes # :nodoc: diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 0fabfe5518..930e4377ff 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -416,7 +416,7 @@ module ActiveRecord raise UnknownPrimaryKey.new(@klass) if primary_key.nil? expects_array = ids.first.kind_of?(Array) - return ids.first if expects_array && ids.first.empty? + return [] if expects_array && ids.first.empty? ids = ids.flatten.compact.uniq diff --git a/activerecord/lib/active_record/result.rb b/activerecord/lib/active_record/result.rb index 3b2556b1c8..da6d10b6ec 100644 --- a/activerecord/lib/active_record/result.rb +++ b/activerecord/lib/active_record/result.rb @@ -21,7 +21,7 @@ module ActiveRecord # ] # # # Get an array of hashes representing the result (column => value): - # result.to_hash + # result.to_a # # => [{"id" => 1, "title" => "title_1", "body" => "body_1"}, # {"id" => 2, "title" => "title_2", "body" => "body_2"}, # ... @@ -65,9 +65,12 @@ module ActiveRecord end end - # Returns an array of hashes representing each row record. def to_hash - hash_rows + ActiveSupport::Deprecation.warn(<<-MSG.squish) + `ActiveRecord::Result#to_hash` has been renamed to `to_a`. + `to_hash` is deprecated and will be removed in Rails 6.1. + MSG + to_a end alias :map! :map @@ -83,6 +86,8 @@ module ActiveRecord hash_rows end + alias :to_a :to_ary + def [](idx) hash_rows[idx] end diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb index a93e5e2b40..217ba22294 100644 --- a/activerecord/test/cases/adapter_test.rb +++ b/activerecord/test/cases/adapter_test.rb @@ -227,7 +227,7 @@ module ActiveRecord post = Post.create!(title: "foo", body: "bar") expected = @connection.select_all("SELECT * FROM posts WHERE id = #{post.id}") result = @connection.select_all("SELECT * FROM posts WHERE id = #{Arel::Nodes::BindParam.new(nil).to_sql}", nil, [[nil, post.id]]) - assert_equal expected.to_hash, result.to_hash + assert_equal expected.to_a, result.to_a end def test_insert_update_delete_with_legacy_binds diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index 0cc4ed7127..43763dc715 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -452,15 +452,39 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase end def test_belongs_to_counter_with_assigning_nil - post = Post.find(1) - comment = Comment.find(1) + topic = Topic.create!(title: "debate") + reply = Reply.create!(title: "blah!", content: "world around!", topic: topic) - assert_equal post.id, comment.post_id - assert_equal 2, Post.find(post.id).comments.size + assert_equal topic.id, reply.parent_id + assert_equal 1, topic.reload.replies.size - comment.post = nil + reply.topic = nil + reply.reload - assert_equal 1, Post.find(post.id).comments.size + assert_equal topic.id, reply.parent_id + assert_equal 1, topic.reload.replies.size + + reply.topic = nil + reply.save! + + assert_equal 0, topic.reload.replies.size + end + + def test_belongs_to_counter_with_assigning_new_object + topic = Topic.create!(title: "debate") + reply = Reply.create!(title: "blah!", content: "world around!", topic: topic) + + assert_equal topic.id, reply.parent_id + assert_equal 1, topic.reload.replies_count + + topic2 = reply.build_topic(title: "debate2") + reply.save! + + assert_not_equal topic.id, reply.parent_id + assert_equal topic2.id, reply.parent_id + + assert_equal 0, topic.reload.replies_count + assert_equal 1, topic2.reload.replies_count end def test_belongs_to_with_primary_key_counter @@ -485,11 +509,13 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert_equal 0, debate2.reload.replies_count reply.topic_with_primary_key = debate2 + reply.save! assert_equal 0, debate.reload.replies_count assert_equal 1, debate2.reload.replies_count reply.topic_with_primary_key = nil + reply.save! assert_equal 0, debate.reload.replies_count assert_equal 0, debate2.reload.replies_count @@ -516,11 +542,13 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert_equal 1, Topic.find(topic2.id).replies.size reply1.topic = nil + reply1.save! assert_equal 0, Topic.find(topic1.id).replies.size assert_equal 0, Topic.find(topic2.id).replies.size reply1.topic = topic1 + reply1.save! assert_equal 1, Topic.find(topic1.id).replies.size assert_equal 0, Topic.find(topic2.id).replies.size @@ -594,6 +622,7 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase debate2.touch(time: time) reply.topic_with_primary_key = debate2 + reply.save! assert_operator debate.reload.updated_at, :>, time assert_operator debate2.reload.updated_at, :>, time @@ -772,6 +801,7 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase reply = Reply.create(title: "re: zoom", content: "speedy quick!") reply.topic = topic + reply.save! assert_equal 1, topic.reload[:replies_count] assert_equal 1, topic.replies.size @@ -827,6 +857,7 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase silly = SillyReply.create(title: "gaga", content: "boo-boo") silly.reply = reply + silly.save! assert_equal 1, reply.reload[:replies_count] assert_equal 1, reply.replies.size diff --git a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb index e717621928..ba2104eb26 100644 --- a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb +++ b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb @@ -117,9 +117,8 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase end def test_eager_association_loading_with_has_many_sti_and_subclasses - silly = SillyReply.new(title: "gaga", content: "boo-boo", parent_id: 1) - silly.parent_id = 1 - assert silly.save + reply = Reply.new(title: "gaga", content: "boo-boo", parent_id: 1) + assert reply.save topics = Topic.all.merge!(includes: :replies, order: ["topics.id", "replies_topics.id"]).to_a assert_no_queries do diff --git a/activerecord/test/cases/attributes_test.rb b/activerecord/test/cases/attributes_test.rb index 3bc56694be..2632aec7ab 100644 --- a/activerecord/test/cases/attributes_test.rb +++ b/activerecord/test/cases/attributes_test.rb @@ -148,6 +148,20 @@ module ActiveRecord assert_equal 2, klass.new.counter end + test "procs for default values are evaluated even after column_defaults is called" do + klass = Class.new(OverloadedType) do + @@counter = 0 + attribute :counter, :integer, default: -> { @@counter += 1 } + end + + assert_equal 1, klass.new.counter + + # column_defaults will increment the counter since the proc is called + klass.column_defaults + + assert_equal 3, klass.new.counter + end + test "procs are memoized before type casting" do klass = Class.new(OverloadedType) do @@counter = 0 diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index e73c88dd5d..355fb4517f 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -371,7 +371,10 @@ class FinderTest < ActiveRecord::TestCase end def test_find_an_empty_array - assert_equal [], Topic.find([]) + empty_array = [] + result = Topic.find(empty_array) + assert_equal [], result + assert_not_same empty_array, result end def test_find_doesnt_have_implicit_ordering diff --git a/activerecord/test/cases/result_test.rb b/activerecord/test/cases/result_test.rb index 68fcafb682..825aee2423 100644 --- a/activerecord/test/cases/result_test.rb +++ b/activerecord/test/cases/result_test.rb @@ -21,12 +21,22 @@ module ActiveRecord assert_equal 3, result.length end - test "to_hash returns row_hashes" do + test "to_a returns row_hashes" do assert_equal [ { "col_1" => "row 1 col 1", "col_2" => "row 1 col 2" }, { "col_1" => "row 2 col 1", "col_2" => "row 2 col 2" }, { "col_1" => "row 3 col 1", "col_2" => "row 3 col 2" }, - ], result.to_hash + ], result.to_a + end + + test "to_hash (deprecated) returns row_hashes" do + assert_deprecated do + assert_equal [ + { "col_1" => "row 1 col 1", "col_2" => "row 1 col 2" }, + { "col_1" => "row 2 col 1", "col_2" => "row 2 col 2" }, + { "col_1" => "row 3 col 1", "col_2" => "row 3 col 2" }, + ], result.to_hash + end end test "first returns first row as a hash" do diff --git a/activerecord/test/models/reply.rb b/activerecord/test/models/reply.rb index 0ea110f4f8..0807bcf875 100644 --- a/activerecord/test/models/reply.rb +++ b/activerecord/test/models/reply.rb @@ -9,6 +9,10 @@ class Reply < Topic has_many :silly_unique_replies, dependent: :destroy, foreign_key: "parent_id" end +class SillyReply < Topic + belongs_to :reply, foreign_key: "parent_id", counter_cache: :replies_count +end + class UniqueReply < Reply belongs_to :topic, foreign_key: "parent_id", counter_cache: true validates_uniqueness_of :content, scope: "parent_id" @@ -54,10 +58,6 @@ class WrongReply < Reply end end -class SillyReply < Reply - belongs_to :reply, foreign_key: "parent_id", counter_cache: :replies_count -end - module Web class Reply < Web::Topic belongs_to :topic, foreign_key: "parent_id", counter_cache: true, class_name: "Web::Topic" diff --git a/guides/source/active_support_instrumentation.md b/guides/source/active_support_instrumentation.md index 69c6a6e414..9963125fa2 100644 --- a/guides/source/active_support_instrumentation.md +++ b/guides/source/active_support_instrumentation.md @@ -467,6 +467,7 @@ Active Job | `:job` | Job object | | `:adapter` | QueueAdapter object processing the job | | `:error` | The error that caused the retry | +| `:wait` | The delay of the retry | ### perform_start.active_job diff --git a/guides/source/configuring.md b/guides/source/configuring.md index e372b3a816..18a377a02e 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -1014,7 +1014,6 @@ If you choose to use MySQL or MariaDB instead of the shipped SQLite3 database, y ```yaml development: adapter: mysql2 - encoding: utf8 database: blog_development pool: 5 username: root @@ -1024,6 +1023,8 @@ development: If your development database has a root user with an empty password, this configuration should work for you. Otherwise, change the username and password in the `development` section as appropriate. +NOTE: If your MySQL version is 5.5 or 5.6 and want to use the `utf8mb4` character set by default, please configure your MySQL server to support the longer key prefix by enabling `innodb_large_prefix` system variable. + Advisory Locks are enabled by default on MySQL and are used to make database migrations concurrent safe. You can disable advisory locks by setting `advisory_locks` to `false`: ```yaml diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index b916fda1cb..691e599ddb 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,12 @@ +* Support environment specific credentials file. + + For `production` environment look first for `config/credentials/production.yml.enc` file that can be decrypted by + `ENV["RAILS_MASTER_KEY"]` or `config/credentials/production.key` master key. + Edit given environment credentials file by command `rails credentials:edit --environment production`. + Default paths can be overwritten by setting `config.credentials.content_path` and `config.credentials.key_path`. + + *Wojciech Wnętrzak* + * Make `ActiveSupport::Cache::NullStore` the default cache store in the test environment. *Michael C. Nelson* diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index 26ed195dcc..656786246d 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -438,8 +438,12 @@ module Rails # Decrypts the credentials hash as kept in +config/credentials.yml.enc+. This file is encrypted with # the Rails master key, which is either taken from <tt>ENV["RAILS_MASTER_KEY"]</tt> or from loading # +config/master.key+. + # If specific credentials file exists for current environment, it takes precedence, thus for +production+ + # environment look first for +config/credentials/production.yml.enc+ with master key taken + # from <tt>ENV["RAILS_MASTER_KEY"]</tt> or from loading +config/credentials/production.key+. + # Default behavior can be overwritten by setting +config.credentials.content_path+ and +config.credentials.key_path+. def credentials - @credentials ||= encrypted("config/credentials.yml.enc") + @credentials ||= encrypted(config.credentials.content_path, key_path: config.credentials.key_path) end # Shorthand to decrypt any encrypted configurations or files. diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index f4cbd2b9d0..eae902a938 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -17,7 +17,7 @@ module Rails :session_options, :time_zone, :reload_classes_only_on_change, :beginning_of_week, :filter_redirect, :x, :enable_dependency_loading, :read_encrypted_secrets, :log_level, :content_security_policy_report_only, - :content_security_policy_nonce_generator, :require_master_key + :content_security_policy_nonce_generator, :require_master_key, :credentials attr_reader :encoding, :api_only, :loaded_config_version @@ -60,6 +60,9 @@ module Rails @content_security_policy_nonce_generator = nil @require_master_key = false @loaded_config_version = nil + @credentials = ActiveSupport::OrderedOptions.new + @credentials.content_path = default_credentials_content_path + @credentials.key_path = default_credentials_key_path end def load_defaults(target_version) @@ -273,6 +276,27 @@ module Rails true end end + + private + def credentials_available_for_current_env? + File.exist?("#{root}/config/credentials/#{Rails.env}.yml.enc") + end + + def default_credentials_content_path + if credentials_available_for_current_env? + File.join(root, "config", "credentials", "#{Rails.env}.yml.enc") + else + File.join(root, "config", "credentials.yml.enc") + end + end + + def default_credentials_key_path + if credentials_available_for_current_env? + File.join(root, "config", "credentials", "#{Rails.env}.key") + else + File.join(root, "config", "master.key") + end + end end end end diff --git a/railties/lib/rails/commands/credentials/USAGE b/railties/lib/rails/commands/credentials/USAGE index ea429f58d8..6b33d1ab74 100644 --- a/railties/lib/rails/commands/credentials/USAGE +++ b/railties/lib/rails/commands/credentials/USAGE @@ -38,3 +38,12 @@ the encrypted credentials. When the temporary file is next saved the contents are encrypted and written to `config/credentials.yml.enc` while the file itself is destroyed to prevent credentials from leaking. + +=== Environment Specific Credentials + +It is possible to have credentials for each environment. If the file for current environment exists it will take +precedence over `config/credentials.yml.enc`, thus for `production` environment first look for +`config/credentials/production.yml.enc` that can be decrypted using master key taken from `ENV["RAILS_MASTER_KEY"]` +or stored in `config/credentials/production.key`. +To edit given file use command `rails credentials:edit --environment production` +Default paths can be overwritten by setting `config.credentials.content_path` and `config.credentials.key_path`. diff --git a/railties/lib/rails/commands/credentials/credentials_command.rb b/railties/lib/rails/commands/credentials/credentials_command.rb index 65c5218fc8..4b30d208e0 100644 --- a/railties/lib/rails/commands/credentials/credentials_command.rb +++ b/railties/lib/rails/commands/credentials/credentials_command.rb @@ -8,6 +8,9 @@ module Rails class CredentialsCommand < Rails::Command::Base # :nodoc: include Helpers::Editor + class_option :environment, aliases: "-e", type: :string, + desc: "Uses credentials from config/credentials/:environment.yml.enc encrypted by config/credentials/:environment.key key" + no_commands do def help say "Usage:\n #{self.class.banner}" @@ -20,58 +23,74 @@ module Rails require_application_and_environment! ensure_editor_available(command: "bin/rails credentials:edit") || (return) - ensure_master_key_has_been_added if Rails.application.credentials.key.nil? - ensure_credentials_have_been_added + + encrypted = Rails.application.encrypted(content_path, key_path: key_path) + + ensure_encryption_key_has_been_added(key_path) if encrypted.key.nil? + ensure_encrypted_file_has_been_added(content_path, key_path) catch_editing_exceptions do - change_credentials_in_system_editor + change_encrypted_file_in_system_editor(content_path, key_path) end - say "New credentials encrypted and saved." + say "File encrypted and saved." + rescue ActiveSupport::MessageEncryptor::InvalidMessage + say "Couldn't decrypt #{content_path}. Perhaps you passed the wrong key?" end def show require_application_and_environment! - say Rails.application.credentials.read.presence || missing_credentials_message + encrypted = Rails.application.encrypted(content_path, key_path: key_path) + + say encrypted.read.presence || missing_encrypted_message(key: encrypted.key, key_path: key_path, file_path: content_path) end private - def ensure_master_key_has_been_added - master_key_generator.add_master_key_file - master_key_generator.ignore_master_key_file + def content_path + options[:environment] ? "config/credentials/#{options[:environment]}.yml.enc" : "config/credentials.yml.enc" + end + + def key_path + options[:environment] ? "config/credentials/#{options[:environment]}.key" : "config/master.key" + end + + + def ensure_encryption_key_has_been_added(key_path) + encryption_key_file_generator.add_key_file(key_path) + encryption_key_file_generator.ignore_key_file(key_path) end - def ensure_credentials_have_been_added - credentials_generator.add_credentials_file_silently + def ensure_encrypted_file_has_been_added(file_path, key_path) + encrypted_file_generator.add_encrypted_file_silently(file_path, key_path) end - def change_credentials_in_system_editor - Rails.application.credentials.change do |tmp_path| + def change_encrypted_file_in_system_editor(file_path, key_path) + Rails.application.encrypted(file_path, key_path: key_path).change do |tmp_path| system("#{ENV["EDITOR"]} #{tmp_path}") end end - def master_key_generator + def encryption_key_file_generator require "rails/generators" - require "rails/generators/rails/master_key/master_key_generator" + require "rails/generators/rails/encryption_key_file/encryption_key_file_generator" - Rails::Generators::MasterKeyGenerator.new + Rails::Generators::EncryptionKeyFileGenerator.new end - def credentials_generator + def encrypted_file_generator require "rails/generators" - require "rails/generators/rails/credentials/credentials_generator" + require "rails/generators/rails/encrypted_file/encrypted_file_generator" - Rails::Generators::CredentialsGenerator.new + Rails::Generators::EncryptedFileGenerator.new end - def missing_credentials_message - if Rails.application.credentials.key.nil? - "Missing master key to decrypt credentials. See `rails credentials:help`" + def missing_encrypted_message(key:, key_path:, file_path:) + if key.nil? + "Missing '#{key_path}' to decrypt credentials. See `rails credentials:help`" else - "No credentials have been added yet. Use `rails credentials:edit` to change that." + "File '#{file_path}' does not exist. Use `rails credentials:edit` to change that." end end end diff --git a/railties/test/commands/credentials_test.rb b/railties/test/commands/credentials_test.rb index 5b8b9e4eda..7842b0db61 100644 --- a/railties/test/commands/credentials_test.rb +++ b/railties/test/commands/credentials_test.rb @@ -55,6 +55,14 @@ class Rails::Command::CredentialsCommandTest < ActiveSupport::TestCase end end + test "edit command modifies file specified by environment option" do + assert_match(/access_key_id: 123/, run_edit_command(environment: "production")) + Dir.chdir(app_path) do + assert File.exist?("config/credentials/production.key") + assert File.exist?("config/credentials/production.yml.enc") + end + end + test "show credentials" do assert_match(/access_key_id: 123/, run_show_command) end @@ -70,17 +78,25 @@ class Rails::Command::CredentialsCommandTest < ActiveSupport::TestCase remove_file "config/master.key" add_to_config "config.require_master_key = false" - assert_match(/Missing master key to decrypt credentials/, run_show_command) + assert_match(/Missing 'config\/master\.key' to decrypt credentials/, run_show_command) + end + + test "show command displays content specified by environment option" do + run_edit_command(environment: "production") + + assert_match(/access_key_id: 123/, run_show_command(environment: "production")) end private - def run_edit_command(editor: "cat") + def run_edit_command(editor: "cat", environment: nil, **options) switch_env("EDITOR", editor) do - rails "credentials:edit" + args = environment ? ["--environment", environment] : [] + rails "credentials:edit", args, **options end end - def run_show_command(**options) - rails "credentials:show", **options + def run_show_command(environment: nil, **options) + args = environment ? ["--environment", environment] : [] + rails "credentials:show", args, **options end end diff --git a/railties/test/credentials_test.rb b/railties/test/credentials_test.rb new file mode 100644 index 0000000000..03370e0fc7 --- /dev/null +++ b/railties/test/credentials_test.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require "isolation/abstract_unit" + +class Rails::CredentialsTest < ActiveSupport::TestCase + include ActiveSupport::Testing::Isolation + + setup :build_app + teardown :teardown_app + + test "reads credentials from environment specific path" do + with_credentials do |content, key| + Dir.chdir(app_path) do + Dir.mkdir("config/credentials") + File.write("config/credentials/production.yml.enc", content) + File.write("config/credentials/production.key", key) + end + + app("production") + + assert_equal "revealed", Rails.application.credentials.mystery + end + end + + test "reads credentials from customized path and key" do + with_credentials do |content, key| + Dir.chdir(app_path) do + Dir.mkdir("config/credentials") + File.write("config/credentials/staging.yml.enc", content) + File.write("config/credentials/staging.key", key) + end + + add_to_env_config("production", "config.credentials.content_path = config.root.join('config/credentials/staging.yml.enc')") + add_to_env_config("production", "config.credentials.key_path = config.root.join('config/credentials/staging.key')") + app("production") + + assert_equal "revealed", Rails.application.credentials.mystery + end + end + + private + def with_credentials + key = "2117e775dc2024d4f49ddf3aeb585919" + # secret_key_base: secret + # mystery: revealed + content = "vgvKu4MBepIgZ5VHQMMPwnQNsLlWD9LKmJHu3UA/8yj6x+3fNhz3DwL9brX7UA==--qLdxHP6e34xeTAiI--nrcAsleXuo9NqiEuhntAhw==" + yield(content, key) + end +end |