diff options
25 files changed, 429 insertions, 33 deletions
diff --git a/actionmailbox/CHANGELOG.md b/actionmailbox/CHANGELOG.md index bca3d1d9ed..0d4799a7b8 100644 --- a/actionmailbox/CHANGELOG.md +++ b/actionmailbox/CHANGELOG.md @@ -1,3 +1,7 @@ +* Fix Bcc header not being included with emails from `create_inbound_email_from` test helpers. + + *jduff* + * Add `ApplicationMailbox.mailbox_for` to expose mailbox routing. *James Dabbs* diff --git a/actionmailbox/lib/action_mailbox/test_helper.rb b/actionmailbox/lib/action_mailbox/test_helper.rb index d248fa8734..dc6a0229f7 100644 --- a/actionmailbox/lib/action_mailbox/test_helper.rb +++ b/actionmailbox/lib/action_mailbox/test_helper.rb @@ -14,7 +14,11 @@ module ActionMailbox # # create_inbound_email_from_mail(from: "david@loudthinking.com", subject: "Hello!") def create_inbound_email_from_mail(status: :processing, **mail_options) - create_inbound_email_from_source Mail.new(mail_options).to_s, status: status + mail = Mail.new(mail_options) + # Bcc header is not encoded by default + mail[:bcc].include_in_headers = true if mail[:bcc] + + create_inbound_email_from_source mail.to_s, status: status end # Create an +InboundEmail+ using the raw rfc822 +source+ as text. diff --git a/actionmailbox/test/controllers/rails/action_mailbox/inbound_emails_controller_test.rb b/actionmailbox/test/controllers/rails/action_mailbox/inbound_emails_controller_test.rb index 06f5a9d6db..33282d36f7 100644 --- a/actionmailbox/test/controllers/rails/action_mailbox/inbound_emails_controller_test.rb +++ b/actionmailbox/test/controllers/rails/action_mailbox/inbound_emails_controller_test.rb @@ -30,6 +30,27 @@ class Rails::Conductor::ActionMailbox::InboundEmailsControllerTest < ActionDispa end end + test "create inbound email with bcc" do + with_rails_env("development") do + assert_difference -> { ActionMailbox::InboundEmail.count }, +1 do + post rails_conductor_inbound_emails_path, params: { + mail: { + from: "Jason Fried <jason@37signals.com>", + bcc: "Replies <replies@example.com>", + subject: "Hey there", + body: "How's it going?" + } + } + end + + mail = ActionMailbox::InboundEmail.last.mail + assert_equal %w[ jason@37signals.com ], mail.from + assert_equal %w[ replies@example.com ], mail.bcc + assert_equal "Hey there", mail.subject + assert_equal "How's it going?", mail.body.decoded + end + end + test "create inbound email with attachments" do with_rails_env("development") do assert_difference -> { ActionMailbox::InboundEmail.count }, +1 do diff --git a/actionmailbox/test/unit/router_test.rb b/actionmailbox/test/unit/router_test.rb index 4dd3730604..7eb8e04a73 100644 --- a/actionmailbox/test/unit/router_test.rb +++ b/actionmailbox/test/unit/router_test.rb @@ -51,6 +51,15 @@ module ActionMailbox assert_equal inbound_email.mail, $processed_mail end + test "single string routing on bcc" do + @router.add_routes("first@example.com" => :first) + + inbound_email = create_inbound_email_from_mail(to: "someone@example.com", bcc: "first@example.com", subject: "This is a reply") + @router.route inbound_email + assert_equal "FirstMailbox", $processed_by + assert_equal inbound_email.mail, $processed_mail + end + test "single string routing case-insensitively" do @router.add_routes("first@example.com" => :first) diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index ca7ae6621b..8bb2c73e52 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,18 @@ +* Add `Vary: Accept` header when using `Accept` header for response + + For some requests like `/users/1`, Rails uses requests' `Accept` + header to determine what to return. And if we don't add `Vary` + in the response header, browsers might accidentally cache different + types of content, which would cause issues: e.g. javascript got displayed + instead of html content. This PR fixes these issues by adding `Vary: Accept` + in these types of requests. For more detailed problem description, please read: + + https://github.com/rails/rails/pull/36213 + + Fixes #25842 + + *Stan Lo* + * Fix IntegrationTest `follow_redirect!` to follow redirection using the same HTTP verb when following a 307 redirection. diff --git a/actionpack/lib/abstract_controller/rendering.rb b/actionpack/lib/abstract_controller/rendering.rb index 8ba2b25552..280af50a44 100644 --- a/actionpack/lib/abstract_controller/rendering.rb +++ b/actionpack/lib/abstract_controller/rendering.rb @@ -28,6 +28,7 @@ module AbstractController else _set_rendered_content_type rendered_format end + _set_vary_header self.response_body = rendered_body end @@ -109,6 +110,9 @@ module AbstractController def _set_html_content_type # :nodoc: end + def _set_vary_header # :nodoc: + end + def _set_rendered_content_type(format) # :nodoc: end diff --git a/actionpack/lib/action_controller/metal/rendering.rb b/actionpack/lib/action_controller/metal/rendering.rb index efa5de313c..fd22c4fa64 100644 --- a/actionpack/lib/action_controller/metal/rendering.rb +++ b/actionpack/lib/action_controller/metal/rendering.rb @@ -77,6 +77,10 @@ module ActionController end end + def _set_vary_header + self.headers["Vary"] = "Accept" if request.should_apply_vary_header? + end + # Normalize arguments by catching blocks and setting them on :update. def _normalize_args(action = nil, options = {}, &blk) options = super diff --git a/actionpack/lib/action_dispatch/http/mime_negotiation.rb b/actionpack/lib/action_dispatch/http/mime_negotiation.rb index a2cac49082..ac0ff133eb 100644 --- a/actionpack/lib/action_dispatch/http/mime_negotiation.rb +++ b/actionpack/lib/action_dispatch/http/mime_negotiation.rb @@ -62,13 +62,7 @@ module ActionDispatch def formats fetch_header("action_dispatch.request.formats") do |k| - params_readable = begin - parameters[:format] - rescue *RESCUABLE_MIME_FORMAT_ERRORS - false - end - - v = if params_readable + v = if params_readable? Array(Mime[parameters[:format]]) elsif use_accept_header && valid_accept_header accepts @@ -153,9 +147,19 @@ module ActionDispatch order.include?(Mime::ALL) ? format : nil end + def should_apply_vary_header? + !params_readable? && use_accept_header && valid_accept_header + end + private BROWSER_LIKE_ACCEPTS = /,\s*\*\/\*|\*\/\*\s*,/ + def params_readable? # :doc: + parameters[:format] + rescue *RESCUABLE_MIME_FORMAT_ERRORS + false + end + def valid_accept_header # :doc: (xhr? && (accept.present? || content_mime_type)) || (accept.present? && accept !~ BROWSER_LIKE_ACCEPTS) diff --git a/actionpack/lib/action_dispatch/journey/route.rb b/actionpack/lib/action_dispatch/journey/route.rb index 4aee7a6f83..9184676801 100644 --- a/actionpack/lib/action_dispatch/journey/route.rb +++ b/actionpack/lib/action_dispatch/journey/route.rb @@ -148,7 +148,7 @@ module ActionDispatch end def glob? - !path.spec.grep(Nodes::Star).empty? + path.spec.any?(Nodes::Star) end def dispatcher? diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index caacd66bd6..cb7c2467ac 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -543,6 +543,32 @@ class IntegrationProcessTest < ActionDispatch::IntegrationTest end end + def test_setting_vary_header_when_request_is_xhr_with_accept_header + with_test_route_set do + get "/get", headers: { "Accept" => "application/json" }, xhr: true + assert_equal "Accept", response.headers["Vary"] + end + end + + def test_not_setting_vary_header_when_format_is_provided + with_test_route_set do + get "/get", params: { format: "json" } + assert_nil response.headers["Vary"] + end + end + + def test_not_setting_vary_header_when_ignore_accept_header_is_set + original_ignore_accept_header = ActionDispatch::Request.ignore_accept_header + ActionDispatch::Request.ignore_accept_header = true + + with_test_route_set do + get "/get", headers: { "Accept" => "application/json" }, xhr: true + assert_nil response.headers["Vary"] + end + ensure + ActionDispatch::Request.ignore_accept_header = original_ignore_accept_header + end + private def with_default_headers(headers) original = ActionDispatch::Response.default_headers diff --git a/actionview/app/assets/javascripts/rails-ujs/utils/form.coffee b/actionview/app/assets/javascripts/rails-ujs/utils/form.coffee index 736cab08db..9e11bfa7ed 100644 --- a/actionview/app/assets/javascripts/rails-ujs/utils/form.coffee +++ b/actionview/app/assets/javascripts/rails-ujs/utils/form.coffee @@ -11,6 +11,7 @@ Rails.serializeElement = (element, additionalParam) -> inputs.forEach (input) -> return if !input.name || input.disabled + return if input.closest('fieldset[disabled]') if matches(input, 'select') toArray(input.options).forEach (option) -> params.push(name: input.name, value: option.value) if option.selected diff --git a/actionview/lib/action_view/digestor.rb b/actionview/lib/action_view/digestor.rb index 7517410ea5..cdf436ccae 100644 --- a/actionview/lib/action_view/digestor.rb +++ b/actionview/lib/action_view/digestor.rb @@ -13,7 +13,7 @@ module ActionView # * <tt>format</tt> - Template format # * <tt>finder</tt> - An instance of <tt>ActionView::LookupContext</tt> # * <tt>dependencies</tt> - An array of dependent views - def digest(name:, format:, finder:, dependencies: nil) + def digest(name:, format: nil, finder:, dependencies: nil) if dependencies.nil? || dependencies.empty? cache_key = "#{name}.#{format}" else diff --git a/actionview/test/ujs/public/test/data-remote.js b/actionview/test/ujs/public/test/data-remote.js index 16ea114f3b..dbbb383995 100644 --- a/actionview/test/ujs/public/test/data-remote.js +++ b/actionview/test/ujs/public/test/data-remote.js @@ -490,4 +490,22 @@ asyncTest('changing a select option without "data-url" attribute still fires aja setTimeout(function() { start() }, 20) }) +asyncTest('inputs inside disabled fieldset are not submited on remote forms', 3, function() { + $('form') + .append('<fieldset>\ + <input name="description" value="A wise man" />\ + </fieldset>') + .append('<fieldset disabled="disabled">\ + <input name="age" />\ + </fieldset>') + .bindNative('ajax:success', function(e, data, status, xhr) { + equal(data.params.user_name, 'john') + equal(data.params.description, 'A wise man') + equal(data.params.age, undefined) + + start() + }) + .triggerNative('submit') +}) + })() diff --git a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb index 6ad4c75fb5..2072f93194 100644 --- a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb +++ b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb @@ -62,7 +62,7 @@ module ActiveRecord::Associations::Builder # :nodoc: def middle_reflection(join_model) middle_name = [lhs_model.name.downcase.pluralize, - association_name].join("_").gsub("::", "_").to_sym + association_name.to_s].sort.join("_").gsub("::", "_").to_sym middle_options = middle_options join_model HasMany.create_reflection(lhs_model, diff --git a/activerecord/lib/active_record/database_configurations.rb b/activerecord/lib/active_record/database_configurations.rb index 8baa0f5af6..3e387782f6 100644 --- a/activerecord/lib/active_record/database_configurations.rb +++ b/activerecord/lib/active_record/database_configurations.rb @@ -91,6 +91,19 @@ module ActiveRecord end alias :blank? :empty? + def each + throw_getter_deprecation(:each) + configurations.each { |config| + yield [config.env_name, config.config] + } + end + + def first + throw_getter_deprecation(:first) + config = configurations.first + [config.env_name, config.config] + end + private def env_with_configs(env = nil) if env @@ -176,9 +189,6 @@ module ActiveRecord def method_missing(method, *args, &blk) case method - when :each, :first - throw_getter_deprecation(method) - configurations.send(method, *args, &blk) when :fetch throw_getter_deprecation(method) configs_for(env_name: args.first) diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index 25cfa0a723..de9742b250 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -700,10 +700,17 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_equal ["id"], developers(:david).projects.select(:id).first.attributes.keys end + def test_join_middle_table_alias + assert_equal( + 2, + Project.includes(:developers_projects).where.not("developers_projects.joined_on": nil).to_a.size + ) + end + def test_join_table_alias assert_equal( 3, - Developer.includes(projects: :developers).where.not("projects_developers_projects_join.joined_on": nil).to_a.size + Developer.includes(projects: :developers).where.not("developers_projects_projects_join.joined_on": nil).to_a.size ) end @@ -716,7 +723,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_equal( 3, - Developer.includes(projects: :developers).where.not("projects_developers_projects_join.joined_on": nil).group(group.join(",")).to_a.size + Developer.includes(projects: :developers).where.not("developers_projects_projects_join.joined_on": nil).group(group.join(",")).to_a.size ) end diff --git a/activerecord/test/cases/database_configurations_test.rb b/activerecord/test/cases/database_configurations_test.rb index ed8151f01a..725d1b5d1b 100644 --- a/activerecord/test/cases/database_configurations_test.rb +++ b/activerecord/test/cases/database_configurations_test.rb @@ -80,17 +80,20 @@ class LegacyDatabaseConfigurationsTest < ActiveRecord::TestCase def test_each_is_deprecated assert_deprecated do - ActiveRecord::Base.configurations.each do |db_config| - assert_equal "primary", db_config.spec_name + all_configs = ActiveRecord::Base.configurations.values + ActiveRecord::Base.configurations.each do |env_name, config| + assert_includes ["arunit", "arunit2", "arunit_without_prepared_statements"], env_name + assert_includes all_configs, config end end end def test_first_is_deprecated + first_config = ActiveRecord::Base.configurations.values.first assert_deprecated do - db_config = ActiveRecord::Base.configurations.first - assert_equal "arunit", db_config.env_name - assert_equal "primary", db_config.spec_name + env_name, config = ActiveRecord::Base.configurations.first + assert_equal "arunit", env_name + assert_equal first_config, config end end diff --git a/activesupport/lib/active_support/inflector/transliterate.rb b/activesupport/lib/active_support/inflector/transliterate.rb index ec6e9ccb59..0751f8a3ad 100644 --- a/activesupport/lib/active_support/inflector/transliterate.rb +++ b/activesupport/lib/active_support/inflector/transliterate.rb @@ -56,14 +56,39 @@ module ActiveSupport # # transliterate('Jürgen', locale: :de) # # => "Juergen" + # + # Transliteration is restricted to UTF-8, US-ASCII and GB18030 strings + # Other encodings will raise an ArgumentError. def transliterate(string, replacement = "?", locale: nil) raise ArgumentError, "Can only transliterate strings. Received #{string.class.name}" unless string.is_a?(String) - I18n.transliterate( + allowed_encodings = [Encoding::UTF_8, Encoding::US_ASCII, Encoding::GB18030] + raise ArgumentError, "Can not transliterate strings with #{string.encoding} encoding" unless allowed_encodings.include?(string.encoding) + + input_encoding = string.encoding + + # US-ASCII is a subset of UTF-8 so we'll force encoding as UTF-8 if + # US-ASCII is given. This way we can let tidy_bytes handle the string + # in the same way as we do for UTF-8 + string.force_encoding(Encoding::UTF_8) if string.encoding == Encoding::US_ASCII + + # GB18030 is Unicode compatible but is not a direct mapping so needs to be + # transcoded. Using invalid/undef :replace will result in loss of data in + # the event of invalid characters, but since tidy_bytes will replace + # invalid/undef with a "?" we're safe to do the same beforehand + string.encode!(Encoding::UTF_8, invalid: :replace, undef: :replace) if string.encoding == Encoding::GB18030 + + transliterated = I18n.transliterate( ActiveSupport::Multibyte::Unicode.tidy_bytes(string).unicode_normalize(:nfc), replacement: replacement, locale: locale ) + + # Restore the string encoding of the input if it was not UTF-8. + # Apply invalid/undef :replace as tidy_bytes does + transliterated.encode!(input_encoding, invalid: :replace, undef: :replace) if input_encoding != transliterated.encoding + + transliterated end # Replaces special characters in a string so that it may be used as part of diff --git a/activesupport/test/transliterate_test.rb b/activesupport/test/transliterate_test.rb index 9e29a93ea0..2e02b5e938 100644 --- a/activesupport/test/transliterate_test.rb +++ b/activesupport/test/transliterate_test.rb @@ -57,4 +57,53 @@ class TransliterateTest < ActiveSupport::TestCase end assert_equal "Can only transliterate strings. Received Object", exception.message end + + def test_transliterate_handles_strings_with_valid_utf8_encodings + string = String.new("A", encoding: Encoding::UTF_8) + assert_equal "A", ActiveSupport::Inflector.transliterate(string) + end + + def test_transliterate_handles_strings_with_valid_us_ascii_encodings + string = String.new("A", encoding: Encoding::US_ASCII) + transcoded = ActiveSupport::Inflector.transliterate(string) + assert_equal "A", transcoded + assert_equal Encoding::US_ASCII, transcoded.encoding + end + + def test_transliterate_handles_strings_with_valid_gb18030_encodings + string = String.new("A", encoding: Encoding::GB18030) + transcoded = ActiveSupport::Inflector.transliterate(string) + assert_equal "A", transcoded + assert_equal Encoding::GB18030, transcoded.encoding + end + + def test_transliterate_handles_strings_with_incompatible_encodings + incompatible_encodings = Encoding.list - [ + Encoding::UTF_8, + Encoding::US_ASCII, + Encoding::GB18030 + ] + incompatible_encodings.each do |encoding| + string = String.new("", encoding: encoding) + exception = assert_raises ArgumentError do + ActiveSupport::Inflector.transliterate(string) + end + assert_equal "Can not transliterate strings with #{encoding} encoding", exception.message + end + end + + def test_transliterate_handles_strings_with_invalid_utf8_bytes + string = String.new("\255", encoding: Encoding::UTF_8) + assert_equal "?", ActiveSupport::Inflector.transliterate(string) + end + + def test_transliterate_handles_strings_with_invalid_us_ascii_bytes + string = String.new("\255", encoding: Encoding::US_ASCII) + assert_equal "?", ActiveSupport::Inflector.transliterate(string) + end + + def test_transliterate_handles_strings_with_invalid_gb18030_bytes + string = String.new("\255", encoding: Encoding::GB18030) + assert_equal "?", ActiveSupport::Inflector.transliterate(string) + end end diff --git a/guides/source/working_with_javascript_in_rails.md b/guides/source/working_with_javascript_in_rails.md index 8cf8efefd0..b740e933ba 100644 --- a/guides/source/working_with_javascript_in_rails.md +++ b/guides/source/working_with_javascript_in_rails.md @@ -14,6 +14,7 @@ After reading this guide, you will know: * How Rails' built-in helpers assist you. * How to handle Ajax on the server side. * The Turbolinks gem. +* How to include your Cross-Site Request Forgery token in request headers ------------------------------------------------------------------------------- @@ -524,6 +525,23 @@ For more details, including other events you can bind to, check out [the Turbolinks README](https://github.com/turbolinks/turbolinks/blob/master/README.md). +Cross-Site Request Forgery (CSRF) token in Ajax +---- + +When using another library to make Ajax calls, it is necessary to add +the security token as a default header for Ajax calls in your library. To get +the token: + +```javascript +var token = document.getElementsByName('csrf-token')[0].content +``` + +You can then submit this token as a X-CSRF-Token in your header for your +Ajax requst. You do not need to add a CSRF for GET requests, only non-GET +requests. + +You can read more about about Cross-Site Request Forgery in [Security](https://guides.rubyonrails.org/security.html#cross-site-request-forgery-csrf) + Other Resources --------------- diff --git a/railties/lib/rails/command/helpers/pretty_credentials.rb b/railties/lib/rails/command/helpers/pretty_credentials.rb new file mode 100644 index 0000000000..873ed0e825 --- /dev/null +++ b/railties/lib/rails/command/helpers/pretty_credentials.rb @@ -0,0 +1,55 @@ +# 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/credentials_command.rb b/railties/lib/rails/commands/credentials/credentials_command.rb index e23a1b3008..772e105007 100644 --- a/railties/lib/rails/commands/credentials/credentials_command.rb +++ b/railties/lib/rails/commands/credentials/credentials_command.rb @@ -2,12 +2,15 @@ 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 self.environment_desc = "Uses credentials from config/credentials/:environment.yml.enc encrypted by config/credentials/:environment.key key" @@ -34,20 +37,29 @@ module Rails 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 - extract_environment_option_from_argument(default_environment: nil) + 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) require_application! - say credentials.read.presence || missing_credentials_message + say credentials(git_textconv_path).read.presence || fallback_message || missing_credentials_message + rescue => e + raise(e) unless git_textconv_path + fallback_message end private - def credentials - Rails.application.encrypted(content_path, key_path: key_path) + def credentials(content = nil) + Rails.application.encrypted(content || content_path, key_path: key_path) end def ensure_encryption_key_has_been_added @@ -77,7 +89,6 @@ module Rails end end - def content_path options[:environment] ? "config/credentials/#{options[:environment]}.yml.enc" : "config/credentials.yml.enc" end @@ -86,6 +97,17 @@ module Rails options[:environment] ? "config/credentials/#{options[:environment]}.key" : "config/master.key" 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) + end def encryption_key_file_generator require "rails/generators" diff --git a/railties/test/application/rake_test.rb b/railties/test/application/rake_test.rb index fe56e3d076..e8456e8c19 100644 --- a/railties/test/application/rake_test.rb +++ b/railties/test/application/rake_test.rb @@ -162,7 +162,6 @@ module ApplicationTests rails "generate", "scaffold", "user", "username:string", "password:string" with_rails_env("test") do rails("db:migrate") - rails("webpacker:compile") end output = rails("test") @@ -194,7 +193,6 @@ module ApplicationTests rails "generate", "scaffold", "LineItems", "product:references", "cart:belongs_to" with_rails_env("test") do rails("db:migrate") - rails("webpacker:compile") end output = rails("test") diff --git a/railties/test/commands/credentials_test.rb b/railties/test/commands/credentials_test.rb index 0ee36081c0..562e2ec382 100644 --- a/railties/test/commands/credentials_test.rb +++ b/railties/test/commands/credentials_test.rb @@ -4,6 +4,7 @@ require "isolation/abstract_unit" require "env_helpers" require "rails/command" require "rails/commands/credentials/credentials_command" +require "fileutils" class Rails::Command::CredentialsCommandTest < ActiveSupport::TestCase include ActiveSupport::Testing::Isolation, EnvHelpers @@ -88,10 +89,107 @@ 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) + end + + test "edit doesn't ask the user to opt in to pretty credentials when alreasy asked" do + app_file("tmp/rails_pretty_credentials", "") + + assert_no_match(/Would you like to make the credentials diff from git/, run_edit_command) + 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) + + assert_no_match(/Would you like to make the credentials diff from git/, run_edit_command) + end + + test "edit ask the user to opt in to pretty credentials, user accepts" do + file = File.open("foo", "w") + file.write("y") + file.rewind + + run_edit_command(stdin: file.path) + + 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.delete(file) + end + + test "edit ask the user to opt in to pretty credentials, user refuses" do + file = File.open("foo", "w") + file.write("n") + file.rewind + + run_edit_command(stdin: file.path) + + git_attributes = app_path(".gitattributes") + assert_not(File.exist?(git_attributes)) + ensure + File.delete(file) + end + test "show credentials" do assert_match(/access_key_id: 123/, run_show_command) end + test "show command when argument is provided (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")) + end + + test "show command when argument is provided (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) + + assert_match(/access_key_id: 123/, run_show_command(file_path)) + ensure + FileUtils.rm_rf(dir) + 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")) + end + + test "show command when argument is provided (git diff) and 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")) + end + + test "show command when argument is provided (git diff) return the raw encrypted content in an error occurs" 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) + + assert_match(file_content, run_show_command(file_path)) + ensure + FileUtils.rm_rf(dir) + 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" @@ -128,8 +226,9 @@ class Rails::Command::CredentialsCommandTest < ActiveSupport::TestCase end end - def run_show_command(environment: nil, **options) + def run_show_command(path = nil, environment: nil, **options) args = environment ? ["--environment", environment] : [] + args.unshift(path) rails "credentials:show", args, **options end end diff --git a/railties/test/isolation/abstract_unit.rb b/railties/test/isolation/abstract_unit.rb index 0fe62df8ba..6077ba3ee7 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) + def rails(*args, allow_failure: false, stderr: false, stdin: File::NULL) args = args.flatten fork = true @@ -328,7 +328,7 @@ module TestHelpers out_read.close err_read.close if err_read - $stdin.reopen(File::NULL, "r") + $stdin.reopen(stdin, "r") $stdout.reopen(out_write) $stderr.reopen(err_write) |