Merge branch 'main' into button-to-authenticity-token

Author: guilleiguaran

actioncable/actioncable.gemspec

@@ -17,7 +17,7 @@ Gem::Specification.new do |s|
   s.email    = ["[email protected]", "[email protected]"]
   s.homepage = "https://rubyonrails.org"
 
-  s.files        = Dir["CHANGELOG.md", "MIT-LICENSE", "README.md", "lib/**/*", "app/assets/javascripts/action_cable.js"]
+  s.files        = Dir["CHANGELOG.md", "MIT-LICENSE", "README.md", "lib/**/*", "app/assets/javascripts/*.js"]
   s.require_path = "lib"
 
   s.metadata = {

actionmailer/lib/action_mailer/base.rb

@@ -435,6 +435,9 @@ module ActionMailer
   # * <tt>deliveries</tt> - Keeps an array of all the emails sent out through the Action Mailer with
   #   <tt>delivery_method :test</tt>. Most useful for unit and functional testing.
   #
+  # * <tt>delivery_job</tt> - The job class used with <tt>deliver_later</tt>. Defaults to
+  #   +ActionMailer::MailDeliveryJob+.
+  #
   # * <tt>deliver_later_queue_name</tt> - The name of the queue used with <tt>deliver_later</tt>.
   class Base < AbstractController::Base
     include DeliveryMethods

actionpack/CHANGELOG.md

@@ -1,3 +1,29 @@
+*   Raise `ActionController::Redirecting::UnsafeRedirectError` for unsafe `redirect_to` redirects.
+
+    This allows `rescue_from` to be used to add a default fallback route:
+
+    ```ruby
+    rescue_from ActionController::Redirecting::UnsafeRedirectError do
+      redirect_to root_url
+    end
+    ```
+
+    *Kasper Timm Hansen*, *Chris Oliver*
+
+*   Add `url_from` to verify a redirect location is internal.
+
+    Takes the open redirect protection from `redirect_to` so users can wrap a
+    param, and fall back to an alternate redirect URL when the param provided
+    one is unsafe.
+
+    ```ruby
+    def create
+      redirect_to url_from(params[:redirect_url]) || root_url
+    end
+    ```
+
+    *dmcge*, *Kasper Timm Hansen*
+
 *   Allow Capybara driver name overrides in `SystemTestCase::driven_by`
 
     Allow users to prevent conflicts among drivers that use the same driver

actionpack/lib/abstract_controller/base.rb

@@ -148,6 +148,7 @@ def process(action, *args)
 
       @_response_body = nil
 
+      ActiveSupport::ExecutionContext[:controller] = self
       process_action(action_name, *args)
     end
 

actionpack/lib/action_controller.rb

@@ -37,7 +37,6 @@ module ActionController
     autoload :Logging
     autoload :MimeResponds
     autoload :ParamsWrapper
-    autoload :QueryTags
     autoload :Redirecting
     autoload :Renderers
     autoload :Rendering

actionpack/lib/action_controller/metal/query_tags.rb

@@ -7,6 +7,8 @@ module Redirecting
     include AbstractController::Logger
     include ActionController::UrlFor
 
+    class UnsafeRedirectError < StandardError; end
+
     included do
       mattr_accessor :raise_on_open_redirects, default: false
     end
@@ -61,20 +63,34 @@ module Redirecting
     #
     #   redirect_to post_url(@post) and return
     #
-    # Passing user input directly into +redirect_to+ is considered dangerous (e.g. `redirect_to(params[:location])`).
-    # Always use regular expressions or a permitted list when redirecting to a user specified location.
+    # === Open Redirect protection
+    #
+    # By default, Rails protects against redirecting to external hosts for your app's safety, so called open redirects.
+    # Note: this was a new default in Rails 7.0, after upgrading opt-in by uncommenting the line with +raise_on_open_redirects+ in <tt>config/initializers/new_framework_defaults_7_0.rb</tt>
+    #
+    # Here #redirect_to automatically validates the potentially-unsafe URL:
+    #
+    #   redirect_to params[:redirect_url]
+    #
+    # Raises UnsafeRedirectError in the case of an unsafe redirect.
+    #
+    # To allow any external redirects pass `allow_other_host: true`, though using a user-provided param in that case is unsafe.
+    #
+    #   redirect_to "https://rubyonrails.org", allow_other_host: true
+    #
+    # See #url_from for more information on what an internal and safe URL is, or how to fall back to an alternate redirect URL in the unsafe case.
     def redirect_to(options = {}, response_options = {})
-      response_options[:allow_other_host] ||= _allow_other_host unless response_options.key?(:allow_other_host)
-
       raise ActionControllerError.new("Cannot redirect to nil!") unless options
       raise AbstractController::DoubleRenderError if response_body
 
+      allow_other_host = response_options.delete(:allow_other_host) { _allow_other_host }
+
       self.status        = _extract_redirect_to_status(options, response_options)
-      self.location      = _compute_safe_redirect_to_location(request, options, response_options)
+      self.location      = _enforce_open_redirect_protection(_compute_redirect_to_location(request, options), allow_other_host: allow_other_host)
       self.response_body = "<html><body>You are being <a href=\"#{ERB::Util.unwrapped_html_escape(response.location)}\">redirected</a>.</body></html>"
     end
 
-    # Soft deprecated alias for <tt>redirect_back_or_to</tt> where the fallback_location location is supplied as a keyword argument instead
+    # Soft deprecated alias for #redirect_back_or_to where the +fallback_location+ location is supplied as a keyword argument instead
     # of the first positional argument.
     def redirect_back(fallback_location:, allow_other_host: _allow_other_host, **args)
       redirect_back_or_to fallback_location, allow_other_host: allow_other_host, **args
@@ -103,23 +119,11 @@ def redirect_back(fallback_location:, allow_other_host: _allow_other_host, **arg
     # All other options that can be passed to #redirect_to are accepted as
     # options and the behavior is identical.
     def redirect_back_or_to(fallback_location, allow_other_host: _allow_other_host, **options)
-      location = request.referer || fallback_location
-      location = fallback_location unless allow_other_host || _url_host_allowed?(request.referer)
-      allow_other_host = true if _allow_other_host && !allow_other_host # if the fallback is an open redirect
-
-      redirect_to location, allow_other_host: allow_other_host, **options
-    end
-
-    def _compute_safe_redirect_to_location(request, options, response_options)
-      location = _compute_redirect_to_location(request, options)
-
-      if response_options[:allow_other_host] || _url_host_allowed?(location)
-        location
+      if request.referer && (allow_other_host || _url_host_allowed?(request.referer))
+        redirect_to request.referer, allow_other_host: allow_other_host, **options
       else
-        raise(ArgumentError, <<~MSG.squish)
-          Unsafe redirect #{location.truncate(100).inspect},
-          use :allow_other_host to redirect anyway.
-        MSG
+        # The method level `allow_other_host` doesn't apply in the fallback case, omit and let the `redirect_to` handling take over.
+        redirect_to fallback_location, **options
       end
     end
 
@@ -143,6 +147,30 @@ def _compute_redirect_to_location(request, options) # :nodoc:
     module_function :_compute_redirect_to_location
     public :_compute_redirect_to_location
 
+    # Verifies the passed +location+ is an internal URL that's safe to redirect to and returns it, or nil if not.
+    # Useful to wrap a params provided redirect URL and fallback to an alternate URL to redirect to:
+    #
+    #   redirect_to url_from(params[:redirect_url]) || root_url
+    #
+    # The +location+ is considered internal, and safe, if it's on the same host as <tt>request.host</tt>:
+    #
+    #   # If request.host is example.com:
+    #   url_from("https://example.com/profile") # => "https://example.com/profile"
+    #   url_from("http://example.com/profile")  # => "http://example.com/profile"
+    #   url_from("http://evil.com/profile")     # => nil
+    #
+    # Subdomains are considered part of the host:
+    #
+    #   # If request.host is on https://example.com or https://app.example.com, you'd get:
+    #   url_from("https://dev.example.com/profile") # => nil
+    #
+    # NOTE: there's a similarity with {url_for}[rdoc-ref:ActionDispatch::Routing::UrlFor#url_for], which generates an internal URL from various options from within the app, e.g. <tt>url_for(@post)</tt>.
+    # However, #url_from is meant to take an external parameter to verify as in <tt>url_from(params[:redirect_url])</tt>.
+    def url_from(location)
+      location = location.presence
+      location if location && _url_host_allowed?(location)
+    end
+
     private
       def _allow_other_host
         !raise_on_open_redirects
@@ -158,6 +186,14 @@ def _extract_redirect_to_status(options, response_options)
         end
       end
 
+      def _enforce_open_redirect_protection(location, allow_other_host:)
+        if allow_other_host || _url_host_allowed?(location)
+          location
+        else
+          raise UnsafeRedirectError, "Unsafe redirect to #{location.truncate(100).inspect}, pass allow_other_host: true to redirect anyway."
+        end
+      end
+
       def _url_host_allowed?(url)
         URI(url.to_s).host == request.host
       rescue ArgumentError, URI::Error

actionpack/lib/action_controller/metal/redirecting.rb

@@ -113,10 +113,6 @@ class Railtie < Rails::Railtie # :nodoc:
       if query_logs_tags_enabled
         app.config.active_record.query_log_tags += [:controller, :action]
 
-        ActiveSupport.on_load(:action_controller) do
-          include ActionController::QueryTags
-        end
-
         ActiveSupport.on_load(:active_record) do
           ActiveRecord::QueryLogs.taggings.merge!(
             controller:            ->(context) { context[:controller]&.controller_name },

actionpack/lib/action_controller/railtie.rb

@@ -88,6 +88,10 @@ def unsafe_redirect_back
     redirect_back_or_to "http://www.rubyonrails.org/"
   end
 
+  def safe_redirect_with_fallback
+    redirect_to url_from(params[:redirect_url]) || "/fallback"
+  end
+
   def redirect_back_with_explicit_fallback_kwarg
     redirect_back(fallback_location: "/things/stuff", status: 307)
   end
@@ -478,27 +482,41 @@ def test_redirect_to_with_block_and_accepted_options
 
   def test_unsafe_redirect
     with_raise_on_open_redirects do
-      error = assert_raise(ArgumentError) do
+      error = assert_raise(ActionController::Redirecting::UnsafeRedirectError) do
         get :unsafe_redirect
       end
 
-      assert_equal(<<~MSG.squish, error.message)
-        Unsafe redirect \"http://www.rubyonrails.org/\",
-        use :allow_other_host to redirect anyway.
-      MSG
+      assert_equal "Unsafe redirect to \"http://www.rubyonrails.org/\", pass allow_other_host: true to redirect anyway.", error.message
     end
   end
 
   def test_unsafe_redirect_back
     with_raise_on_open_redirects do
-      error = assert_raise(ArgumentError) do
+      error = assert_raise(ActionController::Redirecting::UnsafeRedirectError) do
         get :unsafe_redirect_back
       end
 
-      assert_equal(<<~MSG.squish, error.message)
-        Unsafe redirect \"http://www.rubyonrails.org/\",
-        use :allow_other_host to redirect anyway.
-      MSG
+      assert_equal "Unsafe redirect to \"http://www.rubyonrails.org/\", pass allow_other_host: true to redirect anyway.", error.message
+    end
+  end
+
+  def test_url_from
+    with_raise_on_open_redirects do
+      get :safe_redirect_with_fallback, params: { redirect_url: "http://test.host/app" }
+      assert_response :redirect
+      assert_redirected_to "http://test.host/app"
+    end
+  end
+
+  def test_url_from_fallback
+    with_raise_on_open_redirects do
+      get :safe_redirect_with_fallback, params: { redirect_url: "http://www.rubyonrails.org/" }
+      assert_response :redirect
+      assert_redirected_to "http://test.host/fallback"
+
+      get :safe_redirect_with_fallback, params: { redirect_url: "" }
+      assert_response :redirect
+      assert_redirected_to "http://test.host/fallback"
     end
   end
 

actionpack/test/controller/redirect_test.rb

@@ -13,6 +13,15 @@
 
     *Sean Doyle*
 
+*   Support rendering `<form>` elements _without_ `[action]` attributes by:
+
+    * `form_with url: false` or `form_with ..., html: { action: false }`
+    * `form_for ..., url: false` or `form_for ..., html: { action: false }`
+    * `form_tag false` or `form_tag ..., action: false`
+    * `button_to "...", false` or `button_to(false) { ... }`
+
+    *Sean Doyle*
+
 *   Add `:day_format` option to `date_select`
 
         date_select("article", "written_on", day_format: ->(day) { day.ordinalize })