diff options
150 files changed, 1737 insertions, 863 deletions
@@ -13,9 +13,9 @@ gem "rake", ">= 11.1" # This needs to be with require false to ensure correct loading order, as it has to # be loaded after loading the test library. -gem "mocha", "~> 0.14", require: false +gem "mocha", require: false -gem "capybara", "~> 2.13" +gem "capybara", "~> 2.15" gem "rack-cache", "~> 1.2" gem "jquery-rails" diff --git a/Gemfile.lock b/Gemfile.lock index 787631ada5..938b4a71cc 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -309,7 +309,7 @@ GEM path_expander (~> 1.0) minitest-server (1.0.4) minitest (~> 5.0) - mocha (0.14.0) + mocha (1.3.0) metaclass (~> 0.0.1) mono_logger (1.1.0) msgpack (1.1.0) @@ -490,7 +490,7 @@ DEPENDENCIES blade-sauce_labs_plugin bootsnap (>= 1.1.0) byebug - capybara (~> 2.13) + capybara (~> 2.15) coffee-rails dalli (>= 2.2.1) delayed_job @@ -506,7 +506,7 @@ DEPENDENCIES listen (>= 3.0.5, < 3.2) mini_magick minitest-bisect - mocha (~> 0.14) + mocha mysql2 (>= 0.4.4) nokogiri (>= 1.6.8) pg (>= 0.18.0) diff --git a/actioncable/README.md b/actioncable/README.md index a060e8938e..70b39ead57 100644 --- a/actioncable/README.md +++ b/actioncable/README.md @@ -454,9 +454,9 @@ The Ruby side of things is built on top of [websocket-driver](https://github.com ## Deployment Action Cable is powered by a combination of WebSockets and threads. All of the -connection management is handled internally by utilizing Ruby’s native thread +connection management is handled internally by utilizing Ruby's native thread support, which means you can use all your regular Rails models with no problems -as long as you haven’t committed any thread-safety sins. +as long as you haven't committed any thread-safety sins. The Action Cable server does _not_ need to be a multi-threaded application server. This is because Action Cable uses the [Rack socket hijacking API](http://www.rubydoc.info/github/rack/rack/file/SPEC#Hijacking) diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 932968fa35..16090e7946 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,4 +1,20 @@ -* Cookies `:expires` option supports `ActiveSupport::Duration` object. +* Simplify cookies middleware with key rotation support + + Use the `rotate` method for both `MessageEncryptor` and + `MessageVerifier` to add key rotation support for encrypted and + signed cookies. This also helps simplify support for legacy cookie + security. + + *Michael J Coyne* + +* Use Capybara registered `:puma` server config. + + The Capybara registered `:puma` server ensures the puma server is run in process so + connection sharing and open request detection work correctly by default. + + *Thomas Walpole* + +* Cookies `:expires` option supports `ActiveSupport::Duration` object. cookies[:user_name] = { value: "assain", expires: 1.hour } cookies[:key] = { value: "a yummy cookie", expires: 6.months } @@ -7,7 +23,7 @@ *Assain Jaleel* -* Enforce signed/encrypted cookie expiry server side. +* Enforce signed/encrypted cookie expiry server side. Rails can thwart attacks by malicious clients that don't honor a cookie's expiry. diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index 845df500d8..0213987c99 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -49,6 +49,18 @@ module ActionDispatch get_header Cookies::AUTHENTICATED_ENCRYPTED_COOKIE_SALT end + def use_authenticated_cookie_encryption + get_header Cookies::USE_AUTHENTICATED_COOKIE_ENCRYPTION + end + + def encrypted_cookie_cipher + get_header Cookies::ENCRYPTED_COOKIE_CIPHER + end + + def signed_cookie_digest + get_header Cookies::SIGNED_COOKIE_DIGEST + end + def secret_token get_header Cookies::SECRET_TOKEN end @@ -64,6 +76,11 @@ module ActionDispatch def cookies_digest get_header Cookies::COOKIES_DIGEST end + + def cookies_rotations + get_header Cookies::COOKIES_ROTATIONS + end + # :startdoc: end @@ -157,10 +174,14 @@ module ActionDispatch ENCRYPTED_COOKIE_SALT = "action_dispatch.encrypted_cookie_salt".freeze ENCRYPTED_SIGNED_COOKIE_SALT = "action_dispatch.encrypted_signed_cookie_salt".freeze AUTHENTICATED_ENCRYPTED_COOKIE_SALT = "action_dispatch.authenticated_encrypted_cookie_salt".freeze + USE_AUTHENTICATED_COOKIE_ENCRYPTION = "action_dispatch.use_authenticated_cookie_encryption".freeze + ENCRYPTED_COOKIE_CIPHER = "action_dispatch.encrypted_cookie_cipher".freeze + SIGNED_COOKIE_DIGEST = "action_dispatch.signed_cookie_digest".freeze SECRET_TOKEN = "action_dispatch.secret_token".freeze SECRET_KEY_BASE = "action_dispatch.secret_key_base".freeze COOKIES_SERIALIZER = "action_dispatch.cookies_serializer".freeze COOKIES_DIGEST = "action_dispatch.cookies_digest".freeze + COOKIES_ROTATIONS = "action_dispatch.cookies_rotations".freeze # Cookies can typically store 4096 bytes. MAX_COOKIE_SIZE = 4096 @@ -201,12 +222,7 @@ module ActionDispatch # # cookies.signed[:discount] # => 45 def signed - @signed ||= - if upgrade_legacy_signed_cookies? - UpgradeLegacySignedCookieJar.new(self) - else - SignedCookieJar.new(self) - end + @signed ||= SignedKeyRotatingCookieJar.new(self) end # Returns a jar that'll automatically encrypt cookie values before sending them to the client and will decrypt them for read. @@ -223,18 +239,11 @@ module ActionDispatch # Example: # # cookies.encrypted[:discount] = 45 - # # => Set-Cookie: discount=ZS9ZZ1R4cG1pcUJ1bm80anhQang3dz09LS1mbDZDSU5scGdOT3ltQ2dTdlhSdWpRPT0%3D--ab54663c9f4e3bc340c790d6d2b71e92f5b60315; path=/ + # # => Set-Cookie: discount=DIQ7fw==--K3n//8vvnSbGq9dA--7Xh91HfLpwzbj1czhBiwOg==; path=/ # # cookies.encrypted[:discount] # => 45 def encrypted - @encrypted ||= - if upgrade_legacy_signed_cookies? - UpgradeLegacyEncryptedCookieJar.new(self) - elsif upgrade_legacy_hmac_aes_cbc_cookies? - UpgradeLegacyHmacAesCbcCookieJar.new(self) - else - EncryptedCookieJar.new(self) - end + @encrypted ||= EncryptedKeyRotatingCookieJar.new(self) end # Returns the +signed+ or +encrypted+ jar, preferring +encrypted+ if +secret_key_base+ is set. @@ -255,34 +264,18 @@ module ActionDispatch end def upgrade_legacy_hmac_aes_cbc_cookies? - request.secret_key_base.present? && - request.authenticated_encrypted_cookie_salt.present? && - request.encrypted_signed_cookie_salt.present? && - request.encrypted_cookie_salt.present? + request.secret_key_base.present? && + request.encrypted_signed_cookie_salt.present? && + request.encrypted_cookie_salt.present? && + request.use_authenticated_cookie_encryption end - end - - # Passing the ActiveSupport::MessageEncryptor::NullSerializer downstream - # to the Message{Encryptor,Verifier} allows us to handle the - # (de)serialization step within the cookie jar, which gives us the - # opportunity to detect and migrate legacy cookies. - module VerifyAndUpgradeLegacySignedMessage # :nodoc: - def initialize(*args) - super - @legacy_verifier = ActiveSupport::MessageVerifier.new(request.secret_token, serializer: ActiveSupport::MessageEncryptor::NullSerializer) - end - def verify_and_upgrade_legacy_signed_message(name, signed_message) - deserialize(name, @legacy_verifier.verify(signed_message)).tap do |value| - self[name] = { value: value } + def encrypted_cookie_cipher + request.encrypted_cookie_cipher || "aes-256-gcm" end - rescue ActiveSupport::MessageVerifier::InvalidSignature - nil - end - private - def parse(name, signed_message) - super || verify_and_upgrade_legacy_signed_message(name, signed_message) + def signed_cookie_digest + request.signed_cookie_digest || "SHA1" end end @@ -524,6 +517,7 @@ module ActionDispatch module SerializedCookieJars # :nodoc: MARSHAL_SIGNATURE = "\x04\x08".freeze + SERIALIZER = ActiveSupport::MessageEncryptor::NullSerializer protected def needs_migration?(value) @@ -534,12 +528,16 @@ module ActionDispatch serializer.dump(value) end - def deserialize(name, value) + def deserialize(name) + rotate = false + value = yield -> { rotate = true } + if value - if needs_migration?(value) - Marshal.load(value).tap do |v| - self[name] = { value: v } - end + case + when needs_migration?(value) + self[name] = Marshal.load(value) + when rotate + self[name] = serializer.load(value) else serializer.load(value) end @@ -561,24 +559,31 @@ module ActionDispatch def digest request.cookies_digest || "SHA1" end - - def key_generator - request.key_generator - end end - class SignedCookieJar < AbstractCookieJar # :nodoc: + class SignedKeyRotatingCookieJar < AbstractCookieJar # :nodoc: include SerializedCookieJars def initialize(parent_jar) super - secret = key_generator.generate_key(request.signed_cookie_salt) - @verifier = ActiveSupport::MessageVerifier.new(secret, digest: digest, serializer: ActiveSupport::MessageEncryptor::NullSerializer) + + secret = request.key_generator.generate_key(request.signed_cookie_salt) + @verifier = ActiveSupport::MessageVerifier.new(secret, digest: signed_cookie_digest, serializer: SERIALIZER) + + request.cookies_rotations.signed.each do |*secrets, **options| + @verifier.rotate(*secrets, serializer: SERIALIZER, **options) + end + + if upgrade_legacy_signed_cookies? + @verifier.rotate request.secret_token, serializer: SERIALIZER + end end private def parse(name, signed_message) - deserialize name, @verifier.verified(signed_message) + deserialize(name) do |rotate| + @verifier.verified(signed_message, on_rotation: rotate) + end end def commit(options) @@ -588,37 +593,40 @@ module ActionDispatch end end - # UpgradeLegacySignedCookieJar is used instead of SignedCookieJar if - # secrets.secret_token and secret_key_base are both set. It reads - # legacy cookies signed with the old dummy key generator and signs and - # re-saves them using the new key generator to provide a smooth upgrade path. - class UpgradeLegacySignedCookieJar < SignedCookieJar #:nodoc: - include VerifyAndUpgradeLegacySignedMessage - end - - class EncryptedCookieJar < AbstractCookieJar # :nodoc: + class EncryptedKeyRotatingCookieJar < AbstractCookieJar # :nodoc: include SerializedCookieJars def initialize(parent_jar) super - if ActiveSupport::LegacyKeyGenerator === key_generator - raise "You didn't set secret_key_base, which is required for this cookie jar. " \ - "Read the upgrade documentation to learn more about this new config option." + key_len = ActiveSupport::MessageEncryptor.key_len(encrypted_cookie_cipher) + secret = request.key_generator.generate_key(request.authenticated_encrypted_cookie_salt, key_len) + @encryptor = ActiveSupport::MessageEncryptor.new(secret, cipher: encrypted_cookie_cipher, serializer: SERIALIZER) + + request.cookies_rotations.encrypted.each do |*secrets, **options| + @encryptor.rotate(*secrets, serializer: SERIALIZER, **options) end - cipher = "aes-256-gcm" - key_len = ActiveSupport::MessageEncryptor.key_len(cipher) - secret = key_generator.generate_key(request.authenticated_encrypted_cookie_salt || "")[0, key_len] + if upgrade_legacy_hmac_aes_cbc_cookies? + legacy_cipher = "aes-256-cbc" + secret = request.key_generator.generate_key(request.encrypted_cookie_salt, ActiveSupport::MessageEncryptor.key_len(legacy_cipher)) + sign_secret = request.key_generator.generate_key(request.encrypted_signed_cookie_salt) - @encryptor = ActiveSupport::MessageEncryptor.new(secret, cipher: cipher, serializer: ActiveSupport::MessageEncryptor::NullSerializer) + @encryptor.rotate(secret, sign_secret, cipher: legacy_cipher, digest: digest, serializer: SERIALIZER) + end + + if upgrade_legacy_signed_cookies? + @legacy_verifier = ActiveSupport::MessageVerifier.new(request.secret_token, digest: digest, serializer: SERIALIZER) + end end private def parse(name, encrypted_message) - deserialize name, @encryptor.decrypt_and_verify(encrypted_message) - rescue ActiveSupport::MessageVerifier::InvalidSignature, ActiveSupport::MessageEncryptor::InvalidMessage - nil + deserialize(name) do |rotate| + @encryptor.decrypt_and_verify(encrypted_message, on_rotation: rotate) + end + rescue ActiveSupport::MessageEncryptor::InvalidMessage, ActiveSupport::MessageVerifier::InvalidSignature + parse_legacy_signed_message(name, encrypted_message) end def commit(options) @@ -626,39 +634,15 @@ module ActionDispatch raise CookieOverflow if options[:value].bytesize > MAX_COOKIE_SIZE end - end - # UpgradeLegacyEncryptedCookieJar is used by ActionDispatch::Session::CookieStore - # instead of EncryptedCookieJar if secrets.secret_token and secret_key_base - # are both set. It reads legacy cookies signed with the old dummy key generator and - # encrypts and re-saves them using the new key generator to provide a smooth upgrade path. - class UpgradeLegacyEncryptedCookieJar < EncryptedCookieJar #:nodoc: - include VerifyAndUpgradeLegacySignedMessage - end + def parse_legacy_signed_message(name, legacy_signed_message) + if defined?(@legacy_verifier) + deserialize(name) do |rotate| + rotate.call - # UpgradeLegacyHmacAesCbcCookieJar is used by ActionDispatch::Session::CookieStore - # to upgrade cookies encrypted with AES-256-CBC with HMAC to AES-256-GCM - class UpgradeLegacyHmacAesCbcCookieJar < EncryptedCookieJar - def initialize(parent_jar) - super - - secret = key_generator.generate_key(request.encrypted_cookie_salt || "")[0, ActiveSupport::MessageEncryptor.key_len] - sign_secret = key_generator.generate_key(request.encrypted_signed_cookie_salt || "") - - @legacy_encryptor = ActiveSupport::MessageEncryptor.new(secret, sign_secret, cipher: "aes-256-cbc", digest: digest, serializer: ActiveSupport::MessageEncryptor::NullSerializer) - end - - def decrypt_and_verify_legacy_encrypted_message(name, signed_message) - deserialize(name, @legacy_encryptor.decrypt_and_verify(signed_message)).tap do |value| - self[name] = { value: value } - end - rescue ActiveSupport::MessageVerifier::InvalidSignature, ActiveSupport::MessageEncryptor::InvalidMessage - nil - end - - private - def parse(name, signed_message) - super || decrypt_and_verify_legacy_encrypted_message(name, signed_message) + @legacy_verifier.verified(legacy_signed_message) + end + end end end diff --git a/actionpack/lib/action_dispatch/railtie.rb b/actionpack/lib/action_dispatch/railtie.rb index 4743a7ce61..855f2ffa47 100644 --- a/actionpack/lib/action_dispatch/railtie.rb +++ b/actionpack/lib/action_dispatch/railtie.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "action_dispatch" +require "active_support/messages/rotation_configuration" module ActionDispatch class Railtie < Rails::Railtie # :nodoc: @@ -18,6 +19,7 @@ module ActionDispatch config.action_dispatch.signed_cookie_salt = "signed cookie" config.action_dispatch.encrypted_cookie_salt = "encrypted cookie" config.action_dispatch.encrypted_signed_cookie_salt = "signed encrypted cookie" + config.action_dispatch.authenticated_encrypted_cookie_salt = "authenticated encrypted cookie" config.action_dispatch.use_authenticated_cookie_encryption = false config.action_dispatch.perform_deep_munge = true @@ -27,6 +29,8 @@ module ActionDispatch "X-Content-Type-Options" => "nosniff" } + config.action_dispatch.cookies_rotations = ActiveSupport::Messages::RotationConfiguration.new + config.eager_load_namespaces << ActionDispatch initializer "action_dispatch.configure" do |app| @@ -39,8 +43,6 @@ module ActionDispatch ActionDispatch::ExceptionWrapper.rescue_responses.merge!(config.action_dispatch.rescue_responses) ActionDispatch::ExceptionWrapper.rescue_templates.merge!(config.action_dispatch.rescue_templates) - config.action_dispatch.authenticated_encrypted_cookie_salt = "authenticated encrypted cookie" if config.action_dispatch.use_authenticated_cookie_encryption - config.action_dispatch.always_write_cookie = Rails.env.development? if config.action_dispatch.always_write_cookie.nil? ActionDispatch::Cookies::CookieJar.always_write_cookie = config.action_dispatch.always_write_cookie diff --git a/actionpack/lib/action_dispatch/system_testing/server.rb b/actionpack/lib/action_dispatch/system_testing/server.rb index 76bada8df1..32aa6a4dc4 100644 --- a/actionpack/lib/action_dispatch/system_testing/server.rb +++ b/actionpack/lib/action_dispatch/system_testing/server.rb @@ -12,29 +12,17 @@ module ActionDispatch self.silence_puma = false def run - register setup end private - def register - Capybara.register_server :rails_puma do |app, port, host| - Rack::Handler::Puma.run( - app, - Port: port, - Threads: "0:1", - Silent: self.class.silence_puma - ) - end - end - def setup set_server set_port end def set_server - Capybara.server = :rails_puma + Capybara.server = :puma, { Silent: self.class.silence_puma } end def set_port diff --git a/actionpack/test/controller/flash_test.rb b/actionpack/test/controller/flash_test.rb index d92ae0b817..34bc2c0caa 100644 --- a/actionpack/test/controller/flash_test.rb +++ b/actionpack/test/controller/flash_test.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require "abstract_unit" -require "active_support/key_generator" +require "active_support/messages/rotation_configuration" class FlashTest < ActionController::TestCase class TestController < ActionController::Base @@ -243,6 +243,7 @@ end class FlashIntegrationTest < ActionDispatch::IntegrationTest SessionKey = "_myapp_session" Generator = ActiveSupport::LegacyKeyGenerator.new("b3c631c314c0bbca50c1b2843150fe33") + Rotations = ActiveSupport::Messages::RotationConfiguration.new class TestController < ActionController::Base add_flash_types :bar @@ -348,6 +349,7 @@ class FlashIntegrationTest < ActionDispatch::IntegrationTest args[0] ||= {} args[0][:env] ||= {} args[0][:env]["action_dispatch.key_generator"] ||= Generator + args[0][:env]["action_dispatch.cookies_rotations"] = Rotations super(path, *args) end diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index 12ae95d602..eb3d2f34a8 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -2,6 +2,7 @@ require "abstract_unit" require "active_support/log_subscriber/test_helper" +require "active_support/messages/rotation_configuration" # common controller actions module RequestForgeryProtectionActions @@ -630,13 +631,14 @@ end class RequestForgeryProtectionControllerUsingNullSessionTest < ActionController::TestCase class NullSessionDummyKeyGenerator - def generate_key(secret) + def generate_key(secret, length = nil) "03312270731a2ed0d11ed091c2338a06" end end def setup @request.env[ActionDispatch::Cookies::GENERATOR_KEY] = NullSessionDummyKeyGenerator.new + @request.env[ActionDispatch::Cookies::COOKIES_ROTATIONS] = ActiveSupport::Messages::RotationConfiguration.new end test "should allow to set signed cookies" do diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index cb225c0f62..70587fa2b0 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -3,7 +3,7 @@ require "abstract_unit" require "openssl" require "active_support/key_generator" -require "active_support/message_verifier" +require "active_support/messages/rotation_configuration" class CookieJarTest < ActiveSupport::TestCase attr_reader :request @@ -287,15 +287,25 @@ class CookiesTest < ActionController::TestCase tests TestController - SALT = "b3c631c314c0bbca50c1b2843150fe33" + SECRET_KEY_BASE = "b3c631c314c0bbca50c1b2843150fe33" + SIGNED_COOKIE_SALT = "signed cookie" + ENCRYPTED_COOKIE_SALT = "encrypted cookie" + ENCRYPTED_SIGNED_COOKIE_SALT = "sigend encrypted cookie" + AUTHENTICATED_ENCRYPTED_COOKIE_SALT = "authenticated encrypted cookie" def setup super - @request.env["action_dispatch.key_generator"] = ActiveSupport::KeyGenerator.new(SALT, iterations: 2) + @request.env["action_dispatch.key_generator"] = ActiveSupport::KeyGenerator.new(SECRET_KEY_BASE, iterations: 2) + @request.env["action_dispatch.cookies_rotations"] = ActiveSupport::Messages::RotationConfiguration.new - @request.env["action_dispatch.signed_cookie_salt"] = - @request.env["action_dispatch.authenticated_encrypted_cookie_salt"] = SALT + @request.env["action_dispatch.secret_key_base"] = SECRET_KEY_BASE + @request.env["action_dispatch.use_authenticated_cookie_encryption"] = true + + @request.env["action_dispatch.signed_cookie_salt"] = SIGNED_COOKIE_SALT + @request.env["action_dispatch.encrypted_cookie_salt"] = ENCRYPTED_COOKIE_SALT + @request.env["action_dispatch.encrypted_signed_cookie_salt"] = ENCRYPTED_SIGNED_COOKIE_SALT + @request.env["action_dispatch.authenticated_encrypted_cookie_salt"] = AUTHENTICATED_ENCRYPTED_COOKIE_SALT @request.host = "www.nextangle.com" end @@ -430,28 +440,72 @@ class CookiesTest < ActionController::TestCase assert_equal 45, cookies.signed[:user_id] key_generator = @request.env["action_dispatch.key_generator"] - signed_cookie_salt = @request.env["action_dispatch.signed_cookie_salt"] - secret = key_generator.generate_key(signed_cookie_salt) + secret = key_generator.generate_key(@request.env["action_dispatch.signed_cookie_salt"]) verifier = ActiveSupport::MessageVerifier.new(secret, serializer: Marshal, digest: "SHA1") assert_equal verifier.generate(45), cookies[:user_id] end def test_signed_cookie_using_custom_digest - @request.env["action_dispatch.cookies_digest"] = "SHA256" + @request.env["action_dispatch.signed_cookie_digest"] = "SHA256" + get :set_signed_cookie cookies = @controller.send :cookies assert_not_equal 45, cookies[:user_id] assert_equal 45, cookies.signed[:user_id] key_generator = @request.env["action_dispatch.key_generator"] - signed_cookie_salt = @request.env["action_dispatch.signed_cookie_salt"] - secret = key_generator.generate_key(signed_cookie_salt) + secret = key_generator.generate_key(@request.env["action_dispatch.signed_cookie_salt"]) verifier = ActiveSupport::MessageVerifier.new(secret, serializer: Marshal, digest: "SHA256") assert_equal verifier.generate(45), cookies[:user_id] end + def test_signed_cookie_rotating_secret_and_digest + secret = "b3c631c314c0bbca50c1b2843150fe33" + + @request.env["action_dispatch.signed_cookie_digest"] = "SHA256" + @request.env["action_dispatch.cookies_rotations"].rotate :signed, secret, digest: "SHA1" + + old_message = ActiveSupport::MessageVerifier.new(secret, digest: "SHA1", serializer: Marshal).generate(45) + @request.headers["Cookie"] = "user_id=#{old_message}" + + get :get_signed_cookie + assert_equal 45, @controller.send(:cookies).signed[:user_id] + + key_generator = @request.env["action_dispatch.key_generator"] + secret = key_generator.generate_key(@request.env["action_dispatch.signed_cookie_salt"]) + verifier = ActiveSupport::MessageVerifier.new(secret, digest: "SHA256", serializer: Marshal) + assert_equal 45, verifier.verify(@response.cookies["user_id"]) + end + + def test_signed_cookie_with_legacy_secret_scheme + @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33" + + old_message = ActiveSupport::MessageVerifier.new("b3c631c314c0bbca50c1b2843150fe33", digest: "SHA1", serializer: Marshal).generate(45) + + @request.headers["Cookie"] = "user_id=#{old_message}" + get :get_signed_cookie + assert_equal 45, @controller.send(:cookies).signed[:user_id] + + key_generator = @request.env["action_dispatch.key_generator"] + secret = key_generator.generate_key("signed cookie") + verifier = ActiveSupport::MessageVerifier.new(secret, digest: "SHA1", serializer: Marshal) + assert_equal 45, verifier.verify(@response.cookies["user_id"]) + end + + def test_tampered_with_signed_cookie + key_generator = @request.env["action_dispatch.key_generator"] + secret = key_generator.generate_key(@request.env["action_dispatch.signed_cookie_salt"]) + + verifier = ActiveSupport::MessageVerifier.new(secret, serializer: Marshal, digest: "SHA1") + message = verifier.generate(45) + + @request.headers["Cookie"] = "user_id=#{Marshal.dump 45}--#{message.split("--").last}" + get :get_signed_cookie + assert_nil @controller.send(:cookies).signed[:user_id] + end + def test_signed_cookie_using_default_serializer get :set_signed_cookie cookies = @controller.send :cookies @@ -494,8 +548,7 @@ class CookiesTest < ActionController::TestCase @request.env["action_dispatch.cookies_serializer"] = :hybrid key_generator = @request.env["action_dispatch.key_generator"] - signed_cookie_salt = @request.env["action_dispatch.signed_cookie_salt"] - secret = key_generator.generate_key(signed_cookie_salt) + secret = key_generator.generate_key(@request.env["action_dispatch.signed_cookie_salt"]) marshal_value = ActiveSupport::MessageVerifier.new(secret, serializer: Marshal).generate(45) @request.headers["Cookie"] = "user_id=#{marshal_value}" @@ -514,8 +567,8 @@ class CookiesTest < ActionController::TestCase @request.env["action_dispatch.cookies_serializer"] = :hybrid key_generator = @request.env["action_dispatch.key_generator"] - signed_cookie_salt = @request.env["action_dispatch.signed_cookie_salt"] - secret = key_generator.generate_key(signed_cookie_salt) + secret = key_generator.generate_key(@request.env["action_dispatch.signed_cookie_salt"]) + json_value = ActiveSupport::MessageVerifier.new(secret, serializer: JSON).generate(45) @request.headers["Cookie"] = "user_id=#{json_value}" @@ -578,11 +631,10 @@ class CookiesTest < ActionController::TestCase def test_encrypted_cookie_using_hybrid_serializer_can_migrate_marshal_dumped_value_to_json @request.env["action_dispatch.cookies_serializer"] = :hybrid - cipher = "aes-256-gcm" - salt = @request.env["action_dispatch.authenticated_encrypted_cookie_salt"] - secret = @request.env["action_dispatch.key_generator"].generate_key(salt)[0, ActiveSupport::MessageEncryptor.key_len(cipher)] - encryptor = ActiveSupport::MessageEncryptor.new(secret, cipher: cipher, serializer: Marshal) + key_generator = @request.env["action_dispatch.key_generator"] + secret = key_generator.generate_key(@request.env["action_dispatch.authenticated_encrypted_cookie_salt"], 32) + encryptor = ActiveSupport::MessageEncryptor.new(secret, cipher: "aes-256-gcm", serializer: Marshal) marshal_value = encryptor.encrypt_and_sign("bar") @request.headers["Cookie"] = "foo=#{::Rack::Utils.escape marshal_value}" @@ -592,7 +644,7 @@ class CookiesTest < ActionController::TestCase assert_not_equal "bar", cookies[:foo] assert_equal "bar", cookies.encrypted[:foo] - json_encryptor = ActiveSupport::MessageEncryptor.new(secret, cipher: cipher, serializer: JSON) + json_encryptor = ActiveSupport::MessageEncryptor.new(secret, cipher: "aes-256-gcm", serializer: JSON) assert_not_nil @response.cookies["foo"] assert_equal "bar", json_encryptor.decrypt_and_verify(@response.cookies["foo"]) end @@ -600,11 +652,10 @@ class CookiesTest < ActionController::TestCase def test_encrypted_cookie_using_hybrid_serializer_can_read_from_json_dumped_value @request.env["action_dispatch.cookies_serializer"] = :hybrid - cipher = "aes-256-gcm" - salt = @request.env["action_dispatch.authenticated_encrypted_cookie_salt"] - secret = @request.env["action_dispatch.key_generator"].generate_key(salt)[0, ActiveSupport::MessageEncryptor.key_len(cipher)] - encryptor = ActiveSupport::MessageEncryptor.new(secret, cipher: cipher, serializer: JSON) + key_generator = @request.env["action_dispatch.key_generator"] + secret = key_generator.generate_key(@request.env["action_dispatch.authenticated_encrypted_cookie_salt"], 32) + encryptor = ActiveSupport::MessageEncryptor.new(secret, cipher: "aes-256-gcm", serializer: JSON) json_value = encryptor.encrypt_and_sign("bar") @request.headers["Cookie"] = "foo=#{::Rack::Utils.escape json_value}" @@ -691,65 +742,8 @@ class CookiesTest < ActionController::TestCase } end - def test_signed_uses_signed_cookie_jar_if_only_secret_token_is_set - @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33" - @request.env["action_dispatch.secret_key_base"] = nil - get :set_signed_cookie - assert_kind_of ActionDispatch::Cookies::SignedCookieJar, cookies.signed - end - - def test_signed_uses_signed_cookie_jar_if_only_secret_key_base_is_set - @request.env["action_dispatch.secret_token"] = nil - @request.env["action_dispatch.secret_key_base"] = "c3b95688f35581fad38df788add315ff" - get :set_signed_cookie - assert_kind_of ActionDispatch::Cookies::SignedCookieJar, cookies.signed - end - - def test_signed_uses_upgrade_legacy_signed_cookie_jar_if_both_secret_token_and_secret_key_base_are_set - @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33" - @request.env["action_dispatch.secret_key_base"] = "c3b95688f35581fad38df788add315ff" - get :set_signed_cookie - assert_kind_of ActionDispatch::Cookies::UpgradeLegacySignedCookieJar, cookies.signed - end - - def test_signed_or_encrypted_uses_signed_cookie_jar_if_only_secret_token_is_set - @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33" - @request.env["action_dispatch.secret_key_base"] = nil - get :get_encrypted_cookie - assert_kind_of ActionDispatch::Cookies::SignedCookieJar, cookies.signed_or_encrypted - end - - def test_signed_or_encrypted_uses_encrypted_cookie_jar_if_only_secret_key_base_is_set - @request.env["action_dispatch.secret_token"] = nil - @request.env["action_dispatch.secret_key_base"] = "c3b95688f35581fad38df788add315ff" - get :get_encrypted_cookie - assert_kind_of ActionDispatch::Cookies::EncryptedCookieJar, cookies.signed_or_encrypted - end - - def test_signed_or_encrypted_uses_upgrade_legacy_encrypted_cookie_jar_if_both_secret_token_and_secret_key_base_are_set - @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33" - @request.env["action_dispatch.secret_key_base"] = "c3b95688f35581fad38df788add315ff" - get :get_encrypted_cookie - assert_kind_of ActionDispatch::Cookies::UpgradeLegacyEncryptedCookieJar, cookies.signed_or_encrypted - end - - def test_encrypted_uses_encrypted_cookie_jar_if_only_secret_key_base_is_set - @request.env["action_dispatch.secret_token"] = nil - @request.env["action_dispatch.secret_key_base"] = "c3b95688f35581fad38df788add315ff" - get :get_encrypted_cookie - assert_kind_of ActionDispatch::Cookies::EncryptedCookieJar, cookies.encrypted - end - - def test_encrypted_uses_upgrade_legacy_encrypted_cookie_jar_if_both_secret_token_and_secret_key_base_are_set - @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33" - @request.env["action_dispatch.secret_key_base"] = "c3b95688f35581fad38df788add315ff" - get :get_encrypted_cookie - assert_kind_of ActionDispatch::Cookies::UpgradeLegacyEncryptedCookieJar, cookies.encrypted - end - def test_legacy_signed_cookie_is_read_and_transparently_upgraded_by_signed_cookie_jar_if_both_secret_token_and_secret_key_base_are_set @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33" - @request.env["action_dispatch.secret_key_base"] = "c3b95688f35581fad38df788add315ff" legacy_value = ActiveSupport::MessageVerifier.new("b3c631c314c0bbca50c1b2843150fe33").generate(45) @@ -766,9 +760,6 @@ class CookiesTest < ActionController::TestCase def test_legacy_signed_cookie_is_read_and_transparently_encrypted_by_encrypted_cookie_jar_if_both_secret_token_and_secret_key_base_are_set @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33" - @request.env["action_dispatch.secret_key_base"] = "c3b95688f35581fad38df788add315ff" - @request.env["action_dispatch.encrypted_cookie_salt"] = "4433796b79d99a7735553e316522acee" - @request.env["action_dispatch.encrypted_signed_cookie_salt"] = "00646eb40062e1b1deff205a27cd30f9" legacy_value = ActiveSupport::MessageVerifier.new("b3c631c314c0bbca50c1b2843150fe33").generate("bar") @@ -777,17 +768,14 @@ class CookiesTest < ActionController::TestCase assert_equal "bar", @controller.send(:cookies).encrypted[:foo] - cipher = "aes-256-gcm" - salt = @request.env["action_dispatch.authenticated_encrypted_cookie_salt"] - secret = @request.env["action_dispatch.key_generator"].generate_key(salt)[0, ActiveSupport::MessageEncryptor.key_len(cipher)] - encryptor = ActiveSupport::MessageEncryptor.new(secret, cipher: cipher, serializer: Marshal) + secret = @request.env["action_dispatch.key_generator"].generate_key(@request.env["action_dispatch.authenticated_encrypted_cookie_salt"], 32) + encryptor = ActiveSupport::MessageEncryptor.new(secret, cipher: "aes-256-gcm", serializer: Marshal) assert_equal "bar", encryptor.decrypt_and_verify(@response.cookies["foo"]) end def test_legacy_json_signed_cookie_is_read_and_transparently_upgraded_by_signed_json_cookie_jar_if_both_secret_token_and_secret_key_base_are_set @request.env["action_dispatch.cookies_serializer"] = :json @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33" - @request.env["action_dispatch.secret_key_base"] = "c3b95688f35581fad38df788add315ff" legacy_value = ActiveSupport::MessageVerifier.new("b3c631c314c0bbca50c1b2843150fe33", serializer: JSON).generate(45) @@ -805,7 +793,6 @@ class CookiesTest < ActionController::TestCase def test_legacy_json_signed_cookie_is_read_and_transparently_encrypted_by_encrypted_json_cookie_jar_if_both_secret_token_and_secret_key_base_are_set @request.env["action_dispatch.cookies_serializer"] = :json @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33" - @request.env["action_dispatch.secret_key_base"] = "c3b95688f35581fad38df788add315ff" legacy_value = ActiveSupport::MessageVerifier.new("b3c631c314c0bbca50c1b2843150fe33", serializer: JSON).generate("bar") @@ -824,7 +811,6 @@ class CookiesTest < ActionController::TestCase def test_legacy_json_signed_cookie_is_read_and_transparently_upgraded_by_signed_json_hybrid_jar_if_both_secret_token_and_secret_key_base_are_set @request.env["action_dispatch.cookies_serializer"] = :hybrid @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33" - @request.env["action_dispatch.secret_key_base"] = "c3b95688f35581fad38df788add315ff" legacy_value = ActiveSupport::MessageVerifier.new("b3c631c314c0bbca50c1b2843150fe33", serializer: JSON).generate(45) @@ -842,7 +828,6 @@ class CookiesTest < ActionController::TestCase def test_legacy_json_signed_cookie_is_read_and_transparently_encrypted_by_encrypted_hybrid_cookie_jar_if_both_secret_token_and_secret_key_base_are_set @request.env["action_dispatch.cookies_serializer"] = :hybrid @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33" - @request.env["action_dispatch.secret_key_base"] = "c3b95688f35581fad38df788add315ff" legacy_value = ActiveSupport::MessageVerifier.new("b3c631c314c0bbca50c1b2843150fe33", serializer: JSON).generate("bar") @@ -851,17 +836,15 @@ class CookiesTest < ActionController::TestCase assert_equal "bar", @controller.send(:cookies).encrypted[:foo] - cipher = "aes-256-gcm" salt = @request.env["action_dispatch.authenticated_encrypted_cookie_salt"] - secret = @request.env["action_dispatch.key_generator"].generate_key(salt)[0, ActiveSupport::MessageEncryptor.key_len(cipher)] - encryptor = ActiveSupport::MessageEncryptor.new(secret, cipher: cipher, serializer: JSON) + secret = @request.env["action_dispatch.key_generator"].generate_key(salt)[0, ActiveSupport::MessageEncryptor.key_len("aes-256-gcm")] + encryptor = ActiveSupport::MessageEncryptor.new(secret, cipher: "aes-256-gcm", serializer: JSON) assert_equal "bar", encryptor.decrypt_and_verify(@response.cookies["foo"]) end def test_legacy_marshal_signed_cookie_is_read_and_transparently_upgraded_by_signed_json_hybrid_jar_if_both_secret_token_and_secret_key_base_are_set @request.env["action_dispatch.cookies_serializer"] = :hybrid @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33" - @request.env["action_dispatch.secret_key_base"] = "c3b95688f35581fad38df788add315ff" legacy_value = ActiveSupport::MessageVerifier.new("b3c631c314c0bbca50c1b2843150fe33").generate(45) @@ -878,6 +861,8 @@ class CookiesTest < ActionController::TestCase def test_legacy_marshal_signed_cookie_is_read_and_transparently_encrypted_by_encrypted_hybrid_cookie_jar_if_both_secret_token_and_secret_key_base_are_set @request.env["action_dispatch.cookies_serializer"] = :hybrid + + @request.env["action_dispatch.use_authenticated_cookie_encryption"] = true @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33" @request.env["action_dispatch.secret_key_base"] = "c3b95688f35581fad38df788add315ff" @@ -888,16 +873,14 @@ class CookiesTest < ActionController::TestCase assert_equal "bar", @controller.send(:cookies).encrypted[:foo] - cipher = "aes-256-gcm" salt = @request.env["action_dispatch.authenticated_encrypted_cookie_salt"] - secret = @request.env["action_dispatch.key_generator"].generate_key(salt)[0, ActiveSupport::MessageEncryptor.key_len(cipher)] - encryptor = ActiveSupport::MessageEncryptor.new(secret, cipher: cipher, serializer: JSON) + secret = @request.env["action_dispatch.key_generator"].generate_key(salt)[0, ActiveSupport::MessageEncryptor.key_len("aes-256-gcm")] + encryptor = ActiveSupport::MessageEncryptor.new(secret, cipher: "aes-256-gcm", serializer: JSON) assert_equal "bar", encryptor.decrypt_and_verify(@response.cookies["foo"]) end def test_legacy_signed_cookie_is_treated_as_nil_by_signed_cookie_jar_if_tampered @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33" - @request.env["action_dispatch.secret_key_base"] = "c3b95688f35581fad38df788add315ff" @request.headers["Cookie"] = "user_id=45" get :get_signed_cookie @@ -908,7 +891,6 @@ class CookiesTest < ActionController::TestCase def test_legacy_signed_cookie_is_treated_as_nil_by_encrypted_cookie_jar_if_tampered @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33" - @request.env["action_dispatch.secret_key_base"] = "c3b95688f35581fad38df788add315ff" @request.headers["Cookie"] = "foo=baz" get :get_encrypted_cookie @@ -918,17 +900,12 @@ class CookiesTest < ActionController::TestCase end def test_legacy_hmac_aes_cbc_encrypted_marshal_cookie_is_upgraded_to_authenticated_encrypted_cookie - @request.env["action_dispatch.secret_key_base"] = "c3b95688f35581fad38df788add315ff" - - @request.env["action_dispatch.encrypted_cookie_salt"] = - @request.env["action_dispatch.encrypted_signed_cookie_salt"] = SALT - key_generator = @request.env["action_dispatch.key_generator"] encrypted_cookie_salt = @request.env["action_dispatch.encrypted_cookie_salt"] encrypted_signed_cookie_salt = @request.env["action_dispatch.encrypted_signed_cookie_salt"] - secret = key_generator.generate_key(encrypted_cookie_salt) + secret = key_generator.generate_key(encrypted_cookie_salt, ActiveSupport::MessageEncryptor.key_len("aes-256-cbc")) sign_secret = key_generator.generate_key(encrypted_signed_cookie_salt) - marshal_value = ActiveSupport::MessageEncryptor.new(secret[0, ActiveSupport::MessageEncryptor.key_len], sign_secret, serializer: Marshal).encrypt_and_sign("bar") + marshal_value = ActiveSupport::MessageEncryptor.new(secret, sign_secret, cipher: "aes-256-cbc", serializer: Marshal).encrypt_and_sign("bar") @request.headers["Cookie"] = "foo=#{marshal_value}" @@ -938,27 +915,22 @@ class CookiesTest < ActionController::TestCase assert_not_equal "bar", cookies[:foo] assert_equal "bar", cookies.encrypted[:foo] - aead_cipher = "aes-256-gcm" aead_salt = @request.env["action_dispatch.authenticated_encrypted_cookie_salt"] - aead_secret = key_generator.generate_key(aead_salt)[0, ActiveSupport::MessageEncryptor.key_len(aead_cipher)] - aead_encryptor = ActiveSupport::MessageEncryptor.new(aead_secret, cipher: aead_cipher, serializer: Marshal) + aead_secret = key_generator.generate_key(aead_salt, ActiveSupport::MessageEncryptor.key_len("aes-256-gcm")) + aead_encryptor = ActiveSupport::MessageEncryptor.new(aead_secret, cipher: "aes-256-gcm", serializer: Marshal) assert_equal "bar", aead_encryptor.decrypt_and_verify(@response.cookies["foo"]) end def test_legacy_hmac_aes_cbc_encrypted_json_cookie_is_upgraded_to_authenticated_encrypted_cookie @request.env["action_dispatch.cookies_serializer"] = :json - @request.env["action_dispatch.secret_key_base"] = "c3b95688f35581fad38df788add315ff" - - @request.env["action_dispatch.encrypted_cookie_salt"] = - @request.env["action_dispatch.encrypted_signed_cookie_salt"] = SALT key_generator = @request.env["action_dispatch.key_generator"] encrypted_cookie_salt = @request.env["action_dispatch.encrypted_cookie_salt"] encrypted_signed_cookie_salt = @request.env["action_dispatch.encrypted_signed_cookie_salt"] - secret = key_generator.generate_key(encrypted_cookie_salt) + secret = key_generator.generate_key(encrypted_cookie_salt, ActiveSupport::MessageEncryptor.key_len("aes-256-cbc")) sign_secret = key_generator.generate_key(encrypted_signed_cookie_salt) - marshal_value = ActiveSupport::MessageEncryptor.new(secret[0, ActiveSupport::MessageEncryptor.key_len], sign_secret, serializer: JSON).encrypt_and_sign("bar") + marshal_value = ActiveSupport::MessageEncryptor.new(secret, sign_secret, cipher: "aes-256-cbc", serializer: JSON).encrypt_and_sign("bar") @request.headers["Cookie"] = "foo=#{marshal_value}" @@ -968,19 +940,17 @@ class CookiesTest < ActionController::TestCase assert_not_equal "bar", cookies[:foo] assert_equal "bar", cookies.encrypted[:foo] - aead_cipher = "aes-256-gcm" aead_salt = @request.env["action_dispatch.authenticated_encrypted_cookie_salt"] - aead_secret = key_generator.generate_key(aead_salt)[0, ActiveSupport::MessageEncryptor.key_len(aead_cipher)] - aead_encryptor = ActiveSupport::MessageEncryptor.new(aead_secret, cipher: aead_cipher, serializer: JSON) + aead_secret = key_generator.generate_key(aead_salt)[0, ActiveSupport::MessageEncryptor.key_len("aes-256-gcm")] + aead_encryptor = ActiveSupport::MessageEncryptor.new(aead_secret, cipher: "aes-256-gcm", serializer: JSON) assert_equal "bar", aead_encryptor.decrypt_and_verify(@response.cookies["foo"]) end def test_legacy_hmac_aes_cbc_encrypted_cookie_using_64_byte_key_is_upgraded_to_authenticated_encrypted_cookie @request.env["action_dispatch.secret_key_base"] = "c3b95688f35581fad38df788add315ff" - - @request.env["action_dispatch.encrypted_cookie_salt"] = - @request.env["action_dispatch.encrypted_signed_cookie_salt"] = SALT + @request.env["action_dispatch.encrypted_cookie_salt"] = "b3c631c314c0bbca50c1b2843150fe33" + @request.env["action_dispatch.encrypted_signed_cookie_salt"] = "b3c631c314c0bbca50c1b2843150fe33" # Cookie generated with 64 bytes secret message = ["566d4e75536d686e633246564e6b493062557079626c566d51574d30515430394c53315665564a694e4563786555744f57537454576b396a5a31566a626e52525054303d2d2d34663234333330623130623261306163363562316266323335396164666364613564643134623131"].pack("H*") @@ -991,15 +961,35 @@ class CookiesTest < ActionController::TestCase cookies = @controller.send :cookies assert_not_equal "bar", cookies[:foo] assert_equal "bar", cookies.encrypted[:foo] - cipher = "aes-256-gcm" salt = @request.env["action_dispatch.authenticated_encrypted_cookie_salt"] - secret = @request.env["action_dispatch.key_generator"].generate_key(salt)[0, ActiveSupport::MessageEncryptor.key_len(cipher)] - encryptor = ActiveSupport::MessageEncryptor.new(secret, cipher: cipher, serializer: Marshal) + secret = @request.env["action_dispatch.key_generator"].generate_key(salt, ActiveSupport::MessageEncryptor.key_len("aes-256-gcm")) + encryptor = ActiveSupport::MessageEncryptor.new(secret, cipher: "aes-256-gcm", serializer: Marshal) assert_equal "bar", encryptor.decrypt_and_verify(@response.cookies["foo"]) end + def test_encrypted_cookie_rotating_secret + secret = "b3c631c314c0bbca50c1b2843150fe33" + + @request.env["action_dispatch.encrypted_cookie_cipher"] = "aes-256-gcm" + @request.env["action_dispatch.cookies_rotations"].rotate :encrypted, secret + + key_len = ActiveSupport::MessageEncryptor.key_len("aes-256-gcm") + + old_message = ActiveSupport::MessageEncryptor.new(secret, cipher: "aes-256-gcm", serializer: Marshal).encrypt_and_sign(45) + + @request.headers["Cookie"] = "foo=#{::Rack::Utils.escape old_message}" + + get :get_encrypted_cookie + assert_equal 45, @controller.send(:cookies).encrypted[:foo] + + key_generator = @request.env["action_dispatch.key_generator"] + secret = key_generator.generate_key(@request.env["action_dispatch.authenticated_encrypted_cookie_salt"], key_len) + encryptor = ActiveSupport::MessageEncryptor.new(secret, cipher: "aes-256-gcm", serializer: Marshal) + assert_equal 45, encryptor.decrypt_and_verify(@response.cookies["foo"]) + end + def test_cookie_with_all_domain_option get :set_cookie_with_domain assert_response :success diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 446b65a9b9..44f902c163 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -3,6 +3,7 @@ require "erb" require "abstract_unit" require "controller/fake_controllers" +require "active_support/messages/rotation_configuration" class TestRoutingMapper < ActionDispatch::IntegrationTest SprocketsApp = lambda { |env| @@ -4947,6 +4948,7 @@ end class FlashRedirectTest < ActionDispatch::IntegrationTest SessionKey = "_myapp_session" Generator = ActiveSupport::LegacyKeyGenerator.new("b3c631c314c0bbca50c1b2843150fe33") + Rotations = ActiveSupport::Messages::RotationConfiguration.new class KeyGeneratorMiddleware def initialize(app) @@ -4955,6 +4957,8 @@ class FlashRedirectTest < ActionDispatch::IntegrationTest def call(env) env["action_dispatch.key_generator"] ||= Generator + env["action_dispatch.cookies_rotations"] ||= Rotations + @app.call(env) end end diff --git a/actionpack/test/dispatch/session/cookie_store_test.rb b/actionpack/test/dispatch/session/cookie_store_test.rb index 6517cf4c99..cf51c47068 100644 --- a/actionpack/test/dispatch/session/cookie_store_test.rb +++ b/actionpack/test/dispatch/session/cookie_store_test.rb @@ -3,11 +3,13 @@ require "abstract_unit" require "stringio" require "active_support/key_generator" +require "active_support/messages/rotation_configuration" class CookieStoreTest < ActionDispatch::IntegrationTest SessionKey = "_myapp_session" SessionSecret = "b3c631c314c0bbca50c1b2843150fe33" Generator = ActiveSupport::LegacyKeyGenerator.new(SessionSecret) + Rotations = ActiveSupport::Messages::RotationConfiguration.new Verifier = ActiveSupport::MessageVerifier.new(SessionSecret, digest: "SHA1") SignedBar = Verifier.generate(foo: "bar", session_id: SecureRandom.hex(16)) @@ -346,6 +348,8 @@ class CookieStoreTest < ActionDispatch::IntegrationTest args[0] ||= {} args[0][:headers] ||= {} args[0][:headers]["action_dispatch.key_generator"] ||= Generator + args[0][:headers]["action_dispatch.cookies_rotations"] ||= Rotations + super(path, *args) end diff --git a/actionpack/test/dispatch/system_testing/server_test.rb b/actionpack/test/dispatch/system_testing/server_test.rb index ed65d93e49..1866225fc1 100644 --- a/actionpack/test/dispatch/system_testing/server_test.rb +++ b/actionpack/test/dispatch/system_testing/server_test.rb @@ -9,10 +9,6 @@ class ServerTest < ActiveSupport::TestCase ActionDispatch::SystemTesting::Server.new.run end - test "initializing the server port" do - assert_includes Capybara.servers, :rails_puma - end - test "port is always included" do assert Capybara.always_include_port, "expected Capybara.always_include_port to be true" end diff --git a/actionview/test/abstract_unit.rb b/actionview/test/abstract_unit.rb index c98270bd12..f20a66c2d2 100644 --- a/actionview/test/abstract_unit.rb +++ b/actionview/test/abstract_unit.rb @@ -26,14 +26,6 @@ require "active_record" require "pp" # require 'pp' early to prevent hidden_methods from not picking up the pretty-print methods until too late -module Rails - class << self - def env - @_env ||= ActiveSupport::StringInquirer.new(ENV["RAILS_ENV"] || ENV["RACK_ENV"] || "test") - end - end -end - ActiveSupport::Dependencies.hook! Thread.abort_on_exception = true @@ -110,12 +102,6 @@ module ActionDispatch end end -module ActiveSupport - class TestCase - include ActionDispatch::DrawOnce - end -end - class RoutedRackApp attr_reader :routes @@ -162,29 +148,6 @@ class ActionDispatch::IntegrationTest < ActiveSupport::TestCase self.app = build_app - # Stub Rails dispatcher so it does not get controller references and - # simply return the controller#action as Rack::Body. - class StubDispatcher < ::ActionDispatch::Routing::RouteSet::Dispatcher - private - def controller_reference(controller_param) - controller_param - end - - def dispatch(controller, action, env) - [200, { "Content-Type" => "text/html" }, ["#{controller}##{action}"]] - end - end - - def self.stub_controllers - old_dispatcher = ActionDispatch::Routing::RouteSet::Dispatcher - ActionDispatch::Routing::RouteSet.module_eval { remove_const :Dispatcher } - ActionDispatch::Routing::RouteSet.module_eval { const_set :Dispatcher, StubDispatcher } - yield ActionDispatch::Routing::RouteSet.new - ensure - ActionDispatch::Routing::RouteSet.module_eval { remove_const :Dispatcher } - ActionDispatch::Routing::RouteSet.module_eval { const_set :Dispatcher, old_dispatcher } - end - def with_routing(&block) temporary_routes = ActionDispatch::Routing::RouteSet.new old_app, self.class.app = self.class.app, self.class.build_app(temporary_routes) @@ -196,21 +159,6 @@ class ActionDispatch::IntegrationTest < ActiveSupport::TestCase self.class.app = old_app silence_warnings { Object.const_set(:SharedTestRoutes, old_routes) } end - - def with_autoload_path(path) - path = File.join(File.expand_path("fixtures", __dir__), path) - if ActiveSupport::Dependencies.autoload_paths.include?(path) - yield - else - begin - ActiveSupport::Dependencies.autoload_paths << path - yield - ensure - ActiveSupport::Dependencies.autoload_paths.reject! { |p| p == path } - ActiveSupport::Dependencies.clear - end - end - end end ActionView::RoutingUrlFor.include(ActionDispatch::Routing::UrlFor) @@ -274,6 +222,7 @@ module ActionDispatch end class ActiveSupport::TestCase + include ActionDispatch::DrawOnce include ActiveSupport::Testing::MethodCallAssertions # Skips the current run on Rubinius using Minitest::Assertions#skip diff --git a/actionview/test/template/sanitize_helper_test.rb b/actionview/test/template/sanitize_helper_test.rb index c7714cf205..0e690c82cb 100644 --- a/actionview/test/template/sanitize_helper_test.rb +++ b/actionview/test/template/sanitize_helper_test.rb @@ -21,8 +21,8 @@ class SanitizeHelperTest < ActionView::TestCase def test_should_sanitize_illegal_style_properties raw = %(display:block; position:absolute; left:0; top:0; width:100%; height:100%; z-index:1; background-color:black; background-image:url(http://www.ragingplatypus.com/i/cam-full.jpg); background-x:center; background-y:center; background-repeat:repeat;) - expected = %(display: block; width: 100%; height: 100%; background-color: black; background-x: center; background-y: center;) - assert_equal expected, sanitize_css(raw) + expected = %r(\Adisplay:\s?block;\s?width:\s?100%;\s?height:\s?100%;\s?background-color:\s?black;\s?background-x:\s?center;\s?background-y:\s?center;\z) + assert_match expected, sanitize_css(raw) end def test_strip_tags diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index b421fedc96..f73e27b91f 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,41 @@ +* PostgreSQL `tsrange` now preserves subsecond precision + + PostgreSQL 9.1+ introduced range types, and Rails added support for using + this datatype in ActiveRecord. However, the serialization of + PostgreSQL::OID::Range was incomplete, because it did not properly + cast the bounds that make up the range. This led to subseconds being + dropped in SQL commands: + + (byebug) from = type_cast_single_for_database(range.first) + 2010-01-01 13:30:00 UTC + + (byebug) to = type_cast_single_for_database(range.last) + 2011-02-02 19:30:00 UTC + + (byebug) "[#{from},#{to}#{value.exclude_end? ? ')' : ']'}" + "[2010-01-01 13:30:00 UTC,2011-02-02 19:30:00 UTC)" + + (byebug) "[#{type_cast(from)},#{type_cast(to)}#{value.exclude_end? ? ')' : ']'}" + "['2010-01-01 13:30:00.670277','2011-02-02 19:30:00.745125')" + +* Passing a `Set` to `Relation#where` now behaves the same as passing an + array. + + *Sean Griffin* + +* Use given algorithm while removing index from database. + + Fixes #24190. + + *Mehmet Emin İNAÇ* + +* Update payload names for `sql.active_record` instrumentation to be + more descriptive. + + Fixes #30586. + + *Jeremy Green* + * Add new error class `TransactionTimeout` for MySQL adapter which will be raised when lock wait time expires. diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index a61c0336db..ef26f4a20c 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1400,7 +1400,7 @@ module ActiveRecord # has_many :tags, as: :taggable # has_many :reports, -> { readonly } # has_many :subscribers, through: :subscriptions, source: :user - def has_many(name, scope = nil, options = {}, &extension) + def has_many(name, scope = nil, **options, &extension) reflection = Builder::HasMany.build(self, name, scope, options, &extension) Reflection.add_reflection self, name, reflection end @@ -1534,7 +1534,7 @@ module ActiveRecord # has_one :club, through: :membership # has_one :primary_address, -> { where(primary: true) }, through: :addressables, source: :addressable # has_one :credit_card, required: true - def has_one(name, scope = nil, options = {}) + def has_one(name, scope = nil, **options) reflection = Builder::HasOne.build(self, name, scope, options) Reflection.add_reflection self, name, reflection end @@ -1678,7 +1678,7 @@ module ActiveRecord # belongs_to :company, touch: :employees_last_updated_at # belongs_to :user, optional: true # belongs_to :account, default: -> { company.account } - def belongs_to(name, scope = nil, options = {}) + def belongs_to(name, scope = nil, **options) reflection = Builder::BelongsTo.build(self, name, scope, options) Reflection.add_reflection self, name, reflection end diff --git a/activerecord/lib/active_record/associations/builder/association.rb b/activerecord/lib/active_record/associations/builder/association.rb index 496b16b58f..ca3032d967 100644 --- a/activerecord/lib/active_record/associations/builder/association.rb +++ b/activerecord/lib/active_record/associations/builder/association.rb @@ -38,11 +38,6 @@ module ActiveRecord::Associations::Builder # :nodoc: def self.create_reflection(model, name, scope, options, extension = nil) raise ArgumentError, "association names must be a Symbol" unless name.kind_of?(Symbol) - if scope.is_a?(Hash) - options = scope - scope = nil - end - validate_options(options) scope = build_scope(scope, extension) diff --git a/activerecord/lib/active_record/associations/preloader.rb b/activerecord/lib/active_record/associations/preloader.rb index 62caf02a2c..e1754d4a19 100644 --- a/activerecord/lib/active_record/associations/preloader.rb +++ b/activerecord/lib/active_record/associations/preloader.rb @@ -91,13 +91,13 @@ module ActiveRecord # { author: :avatar } # [ :books, { author: :avatar } ] def preload(records, associations, preload_scope = nil) - records = Array.wrap(records).compact.uniq - associations = Array.wrap(associations) + records = records.compact if records.empty? [] else - associations.flat_map { |association| + records.uniq! + Array.wrap(associations).flat_map { |association| preloaders_on association, records, preload_scope } end diff --git a/activerecord/lib/active_record/associations/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb index 9bb6a613e1..607d376a08 100644 --- a/activerecord/lib/active_record/associations/preloader/association.rb +++ b/activerecord/lib/active_record/associations/preloader/association.rb @@ -17,26 +17,20 @@ module ActiveRecord end def run(preloader) - preload(preloader) - end - - def preload(preloader) - raise NotImplementedError - end - - # The name of the key on the associated records - def association_key_name - raise NotImplementedError - end - - # The name of the key on the model which declares the association - def owner_key_name - raise NotImplementedError + associated_records_by_owner(preloader).each do |owner, records| + associate_records_to_owner(owner, records) + end end private - def options - reflection.options + # The name of the key on the associated records + def association_key_name + reflection.join_primary_key(klass) + end + + # The name of the key on the model which declares the association + def owner_key_name + reflection.join_foreign_key end def associated_records_by_owner(preloader) @@ -51,21 +45,19 @@ module ActiveRecord end end + def associate_records_to_owner(owner, records) + raise NotImplementedError + end + def owner_keys - unless defined?(@owner_keys) - @owner_keys = owners.map do |owner| - owner[owner_key_name] - end - @owner_keys.uniq! - @owner_keys.compact! - end - @owner_keys + @owner_keys ||= owners_by_key.keys end def owners_by_key unless defined?(@owners_by_key) @owners_by_key = owners.each_with_object({}) do |owner, h| - h[convert_key(owner[owner_key_name])] = owner + key = convert_key(owner[owner_key_name]) + h[key] = owner if key end end @owners_by_key diff --git a/activerecord/lib/active_record/associations/preloader/belongs_to.rb b/activerecord/lib/active_record/associations/preloader/belongs_to.rb index ae9695f26a..a8e3340b23 100644 --- a/activerecord/lib/active_record/associations/preloader/belongs_to.rb +++ b/activerecord/lib/active_record/associations/preloader/belongs_to.rb @@ -4,13 +4,6 @@ module ActiveRecord module Associations class Preloader class BelongsTo < SingularAssociation #:nodoc: - def association_key_name - options[:primary_key] || klass && klass.primary_key - end - - def owner_key_name - reflection.foreign_key - end end end end diff --git a/activerecord/lib/active_record/associations/preloader/collection_association.rb b/activerecord/lib/active_record/associations/preloader/collection_association.rb index fb920a642c..fc2029f54a 100644 --- a/activerecord/lib/active_record/associations/preloader/collection_association.rb +++ b/activerecord/lib/active_record/associations/preloader/collection_association.rb @@ -5,13 +5,10 @@ module ActiveRecord class Preloader class CollectionAssociation < Association #:nodoc: private - - def preload(preloader) - associated_records_by_owner(preloader).each do |owner, records| - association = owner.association(reflection.name) - association.loaded! - association.target.concat(records) - end + def associate_records_to_owner(owner, records) + association = owner.association(reflection.name) + association.loaded! + association.target.concat(records) end end end diff --git a/activerecord/lib/active_record/associations/preloader/has_many.rb b/activerecord/lib/active_record/associations/preloader/has_many.rb index 29a1ce099d..72f55bc43f 100644 --- a/activerecord/lib/active_record/associations/preloader/has_many.rb +++ b/activerecord/lib/active_record/associations/preloader/has_many.rb @@ -4,13 +4,6 @@ module ActiveRecord module Associations class Preloader class HasMany < CollectionAssociation #:nodoc: - def association_key_name - reflection.foreign_key - end - - def owner_key_name - reflection.active_record_primary_key - end end end end diff --git a/activerecord/lib/active_record/associations/preloader/has_one.rb b/activerecord/lib/active_record/associations/preloader/has_one.rb index d87abf630f..e339b65fb5 100644 --- a/activerecord/lib/active_record/associations/preloader/has_one.rb +++ b/activerecord/lib/active_record/associations/preloader/has_one.rb @@ -4,13 +4,6 @@ module ActiveRecord module Associations class Preloader class HasOne < SingularAssociation #:nodoc: - def association_key_name - reflection.foreign_key - end - - def owner_key_name - reflection.active_record_primary_key - end end end end diff --git a/activerecord/lib/active_record/associations/preloader/singular_association.rb b/activerecord/lib/active_record/associations/preloader/singular_association.rb index 266b5f6b1c..30a92411e3 100644 --- a/activerecord/lib/active_record/associations/preloader/singular_association.rb +++ b/activerecord/lib/active_record/associations/preloader/singular_association.rb @@ -5,14 +5,9 @@ module ActiveRecord class Preloader class SingularAssociation < Association #:nodoc: private - - def preload(preloader) - associated_records_by_owner(preloader).each do |owner, associated_records| - record = associated_records.first - - association = owner.association(reflection.name) - association.target = record - end + def associate_records_to_owner(owner, records) + association = owner.association(reflection.name) + association.target = records.first end end end diff --git a/activerecord/lib/active_record/associations/preloader/through_association.rb b/activerecord/lib/active_record/associations/preloader/through_association.rb index 8aac00d910..fa32cc5553 100644 --- a/activerecord/lib/active_record/associations/preloader/through_association.rb +++ b/activerecord/lib/active_record/associations/preloader/through_association.rb @@ -28,6 +28,8 @@ module ActiveRecord middle_records = through_records.flat_map(&:last) + reflection_scope = reflection_scope() if reflection.scope + preloaders = preloader.preload(middle_records, source_reflection.name, reflection_scope) @@ -49,7 +51,7 @@ module ActiveRecord }.compact # Respect the order on `reflection_scope` if it exists, else use the natural order. - if reflection_scope.values[:order].present? + if reflection_scope && !reflection_scope.order_values.empty? @id_map ||= id_to_index_map @preloaded_records rhs_records.sort_by { |rhs| @id_map[rhs] } else @@ -67,10 +69,7 @@ module ActiveRecord id_map end - def reset_association(owners, association_name, through_scope) - should_reset = (through_scope != through_reflection.klass.unscoped) || - (options[:source_type] && through_reflection.collection?) - + def reset_association(owners, association_name, should_reset) # Don't cache the association - we would only be caching a subset if should_reset owners.each { |owner| @@ -81,6 +80,7 @@ module ActiveRecord def through_scope scope = through_reflection.klass.unscoped + options = reflection.options if options[:source_type] scope.where! reflection.foreign_type => options[:source_type] @@ -113,7 +113,7 @@ module ActiveRecord end end - scope + scope unless scope.empty_scope? end end end diff --git a/activerecord/lib/active_record/attribute_methods/primary_key.rb b/activerecord/lib/active_record/attribute_methods/primary_key.rb index 63c059e291..d8fc046e10 100644 --- a/activerecord/lib/active_record/attribute_methods/primary_key.rb +++ b/activerecord/lib/active_record/attribute_methods/primary_key.rb @@ -17,13 +17,15 @@ module ActiveRecord # Returns the primary key value. def id sync_with_transaction_state - _read_attribute(self.class.primary_key) if self.class.primary_key + primary_key = self.class.primary_key + _read_attribute(primary_key) if primary_key end # Sets the primary key value. def id=(value) sync_with_transaction_state - _write_attribute(self.class.primary_key, value) if self.class.primary_key + primary_key = self.class.primary_key + _write_attribute(primary_key, value) if primary_key end # Queries the primary key value. diff --git a/activerecord/lib/active_record/attribute_methods/read.rb b/activerecord/lib/active_record/attribute_methods/read.rb index b070235684..4077250583 100644 --- a/activerecord/lib/active_record/attribute_methods/read.rb +++ b/activerecord/lib/active_record/attribute_methods/read.rb @@ -58,8 +58,9 @@ module ActiveRecord attr_name.to_s end - name = self.class.primary_key if name == "id".freeze && self.class.primary_key - sync_with_transaction_state if name == self.class.primary_key + primary_key = self.class.primary_key + name = primary_key if name == "id".freeze && primary_key + sync_with_transaction_state if name == primary_key _read_attribute(name, &block) end diff --git a/activerecord/lib/active_record/attribute_methods/write.rb b/activerecord/lib/active_record/attribute_methods/write.rb index 37891ce2ef..bb0ec6a8c3 100644 --- a/activerecord/lib/active_record/attribute_methods/write.rb +++ b/activerecord/lib/active_record/attribute_methods/write.rb @@ -39,8 +39,9 @@ module ActiveRecord attr_name.to_s end - name = self.class.primary_key if name == "id".freeze && self.class.primary_key - sync_with_transaction_state if name == self.class.primary_key + primary_key = self.class.primary_key + name = primary_key if name == "id".freeze && primary_key + sync_with_transaction_state if name == primary_key _write_attribute(name, value) end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index 788a455773..be2f625d74 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -396,6 +396,9 @@ module ActiveRecord alias :belongs_to :references def new_column_definition(name, type, **options) # :nodoc: + if integer_like_primary_key?(type, options) + type = integer_like_primary_key_type(type, options) + end type = aliased_types(type.to_s, type) options[:primary_key] ||= type == :primary_key options[:null] = false if options[:primary_key] @@ -410,6 +413,14 @@ module ActiveRecord def aliased_types(name, fallback) "timestamp" == name ? :datetime : fallback end + + def integer_like_primary_key?(type, options) + options[:primary_key] && [:integer, :bigint].include?(type) && !options.key?(:default) + end + + def integer_like_primary_key_type(type, options) + type + end end class AlterTable # :nodoc: diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index f57c7a5d4d..4f0c1890be 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -2,7 +2,7 @@ require_relative "../../migration/join_table" require "active_support/core_ext/string/access" -require "digest" +require "digest/sha2" module ActiveRecord module ConnectionAdapters # :nodoc: @@ -522,6 +522,8 @@ module ActiveRecord # Specifies the precision for the <tt>:decimal</tt> and <tt>:numeric</tt> columns. # * <tt>:scale</tt> - # Specifies the scale for the <tt>:decimal</tt> and <tt>:numeric</tt> columns. + # * <tt>:comment</tt> - + # Specifies the comment for the column. This option is ignored by some backends. # # Note: The precision is the total number of significant digits, # and the scale is the number of digits that can be stored following diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index 7cd086084a..ae991d3d79 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -284,7 +284,7 @@ module ActiveRecord def table_comment(table_name) # :nodoc: scope = quoted_scope(table_name) - query_value(<<-SQL.strip_heredoc, "SCHEMA") + query_value(<<-SQL.strip_heredoc, "SCHEMA").presence SELECT table_comment FROM information_schema.tables WHERE table_schema = #{scope[:schema]} @@ -311,6 +311,11 @@ module ActiveRecord execute("ALTER TABLE #{quote_table_name(table_name)} #{sqls}") end + def change_table_comment(table_name, comment) #:nodoc: + comment = "" if comment.nil? + execute("ALTER TABLE #{quote_table_name(table_name)} COMMENT #{quote(comment)}") + end + # Renames a table. # # Example: @@ -351,18 +356,19 @@ module ActiveRecord def change_column_default(table_name, column_name, default_or_changes) #:nodoc: default = extract_new_default_value(default_or_changes) - column = column_for(table_name, column_name) - change_column table_name, column_name, column.sql_type, default: default + change_column table_name, column_name, nil, default: default end def change_column_null(table_name, column_name, null, default = nil) #:nodoc: - column = column_for(table_name, column_name) - unless null || default.nil? execute("UPDATE #{quote_table_name(table_name)} SET #{quote_column_name(column_name)}=#{quote(default)} WHERE #{quote_column_name(column_name)} IS NULL") end - change_column table_name, column_name, column.sql_type, null: null + change_column table_name, column_name, nil, null: null + end + + def change_column_comment(table_name, column_name, comment) #:nodoc: + change_column table_name, column_name, nil, comment: comment end def change_column(table_name, column_name, type, options = {}) #:nodoc: @@ -668,6 +674,7 @@ module ActiveRecord def change_column_sql(table_name, column_name, type, options = {}) column = column_for(table_name, column_name) + type ||= column.sql_type unless options.key?(:default) options[:default] = column.default @@ -716,7 +723,7 @@ module ActiveRecord def remove_index_sql(table_name, options = {}) index_name = index_name_for_remove(table_name, options) - "DROP INDEX #{index_name}" + "DROP INDEX #{quote_column_name(index_name)}" end def add_timestamps_sql(table_name, options = {}) diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb index b22a2e4da7..da25e4863c 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb @@ -4,11 +4,6 @@ module ActiveRecord module ConnectionAdapters module MySQL module ColumnMethods - def primary_key(name, type = :primary_key, **options) - options[:auto_increment] = true if [:integer, :bigint].include?(type) && !options.key?(:default) - super - end - def blob(*args, **options) args.each { |name| column(name, :blob, options) } end @@ -68,7 +63,6 @@ module ActiveRecord when :primary_key type = :integer options[:limit] ||= 8 - options[:auto_increment] = true options[:primary_key] = true when /\Aunsigned_(?<type>.+)\z/ type = $~[:type].to_sym @@ -82,6 +76,11 @@ module ActiveRecord def aliased_types(name, fallback) fallback end + + def integer_like_primary_key_type(type, options) + options[:auto_increment] = true + type + end end class Table < ActiveRecord::ConnectionAdapters::Table diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/column.rb b/activerecord/lib/active_record/connection_adapters/postgresql/column.rb index 1b67cee24b..ff95fa4a0e 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/column.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/column.rb @@ -10,8 +10,15 @@ module ActiveRecord def serial? return unless default_function - %r{\Anextval\('"?#{table_name}_#{name}_seq"?'::regclass\)\z} === default_function + if %r{\Anextval\('"?(?<sequence_name>.+_(?<suffix>seq\d*))"?'::regclass\)\z} =~ default_function + sequence_name_from_parts(table_name, name, suffix) == sequence_name + end end + + private + def sequence_name_from_parts(table_name, column_name, suffix) + "#{table_name}_#{column_name}_#{suffix}" + end end end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid/range.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid/range.rb index 7d5d7d91e6..a89aa5ea09 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/oid/range.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid/range.rb @@ -35,7 +35,7 @@ module ActiveRecord if value.is_a?(::Range) from = type_cast_single_for_database(value.begin) to = type_cast_single_for_database(value.end) - "[#{from},#{to}#{value.exclude_end? ? ')' : ']'}" + ::Range.new(from, to, value.exclude_end?) else super end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb index a0a22ba0f1..9fdeab06c1 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb @@ -101,6 +101,8 @@ module ActiveRecord end when OID::Array::Data _quote(encode_array(value)) + when Range + _quote(encode_range(value)) else super end @@ -117,6 +119,8 @@ module ActiveRecord value.to_s when OID::Array::Data encode_array(value) + when Range + encode_range(value) else super end @@ -133,6 +137,10 @@ module ActiveRecord result end + def encode_range(range) + "[#{type_cast(range.first)},#{type_cast(range.last)}#{range.exclude_end? ? ')' : ']'}" + end + def determine_encoding_of_strings_in_array(value) case value when ::Array then determine_encoding_of_strings_in_array(value.first) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb index f1489e4d69..75622eb304 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb @@ -44,15 +44,8 @@ module ActiveRecord # a record (as primary keys cannot be +nil+). This might be done via the # +SecureRandom.uuid+ method and a +before_save+ callback, for instance. def primary_key(name, type = :primary_key, **options) - options[:auto_increment] = true if [:integer, :bigint].include?(type) && !options.key?(:default) if type == :uuid options[:default] = options.fetch(:default, "gen_random_uuid()") - elsif options.delete(:auto_increment) == true && %i(integer bigint).include?(type) - type = if type == :bigint || options[:limit] == 8 - :bigserial - else - :serial - end end super @@ -185,6 +178,15 @@ module ActiveRecord class TableDefinition < ActiveRecord::ConnectionAdapters::TableDefinition include ColumnMethods + + private + def integer_like_primary_key_type(type, options) + if type == :bigint || options[:limit] == 8 + :bigserial + else + :serial + end + end end class Table < ActiveRecord::ConnectionAdapters::Table diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_definitions.rb index 501f17dbad..c9855019c1 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_definitions.rb @@ -3,27 +3,16 @@ module ActiveRecord module ConnectionAdapters module SQLite3 - module ColumnMethods - def primary_key(name, type = :primary_key, **options) - if %i(integer bigint).include?(type) && (options.delete(:auto_increment) == true || !options.key?(:default)) - type = :primary_key - end - - super - end - end - class TableDefinition < ActiveRecord::ConnectionAdapters::TableDefinition - include ColumnMethods - def references(*args, **options) super(*args, type: :integer, **options) end alias :belongs_to :references - end - class Table < ActiveRecord::ConnectionAdapters::Table - include ColumnMethods + private + def integer_like_primary_key_type(type, options) + :primary_key + end end end end diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb index a512702b7b..f4e55147df 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb @@ -39,10 +39,6 @@ module ActiveRecord end end - def update_table_definition(table_name, base) - SQLite3::Table.new(table_name, base) - end - def create_schema_dumper(options) SQLite3::SchemaDumper.create(self, options) end diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 52ca4671c2..8845e26ab7 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -354,9 +354,9 @@ module ActiveRecord # to match the structure of your database. # # To roll the database back to a previous migration version, use - # <tt>rails db:migrate VERSION=X</tt> where <tt>X</tt> is the version to which + # <tt>rails db:rollback VERSION=X</tt> where <tt>X</tt> is the version to which # you wish to downgrade. Alternatively, you can also use the STEP option if you - # wish to rollback last few migrations. <tt>rails db:migrate STEP=2</tt> will rollback + # wish to rollback last few migrations. <tt>rails db:rollback STEP=2</tt> will rollback # the latest two migrations. # # If any of the migrations throw an <tt>ActiveRecord::IrreversibleMigration</tt> exception, diff --git a/activerecord/lib/active_record/migration/command_recorder.rb b/activerecord/lib/active_record/migration/command_recorder.rb index a3a5e0fa16..ac7d506fd1 100644 --- a/activerecord/lib/active_record/migration/command_recorder.rb +++ b/activerecord/lib/active_record/migration/command_recorder.rb @@ -161,8 +161,8 @@ module ActiveRecord table, columns, options = *args options ||= {} - index_name = options[:name] - options_hash = index_name ? { name: index_name } : { column: columns } + options_hash = options.slice(:name, :algorithm) + options_hash[:column] = columns if !options_hash[:name] [:remove_index, [table, options_hash]] end diff --git a/activerecord/lib/active_record/migration/compatibility.rb b/activerecord/lib/active_record/migration/compatibility.rb index 784292f3f9..502cef2e20 100644 --- a/activerecord/lib/active_record/migration/compatibility.rb +++ b/activerecord/lib/active_record/migration/compatibility.rb @@ -20,6 +20,11 @@ module ActiveRecord class V5_0 < V5_1 module TableDefinition + def primary_key(name, type = :primary_key, **options) + type = :integer if type == :primary_key + super + end + def references(*args, **options) super(*args, type: :integer, **options) end @@ -71,6 +76,29 @@ module ActiveRecord end end + def create_join_table(table_1, table_2, column_options: {}, **options) + column_options.reverse_merge!(type: :integer) + + if block_given? + super(table_1, table_2, column_options: column_options, **options) do |t| + class << t + prepend TableDefinition + end + yield t + end + else + super + end + end + + def add_column(table_name, column_name, type, options = {}) + if type == :primary_key + type = :integer + options[:primary_key] = true + end + super + end + def add_reference(table_name, ref_name, **options) super(table_name, ref_name, type: :integer, **options) end diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index fbbf9082cc..b48a137a73 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -71,6 +71,100 @@ module ActiveRecord klass.allocate.init_with("attributes" => attributes, "new_record" => false, &block) end + # Updates an object (or multiple objects) and saves it to the database, if validations pass. + # The resulting object is returned whether the object was saved successfully to the database or not. + # + # ==== Parameters + # + # * +id+ - This should be the id or an array of ids to be updated. + # * +attributes+ - This should be a hash of attributes or an array of hashes. + # + # ==== Examples + # + # # Updates one record + # Person.update(15, user_name: "Samuel", group: "expert") + # + # # Updates multiple records + # people = { 1 => { "first_name" => "David" }, 2 => { "first_name" => "Jeremy" } } + # Person.update(people.keys, people.values) + # + # # Updates multiple records from the result of a relation + # people = Person.where(group: "expert") + # people.update(group: "masters") + # + # Note: Updating a large number of records will run an UPDATE + # query for each record, which may cause a performance issue. + # When running callbacks is not needed for each record update, + # it is preferred to use {update_all}[rdoc-ref:Relation#update_all] + # for updating all records in a single query. + def update(id = :all, attributes) + if id.is_a?(Array) + id.map.with_index { |one_id, idx| update(one_id, attributes[idx]) }.compact + elsif id == :all + all.each { |record| record.update(attributes) } + else + if ActiveRecord::Base === id + raise ArgumentError, + "You are passing an instance of ActiveRecord::Base to `update`. " \ + "Please pass the id of the object by calling `.id`." + end + object = find(id) + object.update(attributes) + object + end + rescue RecordNotFound + end + + # Destroy an object (or multiple objects) that has the given id. The object is instantiated first, + # therefore all callbacks and filters are fired off before the object is deleted. This method is + # less efficient than #delete but allows cleanup methods and other actions to be run. + # + # This essentially finds the object (or multiple objects) with the given id, creates a new object + # from the attributes, and then calls destroy on it. + # + # ==== Parameters + # + # * +id+ - This should be the id or an array of ids to be destroyed. + # + # ==== Examples + # + # # Destroy a single object + # Todo.destroy(1) + # + # # Destroy multiple objects + # todos = [1,2,3] + # Todo.destroy(todos) + def destroy(id) + if id.is_a?(Array) + id.map { |one_id| destroy(one_id) }.compact + else + find(id).destroy + end + rescue RecordNotFound + end + + # Deletes the row with a primary key matching the +id+ argument, using a + # SQL +DELETE+ statement, and returns the number of rows deleted. Active + # Record objects are not instantiated, so the object's callbacks are not + # executed, including any <tt>:dependent</tt> association options. + # + # You can delete multiple rows at once by passing an Array of <tt>id</tt>s. + # + # Note: Although it is often much faster than the alternative, #destroy, + # skipping callbacks might bypass business logic in your application + # that ensures referential integrity or performs other essential jobs. + # + # ==== Examples + # + # # Delete a single row + # Todo.delete(1) + # + # # Delete multiple rows + # Todo.delete([2,3,4]) + def delete(id_or_array) + where(primary_key => id_or_array).delete_all + end + private # Called by +instantiate+ to decide which class to use for a new # record instance. diff --git a/activerecord/lib/active_record/querying.rb b/activerecord/lib/active_record/querying.rb index f780538319..3996d5661f 100644 --- a/activerecord/lib/active_record/querying.rb +++ b/activerecord/lib/active_record/querying.rb @@ -7,7 +7,7 @@ module ActiveRecord delegate :first_or_create, :first_or_create!, :first_or_initialize, to: :all delegate :find_or_create_by, :find_or_create_by!, :find_or_initialize_by, to: :all delegate :find_by, :find_by!, to: :all - delegate :destroy, :destroy_all, :delete, :delete_all, :update, :update_all, to: :all + delegate :destroy_all, :delete_all, :update_all, to: :all delegate :find_each, :find_in_batches, :in_batches, to: :all delegate :select, :group, :order, :except, :reorder, :limit, :offset, :joins, :left_joins, :left_outer_joins, :or, :where, :rewhere, :preload, :eager_load, :includes, :from, :lock, :readonly, :extending, diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 889e24dd1a..97adfb4352 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -292,13 +292,17 @@ module ActiveRecord end def get_join_keys(association_klass) - JoinKeys.new(join_pk(association_klass), join_foreign_key) + JoinKeys.new(join_primary_key(association_klass), join_foreign_key) end def build_scope(table, predicate_builder = predicate_builder(table)) Relation.create(klass, table, predicate_builder) end + def join_primary_key(_) + foreign_key + end + def join_foreign_key active_record_primary_key end @@ -313,10 +317,6 @@ module ActiveRecord PredicateBuilder.new(TableMetadata.new(klass, table)) end - def join_pk(_) - foreign_key - end - def primary_key(klass) klass.primary_key || raise(UnknownPrimaryKey.new(klass)) end @@ -425,9 +425,8 @@ module ActiveRecord def initialize(name, scope, options, active_record) super - @automatic_inverse_of = nil @type = options[:as] && (options[:foreign_type] || "#{options[:as]}_type") - @foreign_type = options[:foreign_type] || "#{name}_type" + @foreign_type = options[:polymorphic] && (options[:foreign_type] || "#{name}_type") @constructable = calculate_constructable(macro, options) @association_scope_cache = Concurrent::Map.new @@ -609,12 +608,14 @@ module ActiveRecord # If it cannot find a suitable inverse association name, it returns # +nil+. def inverse_name - options.fetch(:inverse_of) do - @automatic_inverse_of ||= automatic_inverse_of + unless defined?(@inverse_name) + @inverse_name = options.fetch(:inverse_of) { automatic_inverse_of } end + + @inverse_name end - # returns either false or the inverse association name that it finds. + # returns either +nil+ or the inverse association name that it finds. def automatic_inverse_of if can_find_inverse_of_automatically?(self) inverse_name = ActiveSupport::Inflector.underscore(options[:as] || active_record.name.demodulize).to_sym @@ -631,8 +632,6 @@ module ActiveRecord return inverse_name end end - - false end # Checks if the inverse reflection that is returned from the @@ -736,6 +735,10 @@ module ActiveRecord end end + def join_primary_key(klass) + polymorphic? ? association_primary_key(klass) : association_primary_key + end + def join_foreign_key foreign_key end @@ -745,10 +748,6 @@ module ActiveRecord def calculate_constructable(macro, options) !polymorphic? end - - def join_pk(klass) - polymorphic? ? association_primary_key(klass) : association_primary_key - end end class HasAndBelongsToManyReflection < AssociationReflection # :nodoc: diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 42a9d8492b..3517091a6e 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -63,7 +63,7 @@ module ActiveRecord @klass.connection.insert( im, - "SQL", + "#{@klass} Create", primary_key || false, primary_key_value, nil, @@ -86,7 +86,7 @@ module ActiveRecord @klass.connection.update( um, - "SQL", + "#{@klass} Update", ) end @@ -373,51 +373,7 @@ module ActiveRecord stmt.wheres = arel.constraints end - @klass.connection.update stmt, "SQL" - end - - # Updates an object (or multiple objects) and saves it to the database, if validations pass. - # The resulting object is returned whether the object was saved successfully to the database or not. - # - # ==== Parameters - # - # * +id+ - This should be the id or an array of ids to be updated. - # * +attributes+ - This should be a hash of attributes or an array of hashes. - # - # ==== Examples - # - # # Updates one record - # Person.update(15, user_name: 'Samuel', group: 'expert') - # - # # Updates multiple records - # people = { 1 => { "first_name" => "David" }, 2 => { "first_name" => "Jeremy" } } - # Person.update(people.keys, people.values) - # - # # Updates multiple records from the result of a relation - # people = Person.where(group: 'expert') - # people.update(group: 'masters') - # - # Note: Updating a large number of records will run an - # UPDATE query for each record, which may cause a performance - # issue. When running callbacks is not needed for each record update, - # it is preferred to use #update_all for updating all records - # in a single query. - def update(id = :all, attributes) - if id.is_a?(Array) - id.map.with_index { |one_id, idx| update(one_id, attributes[idx]) } - elsif id == :all - records.each { |record| record.update(attributes) } - else - if ActiveRecord::Base === id - raise ArgumentError, <<-MSG.squish - You are passing an instance of ActiveRecord::Base to `update`. - Please pass the id of the object by calling `.id`. - MSG - end - object = find(id) - object.update(attributes) - object - end + @klass.connection.update stmt, "#{@klass} Update All" end # Destroys the records by instantiating each @@ -440,33 +396,6 @@ module ActiveRecord records.each(&:destroy).tap { reset } end - # Destroy an object (or multiple objects) that has the given id. The object is instantiated first, - # therefore all callbacks and filters are fired off before the object is deleted. This method is - # less efficient than #delete but allows cleanup methods and other actions to be run. - # - # This essentially finds the object (or multiple objects) with the given id, creates a new object - # from the attributes, and then calls destroy on it. - # - # ==== Parameters - # - # * +id+ - Can be either an Integer or an Array of Integers. - # - # ==== Examples - # - # # Destroy a single object - # Todo.destroy(1) - # - # # Destroy multiple objects - # todos = [1,2,3] - # Todo.destroy(todos) - def destroy(id) - if id.is_a?(Array) - id.map { |one_id| destroy(one_id) } - else - find(id).destroy - end - end - # Deletes the records without instantiating the records # first, and hence not calling the {#destroy}[rdoc-ref:Persistence#destroy] # method nor invoking callbacks. @@ -503,35 +432,12 @@ module ActiveRecord stmt.wheres = arel.constraints end - affected = @klass.connection.delete(stmt, "SQL") + affected = @klass.connection.delete(stmt, "#{@klass} Destroy") reset affected end - # Deletes the row with a primary key matching the +id+ argument, using a - # SQL +DELETE+ statement, and returns the number of rows deleted. Active - # Record objects are not instantiated, so the object's callbacks are not - # executed, including any <tt>:dependent</tt> association options. - # - # You can delete multiple rows at once by passing an Array of <tt>id</tt>s. - # - # Note: Although it is often much faster than the alternative, - # #destroy, skipping callbacks might bypass business logic in - # your application that ensures referential integrity or performs other - # essential jobs. - # - # ==== Examples - # - # # Delete a single row - # Todo.delete(1) - # - # # Delete multiple rows - # Todo.delete([2,3,4]) - def delete(id_or_array) - where(primary_key => id_or_array).delete_all - end - # Causes the records to be loaded from the database if they have not # been loaded already. You can use this if for some reason you need # to explicitly load some records before actually using them. The diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 42d43224fa..0889d61c92 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -391,7 +391,7 @@ module ActiveRecord def build_count_subquery(relation, column_name, distinct) relation.select_values = [ if column_name == :all - distinct ? table[Arel.star] : Arel.sql("1") + distinct ? table[Arel.star] : Arel.sql(FinderMethods::ONE_AS_ONE) else column_alias = Arel.sql("count_column") aggregate_column(column_name).as(column_alias) diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb index 5c42414072..be4b169f67 100644 --- a/activerecord/lib/active_record/relation/predicate_builder.rb +++ b/activerecord/lib/active_record/relation/predicate_builder.rb @@ -13,6 +13,7 @@ module ActiveRecord register_handler(Range, RangeHandler.new(self)) register_handler(Relation, RelationHandler.new) register_handler(Array, ArrayHandler.new(self)) + register_handler(Set, ArrayHandler.new(self)) end def build_from_hash(attributes) diff --git a/activerecord/test/cases/adapters/postgresql/range_test.rb b/activerecord/test/cases/adapters/postgresql/range_test.rb index b4a776d04d..a75fdef698 100644 --- a/activerecord/test/cases/adapters/postgresql/range_test.rb +++ b/activerecord/test/cases/adapters/postgresql/range_test.rb @@ -232,6 +232,57 @@ _SQL end end + def test_create_tstzrange_preserve_usec + tstzrange = Time.parse("2010-01-01 14:30:00.670277 +0100")...Time.parse("2011-02-02 14:30:00.745125 CDT") + round_trip(@new_range, :tstz_range, tstzrange) + assert_equal @new_range.tstz_range, tstzrange + assert_equal @new_range.tstz_range, Time.parse("2010-01-01 13:30:00.670277 UTC")...Time.parse("2011-02-02 19:30:00.745125 UTC") + end + + def test_update_tstzrange_preserve_usec + assert_equal_round_trip(@first_range, :tstz_range, + Time.parse("2010-01-01 14:30:00.245124 CDT")...Time.parse("2011-02-02 14:30:00.451274 CET")) + assert_nil_round_trip(@first_range, :tstz_range, + Time.parse("2010-01-01 14:30:00.245124 +0100")...Time.parse("2010-01-01 13:30:00.245124 +0000")) + end + + def test_create_tsrange_preseve_usec + tz = ::ActiveRecord::Base.default_timezone + assert_equal_round_trip(@new_range, :ts_range, + Time.send(tz, 2010, 1, 1, 14, 30, 0, 125435)...Time.send(tz, 2011, 2, 2, 14, 30, 0, 225435)) + end + + def test_update_tsrange_preserve_usec + tz = ::ActiveRecord::Base.default_timezone + assert_equal_round_trip(@first_range, :ts_range, + Time.send(tz, 2010, 1, 1, 14, 30, 0, 142432)...Time.send(tz, 2011, 2, 2, 14, 30, 0, 224242)) + assert_nil_round_trip(@first_range, :ts_range, + Time.send(tz, 2010, 1, 1, 14, 30, 0, 142432)...Time.send(tz, 2010, 1, 1, 14, 30, 0, 142432)) + end + + def test_timezone_awareness_tsrange_preserve_usec + tz = "Pacific Time (US & Canada)" + + in_time_zone tz do + PostgresqlRange.reset_column_information + time_string = "2017-09-26 07:30:59.132451 -0700" + time = Time.zone.parse(time_string) + assert time.usec > 0 + + record = PostgresqlRange.new(ts_range: time_string..time_string) + assert_equal time..time, record.ts_range + assert_equal ActiveSupport::TimeZone[tz], record.ts_range.begin.time_zone + assert_equal time.usec, record.ts_range.begin.usec + + record.save! + record.reload + + assert_equal time..time, record.ts_range + assert_equal ActiveSupport::TimeZone[tz], record.ts_range.begin.time_zone + assert_equal time.usec, record.ts_range.begin.usec + end + end + def test_create_numrange assert_equal_round_trip(@new_range, :num_range, BigDecimal.new("0.5")...BigDecimal.new("1")) diff --git a/activerecord/test/cases/adapters/postgresql/serial_test.rb b/activerecord/test/cases/adapters/postgresql/serial_test.rb index 3c020a88d0..df7875dbf2 100644 --- a/activerecord/test/cases/adapters/postgresql/serial_test.rb +++ b/activerecord/test/cases/adapters/postgresql/serial_test.rb @@ -86,3 +86,39 @@ class PostgresqlBigSerialTest < ActiveRecord::PostgreSQLTestCase assert_match %r{t\.bigint\s+"serials_id",\s+default: -> \{ "nextval\('postgresql_big_serials_id_seq'::regclass\)" \}$}, output end end + +module SequenceNameDetectionTestCases + class CollidedSequenceNameTest < ActiveRecord::PostgreSQLTestCase + include SchemaDumpingHelper + + def setup + @connection = ActiveRecord::Base.connection + @connection.create_table :foo_bar, force: true do |t| + t.serial :baz_id + end + @connection.create_table :foo, force: true do |t| + t.serial :bar_id + t.bigserial :bar_baz_id + end + end + + def teardown + @connection.drop_table :foo_bar, if_exists: true + @connection.drop_table :foo, if_exists: true + end + + def test_serial_columns + columns = @connection.columns(:foo) + columns.each do |column| + assert_equal :integer, column.type + assert column.serial? + end + end + + def test_schema_dump_with_collided_sequence_name + output = dump_table_schema "foo" + assert_match %r{t\.serial\s+"bar_id",\s+null: false$}, output + assert_match %r{t\.bigserial\s+"bar_baz_id",\s+null: false$}, output + end + end +end diff --git a/activerecord/test/cases/adapters/postgresql/type_lookup_test.rb b/activerecord/test/cases/adapters/postgresql/type_lookup_test.rb index 449023b6eb..8212ed4263 100644 --- a/activerecord/test/cases/adapters/postgresql/type_lookup_test.rb +++ b/activerecord/test/cases/adapters/postgresql/type_lookup_test.rb @@ -30,6 +30,6 @@ class PostgresqlTypeLookupTest < ActiveRecord::PostgreSQLTestCase big_range = 0..123456789123456789 assert_raises(ActiveModel::RangeError) { int_range.serialize(big_range) } - assert_equal "[0,123456789123456789]", bigint_range.serialize(big_range) + assert_equal "[0,123456789123456789]", @connection.type_cast(bigint_range.serialize(big_range)) end end diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index 0ea8ef5cea..2d67c57cfb 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -1019,14 +1019,6 @@ class AttributeMethodsTest < ActiveRecord::TestCase ActiveRecord::Base.time_zone_aware_types = old_types end - def cached_columns - Topic.columns.map(&:name) - end - - def time_related_columns_on_topic - Topic.columns.select { |c| [:time, :date, :datetime, :timestamp].include?(c.type) } - end - def privatize(method_signature) @target.class_eval(<<-private_method, __FILE__, __LINE__ + 1) private diff --git a/activerecord/test/cases/attributes_test.rb b/activerecord/test/cases/attributes_test.rb index 29a25b4461..2caf2a63d4 100644 --- a/activerecord/test/cases/attributes_test.rb +++ b/activerecord/test/cases/attributes_test.rb @@ -108,12 +108,14 @@ module ActiveRecord assert_equal 6, klass.attribute_types.length assert_equal 6, klass.column_defaults.length + assert_equal 6, klass.attribute_names.length assert_not klass.attribute_types.include?("wibble") klass.attribute :wibble, Type::Value.new assert_equal 7, klass.attribute_types.length assert_equal 7, klass.column_defaults.length + assert_equal 7, klass.attribute_names.length assert_includes klass.attribute_types, "wibble" end diff --git a/activerecord/test/cases/comment_test.rb b/activerecord/test/cases/comment_test.rb index 1bcafd4b55..584e03d196 100644 --- a/activerecord/test/cases/comment_test.rb +++ b/activerecord/test/cases/comment_test.rb @@ -142,5 +142,27 @@ if ActiveRecord::Base.connection.supports_comments? assert_match %r[t\.string\s+"absent_comment"\n], output assert_no_match %r[t\.string\s+"absent_comment", comment:\n], output end + + def test_change_table_comment + @connection.change_table_comment :commenteds, "Edited table comment" + assert_equal "Edited table comment", @connection.table_comment("commenteds") + end + + def test_change_table_comment_to_nil + @connection.change_table_comment :commenteds, nil + assert_nil @connection.table_comment("commenteds") + end + + def test_change_column_comment + @connection.change_column_comment :commenteds, :name, "Edited column comment" + column = Commented.columns_hash["name"] + assert_equal "Edited column comment", column.comment + end + + def test_change_column_comment_to_nil + @connection.change_column_comment :commenteds, :name, nil + column = Commented.columns_hash["name"] + assert_nil column.comment + end end end diff --git a/activerecord/test/cases/instrumentation_test.rb b/activerecord/test/cases/instrumentation_test.rb new file mode 100644 index 0000000000..e6e8468757 --- /dev/null +++ b/activerecord/test/cases/instrumentation_test.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +require "cases/helper" +require "models/book" + +module ActiveRecord + class InstrumentationTest < ActiveRecord::TestCase + def test_payload_name_on_load + Book.create(name: "test book") + subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") do |*args| + event = ActiveSupport::Notifications::Event.new(*args) + if event.payload[:sql].match "SELECT" + assert_equal "Book Load", event.payload[:name] + end + end + Book.first + ensure + ActiveSupport::Notifications.unsubscribe(subscriber) if subscriber + end + + def test_payload_name_on_create + subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") do |*args| + event = ActiveSupport::Notifications::Event.new(*args) + if event.payload[:sql].match "INSERT" + assert_equal "Book Create", event.payload[:name] + end + end + Book.create(name: "test book") + ensure + ActiveSupport::Notifications.unsubscribe(subscriber) if subscriber + end + + def test_payload_name_on_update + subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") do |*args| + event = ActiveSupport::Notifications::Event.new(*args) + if event.payload[:sql].match "UPDATE" + assert_equal "Book Update", event.payload[:name] + end + end + book = Book.create(name: "test book") + book.update_attribute(:name, "new name") + ensure + ActiveSupport::Notifications.unsubscribe(subscriber) if subscriber + end + + def test_payload_name_on_update_all + subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") do |*args| + event = ActiveSupport::Notifications::Event.new(*args) + if event.payload[:sql].match "UPDATE" + assert_equal "Book Update All", event.payload[:name] + end + end + Book.create(name: "test book") + Book.update_all(name: "new name") + ensure + ActiveSupport::Notifications.unsubscribe(subscriber) if subscriber + end + + def test_payload_name_on_destroy + subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") do |*args| + event = ActiveSupport::Notifications::Event.new(*args) + if event.payload[:sql].match "DELETE" + assert_equal "Book Destroy", event.payload[:name] + end + end + book = Book.create(name: "test book") + book.destroy + ensure + ActiveSupport::Notifications.unsubscribe(subscriber) if subscriber + end + end +end diff --git a/activerecord/test/cases/migration/command_recorder_test.rb b/activerecord/test/cases/migration/command_recorder_test.rb index 0b5e983f14..58bc558619 100644 --- a/activerecord/test/cases/migration/command_recorder_test.rb +++ b/activerecord/test/cases/migration/command_recorder_test.rb @@ -213,6 +213,11 @@ module ActiveRecord assert_equal [:remove_index, [:table, { name: "new_index" }]], remove end + def test_invert_add_index_with_algorithm_option + remove = @recorder.inverse_of :add_index, [:table, :one, algorithm: :concurrently] + assert_equal [:remove_index, [:table, { column: :one, algorithm: :concurrently }]], remove + end + def test_invert_remove_index add = @recorder.inverse_of :remove_index, [:table, :one] assert_equal [:add_index, [:table, :one]], add diff --git a/activerecord/test/cases/migration/compatibility_test.rb b/activerecord/test/cases/migration/compatibility_test.rb index cb3b02c02a..1ae15eb439 100644 --- a/activerecord/test/cases/migration/compatibility_test.rb +++ b/activerecord/test/cases/migration/compatibility_test.rb @@ -199,6 +199,87 @@ class LegacyPrimaryKeyTest < ActiveRecord::TestCase assert_match %r{create_table "legacy_primary_keys", id: :integer, default: nil}, schema end + if current_adapter?(:Mysql2Adapter, :PostgreSQLAdapter) + def test_legacy_primary_key_in_create_table_should_be_integer + @migration = Class.new(ActiveRecord::Migration[5.0]) { + def change + create_table :legacy_primary_keys, id: false do |t| + t.primary_key :id + end + end + }.new + + @migration.migrate(:up) + + schema = dump_table_schema "legacy_primary_keys" + assert_match %r{create_table "legacy_primary_keys", id: :(?:integer|serial), (?!default: nil)}, schema + end + + def test_legacy_primary_key_in_change_table_should_be_integer + @migration = Class.new(ActiveRecord::Migration[5.0]) { + def change + create_table :legacy_primary_keys, id: false do |t| + t.integer :dummy + end + change_table :legacy_primary_keys do |t| + t.primary_key :id + end + end + }.new + + @migration.migrate(:up) + + schema = dump_table_schema "legacy_primary_keys" + assert_match %r{create_table "legacy_primary_keys", id: :(?:integer|serial), (?!default: nil)}, schema + end + + def test_add_column_with_legacy_primary_key_should_be_integer + @migration = Class.new(ActiveRecord::Migration[5.0]) { + def change + create_table :legacy_primary_keys, id: false do |t| + t.integer :dummy + end + add_column :legacy_primary_keys, :id, :primary_key + end + }.new + + @migration.migrate(:up) + + schema = dump_table_schema "legacy_primary_keys" + assert_match %r{create_table "legacy_primary_keys", id: :(?:integer|serial), (?!default: nil)}, schema + end + end + + def test_legacy_join_table_foreign_keys_should_be_integer + @migration = Class.new(ActiveRecord::Migration[5.0]) { + def change + create_join_table :apples, :bananas do |t| + end + end + }.new + + @migration.migrate(:up) + + schema = dump_table_schema "apples_bananas" + assert_match %r{integer "apple_id", null: false}, schema + assert_match %r{integer "banana_id", null: false}, schema + end + + def test_legacy_join_table_column_options_should_be_overwritten + @migration = Class.new(ActiveRecord::Migration[5.0]) { + def change + create_join_table :apples, :bananas, column_options: { type: :bigint } do |t| + end + end + }.new + + @migration.migrate(:up) + + schema = dump_table_schema "apples_bananas" + assert_match %r{bigint "apple_id", null: false}, schema + assert_match %r{bigint "banana_id", null: false}, schema + end + if current_adapter?(:Mysql2Adapter) def test_legacy_bigint_primary_key_should_be_auto_incremented @migration = Class.new(ActiveRecord::Migration[5.0]) { diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb index 170fd02b6f..6cbe18cc8c 100644 --- a/activerecord/test/cases/persistence_test.rb +++ b/activerecord/test/cases/persistence_test.rb @@ -70,10 +70,10 @@ class PersistenceTest < ActiveRecord::TestCase end def test_update_many - topic_data = { 1 => { "content" => "1 updated" }, 2 => { "content" => "2 updated" } } + topic_data = { 1 => { "content" => "1 updated" }, 2 => { "content" => "2 updated" }, nil => {} } updated = Topic.update(topic_data.keys, topic_data.values) - assert_equal 2, updated.size + assert_equal [1, 2], updated.map(&:id) assert_equal "1 updated", Topic.find(1).content assert_equal "2 updated", Topic.find(2).content end @@ -81,9 +81,8 @@ class PersistenceTest < ActiveRecord::TestCase def test_class_level_update_is_affected_by_scoping topic_data = { 1 => { "content" => "1 updated" }, 2 => { "content" => "2 updated" } } - assert_raise(ActiveRecord::RecordNotFound) do - Topic.where("1=0").scoping { Topic.update(topic_data.keys, topic_data.values) } - end + assert_equal [], Topic.where("1=0").scoping { Topic.update(topic_data.keys, topic_data.values) } + assert_not_equal "1 updated", Topic.find(1).content assert_not_equal "2 updated", Topic.find(2).content end @@ -175,7 +174,7 @@ class PersistenceTest < ActiveRecord::TestCase clients = Client.all.merge!(order: "id").find([2, 3]) assert_difference("Client.count", -2) do - destroyed = Client.destroy([2, 3]).sort_by(&:id) + destroyed = Client.destroy([2, 3, nil]).sort_by(&:id) assert_equal clients, destroyed assert destroyed.all?(&:frozen?), "destroyed clients should be frozen" end @@ -917,7 +916,9 @@ class PersistenceTest < ActiveRecord::TestCase should_be_destroyed_reply = Reply.create("title" => "hello", "content" => "world") Topic.find(1).replies << should_be_destroyed_reply - Topic.destroy(1) + topic = Topic.destroy(1) + assert topic.destroyed? + assert_raise(ActiveRecord::RecordNotFound) { Topic.find(1) } assert_raise(ActiveRecord::RecordNotFound) { Reply.find(should_be_destroyed_reply.id) } end @@ -926,9 +927,8 @@ class PersistenceTest < ActiveRecord::TestCase should_not_be_destroyed_reply = Reply.create("title" => "hello", "content" => "world") Topic.find(1).replies << should_not_be_destroyed_reply - assert_raise(ActiveRecord::RecordNotFound) do - Topic.where("1=0").scoping { Topic.destroy(1) } - end + assert_nil Topic.where("1=0").scoping { Topic.destroy(1) } + assert_nothing_raised { Topic.find(1) } assert_nothing_raised { Reply.find(should_not_be_destroyed_reply.id) } end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index ae1dc35bff..4edaf79e9a 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1806,6 +1806,10 @@ class RelationTest < ActiveRecord::TestCase assert_equal post, custom_post_relation.joins(:author).where!(title: post.title).take end + test "arel_attribute respects a custom table" do + assert_equal [posts(:welcome)], custom_post_relation.ranked_by_comments.limit_by(1).to_a + end + test "#load" do relation = Post.all assert_queries(1) do @@ -1912,6 +1916,19 @@ class RelationTest < ActiveRecord::TestCase end end + test "#where with set" do + david = authors(:david) + mary = authors(:mary) + + authors = Author.where(name: ["David", "Mary"].to_set) + assert_equal [david, mary], authors + end + + test "#where with empty set" do + authors = Author.where(name: Set.new) + assert_empty authors + end + private def custom_post_relation table_alias = Post.arel_table.alias("omg_posts") diff --git a/activerecord/test/cases/tasks/postgresql_rake_test.rb b/activerecord/test/cases/tasks/postgresql_rake_test.rb index 6302e84884..ca1defa332 100644 --- a/activerecord/test/cases/tasks/postgresql_rake_test.rb +++ b/activerecord/test/cases/tasks/postgresql_rake_test.rb @@ -229,7 +229,6 @@ if current_adapter?(:PostgreSQLAdapter) ActiveRecord::Base.stubs(:connection).returns(@connection) ActiveRecord::Base.stubs(:establish_connection).returns(true) - Kernel.stubs(:system) end def teardown @@ -333,7 +332,6 @@ if current_adapter?(:PostgreSQLAdapter) ActiveRecord::Base.stubs(:connection).returns(@connection) ActiveRecord::Base.stubs(:establish_connection).returns(true) - Kernel.stubs(:system) end def test_structure_load diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index 4c8e847354..935a11e811 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -21,7 +21,7 @@ class Post < ActiveRecord::Base scope :containing_the_letter_a, -> { where("body LIKE '%a%'") } scope :titled_with_an_apostrophe, -> { where("title LIKE '%''%'") } - scope :ranked_by_comments, -> { order("comments_count DESC") } + scope :ranked_by_comments, -> { order(arel_attribute(:comments_count).desc) } scope :limit_by, lambda { |l| limit(l) } scope :locked, -> { lock } diff --git a/activestorage/README.md b/activestorage/README.md index 17d98978bb..8814887950 100644 --- a/activestorage/README.md +++ b/activestorage/README.md @@ -24,7 +24,7 @@ class User < ApplicationRecord end # Attach an avatar to the user. -user.avatar.attach(io: File.open("~/face.jpg"), filename: "avatar.jpg", content_type: "image/jpg") +user.avatar.attach(io: File.open("/path/to/face.jpg"), filename: "face.jpg", content_type: "image/jpg") # Does the user have an avatar? user.avatar.attached? # => true @@ -63,7 +63,7 @@ end ``` ```erb -<%= form_with model: @message do |form| %> +<%= form_with model: @message, local: true do |form| %> <%= form.text_field :title, placeholder: "Title" %><br> <%= form.text_area :content %><br><br> diff --git a/activestorage/Rakefile b/activestorage/Rakefile index aa71a65f6e..2aa4d2a76f 100644 --- a/activestorage/Rakefile +++ b/activestorage/Rakefile @@ -11,4 +11,6 @@ Rake::TestTask.new do |test| test.warning = false end +task :package + task default: :test diff --git a/activestorage/app/models/active_storage/filename.rb b/activestorage/app/models/active_storage/filename.rb index dead6b6d33..79d55dc889 100644 --- a/activestorage/app/models/active_storage/filename.rb +++ b/activestorage/app/models/active_storage/filename.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true -# Encapsulates a string representing a filename to provide convenience access to parts of it and a sanitized version. -# This is what's returned by ActiveStorage::Blob#filename. A Filename instance is comparable so it can be used for sorting. +# Encapsulates a string representing a filename to provide convenient access to parts of it and sanitization. +# A Filename instance is returned by ActiveStorage::Blob#filename, and is comparable so it can be used for sorting. class ActiveStorage::Filename include Comparable @@ -9,23 +9,31 @@ class ActiveStorage::Filename @filename = filename end - # Returns the basename of the filename. + # Returns the part of the filename preceding any extension. # # ActiveStorage::Filename.new("racecar.jpg").base # => "racecar" + # ActiveStorage::Filename.new("racecar").base # => "racecar" + # ActiveStorage::Filename.new(".gitignore").base # => ".gitignore" def base File.basename @filename, extension_with_delimiter end - # Returns the extension with delimiter of the filename. + # Returns the extension of the filename (i.e. the substring following the last dot, excluding a dot at the + # beginning) with the dot that precedes it. If the filename has no extension, an empty string is returned. # # ActiveStorage::Filename.new("racecar.jpg").extension_with_delimiter # => ".jpg" + # ActiveStorage::Filename.new("racecar").extension_with_delimiter # => "" + # ActiveStorage::Filename.new(".gitignore").extension_with_delimiter # => "" def extension_with_delimiter File.extname @filename end - # Returns the extension without delimiter of the filename. + # Returns the extension of the filename (i.e. the substring following the last dot, excluding a dot at + # the beginning). If the filename has no extension, an empty string is returned. # # ActiveStorage::Filename.new("racecar.jpg").extension_without_delimiter # => "jpg" + # ActiveStorage::Filename.new("racecar").extension_without_delimiter # => "" + # ActiveStorage::Filename.new(".gitignore").extension_without_delimiter # => "" def extension_without_delimiter extension_with_delimiter.from(1).to_s end @@ -37,7 +45,7 @@ class ActiveStorage::Filename # ActiveStorage::Filename.new("foo:bar.jpg").sanitized # => "foo-bar.jpg" # ActiveStorage::Filename.new("foo/bar.jpg").sanitized # => "foo-bar.jpg" # - # ...and any other character unsafe for URLs or storage is converted or stripped. + # Characters considered unsafe for storage (e.g. \, $, and the RTL override character) are replaced with a dash. def sanitized @filename.encode(Encoding::UTF_8, invalid: :replace, undef: :replace, replace: "�").strip.tr("\u{202E}%$|:;/\t\r\n\\", "-") end diff --git a/activestorage/app/models/active_storage/variation.rb b/activestorage/app/models/active_storage/variation.rb index bf269e2a8f..cf04a879eb 100644 --- a/activestorage/app/models/active_storage/variation.rb +++ b/activestorage/app/models/active_storage/variation.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require "active_support/core_ext/object/inclusion" - # A set of transformations that can be applied to a blob to create a variant. This class is exposed via # the ActiveStorage::Blob#variant method and should rarely be used directly. # diff --git a/activestorage/config/routes.rb b/activestorage/config/routes.rb index 168788475c..c3194887be 100644 --- a/activestorage/config/routes.rb +++ b/activestorage/config/routes.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true Rails.application.routes.draw do - get "/rails/active_storage/blobs/:signed_id/*filename" => "active_storage/blobs#show", as: :rails_service_blob + get "/rails/active_storage/blobs/:signed_id/*filename" => "active_storage/blobs#show", as: :rails_service_blob, internal: true direct :rails_blob do |blob| route_for(:rails_service_blob, blob.signed_id, blob.filename) @@ -11,7 +11,7 @@ Rails.application.routes.draw do resolve("ActiveStorage::Attachment") { |attachment| route_for(:rails_blob, attachment.blob) } - get "/rails/active_storage/variants/:signed_blob_id/:variation_key/*filename" => "active_storage/variants#show", as: :rails_blob_variation + get "/rails/active_storage/variants/:signed_blob_id/:variation_key/*filename" => "active_storage/variants#show", as: :rails_blob_variation, internal: true direct :rails_variant do |variant| signed_blob_id = variant.blob.signed_id @@ -24,7 +24,7 @@ Rails.application.routes.draw do resolve("ActiveStorage::Variant") { |variant| route_for(:rails_variant, variant) } - get "/rails/active_storage/disk/:encoded_key/*filename" => "active_storage/disk#show", as: :rails_disk_service - put "/rails/active_storage/disk/:encoded_token" => "active_storage/disk#update", as: :update_rails_disk_service - post "/rails/active_storage/direct_uploads" => "active_storage/direct_uploads#create", as: :rails_direct_uploads + get "/rails/active_storage/disk/:encoded_key/*filename" => "active_storage/disk#show", as: :rails_disk_service, internal: true + put "/rails/active_storage/disk/:encoded_token" => "active_storage/disk#update", as: :update_rails_disk_service, internal: true + post "/rails/active_storage/direct_uploads" => "active_storage/direct_uploads#create", as: :rails_direct_uploads, internal: true end diff --git a/activestorage/lib/active_storage/attached/macros.rb b/activestorage/lib/active_storage/attached/macros.rb index 35a081adc4..f0256718ac 100644 --- a/activestorage/lib/active_storage/attached/macros.rb +++ b/activestorage/lib/active_storage/attached/macros.rb @@ -12,6 +12,10 @@ module ActiveStorage # There is no column defined on the model side, Active Storage takes # care of the mapping between your records and the attachment. # + # To avoid N+1 queries, you can include the attached blobs in your query like so: + # + # User.with_attached_avatar + # # Under the covers, this relationship is implemented as a +has_one+ association to a # ActiveStorage::Attachment record and a +has_one-through+ association to a # ActiveStorage::Blob record. These associations are available as +avatar_attachment+ @@ -33,6 +37,8 @@ module ActiveStorage has_one :"#{name}_attachment", -> { where(name: name) }, class_name: "ActiveStorage::Attachment", as: :record has_one :"#{name}_blob", through: :"#{name}_attachment", class_name: "ActiveStorage::Blob", source: :blob + scope :"with_attached_#{name}", -> { includes("#{name}_attachment": :blob) } + if dependent == :purge_later before_destroy { public_send(name).purge_later } end diff --git a/activestorage/lib/active_storage/attached/many.rb b/activestorage/lib/active_storage/attached/many.rb index 59b7d7d559..1e0657c33c 100644 --- a/activestorage/lib/active_storage/attached/many.rb +++ b/activestorage/lib/active_storage/attached/many.rb @@ -17,7 +17,7 @@ module ActiveStorage # # document.images.attach(params[:images]) # Array of ActionDispatch::Http::UploadedFile objects # document.images.attach(params[:signed_blob_id]) # Signed reference to blob from direct upload - # document.images.attach(io: File.open("~/racecar.jpg"), filename: "racecar.jpg", content_type: "image/jpg") + # document.images.attach(io: File.open("/path/to/racecar.jpg"), filename: "racecar.jpg", content_type: "image/jpg") # document.images.attach([ first_blob, second_blob ]) def attach(*attachables) attachables.flatten.collect do |attachable| diff --git a/activestorage/lib/active_storage/attached/one.rb b/activestorage/lib/active_storage/attached/one.rb index ac90f32d95..c66be08f58 100644 --- a/activestorage/lib/active_storage/attached/one.rb +++ b/activestorage/lib/active_storage/attached/one.rb @@ -18,7 +18,7 @@ module ActiveStorage # # person.avatar.attach(params[:avatar]) # ActionDispatch::Http::UploadedFile object # person.avatar.attach(params[:signed_blob_id]) # Signed reference to blob from direct upload - # person.avatar.attach(io: File.open("~/face.jpg"), filename: "face.jpg", content_type: "image/jpg") + # person.avatar.attach(io: File.open("/path/to/face.jpg"), filename: "face.jpg", content_type: "image/jpg") # person.avatar.attach(avatar_blob) # ActiveStorage::Blob object def attach(attachable) if attached? && dependent == :purge_later diff --git a/activestorage/test/models/attachments_test.rb b/activestorage/test/models/attachments_test.rb index ac346c0087..379ae0a416 100644 --- a/activestorage/test/models/attachments_test.rb +++ b/activestorage/test/models/attachments_test.rb @@ -84,6 +84,19 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase end end + test "find with attached blob" do + records = %w[alice bob].map do |name| + User.create!(name: name).tap do |user| + user.avatar.attach create_blob(filename: "#{name}.jpg") + end + end + + users = User.where(id: records.map(&:id)).with_attached_avatar.all + + assert_equal "alice.jpg", users.first.avatar.filename.to_s + assert_equal "bob.jpg", users.second.avatar.filename.to_s + end + test "attach existing blobs" do @user.highlights.attach create_blob(filename: "funky.jpg"), create_blob(filename: "wonky.jpg") diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index f158d5357d..493ebeb01f 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,22 @@ +* Return an instance of `HashWithIndifferentAccess` from `HashWithIndifferentAccess#transform_keys`. + + *Yuji Yaginuma* + +* Add key rotation support to `MessageEncryptor` and `MessageVerifier` + + This change introduces a `rotate` method to both the `MessageEncryptor` and + `MessageVerifier` classes. This method accepts the same arguments and + options as the given classes' constructor. The `encrypt_and_verify` method + for `MessageEncryptor` and the `verified` method for `MessageVerifier` also + accept an optional keyword argument `:on_rotation` block which is called + when a rotated instance is used to decrypt or verify the message. + + *Michael J Coyne* + +* Deprecate `Module#reachable?` method. + + *bogdanvlviv* + * Add `config/credentials.yml.enc` to store production app secrets. Allows saving any authentication credentials for third party services diff --git a/activesupport/lib/active_support/core_ext/class/subclasses.rb b/activesupport/lib/active_support/core_ext/class/subclasses.rb index 4c910feb44..75e65337b7 100644 --- a/activesupport/lib/active_support/core_ext/class/subclasses.rb +++ b/activesupport/lib/active_support/core_ext/class/subclasses.rb @@ -1,8 +1,5 @@ # frozen_string_literal: true -require_relative "../module/anonymous" -require_relative "../module/reachable" - class Class begin # Test if this Ruby supports each_object against singleton_class diff --git a/activesupport/lib/active_support/core_ext/module/reachable.rb b/activesupport/lib/active_support/core_ext/module/reachable.rb index 91b230b46c..790a3cc561 100644 --- a/activesupport/lib/active_support/core_ext/module/reachable.rb +++ b/activesupport/lib/active_support/core_ext/module/reachable.rb @@ -7,4 +7,5 @@ class Module def reachable? #:nodoc: !anonymous? && name.safe_constantize.equal?(self) end + deprecate :reachable? end diff --git a/activesupport/lib/active_support/hash_with_indifferent_access.rb b/activesupport/lib/active_support/hash_with_indifferent_access.rb index 12291af443..fcc13feb8c 100644 --- a/activesupport/lib/active_support/hash_with_indifferent_access.rb +++ b/activesupport/lib/active_support/hash_with_indifferent_access.rb @@ -306,6 +306,11 @@ module ActiveSupport dup.tap { |hash| hash.transform_values!(*args, &block) } end + def transform_keys(*args, &block) + return to_enum(:transform_keys) unless block_given? + dup.tap { |hash| hash.transform_keys!(*args, &block) } + end + def compact dup.tap(&:compact!) end diff --git a/activesupport/lib/active_support/message_encryptor.rb b/activesupport/lib/active_support/message_encryptor.rb index 27620f56be..8a1918039c 100644 --- a/activesupport/lib/active_support/message_encryptor.rb +++ b/activesupport/lib/active_support/message_encryptor.rb @@ -54,7 +54,33 @@ module ActiveSupport # # Then the messages can be verified and returned upto the expire time. # Thereafter, verifying returns +nil+. + # + # === Rotating keys + # + # MessageEncryptor also supports rotating out old configurations by falling + # back to a stack of encryptors. Call `rotate` to build and add an encryptor + # so `decrypt_and_verify` will also try the fallback. + # + # By default any rotated encryptors use the values of the primary + # encryptor unless specified otherwise. + # + # You'd give your encryptor the new defaults: + # + # crypt = ActiveSupport::MessageEncryptor.new(@secret, cipher: "aes-256-gcm") + # + # Then gradually rotate the old values out by adding them as fallbacks. Any message + # generated with the old values will then work until the rotation is removed. + # + # crypt.rotate old_secret # Fallback to an old secret instead of @secret. + # crypt.rotate cipher: "aes-256-cbc" # Fallback to an old cipher instead of aes-256-gcm. + # + # Though if both the secret and the cipher was changed at the same time, + # the above should be combined into: + # + # verifier.rotate old_secret, cipher: "aes-256-cbc" class MessageEncryptor + prepend Messages::Rotator::Encryptor + class << self attr_accessor :use_authenticated_message_encryption #:nodoc: @@ -126,7 +152,7 @@ module ActiveSupport # Decrypt and verify a message. We need to verify the message in order to # avoid padding attacks. Reference: https://www.limited-entropy.com/padding-oracle-attacks/. - def decrypt_and_verify(data, purpose: nil) + def decrypt_and_verify(data, purpose: nil, **) _decrypt(verifier.verify(data), purpose) end diff --git a/activesupport/lib/active_support/message_verifier.rb b/activesupport/lib/active_support/message_verifier.rb index 7110d6d2c9..f0b6503b96 100644 --- a/activesupport/lib/active_support/message_verifier.rb +++ b/activesupport/lib/active_support/message_verifier.rb @@ -4,6 +4,7 @@ require "base64" require_relative "core_ext/object/blank" require_relative "security_utils" require_relative "messages/metadata" +require_relative "messages/rotator" module ActiveSupport # +MessageVerifier+ makes it easy to generate and verify messages which are @@ -73,7 +74,33 @@ module ActiveSupport # Then the messages can be verified and returned upto the expire time. # Thereafter, the +verified+ method returns +nil+ while +verify+ raises # <tt>ActiveSupport::MessageVerifier::InvalidSignature</tt>. + # + # === Rotating keys + # + # MessageVerifier also supports rotating out old configurations by falling + # back to a stack of verifiers. Call `rotate` to build and add a verifier to + # so either `verified` or `verify` will also try verifying with the fallback. + # + # By default any rotated verifiers use the values of the primary + # verifier unless specified otherwise. + # + # You'd give your verifier the new defaults: + # + # verifier = ActiveSupport::MessageVerifier.new(@secret, digest: "SHA512", serializer: JSON) + # + # Then gradually rotate the old values out by adding them as fallbacks. Any message + # generated with the old values will then work until the rotation is removed. + # + # verifier.rotate old_secret # Fallback to an old secret instead of @secret. + # verifier.rotate digest: "SHA256" # Fallback to an old digest instead of SHA512. + # verifier.rotate serializer: Marshal # Fallback to an old serializer instead of JSON. + # + # Though the above would most likely be combined into one rotation: + # + # verifier.rotate old_secret, digest: "SHA256", serializer: Marshal class MessageVerifier + prepend Messages::Rotator::Verifier + class InvalidSignature < StandardError; end def initialize(secret, options = {}) @@ -120,7 +147,7 @@ module ActiveSupport # # incompatible_message = "test--dad7b06c94abba8d46a15fafaef56c327665d5ff" # verifier.verified(incompatible_message) # => TypeError: incompatible marshal file format - def verified(signed_message, purpose: nil) + def verified(signed_message, purpose: nil, **) if valid_message?(signed_message) begin data = signed_message.split("--".freeze)[0] @@ -145,8 +172,8 @@ module ActiveSupport # # other_verifier = ActiveSupport::MessageVerifier.new 'd1ff3r3nt-s3Krit' # other_verifier.verify(signed_message) # => ActiveSupport::MessageVerifier::InvalidSignature - def verify(signed_message, purpose: nil) - verified(signed_message, purpose: purpose) || raise(InvalidSignature) + def verify(*args) + verified(*args) || raise(InvalidSignature) end # Generates a signed message for the provided value. diff --git a/activesupport/lib/active_support/messages/rotation_configuration.rb b/activesupport/lib/active_support/messages/rotation_configuration.rb new file mode 100644 index 0000000000..bd50d6d348 --- /dev/null +++ b/activesupport/lib/active_support/messages/rotation_configuration.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module ActiveSupport + module Messages + class RotationConfiguration # :nodoc: + attr_reader :signed, :encrypted + + def initialize + @signed, @encrypted = [], [] + end + + def rotate(kind, *args) + case kind + when :signed + @signed << args + when :encrypted + @encrypted << args + end + end + end + end +end diff --git a/activesupport/lib/active_support/messages/rotator.rb b/activesupport/lib/active_support/messages/rotator.rb new file mode 100644 index 0000000000..823a399d67 --- /dev/null +++ b/activesupport/lib/active_support/messages/rotator.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +module ActiveSupport + module Messages + module Rotator # :nodoc: + def initialize(*, **options) + super + + @options = options + @rotations = [] + end + + def rotate(*secrets, **options) + @rotations << build_rotation(*secrets, @options.merge(options)) + end + + module Encryptor + include Rotator + + def decrypt_and_verify(*args, on_rotation: nil, **options) + super + rescue MessageEncryptor::InvalidMessage, MessageVerifier::InvalidSignature + run_rotations(on_rotation) { |encryptor| encryptor.decrypt_and_verify(*args, options) } || raise + end + + private + def build_rotation(secret = @secret, sign_secret = @sign_secret, options) + self.class.new(secret, sign_secret, options) + end + end + + module Verifier + include Rotator + + def verified(*args, on_rotation: nil, **options) + super || run_rotations(on_rotation) { |verifier| verifier.verified(*args, options) } + end + + private + def build_rotation(secret = @secret, options) + self.class.new(secret, options) + end + end + + private + def run_rotations(on_rotation) + @rotations.find do |rotation| + if message = yield(rotation) rescue next + on_rotation.call if on_rotation + return message + end + end + end + end + end +end diff --git a/activesupport/lib/active_support/number_helper/number_to_rounded_converter.rb b/activesupport/lib/active_support/number_helper/number_to_rounded_converter.rb index 3b62fe6819..b7ad76bb62 100644 --- a/activesupport/lib/active_support/number_helper/number_to_rounded_converter.rb +++ b/activesupport/lib/active_support/number_helper/number_to_rounded_converter.rb @@ -37,18 +37,6 @@ module ActiveSupport private - def digits_and_rounded_number(precision) - if zero? - [1, 0] - else - digits = digit_count(number) - multiplier = 10**(digits - precision) - rounded_number = calculate_rounded_number(multiplier) - digits = digit_count(rounded_number) # After rounding, the number of digits may have changed - [digits, rounded_number] - end - end - def calculate_rounded_number(multiplier) (number / BigDecimal.new(multiplier.to_f.to_s)).round * multiplier end diff --git a/activesupport/lib/active_support/ordered_options.rb b/activesupport/lib/active_support/ordered_options.rb index fa7825b3ba..b74510fdb2 100644 --- a/activesupport/lib/active_support/ordered_options.rb +++ b/activesupport/lib/active_support/ordered_options.rb @@ -24,7 +24,7 @@ module ActiveSupport # To raise an exception when the value is blank, append a # bang to the key name, like: # - # h.dog! # => raises KeyError: key not found: :dog + # h.dog! # => raises KeyError: :dog is blank # class OrderedOptions < Hash alias_method :_get, :[] # preserve the original #[] method @@ -46,7 +46,7 @@ module ActiveSupport bangs = name_string.chomp!("!") if bangs - fetch(name_string.to_sym).presence || raise(KeyError.new("#{name_string} is blank.")) + self[name_string].presence || raise(KeyError.new(":#{name_string} is blank")) else self[name_string] end diff --git a/activesupport/lib/active_support/security_utils.rb b/activesupport/lib/active_support/security_utils.rb index 51870559ec..b6b31ef140 100644 --- a/activesupport/lib/active_support/security_utils.rb +++ b/activesupport/lib/active_support/security_utils.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require "digest" +require "digest/sha2" module ActiveSupport module SecurityUtils diff --git a/activesupport/test/core_ext/module/reachable_test.rb b/activesupport/test/core_ext/module/reachable_test.rb index a69fc6839e..097a72fa5b 100644 --- a/activesupport/test/core_ext/module/reachable_test.rb +++ b/activesupport/test/core_ext/module/reachable_test.rb @@ -5,13 +5,17 @@ require "active_support/core_ext/module/reachable" class AnonymousTest < ActiveSupport::TestCase test "an anonymous class or module is not reachable" do - assert !Module.new.reachable? - assert !Class.new.reachable? + assert_deprecated do + assert !Module.new.reachable? + assert !Class.new.reachable? + end end test "ordinary named classes or modules are reachable" do - assert Kernel.reachable? - assert Object.reachable? + assert_deprecated do + assert Kernel.reachable? + assert Object.reachable? + end end test "a named class or module whose constant has gone is not reachable" do @@ -21,8 +25,10 @@ class AnonymousTest < ActiveSupport::TestCase self.class.send(:remove_const, :C) self.class.send(:remove_const, :M) - assert !c.reachable? - assert !m.reachable? + assert_deprecated do + assert !c.reachable? + assert !m.reachable? + end end test "a named class or module whose constants store different objects are not reachable" do @@ -35,9 +41,11 @@ class AnonymousTest < ActiveSupport::TestCase eval "class C; end" eval "module M; end" - assert C.reachable? - assert M.reachable? - assert !c.reachable? - assert !m.reachable? + assert_deprecated do + assert C.reachable? + assert M.reachable? + assert !c.reachable? + assert !m.reachable? + end end end diff --git a/activesupport/test/hash_with_indifferent_access_test.rb b/activesupport/test/hash_with_indifferent_access_test.rb index b3788ee65c..b878ac20fa 100644 --- a/activesupport/test/hash_with_indifferent_access_test.rb +++ b/activesupport/test/hash_with_indifferent_access_test.rb @@ -399,6 +399,13 @@ class HashWithIndifferentAccessTest < ActiveSupport::TestCase assert_instance_of ActiveSupport::HashWithIndifferentAccess, indifferent_strings end + def test_indifferent_transform_keys + hash = ActiveSupport::HashWithIndifferentAccess.new(@strings).transform_keys { |k| k * 2 } + + assert_equal({ "aa" => 1, "bb" => 2 }, hash) + assert_instance_of ActiveSupport::HashWithIndifferentAccess, hash + end + def test_indifferent_compact hash_contain_nil_value = @strings.merge("z" => nil) hash = ActiveSupport::HashWithIndifferentAccess.new(hash_contain_nil_value) diff --git a/activesupport/test/message_encryptor_test.rb b/activesupport/test/message_encryptor_test.rb index 1fbe655642..9edf07f762 100644 --- a/activesupport/test/message_encryptor_test.rb +++ b/activesupport/test/message_encryptor_test.rb @@ -115,6 +115,72 @@ class MessageEncryptorTest < ActiveSupport::TestCase assert_equal "Ruby on Rails", encryptor.decrypt_and_verify(encrypted_message) end + def test_rotating_secret + old_message = ActiveSupport::MessageEncryptor.new(secrets[:old], cipher: "aes-256-gcm").encrypt_and_sign("old") + + encryptor = ActiveSupport::MessageEncryptor.new(@secret, cipher: "aes-256-gcm") + encryptor.rotate secrets[:old] + + assert_equal "old", encryptor.decrypt_and_verify(old_message) + end + + def test_rotating_serializer + old_message = ActiveSupport::MessageEncryptor.new(secrets[:old], cipher: "aes-256-gcm", serializer: JSON). + encrypt_and_sign(ahoy: :hoy) + + encryptor = ActiveSupport::MessageEncryptor.new(@secret, cipher: "aes-256-gcm", serializer: JSON) + encryptor.rotate secrets[:old] + + assert_equal({ "ahoy" => "hoy" }, encryptor.decrypt_and_verify(old_message)) + end + + def test_rotating_aes_cbc_secrets + old_encryptor = ActiveSupport::MessageEncryptor.new(secrets[:old], "old sign", cipher: "aes-256-cbc") + old_message = old_encryptor.encrypt_and_sign("old") + + encryptor = ActiveSupport::MessageEncryptor.new(@secret) + encryptor.rotate secrets[:old], "old sign", cipher: "aes-256-cbc" + + assert_equal "old", encryptor.decrypt_and_verify(old_message) + end + + def test_multiple_rotations + older_message = ActiveSupport::MessageEncryptor.new(secrets[:older], "older sign").encrypt_and_sign("older") + old_message = ActiveSupport::MessageEncryptor.new(secrets[:old], "old sign").encrypt_and_sign("old") + + encryptor = ActiveSupport::MessageEncryptor.new(@secret) + encryptor.rotate secrets[:old], "old sign" + encryptor.rotate secrets[:older], "older sign" + + assert_equal "new", encryptor.decrypt_and_verify(encryptor.encrypt_and_sign("new")) + assert_equal "old", encryptor.decrypt_and_verify(old_message) + assert_equal "older", encryptor.decrypt_and_verify(older_message) + end + + def test_on_rotation_is_called_and_returns_modified_messages + older_message = ActiveSupport::MessageEncryptor.new(secrets[:older], "older sign").encrypt_and_sign(encoded: "message") + + encryptor = ActiveSupport::MessageEncryptor.new(@secret) + encryptor.rotate secrets[:old] + encryptor.rotate secrets[:older], "older sign" + + rotated = false + message = encryptor.decrypt_and_verify(older_message, on_rotation: proc { rotated = true }) + + assert_equal({ encoded: "message" }, message) + assert rotated + end + + def test_with_rotated_metadata + old_message = ActiveSupport::MessageEncryptor.new(secrets[:old], cipher: "aes-256-gcm"). + encrypt_and_sign("metadata", purpose: :rotation) + + encryptor = ActiveSupport::MessageEncryptor.new(@secret, cipher: "aes-256-gcm") + encryptor.rotate secrets[:old] + + assert_equal "metadata", encryptor.decrypt_and_verify(old_message, purpose: :rotation) + end + private def assert_aead_not_decrypted(encryptor, value) assert_raise(ActiveSupport::MessageEncryptor::InvalidMessage) do @@ -134,6 +200,10 @@ class MessageEncryptorTest < ActiveSupport::TestCase end end + def secrets + @secrets ||= Hash.new { |h, k| h[k] = SecureRandom.random_bytes(32) } + end + def munge(base64_string) bits = ::Base64.strict_decode64(base64_string) bits.reverse! diff --git a/activesupport/test/message_verifier_test.rb b/activesupport/test/message_verifier_test.rb index fbeafca203..05d5c1cbc3 100644 --- a/activesupport/test/message_verifier_test.rb +++ b/activesupport/test/message_verifier_test.rb @@ -20,6 +20,7 @@ class MessageVerifierTest < ActiveSupport::TestCase def setup @verifier = ActiveSupport::MessageVerifier.new("Hey, I'm a secret!") @data = { some: "data", now: Time.utc(2010) } + @secret = SecureRandom.random_bytes(32) end def test_valid_message @@ -90,6 +91,51 @@ class MessageVerifierTest < ActiveSupport::TestCase signed_message = "BAh7BzoJc29tZUkiCWRhdGEGOgZFVDoIbm93SXU6CVRpbWUNIIAbgAAAAAAHOgtvZmZzZXRpADoJem9uZUkiCFVUQwY7BkY=--d03c52c91dfe4ccc5159417c660461bcce005e96" assert_equal @data, @verifier.verify(signed_message) end + + def test_rotating_secret + old_message = ActiveSupport::MessageVerifier.new("old", digest: "SHA1").generate("old") + + verifier = ActiveSupport::MessageVerifier.new(@secret, digest: "SHA1") + verifier.rotate "old" + + assert_equal "old", verifier.verified(old_message) + end + + def test_multiple_rotations + old_message = ActiveSupport::MessageVerifier.new("old", digest: "SHA256").generate("old") + older_message = ActiveSupport::MessageVerifier.new("older", digest: "SHA1").generate("older") + + verifier = ActiveSupport::MessageVerifier.new(@secret, digest: "SHA512") + verifier.rotate "old", digest: "SHA256" + verifier.rotate "older", digest: "SHA1" + + assert_equal "new", verifier.verified(verifier.generate("new")) + assert_equal "old", verifier.verified(old_message) + assert_equal "older", verifier.verified(older_message) + end + + def test_on_rotation_is_called_and_verified_returns_message + older_message = ActiveSupport::MessageVerifier.new("older", digest: "SHA1").generate(encoded: "message") + + verifier = ActiveSupport::MessageVerifier.new(@secret, digest: "SHA512") + verifier.rotate "old", digest: "SHA256" + verifier.rotate "older", digest: "SHA1" + + rotated = false + message = verifier.verified(older_message, on_rotation: proc { rotated = true }) + + assert_equal({ encoded: "message" }, message) + assert rotated + end + + def test_rotations_with_metadata + old_message = ActiveSupport::MessageVerifier.new("old").generate("old", purpose: :rotation) + + verifier = ActiveSupport::MessageVerifier.new(@secret) + verifier.rotate "old" + + assert_equal "old", verifier.verified(old_message, purpose: :rotation) + end end class MessageVerifierMetadataTest < ActiveSupport::TestCase diff --git a/activesupport/test/messages/rotation_configuration_test.rb b/activesupport/test/messages/rotation_configuration_test.rb new file mode 100644 index 0000000000..2f6824ed21 --- /dev/null +++ b/activesupport/test/messages/rotation_configuration_test.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +require "abstract_unit" +require "active_support/messages/rotation_configuration" + +class MessagesRotationConfiguration < ActiveSupport::TestCase + def setup + @config = ActiveSupport::Messages::RotationConfiguration.new + end + + def test_signed_configurations + @config.rotate :signed, "older secret", salt: "salt", digest: "SHA1" + @config.rotate :signed, "old secret", salt: "salt", digest: "SHA256" + + assert_equal [ + [ "older secret", salt: "salt", digest: "SHA1" ], + [ "old secret", salt: "salt", digest: "SHA256" ] ], @config.signed + end + + def test_encrypted_configurations + @config.rotate :encrypted, "old raw key", cipher: "aes-256-gcm" + + assert_equal [ [ "old raw key", cipher: "aes-256-gcm" ] ], @config.encrypted + end +end diff --git a/activesupport/test/ordered_options_test.rb b/activesupport/test/ordered_options_test.rb index 7f2e774c02..2c67bb02ac 100644 --- a/activesupport/test/ordered_options_test.rb +++ b/activesupport/test/ordered_options_test.rb @@ -102,4 +102,17 @@ class OrderedOptionsTest < ActiveSupport::TestCase end assert_raises(KeyError) { a.non_existing_key! } end + + def test_inheritable_options_with_bang + a = ActiveSupport::InheritableOptions.new(foo: :bar) + + assert_nothing_raised { a.foo! } + assert_equal a.foo, a.foo! + + assert_raises(KeyError) do + a.foo = nil + a.foo! + end + assert_raises(KeyError) { a.non_existing_key! } + end end diff --git a/guides/bug_report_templates/action_controller_gem.rb b/guides/bug_report_templates/action_controller_gem.rb index 4d8d8db3e5..341724cdcd 100644 --- a/guides/bug_report_templates/action_controller_gem.rb +++ b/guides/bug_report_templates/action_controller_gem.rb @@ -9,6 +9,9 @@ end gemfile(true) do source "https://rubygems.org" + + git_source(:github) { |repo| "https://github.com/#{repo}.git" } + # Activate the gem you are reporting the issue against. gem "rails", "5.1.0" end diff --git a/guides/bug_report_templates/action_controller_master.rb b/guides/bug_report_templates/action_controller_master.rb index 1f862e07da..558d9bf3e2 100644 --- a/guides/bug_report_templates/action_controller_master.rb +++ b/guides/bug_report_templates/action_controller_master.rb @@ -9,6 +9,9 @@ end gemfile(true) do source "https://rubygems.org" + + git_source(:github) { |repo| "https://github.com/#{repo}.git" } + gem "rails", github: "rails/rails" gem "arel", github: "rails/arel" end diff --git a/guides/bug_report_templates/active_job_gem.rb b/guides/bug_report_templates/active_job_gem.rb index af777a86ef..013d1f8602 100644 --- a/guides/bug_report_templates/active_job_gem.rb +++ b/guides/bug_report_templates/active_job_gem.rb @@ -9,6 +9,9 @@ end gemfile(true) do source "https://rubygems.org" + + git_source(:github) { |repo| "https://github.com/#{repo}.git" } + # Activate the gem you are reporting the issue against. gem "activejob", "5.1.0" end diff --git a/guides/bug_report_templates/active_job_master.rb b/guides/bug_report_templates/active_job_master.rb index 39fb3f60a6..ce480cbb52 100644 --- a/guides/bug_report_templates/active_job_master.rb +++ b/guides/bug_report_templates/active_job_master.rb @@ -9,6 +9,9 @@ end gemfile(true) do source "https://rubygems.org" + + git_source(:github) { |repo| "https://github.com/#{repo}.git" } + gem "rails", github: "rails/rails" gem "arel", github: "rails/arel" end diff --git a/guides/bug_report_templates/active_record_gem.rb b/guides/bug_report_templates/active_record_gem.rb index 168e2dcc66..921917fbe9 100644 --- a/guides/bug_report_templates/active_record_gem.rb +++ b/guides/bug_report_templates/active_record_gem.rb @@ -9,6 +9,9 @@ end gemfile(true) do source "https://rubygems.org" + + git_source(:github) { |repo| "https://github.com/#{repo}.git" } + # Activate the gem you are reporting the issue against. gem "activerecord", "5.1.0" gem "sqlite3" diff --git a/guides/bug_report_templates/active_record_master.rb b/guides/bug_report_templates/active_record_master.rb index cbd2cff2b8..78411e2d57 100644 --- a/guides/bug_report_templates/active_record_master.rb +++ b/guides/bug_report_templates/active_record_master.rb @@ -9,6 +9,9 @@ end gemfile(true) do source "https://rubygems.org" + + git_source(:github) { |repo| "https://github.com/#{repo}.git" } + gem "rails", github: "rails/rails" gem "arel", github: "rails/arel" gem "sqlite3" diff --git a/guides/bug_report_templates/active_record_migrations_gem.rb b/guides/bug_report_templates/active_record_migrations_gem.rb index b931ed0beb..f75b6fd932 100644 --- a/guides/bug_report_templates/active_record_migrations_gem.rb +++ b/guides/bug_report_templates/active_record_migrations_gem.rb @@ -9,6 +9,9 @@ end gemfile(true) do source "https://rubygems.org" + + git_source(:github) { |repo| "https://github.com/#{repo}.git" } + # Activate the gem you are reporting the issue against. gem "activerecord", "5.1.0" gem "sqlite3" diff --git a/guides/bug_report_templates/active_record_migrations_master.rb b/guides/bug_report_templates/active_record_migrations_master.rb index 2c009c0563..60416ed42f 100644 --- a/guides/bug_report_templates/active_record_migrations_master.rb +++ b/guides/bug_report_templates/active_record_migrations_master.rb @@ -9,6 +9,9 @@ end gemfile(true) do source "https://rubygems.org" + + git_source(:github) { |repo| "https://github.com/#{repo}.git" } + gem "rails", github: "rails/rails" gem "arel", github: "rails/arel" gem "sqlite3" diff --git a/guides/bug_report_templates/benchmark.rb b/guides/bug_report_templates/benchmark.rb index d0f5a634bc..fb51273e3e 100644 --- a/guides/bug_report_templates/benchmark.rb +++ b/guides/bug_report_templates/benchmark.rb @@ -9,6 +9,9 @@ end gemfile(true) do source "https://rubygems.org" + + git_source(:github) { |repo| "https://github.com/#{repo}.git" } + gem "rails", github: "rails/rails" gem "arel", github: "rails/arel" gem "benchmark-ips" diff --git a/guides/bug_report_templates/generic_gem.rb b/guides/bug_report_templates/generic_gem.rb index c990bda005..60e8322c2a 100644 --- a/guides/bug_report_templates/generic_gem.rb +++ b/guides/bug_report_templates/generic_gem.rb @@ -9,6 +9,9 @@ end gemfile(true) do source "https://rubygems.org" + + git_source(:github) { |repo| "https://github.com/#{repo}.git" } + # Activate the gem you are reporting the issue against. gem "activesupport", "5.1.0" end diff --git a/guides/bug_report_templates/generic_master.rb b/guides/bug_report_templates/generic_master.rb index 1a9b99b624..384c8b1833 100644 --- a/guides/bug_report_templates/generic_master.rb +++ b/guides/bug_report_templates/generic_master.rb @@ -9,6 +9,9 @@ end gemfile(true) do source "https://rubygems.org" + + git_source(:github) { |repo| "https://github.com/#{repo}.git" } + gem "rails", github: "rails/rails" gem "arel", github: "rails/arel" end diff --git a/guides/source/5_1_release_notes.md b/guides/source/5_1_release_notes.md index fa92b9e5f8..80c9da6446 100644 --- a/guides/source/5_1_release_notes.md +++ b/guides/source/5_1_release_notes.md @@ -602,7 +602,7 @@ Please refer to the [Changelog][active-support] for detailed changes. ([Pull Request](https://github.com/rails/rails/pull/28157)) * Deprecated passing string to `:if` and `:unless` conditional options on `set_callback` and `skip_callback`. - ([Commit](https://github.com/rails/rails/commit/0952552) + ([Commit](https://github.com/rails/rails/commit/0952552)) ### Notable changes diff --git a/guides/source/action_controller_overview.md b/guides/source/action_controller_overview.md index 2c3f74c3e1..5fb8e300de 100644 --- a/guides/source/action_controller_overview.md +++ b/guides/source/action_controller_overview.md @@ -400,9 +400,9 @@ Rails.application.config.session_store :cookie_store, key: '_your_app_session', Rails sets up (for the CookieStore) a secret key used for signing the session data in `config/credentials.yml.enc`. This can be changed with `bin/rails credentials:edit`. ```ruby -# amazon: -# access_key_id: 123 -# secret_access_key: 345 +# aws: +# access_key_id: 123 +# secret_access_key: 345 # Used as the base secret for all MessageVerifiers in Rails, including the one protecting cookies. secret_key_base: 492f... diff --git a/guides/source/action_mailer_basics.md b/guides/source/action_mailer_basics.md index 96ef9c4450..cb07781d1c 100644 --- a/guides/source/action_mailer_basics.md +++ b/guides/source/action_mailer_basics.md @@ -92,8 +92,8 @@ registered email address: class UserMailer < ApplicationMailer default from: 'notifications@example.com' - def welcome_email(user) - @user = user + def welcome_email + @user = params[:user] @url = 'http://example.com/login' mail(to: @user.email, subject: 'Welcome to My Awesome Site') end @@ -176,7 +176,7 @@ $ bin/rails db:migrate Now that we have a user model to play with, we will just edit the `app/controllers/users_controller.rb` make it instruct the `UserMailer` to deliver an email to the newly created user by editing the create action and inserting a -call to `UserMailer.welcome_email` right after the user is successfully saved. +call to `UserMailer.with(user: @user).welcome_email` right after the user is successfully saved. Action Mailer is nicely integrated with Active Job so you can send emails outside of the request-response cycle, so the user doesn't have to wait on it: @@ -191,7 +191,7 @@ class UsersController < ApplicationController respond_to do |format| if @user.save # Tell the UserMailer to send a welcome email after save - UserMailer.welcome_email(@user).deliver_later + UserMailer.with(user: @user).welcome_email.deliver_later format.html { redirect_to(@user, notice: 'User was successfully created.') } format.json { render json: @user, status: :created, location: @user } @@ -220,12 +220,17 @@ If you want to send emails right away (from a cronjob for example) just call class SendWeeklySummary def run User.find_each do |user| - UserMailer.weekly_summary(user).deliver_now + UserMailer.with(user: user).weekly_summary.deliver_now end end end ``` +Any key value pair passed to `with` just becomes the `params` for the mailer +action. So `with(user: @user, account: @user.account)` makes `params[:user]` and +`params[:account]` available in the mailer action. Just like controllers have +params. + The method `welcome_email` returns an `ActionMailer::MessageDelivery` object which can then just be told `deliver_now` or `deliver_later` to send itself out. The `ActionMailer::MessageDelivery` object is just a wrapper around a `Mail::Message`. If @@ -331,7 +336,7 @@ with the addresses separated by commas. ```ruby class AdminMailer < ApplicationMailer - default to: Proc.new { Admin.pluck(:email) }, + default to: -> { Admin.pluck(:email) }, from: 'notification@example.com' def new_registration(user) @@ -351,8 +356,8 @@ address when they receive the email. The trick to doing that is to format the email address in the format `"Full Name" <email>`. ```ruby -def welcome_email(user) - @user = user +def welcome_email + @user = params[:user] email_with_name = %("#{@user.name}" <#{@user.email}>) mail(to: email_with_name, subject: 'Welcome to My Awesome Site') end @@ -372,8 +377,8 @@ To change the default mailer view for your action you do something like: class UserMailer < ApplicationMailer default from: 'notifications@example.com' - def welcome_email(user) - @user = user + def welcome_email + @user = params[:user] @url = 'http://example.com/login' mail(to: @user.email, subject: 'Welcome to My Awesome Site', @@ -394,8 +399,8 @@ templates or even render inline or text without using a template file: class UserMailer < ApplicationMailer default from: 'notifications@example.com' - def welcome_email(user) - @user = user + def welcome_email + @user = params[:user] @url = 'http://example.com/login' mail(to: @user.email, subject: 'Welcome to My Awesome Site') do |format| @@ -453,8 +458,8 @@ the format block to specify different layouts for different formats: ```ruby class UserMailer < ApplicationMailer - def welcome_email(user) - mail(to: user.email) do |format| + def welcome_email + mail(to: params[:user].email) do |format| format.html { render layout: 'my_layout' } format.text end @@ -477,7 +482,7 @@ special URL that renders them. In the above example, the preview class for ```ruby class UserMailerPreview < ActionMailer::Preview def welcome_email - UserMailer.welcome_email(User.first) + UserMailer.with(user: User.first).welcome_email end end ``` @@ -594,12 +599,12 @@ mailer action. ```ruby class UserMailer < ApplicationMailer - def welcome_email(user, company) - @user = user + def welcome_email + @user = params[:user] @url = user_url(@user) - delivery_options = { user_name: company.smtp_user, - password: company.smtp_password, - address: company.smtp_host } + delivery_options = { user_name: params[:company].smtp_user, + password: params[:company].smtp_password, + address: params[:company].smtp_host } mail(to: @user.email, subject: "Please see the Terms and Conditions attached", delivery_method_options: delivery_options) @@ -616,9 +621,9 @@ will default to `text/plain` otherwise. ```ruby class UserMailer < ApplicationMailer - def welcome_email(user, email_body) - mail(to: user.email, - body: email_body, + def welcome_email + mail(to: params[:user].email, + body: params[:email_body], content_type: "text/html", subject: "Already rendered!") end @@ -677,24 +682,43 @@ Action Mailer allows for you to specify a `before_action`, `after_action` and * You could use a `before_action` to populate the mail object with defaults, delivery_method_options or insert default headers and attachments. +```ruby +class InvitationsMailer < ApplicationMailer + before_action { @inviter, @invitee = params[:inviter], params[:invitee] } + before_action { @account = params[:inviter].account } + + default to: -> { @invitee.email_address }, + from: -> { common_address(@inviter) }, + reply_to: -> { @inviter.email_address_with_name } + + def account_invitation + mail subject: "#{@inviter.name} invited you to their Basecamp (#{@account.name})" + end + + def project_invitation + @project = params[:project] + @summarizer = ProjectInvitationSummarizer.new(@project.bucket) + + mail subject: "#{@inviter.name.familiar} added you to a project in Basecamp (#{@account.name})" + end +end +``` + * You could use an `after_action` to do similar setup as a `before_action` but using instance variables set in your mailer action. ```ruby class UserMailer < ApplicationMailer + before_action { @business, @user = params[:business], params[:user] } + after_action :set_delivery_options, :prevent_delivery_to_guests, :set_business_headers - def feedback_message(business, user) - @business = business - @user = user - mail + def feedback_message end - def campaign_message(business, user) - @business = business - @user = user + def campaign_message end private diff --git a/guides/source/active_job_basics.md b/guides/source/active_job_basics.md index 7a3ff12b63..914ef2c327 100644 --- a/guides/source/active_job_basics.md +++ b/guides/source/active_job_basics.md @@ -389,6 +389,25 @@ class GuestsCleanupJob < ApplicationJob end ``` +### Retrying or Discarding failed jobs + +It's also possible to retry or discard a job if an exception is raised during execution. +For example: + +```ruby +class RemoteServiceJob < ApplicationJob + retry_on CustomAppException # defaults to 3s wait, 5 attempts + + discard_on ActiveJob::DeserializationError + + def perform(*args) + # Might raise CustomAppException or ActiveJob::DeserializationError + end +end +``` + +To get more details see the API Documentation for [ActiveJob::Exceptions](http://api.rubyonrails.org/classes/ActiveJob/Exceptions/ClassMethods.html). + ### Deserialization GlobalID allows serializing full Active Record objects passed to `#perform`. diff --git a/guides/source/active_record_querying.md b/guides/source/active_record_querying.md index 678b80516f..3573c3c77b 100644 --- a/guides/source/active_record_querying.md +++ b/guides/source/active_record_querying.md @@ -414,7 +414,7 @@ end `find_in_batches` works on model classes, as seen above, and also on relations: ```ruby -Invoice.pending.find_in_batches do |invoice| +Invoice.pending.find_in_batches do |invoices| pending_invoices_export.add_invoices(invoices) end ``` diff --git a/guides/source/active_support_core_extensions.md b/guides/source/active_support_core_extensions.md index 1438245f9c..ae573cc77c 100644 --- a/guides/source/active_support_core_extensions.md +++ b/guides/source/active_support_core_extensions.md @@ -674,44 +674,6 @@ M.parents # => [X::Y, X, Object] NOTE: Defined in `active_support/core_ext/module/introspection.rb`. -### Reachable - -A named module is reachable if it is stored in its corresponding constant. It means you can reach the module object via the constant. - -That is what ordinarily happens, if a module is called "M", the `M` constant exists and holds it: - -```ruby -module M -end - -M.reachable? # => true -``` - -But since constants and modules are indeed kind of decoupled, module objects can become unreachable: - -```ruby -module M -end - -orphan = Object.send(:remove_const, :M) - -# The module object is orphan now but it still has a name. -orphan.name # => "M" - -# You cannot reach it via the constant M because it does not even exist. -orphan.reachable? # => false - -# Let's define a module called "M" again. -module M -end - -# The constant M exists now again, and it stores a module -# object called "M", but it is a new instance. -orphan.reachable? # => false -``` - -NOTE: Defined in `active_support/core_ext/module/reachable.rb`. - ### Anonymous A module may or may not have a name: @@ -745,7 +707,6 @@ end m = Object.send(:remove_const, :M) -m.reachable? # => false m.anonymous? # => false ``` diff --git a/guides/source/active_support_instrumentation.md b/guides/source/active_support_instrumentation.md index 03c9183eb3..ff4288a7f5 100644 --- a/guides/source/active_support_instrumentation.md +++ b/guides/source/active_support_instrumentation.md @@ -304,7 +304,7 @@ Action Mailer mailer: "Notification", message_id: "4f5b5491f1774_181b23fc3d4434d38138e5@mba.local.mail", subject: "Rails Guides", - to: ["users@rails.com", "ddh@rails.com"], + to: ["users@rails.com", "dhh@rails.com"], from: ["me@rails.com"], date: Sat, 10 Mar 2012 14:18:09 +0100, mail: "..." # omitted for brevity @@ -330,7 +330,7 @@ Action Mailer mailer: "Notification", message_id: "4f5b5491f1774_181b23fc3d4434d38138e5@mba.local.mail", subject: "Rails Guides", - to: ["users@rails.com", "ddh@rails.com"], + to: ["users@rails.com", "dhh@rails.com"], from: ["me@rails.com"], date: Sat, 10 Mar 2012 14:18:09 +0100, mail: "..." # omitted for brevity diff --git a/guides/source/api_app.md b/guides/source/api_app.md index da1b7b25ef..43a7de88b0 100644 --- a/guides/source/api_app.md +++ b/guides/source/api_app.md @@ -216,7 +216,6 @@ An API application comes with the following middleware by default: - `Rack::Head` - `Rack::ConditionalGet` - `Rack::ETag` -- `MyApi::Application::Routes` See the [internal middleware](rails_on_rack.html#internal-middleware-stack) section of the Rack guide for further information on them. diff --git a/guides/source/asset_pipeline.md b/guides/source/asset_pipeline.md index 17ab9c7600..8bd1f91304 100644 --- a/guides/source/asset_pipeline.md +++ b/guides/source/asset_pipeline.md @@ -154,7 +154,7 @@ environments. You can enable or disable it in your configuration through the More reading: -* [Optimize caching](http://code.google.com/speed/page-speed/docs/caching.html) +* [Optimize caching](https://developers.google.com/speed/docs/insights/LeverageBrowserCaching) * [Revving Filenames: don't use querystring](http://www.stevesouders.com/blog/2008/08/23/revving-filenames-dont-use-querystring/) diff --git a/guides/source/association_basics.md b/guides/source/association_basics.md index b5bd24d027..9616647f15 100644 --- a/guides/source/association_basics.md +++ b/guides/source/association_basics.md @@ -906,7 +906,7 @@ The `belongs_to` association supports these options: ##### `:autosave` -If you set the `:autosave` option to `true`, Rails will save any loaded members and destroy members that are marked for destruction whenever you save the parent object. +If you set the `:autosave` option to `true`, Rails will save any loaded association members and destroy members that are marked for destruction whenever you save the parent object. Setting `:autosave` to `false` is not the same as not setting the `:autosave` option. If the `:autosave` option is not present, then new associated objects will be saved, but updated associated objects will not be saved. ##### `:class_name` @@ -1257,7 +1257,7 @@ Setting the `:as` option indicates that this is a polymorphic association. Polym ##### `:autosave` -If you set the `:autosave` option to `true`, Rails will save any loaded members and destroy members that are marked for destruction whenever you save the parent object. +If you set the `:autosave` option to `true`, Rails will save any loaded association members and destroy members that are marked for destruction whenever you save the parent object. Setting `:autosave` to `false` is not the same as not setting the `:autosave` option. If the `:autosave` option is not present, then new associated objects will be saved, but updated associated objects will not be saved. ##### `:class_name` @@ -1653,7 +1653,7 @@ Setting the `:as` option indicates that this is a polymorphic association, as di ##### `:autosave` -If you set the `:autosave` option to `true`, Rails will save any loaded members and destroy members that are marked for destruction whenever you save the parent object. +If you set the `:autosave` option to `true`, Rails will save any loaded association members and destroy members that are marked for destruction whenever you save the parent object. Setting `:autosave` to `false` is not the same as not setting the `:autosave` option. If the `:autosave` option is not present, then new associated objects will be saved, but updated associated objects will not be saved. ##### `:class_name` @@ -2176,7 +2176,7 @@ end ##### `:autosave` -If you set the `:autosave` option to `true`, Rails will save any loaded members and destroy members that are marked for destruction whenever you save the parent object. +If you set the `:autosave` option to `true`, Rails will save any loaded association members and destroy members that are marked for destruction whenever you save the parent object. Setting `:autosave` to `false` is not the same as not setting the `:autosave` option. If the `:autosave` option is not present, then new associated objects will be saved, but updated associated objects will not be saved. ##### `:class_name` diff --git a/guides/source/autoloading_and_reloading_constants.md b/guides/source/autoloading_and_reloading_constants.md index c62194faf4..ede0324a51 100644 --- a/guides/source/autoloading_and_reloading_constants.md +++ b/guides/source/autoloading_and_reloading_constants.md @@ -954,7 +954,7 @@ to work on some subclass, things get interesting. While working with `Polygon` you do not need to be aware of all its descendants, because anything in the table is by definition a polygon, but when working with subclasses Active Record needs to be able to enumerate the types it is looking -for. Let’s see an example. +for. Let's see an example. `Rectangle.all` only loads rectangles by adding a type constraint to the query: @@ -963,7 +963,7 @@ SELECT "polygons".* FROM "polygons" WHERE "polygons"."type" IN ("Rectangle") ``` -Let’s introduce now a subclass of `Rectangle`: +Let's introduce now a subclass of `Rectangle`: ```ruby # app/models/square.rb @@ -978,7 +978,7 @@ SELECT "polygons".* FROM "polygons" WHERE "polygons"."type" IN ("Rectangle", "Square") ``` -But there’s a caveat here: How does Active Record know that the class `Square` +But there's a caveat here: How does Active Record know that the class `Square` exists at all? Even if the file `app/models/square.rb` exists and defines the `Square` class, @@ -1049,7 +1049,7 @@ end The purpose of this setup would be that the application uses the class that corresponds to the environment via `AUTH_SERVICE`. In development mode -`MockedAuthService` gets autoloaded when the initializer runs. Let’s suppose +`MockedAuthService` gets autoloaded when the initializer runs. Let's suppose we do some requests, change its implementation, and hit the application again. To our surprise the changes are not reflected. Why? diff --git a/guides/source/caching_with_rails.md b/guides/source/caching_with_rails.md index 910a531068..96650b5be9 100644 --- a/guides/source/caching_with_rails.md +++ b/guides/source/caching_with_rails.md @@ -181,7 +181,7 @@ cache. ### Shared Partial Caching -It is possible to share partials and associated caching between files with different mime types. For example shared partial caching allows template writers to share a partial between HTML and Javascript files. When templates are collected in the template resolver file paths they only include the template language extension and not the mime type. Because of this templates can be used for multiple mime types. Both HTML and JavaScript requests will respond to the following code: +It is possible to share partials and associated caching between files with different mime types. For example shared partial caching allows template writers to share a partial between HTML and JavaScript files. When templates are collected in the template resolver file paths they only include the template language extension and not the mime type. Because of this templates can be used for multiple mime types. Both HTML and JavaScript requests will respond to the following code: ```ruby render(partial: 'hotels/hotel', collection: @hotels, cached: true) @@ -195,7 +195,7 @@ Another option is to include the full filename of the partial to render. render(partial: 'hotels/hotel.html.erb', collection: @hotels, cached: true) ``` -Will load a file named `hotels/hotel.html.erb` in any file mime type, for example you could include this partial in a Javascript file. +Will load a file named `hotels/hotel.html.erb` in any file mime type, for example you could include this partial in a JavaScript file. ### Managing dependencies diff --git a/guides/source/configuring.md b/guides/source/configuring.md index 1c720ad82f..0f87d73d6e 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -487,6 +487,15 @@ Defaults to `'signed cookie'`. authenticated encrypted cookie salt. Defaults to `'authenticated encrypted cookie'`. +* `config.action_dispatch.encrypted_cookie_cipher` sets the cipher to be + used for encrypted cookies. This defaults to `"aes-256-gcm"`. + +* `config.action_dispatch.signed_cookie_digest` sets the digest to be + used for signed cookies. This defaults to `"SHA1"`. + +* `config.action_dispatch.cookies_rotations` allows rotating + secrets, ciphers, and digests for encrypted and signed cookies. + * `config.action_dispatch.perform_deep_munge` configures whether `deep_munge` method should be performed on the parameters. See [Security Guide](security.html#unsafe-query-generation) for more information. It defaults to `true`. diff --git a/guides/source/engines.md b/guides/source/engines.md index c7331b6ca4..188620a683 100644 --- a/guides/source/engines.md +++ b/guides/source/engines.md @@ -63,7 +63,7 @@ authentication for its parent applications, or [Thredded](https://github.com/thredded/thredded), an engine that provides forum functionality. There's also [Spree](https://github.com/spree/spree) which provides an e-commerce platform, and -[RefineryCMS](https://github.com/refinery/refinerycms), a CMS engine. +[Refinery CMS](https://github.com/refinery/refinerycms), a CMS engine. Finally, engines would not have been possible without the work of James Adam, Piotr Sarnacki, the Rails Core Team, and a number of other people. If you ever @@ -1322,7 +1322,7 @@ engine. Assets within an engine work in an identical way to a full application. Because the engine class inherits from `Rails::Engine`, the application will know to -look up assets in the engine's 'app/assets' and 'lib/assets' directories. +look up assets in the engine's `app/assets` and `lib/assets` directories. Like all of the other components of an engine, the assets should be namespaced. This means that if you have an asset called `style.css`, it should be placed at diff --git a/guides/source/form_helpers.md b/guides/source/form_helpers.md index f46f1648b3..4ce67df93a 100644 --- a/guides/source/form_helpers.md +++ b/guides/source/form_helpers.md @@ -274,10 +274,12 @@ There are a few things to note here: The resulting HTML is: ```html -<form accept-charset="UTF-8" action="/articles" method="post" class="nifty_form"> - <input id="article_title" name="article[title]" type="text" /> - <textarea id="article_body" name="article[body]" cols="60" rows="12"></textarea> - <input name="commit" type="submit" value="Create" /> +<form class="nifty_form" id="new_article" action="/articles" accept-charset="UTF-8" method="post"> + <input name="utf8" type="hidden" value="✓" /> + <input type="hidden" name="authenticity_token" value="NRkFyRWxdYNfUg7vYxLOp2SLf93lvnl+QwDWorR42Dp6yZXPhHEb6arhDOIWcqGit8jfnrPwL781/xlrzj63TA==" /> + <input type="text" name="article[title]" id="article_title" /> + <textarea name="article[body]" id="article_body" cols="60" rows="12"></textarea> + <input type="submit" name="commit" value="Create" data-disable-with="Create" /> </form> ``` @@ -299,9 +301,11 @@ You can create a similar binding without actually creating `<form>` tags with th which produces the following output: ```html -<form accept-charset="UTF-8" action="/people" class="new_person" id="new_person" method="post"> - <input id="person_name" name="person[name]" type="text" /> - <input id="contact_detail_phone_number" name="contact_detail[phone_number]" type="text" /> +<form class="new_person" id="new_person" action="/people" accept-charset="UTF-8" method="post"> + <input name="utf8" type="hidden" value="✓" /> + <input type="hidden" name="authenticity_token" value="bL13x72pldyDD8bgtkjKQakJCpd4A8JdXGbfksxBDHdf1uC0kCMqe2tvVdUYfidJt0fj3ihC4NxiVHv8GVYxJA==" /> + <input type="text" name="person[name]" id="person_name" /> + <input type="text" name="contact_detail[phone_number]" id="contact_detail_phone_number" /> </form> ``` diff --git a/guides/source/getting_started.md b/guides/source/getting_started.md index 7c7b3a4c01..70a945ad9e 100644 --- a/guides/source/getting_started.md +++ b/guides/source/getting_started.md @@ -594,7 +594,7 @@ familiar error: You now need to create the `create` action within the `ArticlesController` for this to work. -NOTE: by default `form_with` submits forms using Ajax thereby skipping full page +NOTE: By default `form_with` submits forms using Ajax thereby skipping full page redirects. To make this guide easier to get into we've disabled that with `local: true` for now. diff --git a/guides/source/i18n.md b/guides/source/i18n.md index cb24822f86..0153f52249 100644 --- a/guides/source/i18n.md +++ b/guides/source/i18n.md @@ -1187,7 +1187,7 @@ If you find your own locale (language) missing from our [example translations da Resources --------- -* [Google group: rails-i18n](https://groups.google.com/forum/#!forum/rails-i18n) - The project's mailing list. +* [Google group: rails-i18n](https://groups.google.com/group/rails-i18n) - The project's mailing list. * [GitHub: rails-i18n](https://github.com/svenfuchs/rails-i18n) - Code repository and issue tracker for the rails-i18n project. Most importantly you can find lots of [example translations](https://github.com/svenfuchs/rails-i18n/tree/master/rails/locale) for Rails that should work for your application in most cases. * [GitHub: i18n](https://github.com/svenfuchs/i18n) - Code repository and issue tracker for the i18n gem. diff --git a/guides/source/layouts_and_rendering.md b/guides/source/layouts_and_rendering.md index 76b325d0bf..fe2477f2ae 100644 --- a/guides/source/layouts_and_rendering.md +++ b/guides/source/layouts_and_rendering.md @@ -71,23 +71,25 @@ If we want to display the properties of all the books in our view, we can do so <h1>Listing Books</h1> <table> - <tr> - <th>Title</th> - <th>Summary</th> - <th></th> - <th></th> - <th></th> - </tr> - -<% @books.each do |book| %> - <tr> - <td><%= book.title %></td> - <td><%= book.content %></td> - <td><%= link_to "Show", book %></td> - <td><%= link_to "Edit", edit_book_path(book) %></td> - <td><%= link_to "Remove", book, method: :delete, data: { confirm: "Are you sure?" } %></td> - </tr> -<% end %> + <thead> + <tr> + <th>Title</th> + <th>Content</th> + <th colspan="3"></th> + </tr> + </thead> + + <tbody> + <% @books.each do |book| %> + <tr> + <td><%= book.title %></td> + <td><%= book.content %></td> + <td><%= link_to "Show", book %></td> + <td><%= link_to "Edit", edit_book_path(book) %></td> + <td><%= link_to "Destroy", book, method: :delete, data: { confirm: "Are you sure?" } %></td> + </tr> + <% end %> + </tbody> </table> <br> diff --git a/guides/source/plugins.md b/guides/source/plugins.md index 0f0cde7634..5048444cb2 100644 --- a/guides/source/plugins.md +++ b/guides/source/plugins.md @@ -237,7 +237,7 @@ Finished in 0.004812s, 831.2949 runs/s, 415.6475 assertions/s. This tells us that we don't have the necessary models (Hickwall and Wickwall) that we are trying to test. We can easily generate these models in our "dummy" Rails application by running the following commands from the -test/dummy directory: +`test/dummy` directory: ```bash $ cd test/dummy @@ -359,7 +359,7 @@ When you run `bin/test`, you should see the tests all pass: ### Add an Instance Method -This plugin will add a method named 'squawk' to any Active Record object that calls 'acts_as_yaffle'. The 'squawk' +This plugin will add a method named 'squawk' to any Active Record object that calls `acts_as_yaffle`. The 'squawk' method will simply set the value of one of the fields in the database. To start out, write a failing test that shows the behavior you'd like: @@ -392,7 +392,7 @@ end ``` Run the test to make sure the last two tests fail with an error that contains "NoMethodError: undefined method `squawk'", -then update 'acts_as_yaffle.rb' to look like this: +then update `acts_as_yaffle.rb` to look like this: ```ruby # yaffle/lib/yaffle/acts_as_yaffle.rb diff --git a/guides/source/security.md b/guides/source/security.md index a74de22ac0..a07d583f15 100644 --- a/guides/source/security.md +++ b/guides/source/security.md @@ -85,37 +85,116 @@ This will also be a good idea, if you modify the structure of an object and old * _Critical data should not be stored in session_. If the user clears their cookies or closes the browser, they will be lost. And with a client-side session storage, the user can read the data. -### Session Storage +### Encrypted Session Storage NOTE: _Rails provides several storage mechanisms for the session hashes. The most important is `ActionDispatch::Session::CookieStore`._ -Rails 2 introduced a new default session storage, CookieStore. CookieStore saves the session hash directly in a cookie on the client-side. The server retrieves the session hash from the cookie and eliminates the need for a session ID. That will greatly increase the speed of the application, but it is a controversial storage option and you have to think about the security implications of it: +The `CookieStore` saves the session hash directly in a cookie on the +client-side. The server retrieves the session hash from the cookie and +eliminates the need for a session ID. That will greatly increase the +speed of the application, but it is a controversial storage option and +you have to think about the security implications and storage +limitations of it: + +* Cookies imply a strict size limit of 4kB. This is fine as you should + not store large amounts of data in a session anyway, as described + before. Storing the current user's database id in a session is common + practice. + +* Session cookies do not invalidate themselves and can be maliciously + reused. It may be a good idea to have your application invalidate old + session cookies using a stored timestamp. + +The `CookieStore` uses the +[encrypted](http://api.rubyonrails.org/classes/ActionDispatch/Cookies/ChainedCookieJars.html#method-i-encrypted) +cookie jar to provide a secure, encrypted location to store session +data. Cookie-based sessions thus provide both integrity as well as +confidentiality to their contents. The encryption key, as well as the +verification key used for +[signed](http://api.rubyonrails.org/classes/ActionDispatch/Cookies/ChainedCookieJars.html#method-i-signed) +cookies, is derived from the `secret_key_base` configuration value. + +As of Rails 5.2 encrypted cookies and sessions are protected using AES +GCM encryption. This form of encryption is a type of Authenticated +Encryption and couples authentication and encryption in single step +while also producing shorter ciphertexts as compared to other +algorithms previously used. The key for cookies encrypted with AES GCM +are derived using a salt value defined by the +`config.action_dispatch.authenticated_encrypted_cookie_salt` +configuration value. + +Prior to this version, encrypted cookies were secured using AES in CBC +mode with HMAC using SHA1 for authentication. The keys for this type of +encryption and for HMAC verification were derived via the salts defined +by `config.action_dispatch.encrypted_cookie_salt` and +`config.action_dispatch.encrypted_signed_cookie_salt` respectively. + +Prior to Rails version 4 in both versions 2 and 3, session cookies were +protected using only HMAC verification. As such, these session cookies +only provided integrity to their content because the actual session data +was stored in plaintext encoded as base64. This is how `signed` cookies +work in the current version of Rails. These kinds of cookies are still +useful for protecting the integrity of certain client-stored data and +information. + +__Do not use a trivial secret for the `secret_key_base`, i.e. a word +from a dictionary, or one which is shorter than 30 characters! Instead +use `rails secret` to generate secret keys!__ + +It is also important to use different salt values for encrypted and +signed cookies. Using the same value for different salt configuration +values may lead to the same derived key being used for different +security features which in turn may weaken the strength of the key. -* Cookies imply a strict size limit of 4kB. This is fine as you should not store large amounts of data in a session anyway, as described before. _Storing the current user's database id in a session is usually ok_. +In test and development applications get a `secret_key_base` derived from the app name. Other environments must use a random key present in `config/credentials.yml.enc`, shown here in its decrypted state: -* The client can see everything you store in a session, because it is stored in clear-text (actually Base64-encoded, so not encrypted). So, of course, _you don't want to store any secrets here_. To prevent session hash tampering, a digest is calculated from the session with a server-side secret (`secrets.secret_token`) and inserted into the end of the cookie. + secret_key_base: 492f... -In Rails 4, encrypted cookies through AES in CBC mode with HMAC using SHA1 for -verification was introduced. This prevents the user from accessing and tampering -the content of the cookie. Thus the session becomes a more secure place to store -data. The encryption is performed using a server-side `secret_key_base`. -Two salts are used when deriving keys for encryption and verification. These -salts are set via the `config.action_dispatch.encrypted_cookie_salt` and -`config.action_dispatch.encrypted_signed_cookie_salt` configuration values. +If you have received an application where the secret was exposed (e.g. an application whose source was shared), strongly consider changing the secret. -Rails 5.2 uses AES-GCM for the encryption which couples authentication -and encryption in one faster step and produces shorter ciphertexts. +### Rotating Encrypted and Signed Cookies Configurations -Encrypted cookies are automatically upgraded if the -`config.action_dispatch.use_authenticated_cookie_encryption` is enabled. +Rotation is ideal for changing cookie configurations and ensuring old cookies +aren't immediately invalid. Your users then have a chance to visit your site, +get their cookie read with an old configuration and have it rewritten with the +new change. The rotation can then be removed once you're comfortable enough +users have had their chance to get their cookies upgraded. -_Do not use a trivial secret, i.e. a word from a dictionary, or one which is shorter than 30 characters! Instead use `rails secret` to generate secret keys!_ +It's possible to rotate the ciphers and digests used for encrypted and signed cookies. -In test and development applications get a `secret_key_base` derived from the app name. Other environments must use a random key present in `config/credentials.yml.enc`, shown here in its decrypted state: +For instance to change the digest used for signed cookies from SHA1 to SHA256, +you would first assign the new configuration value: - secret_key_base: 492f... +```ruby +Rails.application.config.action_dispatch.signed_cookie_digest = "SHA256" +``` -If you have received an application where the secret was exposed (e.g. an application whose source was shared), strongly consider changing the secret. +Then you'd set up a rotation with the old configuration to keep it alive. + +```ruby +Rails.application.config.action_dispatch.cookies_rotations.tap do |cookies| + cookies.rotate :signed, digest: "SHA256" +end +``` + +Then any written signed cookies will be digested with SHA256. Old cookies +that were written with SHA1 can still be read, and if accessed will be written +with the new digest so they're upgraded and won't be invalid when you remove the +rotation. + +Once users with SHA1 digested signed cookies should no longer have a chance to +have their cookies rewritten, remove the rotation. + +While you can setup as many rotations as you'd like it's not common to have many +rotations going at any one time. + +For more details on key rotation with encrypted and signed messages as +well as the various options the `rotate` method accepts, please refer to +the +[MessageEncryptor API](api.rubyonrails.org/classes/ActiveSupport/MessageEncryptor.html) +and +[MessageVerifier API](api.rubyonrails.org/classes/ActiveSupport/MessageVerifier.html) +documentation. ### Replay Attacks for CookieStore Sessions @@ -1029,7 +1108,7 @@ Rails generates a `config/credentials.yml.enc` to store third-party credentials within the repo. This is only viable because Rails encrypts the file with a master key that's generated into a version control ignored `config/master.key` — Rails will also look for that key in `ENV["RAILS_MASTER_KEY"]`. Rails also requires the -the key to boot in production, so the credentials can be read. +key to boot in production, so the credentials can be read. To edit stored credentials use `bin/rails credentials:edit`. @@ -1038,18 +1117,18 @@ By default, this file contains the application's access keys for external APIs. The credentials added to this file are accessible via `Rails.application.credentials`. -For example, with the following decrypted `config/credentails.yml.enc`: +For example, with the following decrypted `config/credentials.yml.enc`: secret_key_base: 3b7cd727ee24e8444053437c36cc66c3 some_api_key: SOMEKEY -`Rails.application.credentails.some_api_key` returns `SOMEKEY` in any environment. +`Rails.application.credentials.some_api_key` returns `SOMEKEY` in any environment. If you want an exception to be raised when some key is blank, use the bang version: ```ruby -Rails.application.credentails.some_api_key! # => raises KeyError: key not found: :some_api_key +Rails.application.credentials.some_api_key! # => raises KeyError: :some_api_key is blank ``` Additional Resources diff --git a/guides/source/working_with_javascript_in_rails.md b/guides/source/working_with_javascript_in_rails.md index 27cef2bd27..098366ec1b 100644 --- a/guides/source/working_with_javascript_in_rails.md +++ b/guides/source/working_with_javascript_in_rails.md @@ -382,7 +382,7 @@ Rails 5.1 introduced rails-ujs and dropped jQuery as a dependency. As a result the Unobtrusive JavaScript (UJS) driver has been rewritten to operate without jQuery. These introductions cause small changes to `custom events` fired during the request: -NOTE: Signature of calls to UJS’s event handlers has changed. +NOTE: Signature of calls to UJS's event handlers has changed. Unlike the version with jQuery, all custom events return only one parameter: `event`. In this parameter, there is an additional attribute `detail` which contains an array of extra parameters. diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 5057059898..ff440b7939 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,7 @@ +* Add `mini_magick` to default `Gemfile` as comment. + + *Yoshiyuki Hirano* + * Derive `secret_key_base` from the app name in development and test environments. Spares away needless secret configs. diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index abfec90b6d..24f5eeae87 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -259,8 +259,12 @@ module Rails "action_dispatch.encrypted_cookie_salt" => config.action_dispatch.encrypted_cookie_salt, "action_dispatch.encrypted_signed_cookie_salt" => config.action_dispatch.encrypted_signed_cookie_salt, "action_dispatch.authenticated_encrypted_cookie_salt" => config.action_dispatch.authenticated_encrypted_cookie_salt, + "action_dispatch.use_authenticated_cookie_encryption" => config.action_dispatch.use_authenticated_cookie_encryption, + "action_dispatch.encrypted_cookie_cipher" => config.action_dispatch.encrypted_cookie_cipher, + "action_dispatch.signed_cookie_digest" => config.action_dispatch.signed_cookie_digest, "action_dispatch.cookies_serializer" => config.action_dispatch.cookies_serializer, - "action_dispatch.cookies_digest" => config.action_dispatch.cookies_digest + "action_dispatch.cookies_digest" => config.action_dispatch.cookies_digest, + "action_dispatch.cookies_rotations" => config.action_dispatch.cookies_rotations ) end end diff --git a/railties/lib/rails/commands/runner/runner_command.rb b/railties/lib/rails/commands/runner/runner_command.rb index cd9462e08f..30fbf04982 100644 --- a/railties/lib/rails/commands/runner/runner_command.rb +++ b/railties/lib/rails/commands/runner/runner_command.rb @@ -32,13 +32,13 @@ module Rails ARGV.replace(command_argv) if code_or_file == "-" - eval($stdin.read, binding, "stdin") + eval($stdin.read, TOPLEVEL_BINDING, "stdin") elsif File.exist?(code_or_file) $0 = code_or_file Kernel.load code_or_file else begin - eval(code_or_file, binding, __FILE__, __LINE__) + eval(code_or_file, TOPLEVEL_BINDING, __FILE__, __LINE__) rescue SyntaxError, NameError => error $stderr.puts "Please specify a valid ruby command or the path of a script to run." $stderr.puts "Run '#{self.class.executable} -h' for help." diff --git a/railties/lib/rails/generators/css/scaffold/scaffold_generator.rb b/railties/lib/rails/generators/css/scaffold/scaffold_generator.rb index 5996cb1483..d8eb4f2c7b 100644 --- a/railties/lib/rails/generators/css/scaffold/scaffold_generator.rb +++ b/railties/lib/rails/generators/css/scaffold/scaffold_generator.rb @@ -5,13 +5,13 @@ require_relative "../../named_base" module Css # :nodoc: module Generators # :nodoc: class ScaffoldGenerator < Rails::Generators::NamedBase # :nodoc: + source_root Rails::Generators::ScaffoldGenerator.source_root + # In order to allow the Sass generators to pick up the default Rails CSS and # transform it, we leave it in a standard location for the CSS stylesheet # generators to handle. For the simple, default case, just copy it over. def copy_stylesheet - dir = Rails::Generators::ScaffoldGenerator.source_root - file = File.join(dir, "scaffold.css") - create_file "app/assets/stylesheets/scaffold.css", File.read(file) + copy_file "scaffold.css", "app/assets/stylesheets/scaffold.css" end end end diff --git a/railties/lib/rails/generators/rails/app/app_generator.rb b/railties/lib/rails/generators/rails/app/app_generator.rb index ac82ff6633..23fdf03b05 100644 --- a/railties/lib/rails/generators/rails/app/app_generator.rb +++ b/railties/lib/rails/generators/rails/app/app_generator.rb @@ -69,7 +69,7 @@ module Rails def version_control if !options[:skip_git] && !options[:pretend] - run "git init" + run "git init", capture: options[:quiet] end end @@ -164,7 +164,7 @@ module Rails require_relative "../master_key/master_key_generator" after_bundle do - Rails::Generators::MasterKeyGenerator.new.add_master_key_file + Rails::Generators::MasterKeyGenerator.new([], quiet: options[:quiet]).add_master_key_file end end @@ -174,7 +174,7 @@ module Rails require_relative "../credentials/credentials_generator" after_bundle do - Rails::Generators::CredentialsGenerator.new.add_credentials_file_silently + Rails::Generators::CredentialsGenerator.new([], quiet: options[:quiet]).add_credentials_file_silently end end diff --git a/railties/lib/rails/generators/rails/app/templates/Gemfile b/railties/lib/rails/generators/rails/app/templates/Gemfile index 7b7bebc957..bfbba789b0 100644 --- a/railties/lib/rails/generators/rails/app/templates/Gemfile +++ b/railties/lib/rails/generators/rails/app/templates/Gemfile @@ -21,6 +21,9 @@ ruby <%= "'#{RUBY_VERSION}'" -%> # Use ActiveModel has_secure_password # gem 'bcrypt', '~> 3.1.7' +# Use ActiveStorage variant +# gem 'mini_magick', '~> 4.8' + # Use Capistrano for deployment # gem 'capistrano-rails', group: :development @@ -38,7 +41,7 @@ group :development, :test do gem 'byebug', platforms: [:mri, :mingw, :x64_mingw] <%- if depends_on_system_test? -%> # Adds support for Capybara system testing and selenium driver - gem 'capybara', '~> 2.13' + gem 'capybara', '~> 2.15' gem 'selenium-webdriver' <%- end -%> end diff --git a/railties/lib/rails/generators/rails/app/templates/config/storage.yml b/railties/lib/rails/generators/rails/app/templates/config/storage.yml index 089ed4567a..9bada4b66d 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/storage.yml +++ b/railties/lib/rails/generators/rails/app/templates/config/storage.yml @@ -6,11 +6,11 @@ local: service: Disk root: <%%= Rails.root.join("storage") %> -# Use rails secrets:edit to set the AWS secrets (as shared:aws:access_key_id|secret_access_key) +# Use rails credentials:edit to set the AWS secrets (as aws:access_key_id|secret_access_key) # amazon: # service: S3 -# access_key_id: <%%= Rails.application.secrets.dig(:aws, :access_key_id) %> -# secret_access_key: <%%= Rails.application.secrets.dig(:aws, :secret_access_key) %> +# access_key_id: <%%= Rails.application.credentials.dig(:aws, :access_key_id) %> +# secret_access_key: <%%= Rails.application.credentials.dig(:aws, :secret_access_key) %> # region: us-east-1 # bucket: your_own_bucket @@ -21,12 +21,12 @@ local: # keyfile: <%%= Rails.root.join("path/to/gcs.keyfile") %> # bucket: your_own_bucket -# Use rails secrets:edit to set the Azure Storage secret (as shared:azure_storage:storage_access_key) +# Use rails credentials:edit to set the Azure Storage secret (as azure_storage:storage_access_key) # microsoft: # service: AzureStorage # path: your_azure_storage_path # storage_account_name: your_account_name -# storage_access_key: <%%= Rails.application.secrets.dig(:azure_storage, :storage_access_key) %> +# storage_access_key: <%%= Rails.application.credentials.dig(:azure_storage, :storage_access_key) %> # container: your_container_name # mirror: diff --git a/railties/lib/rails/generators/rails/credentials/credentials_generator.rb b/railties/lib/rails/generators/rails/credentials/credentials_generator.rb index ddcccd5ce5..21ca566818 100644 --- a/railties/lib/rails/generators/rails/credentials/credentials_generator.rb +++ b/railties/lib/rails/generators/rails/credentials/credentials_generator.rb @@ -37,7 +37,7 @@ module Rails private def credentials_template - "# amazon:\n# access_key_id: 123\n# secret_access_key: 345\n\n" + + "# aws:\n# access_key_id: 123\n# secret_access_key: 345\n\n" + "# Used as the base secret for all MessageVerifiers in Rails, including the one protecting cookies.\n" + "secret_key_base: #{SecureRandom.hex(64)}" end diff --git a/railties/lib/rails/generators/rails/master_key/master_key_generator.rb b/railties/lib/rails/generators/rails/master_key/master_key_generator.rb index e49d3b39e0..395687974a 100644 --- a/railties/lib/rails/generators/rails/master_key/master_key_generator.rb +++ b/railties/lib/rails/generators/rails/master_key/master_key_generator.rb @@ -13,15 +13,15 @@ module Rails unless MASTER_KEY_PATH.exist? key = ActiveSupport::EncryptedFile.generate_key - say "Adding #{MASTER_KEY_PATH} to store the master encryption key: #{key}" - say "" - say "Save this in a password manager your team can access." - say "" - say "If you lose the key, no one, including you, can access anything encrypted with it." + log "Adding #{MASTER_KEY_PATH} to store the master encryption key: #{key}" + log "" + log "Save this in a password manager your team can access." + log "" + log "If you lose the key, no one, including you, can access anything encrypted with it." - say "" + log "" create_file MASTER_KEY_PATH, key - say "" + log "" ignore_master_key_file end @@ -31,15 +31,15 @@ module Rails def ignore_master_key_file if File.exist?(".gitignore") unless File.read(".gitignore").include?(key_ignore) - say "Ignoring #{MASTER_KEY_PATH} so it won't end up in Git history:" - say "" + log "Ignoring #{MASTER_KEY_PATH} so it won't end up in Git history:" + log "" append_to_file ".gitignore", key_ignore - say "" + log "" end else - say "IMPORTANT: Don't commit #{MASTER_KEY_PATH}. Add this to your ignore file:" - say key_ignore, :on_green - say "" + log "IMPORTANT: Don't commit #{MASTER_KEY_PATH}. Add this to your ignore file:" + log key_ignore, :on_green + log "" end end diff --git a/railties/lib/rails/generators/rails/resource/USAGE b/railties/lib/rails/generators/rails/resource/USAGE index e359cd574f..66d0ee546a 100644 --- a/railties/lib/rails/generators/rails/resource/USAGE +++ b/railties/lib/rails/generators/rails/resource/USAGE @@ -1,6 +1,6 @@ Description: Stubs out a new resource including an empty model and controller suitable - for a restful, resource-oriented application. Pass the singular model name, + for a RESTful, resource-oriented application. Pass the singular model name, either CamelCased or under_scored, as the first argument, and an optional list of attribute pairs. diff --git a/railties/lib/rails/generators/test_unit/controller/templates/functional_test.rb b/railties/lib/rails/generators/test_unit/controller/templates/functional_test.rb index 4efa977a89..ff41fef9e9 100644 --- a/railties/lib/rails/generators/test_unit/controller/templates/functional_test.rb +++ b/railties/lib/rails/generators/test_unit/controller/templates/functional_test.rb @@ -1,5 +1,3 @@ -# frozen_string_literal: true - require 'test_helper' <% module_namespacing do -%> diff --git a/railties/lib/rails/generators/test_unit/generator/templates/generator_test.rb b/railties/lib/rails/generators/test_unit/generator/templates/generator_test.rb index e6fb6c5ff4..a7f1fc4fba 100644 --- a/railties/lib/rails/generators/test_unit/generator/templates/generator_test.rb +++ b/railties/lib/rails/generators/test_unit/generator/templates/generator_test.rb @@ -1,5 +1,3 @@ -# frozen_string_literal: true - require 'test_helper' require '<%= generator_path %>' diff --git a/railties/lib/rails/generators/test_unit/integration/templates/integration_test.rb b/railties/lib/rails/generators/test_unit/integration/templates/integration_test.rb index 65708b6c3b..118e0f1271 100644 --- a/railties/lib/rails/generators/test_unit/integration/templates/integration_test.rb +++ b/railties/lib/rails/generators/test_unit/integration/templates/integration_test.rb @@ -1,5 +1,3 @@ -# frozen_string_literal: true - require 'test_helper' <% module_namespacing do -%> diff --git a/railties/lib/rails/generators/test_unit/mailer/templates/functional_test.rb b/railties/lib/rails/generators/test_unit/mailer/templates/functional_test.rb index 1ec3a2f360..a2f2d30de5 100644 --- a/railties/lib/rails/generators/test_unit/mailer/templates/functional_test.rb +++ b/railties/lib/rails/generators/test_unit/mailer/templates/functional_test.rb @@ -1,5 +1,3 @@ -# frozen_string_literal: true - require 'test_helper' <% module_namespacing do -%> diff --git a/railties/lib/rails/generators/test_unit/mailer/templates/preview.rb b/railties/lib/rails/generators/test_unit/mailer/templates/preview.rb index 9876210b6c..b063cbc47b 100644 --- a/railties/lib/rails/generators/test_unit/mailer/templates/preview.rb +++ b/railties/lib/rails/generators/test_unit/mailer/templates/preview.rb @@ -1,5 +1,3 @@ -# frozen_string_literal: true - <% module_namespacing do -%> # Preview all emails at http://localhost:3000/rails/mailers/<%= file_path %>_mailer class <%= class_name %>MailerPreview < ActionMailer::Preview diff --git a/railties/lib/rails/generators/test_unit/model/templates/unit_test.rb b/railties/lib/rails/generators/test_unit/model/templates/unit_test.rb index 5f1ffeb33b..c9bc7d5b90 100644 --- a/railties/lib/rails/generators/test_unit/model/templates/unit_test.rb +++ b/railties/lib/rails/generators/test_unit/model/templates/unit_test.rb @@ -1,5 +1,3 @@ -# frozen_string_literal: true - require 'test_helper' <% module_namespacing do -%> diff --git a/railties/lib/rails/generators/test_unit/plugin/templates/test_helper.rb b/railties/lib/rails/generators/test_unit/plugin/templates/test_helper.rb index 2147b09568..30a861f09d 100644 --- a/railties/lib/rails/generators/test_unit/plugin/templates/test_helper.rb +++ b/railties/lib/rails/generators/test_unit/plugin/templates/test_helper.rb @@ -1,4 +1,2 @@ -# frozen_string_literal: true - require 'active_support/testing/autorun' require 'active_support' diff --git a/railties/lib/rails/generators/test_unit/scaffold/templates/api_functional_test.rb b/railties/lib/rails/generators/test_unit/scaffold/templates/api_functional_test.rb index 2ef93b8aea..f21861d8e6 100644 --- a/railties/lib/rails/generators/test_unit/scaffold/templates/api_functional_test.rb +++ b/railties/lib/rails/generators/test_unit/scaffold/templates/api_functional_test.rb @@ -1,5 +1,3 @@ -# frozen_string_literal: true - require 'test_helper' <% module_namespacing do -%> diff --git a/railties/lib/rails/generators/test_unit/scaffold/templates/functional_test.rb b/railties/lib/rails/generators/test_unit/scaffold/templates/functional_test.rb index bcf9392bd1..195d60be20 100644 --- a/railties/lib/rails/generators/test_unit/scaffold/templates/functional_test.rb +++ b/railties/lib/rails/generators/test_unit/scaffold/templates/functional_test.rb @@ -1,5 +1,3 @@ -# frozen_string_literal: true - require 'test_helper' <% module_namespacing do -%> diff --git a/railties/lib/rails/generators/test_unit/scaffold/templates/system_test.rb b/railties/lib/rails/generators/test_unit/scaffold/templates/system_test.rb index ba8bdc192e..f83f5a5c62 100644 --- a/railties/lib/rails/generators/test_unit/scaffold/templates/system_test.rb +++ b/railties/lib/rails/generators/test_unit/scaffold/templates/system_test.rb @@ -1,5 +1,3 @@ -# frozen_string_literal: true - require "application_system_test_case" <% module_namespacing do -%> diff --git a/railties/lib/rails/generators/test_unit/system/templates/application_system_test_case.rb b/railties/lib/rails/generators/test_unit/system/templates/application_system_test_case.rb index c05709aff8..d19212abd5 100644 --- a/railties/lib/rails/generators/test_unit/system/templates/application_system_test_case.rb +++ b/railties/lib/rails/generators/test_unit/system/templates/application_system_test_case.rb @@ -1,5 +1,3 @@ -# frozen_string_literal: true - require "test_helper" class ApplicationSystemTestCase < ActionDispatch::SystemTestCase diff --git a/railties/lib/rails/generators/test_unit/system/templates/system_test.rb b/railties/lib/rails/generators/test_unit/system/templates/system_test.rb index cfac061cd1..b5ce2ba5c8 100644 --- a/railties/lib/rails/generators/test_unit/system/templates/system_test.rb +++ b/railties/lib/rails/generators/test_unit/system/templates/system_test.rb @@ -1,5 +1,3 @@ -# frozen_string_literal: true - require "application_system_test_case" class <%= class_name.pluralize %>Test < ApplicationSystemTestCase diff --git a/railties/test/application/middleware/cookies_test.rb b/railties/test/application/middleware/cookies_test.rb index 23f1ec3e35..ecb4ee3446 100644 --- a/railties/test/application/middleware/cookies_test.rb +++ b/railties/test/application/middleware/cookies_test.rb @@ -1,10 +1,12 @@ # frozen_string_literal: true require "isolation/abstract_unit" +require "rack/test" module ApplicationTests class CookiesTest < ActiveSupport::TestCase include ActiveSupport::Testing::Isolation + include Rack::Test::Methods def new_app File.expand_path("#{app_path}/../new_app") @@ -15,6 +17,10 @@ module ApplicationTests FileUtils.rm_rf("#{app_path}/config/environments") end + def app + Rails.application + end + def teardown teardown_app FileUtils.rm_rf(new_app) if File.directory?(new_app) @@ -44,5 +50,144 @@ module ApplicationTests require "#{app_path}/config/environment" assert_equal false, ActionDispatch::Cookies::CookieJar.always_write_cookie end + + test "signed cookies with SHA512 digest and rotated out SHA256 and SHA1 digests" do + app_file "config/routes.rb", <<-RUBY + Rails.application.routes.draw do + get ':controller(/:action)' + post ':controller(/:action)' + end + RUBY + + controller :foo, <<-RUBY + class FooController < ActionController::Base + protect_from_forgery with: :null_session + + def write_raw_cookie_sha1 + cookies[:signed_cookie] = TestVerifiers.sha1.generate("signed cookie") + head :ok + end + + def write_raw_cookie_sha256 + cookies[:signed_cookie] = TestVerifiers.sha256.generate("signed cookie") + head :ok + end + + def read_signed + render plain: cookies.signed[:signed_cookie].inspect + end + + def read_raw_cookie + render plain: cookies[:signed_cookie] + end + end + RUBY + + add_to_config <<-RUBY + sha1_secret = Rails.application.key_generator.generate_key("sha1") + sha256_secret = Rails.application.key_generator.generate_key("sha256") + + ::TestVerifiers = Class.new do + class_attribute :sha1, default: ActiveSupport::MessageVerifier.new(sha1_secret, digest: "SHA1") + class_attribute :sha256, default: ActiveSupport::MessageVerifier.new(sha256_secret, digest: "SHA256") + end + + config.action_dispatch.signed_cookie_digest = "SHA512" + config.action_dispatch.signed_cookie_salt = "sha512 salt" + + config.action_dispatch.cookies_rotations.tap do |cookies| + cookies.rotate :signed, sha1_secret, digest: "SHA1" + cookies.rotate :signed, sha256_secret, digest: "SHA256" + end + RUBY + + require "#{app_path}/config/environment" + + verifier_sha512 = ActiveSupport::MessageVerifier.new(app.key_generator.generate_key("sha512 salt"), digest: :SHA512) + + get "/foo/write_raw_cookie_sha1" + get "/foo/read_signed" + assert_equal "signed cookie".inspect, last_response.body + + get "/foo/read_raw_cookie" + assert_equal "signed cookie", verifier_sha512.verify(last_response.body) + + get "/foo/write_raw_cookie_sha256" + get "/foo/read_signed" + assert_equal "signed cookie".inspect, last_response.body + + get "/foo/read_raw_cookie" + assert_equal "signed cookie", verifier_sha512.verify(last_response.body) + end + + test "encrypted cookies rotating multiple encryption keys" do + app_file "config/routes.rb", <<-RUBY + Rails.application.routes.draw do + get ':controller(/:action)' + post ':controller(/:action)' + end + RUBY + + controller :foo, <<-RUBY + class FooController < ActionController::Base + protect_from_forgery with: :null_session + + def write_raw_cookie_one + cookies[:encrypted_cookie] = TestEncryptors.first_gcm.encrypt_and_sign("encrypted cookie") + head :ok + end + + def write_raw_cookie_two + cookies[:encrypted_cookie] = TestEncryptors.second_gcm.encrypt_and_sign("encrypted cookie") + head :ok + end + + def read_encrypted + render plain: cookies.encrypted[:encrypted_cookie].inspect + end + + def read_raw_cookie + render plain: cookies[:encrypted_cookie] + end + end + RUBY + + add_to_config <<-RUBY + first_secret = Rails.application.key_generator.generate_key("first", 32) + second_secret = Rails.application.key_generator.generate_key("second", 32) + + ::TestEncryptors = Class.new do + class_attribute :first_gcm, default: ActiveSupport::MessageEncryptor.new(first_secret, cipher: "aes-256-gcm") + class_attribute :second_gcm, default: ActiveSupport::MessageEncryptor.new(second_secret, cipher: "aes-256-gcm") + end + + config.action_dispatch.use_authenticated_cookie_encryption = true + config.action_dispatch.encrypted_cookie_cipher = "aes-256-gcm" + config.action_dispatch.authenticated_encrypted_cookie_salt = "salt" + + config.action_dispatch.cookies_rotations.tap do |cookies| + cookies.rotate :encrypted, first_secret + cookies.rotate :encrypted, second_secret + end + RUBY + + require "#{app_path}/config/environment" + + encryptor = ActiveSupport::MessageEncryptor.new(app.key_generator.generate_key("salt", 32), cipher: "aes-256-gcm") + + get "/foo/write_raw_cookie_one" + get "/foo/read_encrypted" + assert_equal "encrypted cookie".inspect, last_response.body + + get "/foo/read_raw_cookie" + assert_equal "encrypted cookie", encryptor.decrypt_and_verify(last_response.body) + + get "/foo/write_raw_cookie_sha256" + get "/foo/read_encrypted" + assert_equal "encrypted cookie".inspect, last_response.body + + get "/foo/read_raw_cookie" + assert_equal "encrypted cookie", encryptor.decrypt_and_verify(last_response.body) + end end end diff --git a/railties/test/application/rake_test.rb b/railties/test/application/rake_test.rb index f9b14f98cb..76bc0ce1d7 100644 --- a/railties/test/application/rake_test.rb +++ b/railties/test/application/rake_test.rb @@ -133,13 +133,8 @@ module ApplicationTests output = rails("routes") assert_equal <<-MESSAGE.strip_heredoc, output - Prefix Verb URI Pattern Controller#Action - cart GET /cart(.:format) cart#show - rails_service_blob GET /rails/active_storage/blobs/:signed_id/*filename(.:format) active_storage/blobs#show - rails_blob_variation GET /rails/active_storage/variants/:signed_blob_id/:variation_key/*filename(.:format) active_storage/variants#show - rails_disk_service GET /rails/active_storage/disk/:encoded_key/*filename(.:format) active_storage/disk#show - update_rails_disk_service PUT /rails/active_storage/disk/:encoded_token(.:format) active_storage/disk#update - rails_direct_uploads POST /rails/active_storage/direct_uploads(.:format) active_storage/direct_uploads#create + Prefix Verb URI Pattern Controller#Action + cart GET /cart(.:format) cart#show MESSAGE end @@ -174,18 +169,14 @@ module ApplicationTests output = rails("routes", "-g", "show", allow_failure: true) assert_equal <<-MESSAGE.strip_heredoc, output - Prefix Verb URI Pattern Controller#Action - cart GET /cart(.:format) cart#show - rails_service_blob GET /rails/active_storage/blobs/:signed_id/*filename(.:format) active_storage/blobs#show - rails_blob_variation GET /rails/active_storage/variants/:signed_blob_id/:variation_key/*filename(.:format) active_storage/variants#show - rails_disk_service GET /rails/active_storage/disk/:encoded_key/*filename(.:format) active_storage/disk#show + Prefix Verb URI Pattern Controller#Action + cart GET /cart(.:format) cart#show MESSAGE output = rails("routes", "-g", "POST") assert_equal <<-MESSAGE.strip_heredoc, output - Prefix Verb URI Pattern Controller#Action - POST /cart(.:format) cart#create - rails_direct_uploads POST /rails/active_storage/direct_uploads(.:format) active_storage/direct_uploads#create + Prefix Verb URI Pattern Controller#Action + POST /cart(.:format) cart#create MESSAGE output = rails("routes", "-g", "basketballs") @@ -242,12 +233,11 @@ module ApplicationTests RUBY assert_equal <<-MESSAGE.strip_heredoc, rails("routes") - Prefix Verb URI Pattern Controller#Action - rails_service_blob GET /rails/active_storage/blobs/:signed_id/*filename(.:format) active_storage/blobs#show - rails_blob_variation GET /rails/active_storage/variants/:signed_blob_id/:variation_key/*filename(.:format) active_storage/variants#show - rails_disk_service GET /rails/active_storage/disk/:encoded_key/*filename(.:format) active_storage/disk#show - update_rails_disk_service PUT /rails/active_storage/disk/:encoded_token(.:format) active_storage/disk#update - rails_direct_uploads POST /rails/active_storage/direct_uploads(.:format) active_storage/direct_uploads#create + You don't have any routes defined! + + Please add some routes in config/routes.rb. + + For more information about routes, see the Rails guide: http://guides.rubyonrails.org/routing.html. MESSAGE end @@ -261,13 +251,8 @@ module ApplicationTests output = Dir.chdir(app_path) { `bin/rake --rakefile Rakefile routes` } assert_equal <<-MESSAGE.strip_heredoc, output - Prefix Verb URI Pattern Controller#Action - cart GET /cart(.:format) cart#show - rails_service_blob GET /rails/active_storage/blobs/:signed_id/*filename(.:format) active_storage/blobs#show - rails_blob_variation GET /rails/active_storage/variants/:signed_blob_id/:variation_key/*filename(.:format) active_storage/variants#show - rails_disk_service GET /rails/active_storage/disk/:encoded_key/*filename(.:format) active_storage/disk#show - update_rails_disk_service PUT /rails/active_storage/disk/:encoded_token(.:format) active_storage/disk#update - rails_direct_uploads POST /rails/active_storage/direct_uploads(.:format) active_storage/direct_uploads#create + Prefix Verb URI Pattern Controller#Action + cart GET /cart(.:format) cart#show MESSAGE end diff --git a/railties/test/application/runner_test.rb b/railties/test/application/runner_test.rb index 64c46c4b45..aa5d495c97 100644 --- a/railties/test/application/runner_test.rb +++ b/railties/test/application/runner_test.rb @@ -128,5 +128,17 @@ module ApplicationTests assert_match "production", rails("runner", "puts Rails.env") end end + + def test_can_call_same_name_class_as_defined_in_thor + app_file "app/models/task.rb", <<-MODEL + class Task + def self.count + 42 + end + end + MODEL + + assert_match "42", rails("runner", "puts Task.count") + end end end diff --git a/railties/test/commands/credentials_test.rb b/railties/test/commands/credentials_test.rb index fe52c306d2..743fb5f788 100644 --- a/railties/test/commands/credentials_test.rb +++ b/railties/test/commands/credentials_test.rb @@ -12,6 +12,21 @@ class Rails::Command::CredentialsCommandTest < ActiveSupport::TestCase teardown { teardown_app } + test "edit without editor gives hint" do + assert_match "No $EDITOR to open credentials in", run_edit_command(editor: "") + end + + test "edit credentials" do + # Run twice to ensure credentials can be reread after first edit pass. + 2.times do + assert_match(/access_key_id: 123/, run_edit_command) + end + end + + test "show credentials" do + assert_match(/access_key_id: 123/, run_show_command) + end + test "edit command does not add master key to gitignore when already exist" do run_edit_command @@ -27,4 +42,8 @@ class Rails::Command::CredentialsCommandTest < ActiveSupport::TestCase rails "credentials:edit" end end + + def run_show_command + rails "credentials:show" + end end diff --git a/railties/test/fixtures/about_yml_plugins/bad_about_yml/about.yml b/railties/test/fixtures/about_yml_plugins/bad_about_yml/about.yml deleted file mode 100644 index fe80872a16..0000000000 --- a/railties/test/fixtures/about_yml_plugins/bad_about_yml/about.yml +++ /dev/null @@ -1 +0,0 @@ -# an empty YAML file - any content in here seems to get parsed as a string
\ No newline at end of file diff --git a/railties/test/fixtures/about_yml_plugins/bad_about_yml/init.rb b/railties/test/fixtures/about_yml_plugins/bad_about_yml/init.rb deleted file mode 100644 index 1a82a2bdd4..0000000000 --- a/railties/test/fixtures/about_yml_plugins/bad_about_yml/init.rb +++ /dev/null @@ -1,3 +0,0 @@ -# frozen_string_literal: true - -# intentionally empty diff --git a/railties/test/fixtures/about_yml_plugins/plugin_without_about_yml/init.rb b/railties/test/fixtures/about_yml_plugins/plugin_without_about_yml/init.rb deleted file mode 100644 index 1a82a2bdd4..0000000000 --- a/railties/test/fixtures/about_yml_plugins/plugin_without_about_yml/init.rb +++ /dev/null @@ -1,3 +0,0 @@ -# frozen_string_literal: true - -# intentionally empty diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 904e2a5c84..20f593f25c 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -560,6 +560,11 @@ class AppGeneratorTest < Rails::Generators::TestCase assert_no_match(/run git init/, output) end + def test_quiet_option + output = run_generator [File.join(destination_root, "myapp"), "--quiet"] + assert_empty output + end + def test_application_name_with_spaces path = File.join(destination_root, "foo bar") @@ -737,7 +742,7 @@ class AppGeneratorTest < Rails::Generators::TestCase sequence = ["git init", "install", "exec spring binstub --all", "echo ran after_bundle"] @sequence_step ||= 0 - ensure_bundler_first = -> command do + ensure_bundler_first = -> command, options = nil do assert_equal sequence[@sequence_step], command, "commands should be called in sequence #{sequence}" @sequence_step += 1 end diff --git a/tasks/release.rb b/tasks/release.rb index aa8ba44c1a..6ff06f3c4a 100644 --- a/tasks/release.rb +++ b/tasks/release.rb @@ -6,7 +6,6 @@ FRAMEWORK_NAMES = Hash.new { |h, k| k.split(/(?<=active|action)/).map(&:capitali root = File.expand_path("..", __dir__) version = File.read("#{root}/RAILS_VERSION").strip tag = "v#{version}" -gem_version = Gem::Version.new(version) directory "pkg" |