diff options
14 files changed, 198 insertions, 185 deletions
diff --git a/actionpack/lib/action_controller/metal/renderers.rb b/actionpack/lib/action_controller/metal/renderers.rb index a251c29d23..660aef4106 100644 --- a/actionpack/lib/action_controller/metal/renderers.rb +++ b/actionpack/lib/action_controller/metal/renderers.rb @@ -163,18 +163,18 @@ module ActionController "/**/#{options[:callback]}(#{json})" else - self.content_type ||= Mime[:json] + self.content_type = Mime[:json] if media_type.nil? json end end add :js do |js, options| - self.content_type ||= Mime[:js] + self.content_type = Mime[:js] if media_type.nil? js.respond_to?(:to_js) ? js.to_js(options) : js end add :xml do |xml, options| - self.content_type ||= Mime[:xml] + self.content_type = Mime[:xml] if media_type.nil? xml.respond_to?(:to_xml) ? xml.to_xml(options) : xml end end diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index e923afb17c..31df6cea0f 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -280,7 +280,7 @@ module ActionController #:nodoc: # Check for cross-origin JavaScript responses. def non_xhr_javascript_response? # :doc: - %r(\A(?:text|application)/javascript).match?(content_type) && !request.xhr? + %r(\A(?:text|application)/javascript).match?(media_type) && !request.xhr? end AUTHENTICITY_TOKEN_LENGTH = 32 diff --git a/actionpack/test/controller/render_js_test.rb b/actionpack/test/controller/render_js_test.rb index da8f6e8062..02bc0c6ade 100644 --- a/actionpack/test/controller/render_js_test.rb +++ b/actionpack/test/controller/render_js_test.rb @@ -32,4 +32,13 @@ class RenderJSTest < ActionController::TestCase get :show_partial, format: "js", xhr: true assert_equal "partial js", @response.body end + + def test_should_not_trigger_content_type_deprecation + original = ActionDispatch::Response.return_only_media_type_on_content_type + ActionDispatch::Response.return_only_media_type_on_content_type = true + + assert_not_deprecated { get :render_vanilla_js_hello, xhr: true } + ensure + ActionDispatch::Response.return_only_media_type_on_content_type = original + end end diff --git a/actionpack/test/controller/render_json_test.rb b/actionpack/test/controller/render_json_test.rb index 82c6aaafe5..0045d2be34 100644 --- a/actionpack/test/controller/render_json_test.rb +++ b/actionpack/test/controller/render_json_test.rb @@ -133,4 +133,22 @@ class RenderJsonTest < ActionController::TestCase get :render_json_without_options assert_equal '{"a":"b"}', @response.body end + + def test_should_not_trigger_content_type_deprecation + original = ActionDispatch::Response.return_only_media_type_on_content_type + ActionDispatch::Response.return_only_media_type_on_content_type = true + + assert_not_deprecated { get :render_json_hello_world } + ensure + ActionDispatch::Response.return_only_media_type_on_content_type = original + end + + def test_should_not_trigger_content_type_deprecation_with_callback + original = ActionDispatch::Response.return_only_media_type_on_content_type + ActionDispatch::Response.return_only_media_type_on_content_type = true + + assert_not_deprecated { get :render_json_hello_world_with_callback, xhr: true } + ensure + ActionDispatch::Response.return_only_media_type_on_content_type = original + end end diff --git a/actionpack/test/controller/render_xml_test.rb b/actionpack/test/controller/render_xml_test.rb index 28d8e281ab..16da41e7c7 100644 --- a/actionpack/test/controller/render_xml_test.rb +++ b/actionpack/test/controller/render_xml_test.rb @@ -98,4 +98,13 @@ class RenderXmlTest < ActionController::TestCase get :implicit_content_type, format: "atom" assert_equal Mime[:atom], @response.media_type end + + def test_should_not_trigger_content_type_deprecation + original = ActionDispatch::Response.return_only_media_type_on_content_type + ActionDispatch::Response.return_only_media_type_on_content_type = true + + assert_not_deprecated { get :render_with_to_xml } + ensure + ActionDispatch::Response.return_only_media_type_on_content_type = original + end end diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index 01250880f5..145d04f5be 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -591,6 +591,15 @@ module RequestForgeryProtectionTests end end + def test_should_not_trigger_content_type_deprecation + original = ActionDispatch::Response.return_only_media_type_on_content_type + ActionDispatch::Response.return_only_media_type_on_content_type = true + + assert_not_deprecated { get :same_origin_js, xhr: true } + ensure + ActionDispatch::Response.return_only_media_type_on_content_type = original + end + def test_should_not_raise_error_if_token_is_not_a_string assert_blocked do patch :index, params: { custom_authenticity_token: { foo: "bar" } } diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb index 6aca7d93ad..38baf0509c 100644 --- a/activerecord/test/cases/dirty_test.rb +++ b/activerecord/test/cases/dirty_test.rb @@ -491,7 +491,7 @@ class DirtyTest < ActiveRecord::TestCase assert_equal 4, pirate.previous_changes.size assert_equal [nil, "arrr"], pirate.previous_changes["catchphrase"] - assert_equal nil, pirate.catchphrase_previously_was + assert_nil pirate.catchphrase_previously_was assert_equal [nil, pirate.id], pirate.previous_changes["id"] assert_nil pirate.previous_changes["updated_on"][0] assert_not_nil pirate.previous_changes["updated_on"][1] @@ -508,7 +508,7 @@ class DirtyTest < ActiveRecord::TestCase assert_equal 4, pirate.previous_changes.size assert_equal [nil, "arrr"], pirate.previous_changes["catchphrase"] - assert_equal nil, pirate.catchphrase_previously_was + assert_nil pirate.catchphrase_previously_was assert_equal [nil, pirate.id], pirate.previous_changes["id"] assert_includes pirate.previous_changes, "updated_on" assert_includes pirate.previous_changes, "created_on" diff --git a/guides/source/autoloading_and_reloading_constants.md b/guides/source/autoloading_and_reloading_constants.md index 212cbfaf43..2556119d33 100644 --- a/guides/source/autoloading_and_reloading_constants.md +++ b/guides/source/autoloading_and_reloading_constants.md @@ -277,7 +277,7 @@ Rails.autoloaders.main Rails.autoloaders.once ``` -The former is the main one. The latter is there mostly for backwards compatibily reasons, in case the application has something in `config.autoload_once_paths` (this is discouraged nowadays). +The former is the main one. The latter is there mostly for backwards compatibility reasons, in case the application has something in `config.autoload_once_paths` (this is discouraged nowadays). You can check if `zeitwerk` mode is enabled with diff --git a/railties/lib/rails/command/helpers/pretty_credentials.rb b/railties/lib/rails/command/helpers/pretty_credentials.rb deleted file mode 100644 index 873ed0e825..0000000000 --- a/railties/lib/rails/command/helpers/pretty_credentials.rb +++ /dev/null @@ -1,55 +0,0 @@ -# frozen_string_literal: true - -require "fileutils" - -module Rails - module Command - module Helpers - module PrettyCredentials - Error = Class.new(StandardError) - - def opt_in_pretty_credentials - unless already_answered? || already_opted_in? - answer = yes?("Would you like to make the credentials diff from git more readable in the future? [Y/n]") - end - - opt_in! if answer - FileUtils.touch(tracker) unless answer.nil? - rescue Error - say("Couldn't setup git to prettify the credentials diff") - end - - private - def already_answered? - tracker.exist? - end - - def already_opted_in? - system_call("git config --get 'diff.rails_credentials.textconv'", accepted_codes: [0, 1]) - end - - def opt_in! - system_call("git config diff.rails_credentials.textconv 'bin/rails credentials:show'", accepted_codes: [0]) - - git_attributes = Rails.root.join(".gitattributes") - File.open(git_attributes, "a+") do |file| - file.write(<<~EOM) - config/credentials/*.yml.enc diff=rails_credentials - config/credentials.yml.enc diff=rails_credentials - EOM - end - end - - def tracker - Rails.root.join("tmp", "rails_pretty_credentials") - end - - def system_call(command_line, accepted_codes:) - result = system(command_line) - raise(Error) if accepted_codes.exclude?($?.exitstatus) - result - end - end - end - end -end diff --git a/railties/lib/rails/commands/credentials/USAGE b/railties/lib/rails/commands/credentials/USAGE index c8d3fb9eda..6b896ab02a 100644 --- a/railties/lib/rails/commands/credentials/USAGE +++ b/railties/lib/rails/commands/credentials/USAGE @@ -30,6 +30,21 @@ You could prepend that to your server's start command like this: RAILS_MASTER_KEY="very-secret-and-secure" server.start +=== Set up Git to Diff Credentials + +Rails provides `rails credentials:diff --enable` to instruct Git to call `rails credentials:diff` +when `git diff` is run on a credentials file. + +Running the command enrolls the project such that all credentials files use the +"rails_credentials" diff driver in .gitattributes. + +Additionally since Git requires the driver itself to be set up in a config file +that isn't tracked Rails automatically ensures it's configured when running +`credentials:edit`. + +Otherwise each co-worker would have to run enable manually, including on each new +repo clone. + === Editing Credentials This will open a temporary file in `$EDITOR` with the decrypted contents to edit diff --git a/railties/lib/rails/commands/credentials/credentials_command.rb b/railties/lib/rails/commands/credentials/credentials_command.rb index 772e105007..9cde44558b 100644 --- a/railties/lib/rails/commands/credentials/credentials_command.rb +++ b/railties/lib/rails/commands/credentials/credentials_command.rb @@ -1,18 +1,19 @@ # frozen_string_literal: true +require "pathname" require "active_support" require "rails/command/helpers/editor" -require "rails/command/helpers/pretty_credentials" require "rails/command/environment_argument" -require "pathname" module Rails module Command class CredentialsCommand < Rails::Command::Base # :nodoc: include Helpers::Editor - include Helpers::PrettyCredentials include EnvironmentArgument + require_relative "credentials_command/diffing" + include Diffing + self.environment_desc = "Uses credentials from config/credentials/:environment.yml.enc encrypted by config/credentials/:environment.key key" no_commands do @@ -31,35 +32,44 @@ module Rails ensure_encryption_key_has_been_added if credentials.key.nil? ensure_credentials_have_been_added + ensure_rails_credentials_driver_is_set catch_editing_exceptions do change_credentials_in_system_editor end say "File encrypted and saved." - opt_in_pretty_credentials rescue ActiveSupport::MessageEncryptor::InvalidMessage say "Couldn't decrypt #{content_path}. Perhaps you passed the wrong key?" end - def show(git_textconv_path = nil) - if git_textconv_path - default_environment = extract_environment_from_path(git_textconv_path) - fallback_message = File.read(git_textconv_path) - end - - extract_environment_option_from_argument(default_environment: default_environment) + def show + extract_environment_option_from_argument(default_environment: nil) require_application! - say credentials(git_textconv_path).read.presence || fallback_message || missing_credentials_message - rescue => e - raise(e) unless git_textconv_path - fallback_message + say credentials.read.presence || missing_credentials_message + end + + option :enroll, type: :boolean, default: false, + desc: "Enrolls project in credential file diffing with `git diff`" + + def diff(content_path = nil) + if @content_path = content_path + extract_environment_option_from_argument(default_environment: extract_environment_from_path(content_path)) + require_application! + + say credentials.read.presence || credentials.content_path.read + else + require_application! + enroll_project_in_credentials_diffing if options[:enroll] + end + rescue ActiveSupport::MessageEncryptor::InvalidMessage + say credentials.content_path.read end private - def credentials(content = nil) - Rails.application.encrypted(content || content_path, key_path: key_path) + def credentials + Rails.application.encrypted(content_path, key_path: key_path) end def ensure_encryption_key_has_been_added @@ -89,8 +99,9 @@ module Rails end end + def content_path - options[:environment] ? "config/credentials/#{options[:environment]}.yml.enc" : "config/credentials.yml.enc" + @content_path ||= options[:environment] ? "config/credentials/#{options[:environment]}.yml.enc" : "config/credentials.yml.enc" end def key_path @@ -98,15 +109,7 @@ module Rails end def extract_environment_from_path(path) - regex = %r{ - ([A-Za-z0-9]+) # match the environment - (?<!credentials) # don't match if file contains the word "credentials" - # in such case, the environment should be the default one - \.yml\.enc # look for `.yml.enc` file extension - }x - path.match(regex) - - Regexp.last_match(1) + available_environments.find { |env| path.include? env } if path.match?(/\.yml\.enc$/) end def encryption_key_file_generator diff --git a/railties/lib/rails/commands/credentials/credentials_command/diffing.rb b/railties/lib/rails/commands/credentials/credentials_command/diffing.rb new file mode 100644 index 0000000000..1d34c68074 --- /dev/null +++ b/railties/lib/rails/commands/credentials/credentials_command/diffing.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +module Rails::Command::CredentialsCommand::Diffing # :nodoc: + def enroll_project_in_credentials_diffing + if enrolled? + true + else + gitattributes.write(<<~end_of_template, mode: "a") + config/credentials/*.yml.enc diff=rails_credentials + config/credentials.yml.enc diff=rails_credentials + end_of_template + + say "Project successfully enrolled!" + say "Rails ensures the rails_credentials diff driver is set when running `credentials:edit`. See `credentials:help` for more." + end + end + + def ensure_rails_credentials_driver_is_set + set_driver if enrolled? && !driver_configured? + end + + private + def enrolled? + gitattributes.read.match?(/config\/credentials(\/\*)?\.yml\.enc diff=rails_credentials/) + rescue Errno::ENOENT + false + end + + def driver_configured? + system "git config --get diff.rails_credentials.textconv", out: File::NULL + end + + def set_driver + puts "running" + system "git config diff.rails_credentials.textconv 'bin/rails credentials:diff'" + end + + def gitattributes + Rails.root.join(".gitattributes") + end +end diff --git a/railties/test/commands/credentials_test.rb b/railties/test/commands/credentials_test.rb index 3dec6fbe10..974b34836b 100644 --- a/railties/test/commands/credentials_test.rb +++ b/railties/test/commands/credentials_test.rb @@ -10,9 +10,8 @@ require "tempfile" class Rails::Command::CredentialsCommandTest < ActiveSupport::TestCase include ActiveSupport::Testing::Isolation, EnvHelpers - setup { build_app } - - teardown { teardown_app } + setup :build_app + teardown :teardown_app test "edit without editor gives hint" do run_edit_command(editor: "").tap do |output| @@ -90,134 +89,95 @@ class Rails::Command::CredentialsCommandTest < ActiveSupport::TestCase assert_match(/secret_key_base/, output) end - test "edit ask the user to opt in to pretty credentials" do - assert_match(/Would you like to make the credentials diff from git/, run_edit_command) + + test "show credentials" do + assert_match(/access_key_id: 123/, run_show_command) end - test "edit doesn't ask the user to opt in to pretty credentials when alreasy asked" do - app_file("tmp/rails_pretty_credentials", "") + test "show command raises error when require_master_key is specified and key does not exist" do + remove_file "config/master.key" + add_to_config "config.require_master_key = true" - assert_no_match(/Would you like to make the credentials diff from git/, run_edit_command) + assert_match(/Missing encryption key to decrypt file with/, run_show_command(allow_failure: true)) end - test "edit doesn't ask the user to opt in when user already opted in" do - content = <<~EOM - [diff "rails_credentials"] - textconv = bin/rails credentials:show - EOM - app_file(".git/config", content) + test "show command does not raise error when require_master_key is false and master key does not exist" do + remove_file "config/master.key" + add_to_config "config.require_master_key = false" - assert_no_match(/Would you like to make the credentials diff from git/, run_edit_command) + assert_match(/Missing 'config\/master\.key' to decrypt credentials/, run_show_command) end - test "edit ask the user to opt in to pretty credentials, user accepts" do - file = Tempfile.open("credentials_test") - file.write("y") - file.rewind + test "show command displays content specified by environment option" do + run_edit_command(environment: "production") - run_edit_command(stdin: file.path) + assert_match(/access_key_id: 123/, run_show_command(environment: "production")) + end - git_attributes = app_path(".gitattributes") - expected = <<~EOM - config/credentials/*.yml.enc diff=rails_credentials - config/credentials.yml.enc diff=rails_credentials - EOM - assert(File.exist?(git_attributes)) - assert_equal(expected, File.read(git_attributes)) - Dir.chdir(app_path) do - assert_equal("bin/rails credentials:show\n", `git config --get 'diff.rails_credentials.textconv'`) - end - ensure - file.close! + test "show command properly expands environment option" do + run_edit_command(environment: "production") + + output = run_show_command(environment: "prod") + assert_match(/access_key_id: 123/, output) + assert_no_match(/secret_key_base/, output) end - test "edit ask the user to opt in to pretty credentials, user refuses" do - file = Tempfile.open("credentials_test") - file.write("n") - file.rewind - run_edit_command(stdin: file.path) + test "diff enroll diffing" do + assert_match("successfully enrolled", run_diff_command(enroll: true)) - git_attributes = app_path(".gitattributes") - assert_not(File.exist?(git_attributes)) - ensure - file.close! + assert_equal <<~EOM, File.read(app_path(".gitattributes")) + config/credentials/*.yml.enc diff=rails_credentials + config/credentials.yml.enc diff=rails_credentials + EOM end - test "show credentials" do - assert_match(/access_key_id: 123/, run_show_command) + test "running edit after enrolling in diffing sets diff driver" do + run_diff_command(enroll: true) + run_edit_command + + Dir.chdir(app_path) do + assert_equal "bin/rails credentials:diff", `git config --get 'diff.rails_credentials.textconv'`.strip + end end - test "show command when argument is provided (from git diff left file)" do + test "diff from git diff left file" do run_edit_command(environment: "development") - assert_match(/access_key_id: 123/, run_show_command("config/credentials/development.yml.enc")) + assert_match(/access_key_id: 123/, run_diff_command("config/credentials/development.yml.enc")) end - test "show command when argument is provided (from git diff right file)" do + test "diff from git diff right file" do run_edit_command(environment: "development") - dir = Dir.mktmpdir - file_path = File.join(dir, "KnAM4a_development.yml.enc") - file_content = File.read(app_path("config", "credentials", "development.yml.enc")) - File.write(file_path, file_content) + content_path = app_path("config", "credentials", "KnAM4a_development.yml.enc") + File.write(content_path, + File.read(app_path("config", "credentials", "development.yml.enc"))) - assert_match(/access_key_id: 123/, run_show_command(file_path)) - ensure - FileUtils.rm_rf(dir) + assert_match(/access_key_id: 123/, run_diff_command(content_path)) end - test "show command when argument is provided (git diff) and filename is the master credentials" do - assert_match(/access_key_id: 123/, run_show_command("config/credentials.yml.enc")) + test "diff for main credentials" do + assert_match(/access_key_id: 123/, run_diff_command("config/credentials.yml.enc")) end - test "show command when argument is provided (git diff) and master key is not available" do + test "diff when master key is not available" do remove_file "config/master.key" raw_content = File.read(app_path("config", "credentials.yml.enc")) - assert_match(raw_content, run_show_command("config/credentials.yml.enc")) + assert_match(raw_content, run_diff_command("config/credentials.yml.enc")) end - test "show command when argument is provided (git diff) return the raw encrypted content in an error occurs" do + test "diff returns raw encrypted content when errors occur" do run_edit_command(environment: "development") - dir = Dir.mktmpdir - file_path = File.join(dir, "20190807development.yml.enc") - file_content = File.read(app_path("config", "credentials", "development.yml.enc")) - File.write(file_path, file_content) + content_path = app_path("20190807development.yml.enc") + encrypted_content = File.read(app_path("config", "credentials", "development.yml.enc")) + File.write(content_path, encrypted_content + "ruin decryption") - assert_match(file_content, run_show_command(file_path)) - ensure - FileUtils.rm_rf(dir) + assert_match(encrypted_content, run_diff_command(content_path)) end - test "show command raises error when require_master_key is specified and key does not exist" do - remove_file "config/master.key" - add_to_config "config.require_master_key = true" - - assert_match(/Missing encryption key to decrypt file with/, run_show_command(allow_failure: true)) - end - - test "show command does not raise error when require_master_key is false and master key does not exist" do - remove_file "config/master.key" - add_to_config "config.require_master_key = false" - - 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 - - test "show command properly expands environment option" do - run_edit_command(environment: "production") - - output = run_show_command(environment: "prod") - assert_match(/access_key_id: 123/, output) - assert_no_match(/secret_key_base/, output) - end private def run_edit_command(editor: "cat", environment: nil, **options) @@ -227,9 +187,13 @@ class Rails::Command::CredentialsCommandTest < ActiveSupport::TestCase end end - def run_show_command(path = nil, environment: nil, **options) + def run_show_command(environment: nil, **options) args = environment ? ["--environment", environment] : [] - args.unshift(path) rails "credentials:show", args, **options end + + def run_diff_command(path = nil, enroll: nil, **options) + args = enroll ? ["--enroll"] : [path] + rails "credentials:diff", args, **options + end end diff --git a/railties/test/isolation/abstract_unit.rb b/railties/test/isolation/abstract_unit.rb index 6077ba3ee7..0fe62df8ba 100644 --- a/railties/test/isolation/abstract_unit.rb +++ b/railties/test/isolation/abstract_unit.rb @@ -301,7 +301,7 @@ module TestHelpers # stderr:: true to pass STDERR output straight to the "real" STDERR. # By default, the STDERR and STDOUT of the process will be # combined in the returned string. - def rails(*args, allow_failure: false, stderr: false, stdin: File::NULL) + def rails(*args, allow_failure: false, stderr: false) args = args.flatten fork = true @@ -328,7 +328,7 @@ module TestHelpers out_read.close err_read.close if err_read - $stdin.reopen(stdin, "r") + $stdin.reopen(File::NULL, "r") $stdout.reopen(out_write) $stderr.reopen(err_write) |