aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--actionmailbox/CHANGELOG.md4
-rw-r--r--actionmailbox/lib/action_mailbox/test_helper.rb6
-rw-r--r--actionmailbox/test/controllers/rails/action_mailbox/inbound_emails_controller_test.rb21
-rw-r--r--actionmailbox/test/unit/router_test.rb9
-rw-r--r--actionpack/CHANGELOG.md15
-rw-r--r--actionpack/lib/abstract_controller/rendering.rb4
-rw-r--r--actionpack/lib/action_controller/metal/rendering.rb4
-rw-r--r--actionpack/lib/action_dispatch/http/mime_negotiation.rb18
-rw-r--r--actionpack/lib/action_dispatch/journey/route.rb2
-rw-r--r--actionpack/test/controller/integration_test.rb26
-rw-r--r--actionview/app/assets/javascripts/rails-ujs/utils/form.coffee1
-rw-r--r--actionview/lib/action_view/digestor.rb2
-rw-r--r--actionview/test/ujs/public/test/data-remote.js18
-rw-r--r--activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb2
-rw-r--r--activerecord/lib/active_record/database_configurations.rb16
-rw-r--r--activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb11
-rw-r--r--activerecord/test/cases/database_configurations_test.rb13
-rw-r--r--activesupport/lib/active_support/inflector/transliterate.rb27
-rw-r--r--activesupport/test/transliterate_test.rb49
-rw-r--r--guides/source/working_with_javascript_in_rails.md18
-rw-r--r--railties/lib/rails/command/helpers/pretty_credentials.rb55
-rw-r--r--railties/lib/rails/commands/credentials/credentials_command.rb34
-rw-r--r--railties/test/application/rake_test.rb2
-rw-r--r--railties/test/commands/credentials_test.rb101
-rw-r--r--railties/test/isolation/abstract_unit.rb4
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)