Merge pull request #32277 from derekprior/dp-deprecate-force-ssl

Deprecate controller level force_ssl

4 files changed, 48 insertions, 70 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 61451dd673..9835733b17 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,3 +1,8 @@
+*   Controller level `force_ssl` has been deprecated in favor of
+    `config.force_ssl`.
+
+    *Derek Prior*
+
 *   Rails 6 requires Ruby 2.4.1 or newer.
 
     *Jeremy Daer*
diff --git a/actionpack/lib/action_controller/metal/force_ssl.rb b/actionpack/lib/action_controller/metal/force_ssl.rb
index 7de500d119..8d53a30e93 100644
--- a/actionpack/lib/action_controller/metal/force_ssl.rb
+++ b/actionpack/lib/action_controller/metal/force_ssl.rb
@@ -4,18 +4,10 @@ require "active_support/core_ext/hash/except"
 require "active_support/core_ext/hash/slice"
 
 module ActionController
-  # This module provides a method which will redirect the browser to use the secured HTTPS
-  # protocol. This will ensure that users' sensitive information will be
-  # transferred safely over the internet. You _should_ always force the browser
-  # to use HTTPS when you're transferring sensitive information such as
-  # user authentication, account information, or credit card information.
-  #
-  # Note that if you are really concerned about your application security,
-  # you might consider using +config.force_ssl+ in your config file instead.
-  # That will ensure all the data is transferred via HTTPS, and will
-  # prevent the user from getting their session hijacked when accessing the
-  # site over unsecured HTTP protocol.
-  module ForceSSL
+  # This module is deprecated in favor of +config.force_ssl+ in your environment
+  # config file. This will ensure all communication to non-whitelisted endpoints
+  # served by your application occurs over HTTPS.
+  module ForceSSL # :nodoc:
     extend ActiveSupport::Concern
     include AbstractController::Callbacks
 
@@ -23,45 +15,17 @@ module ActionController
     URL_OPTIONS = [:protocol, :host, :domain, :subdomain, :port, :path]
     REDIRECT_OPTIONS = [:status, :flash, :alert, :notice]
 
-    module ClassMethods
-      # Force the request to this particular controller or specified actions to be
-      # through the HTTPS protocol.
-      #
-      # If you need to disable this for any reason (e.g. development) then you can use
-      # an +:if+ or +:unless+ condition.
-      #
-      #     class AccountsController < ApplicationController
-      #       force_ssl if: :ssl_configured?
-      #
-      #       def ssl_configured?
-      #         !Rails.env.development?
-      #       end
-      #     end
-      #
-      # ==== URL Options
-      # You can pass any of the following options to affect the redirect URL
-      # * <tt>host</tt>       - Redirect to a different host name
-      # * <tt>subdomain</tt>  - Redirect to a different subdomain
-      # * <tt>domain</tt>     - Redirect to a different domain
-      # * <tt>port</tt>       - Redirect to a non-standard port
-      # * <tt>path</tt>       - Redirect to a different path
-      #
-      # ==== Redirect Options
-      # You can pass any of the following options to affect the redirect status and response
-      # * <tt>status</tt>     - Redirect with a custom status (default is 301 Moved Permanently)
-      # * <tt>flash</tt>      - Set a flash message when redirecting
-      # * <tt>alert</tt>      - Set an alert message when redirecting
-      # * <tt>notice</tt>     - Set a notice message when redirecting
-      #
-      # ==== Action Options
-      # You can pass any of the following options to affect the before_action callback
-      # * <tt>only</tt>       - The callback should be run only for this action
-      # * <tt>except</tt>     - The callback should be run for all actions except this action
-      # * <tt>if</tt>         - A symbol naming an instance method or a proc; the
-      #   callback will be called only when it returns a true value.
-      # * <tt>unless</tt>     - A symbol naming an instance method or a proc; the
-      #   callback will be called only when it returns a false value.
+    module ClassMethods # :nodoc:
       def force_ssl(options = {})
+        ActiveSupport::Deprecation.warn(<<-MESSAGE.squish)
+          Controller-level `force_ssl` is deprecated and will be removed from
+          Rails 6.1. Please enable `config.force_ssl` in your environment
+          configuration to enable the ActionDispatch::SSL middleware to more
+          fully enforce that your application communicate over HTTPS. If needed,
+          you can use `config.ssl_options` to exempt matching endpoints from
+          being redirected to HTTPS.
+        MESSAGE
+
         action_options = options.slice(*ACTION_OPTIONS)
         redirect_options = options.except(*ACTION_OPTIONS)
         before_action(action_options) do
@@ -70,11 +34,6 @@ module ActionController
       end
     end
 
-    # Redirect the existing request to use the HTTPS protocol.
-    #
-    # ==== Parameters
-    # * <tt>host_or_options</tt> - Either a host name or any of the URL and
-    #   redirect options available to the <tt>force_ssl</tt> method.
     def force_ssl_redirect(host_or_options = nil)
       unless request.ssl?
         options = {
diff --git a/actionpack/test/controller/api/force_ssl_test.rb b/actionpack/test/controller/api/force_ssl_test.rb
index 07459c3753..8191578eb0 100644
--- a/actionpack/test/controller/api/force_ssl_test.rb
+++ b/actionpack/test/controller/api/force_ssl_test.rb
@@ -3,7 +3,9 @@
 require "abstract_unit"
 
 class ForceSSLApiController < ActionController::API
-  force_ssl
+  ActiveSupport::Deprecation.silence do
+    force_ssl
+  end
 
   def one; end
   def two
diff --git a/actionpack/test/controller/force_ssl_test.rb b/actionpack/test/controller/force_ssl_test.rb
index 84ac1fda3c..7f59f6acaf 100644
--- a/actionpack/test/controller/force_ssl_test.rb
+++ b/actionpack/test/controller/force_ssl_test.rb
@@ -13,19 +13,23 @@ class ForceSSLController < ActionController::Base
 end
 
 class ForceSSLControllerLevel < ForceSSLController
-  force_ssl
+  ActiveSupport::Deprecation.silence do
+    force_ssl
+  end
 end
 
 class ForceSSLCustomOptions < ForceSSLController
-  force_ssl host: "secure.example.com", only: :redirect_host
-  force_ssl port: 8443, only: :redirect_port
-  force_ssl subdomain: "secure", only: :redirect_subdomain
-  force_ssl domain: "secure.com", only: :redirect_domain
-  force_ssl path: "/foo", only: :redirect_path
-  force_ssl status: :found, only: :redirect_status
-  force_ssl flash: { message: "Foo, Bar!" }, only: :redirect_flash
-  force_ssl alert: "Foo, Bar!", only: :redirect_alert
-  force_ssl notice: "Foo, Bar!", only: :redirect_notice
+  ActiveSupport::Deprecation.silence do
+    force_ssl host: "secure.example.com", only: :redirect_host
+    force_ssl port: 8443, only: :redirect_port
+    force_ssl subdomain: "secure", only: :redirect_subdomain
+    force_ssl domain: "secure.com", only: :redirect_domain
+    force_ssl path: "/foo", only: :redirect_path
+    force_ssl status: :found, only: :redirect_status
+    force_ssl flash: { message: "Foo, Bar!" }, only: :redirect_flash
+    force_ssl alert: "Foo, Bar!", only: :redirect_alert
+    force_ssl notice: "Foo, Bar!", only: :redirect_notice
+  end
 
   def force_ssl_action
     render plain: action_name
@@ -55,15 +59,21 @@ class ForceSSLCustomOptions < ForceSSLController
 end
 
 class ForceSSLOnlyAction < ForceSSLController
-  force_ssl only: :cheeseburger
+  ActiveSupport::Deprecation.silence do
+    force_ssl only: :cheeseburger
+  end
 end
 
 class ForceSSLExceptAction < ForceSSLController
-  force_ssl except: :banana
+  ActiveSupport::Deprecation.silence do
+    force_ssl except: :banana
+  end
 end
 
 class ForceSSLIfCondition < ForceSSLController
-  force_ssl if: :use_force_ssl?
+  ActiveSupport::Deprecation.silence do
+    force_ssl if: :use_force_ssl?
+  end
 
   def use_force_ssl?
     action_name == "cheeseburger"
@@ -71,7 +81,9 @@ class ForceSSLIfCondition < ForceSSLController
 end
 
 class ForceSSLFlash < ForceSSLController
-  force_ssl except: [:banana, :set_flash, :use_flash]
+  ActiveSupport::Deprecation.silence do
+    force_ssl except: [:banana, :set_flash, :use_flash]
+  end
 
   def set_flash
     flash["that"] = "hello"