Deprecate controller level force_ssl

Today there are two common ways for Rails developers to force their
applications to communicate over HTTPS:

* `config.force_ssl` is a setting in environment configurations that
  enables the `ActionDispatch::SSL` middleware. With this middleware
  enabled, all HTTP communication to your application will be redirected
  to HTTPS. The middleware also takes care of other best practices by
  setting HSTS headers, upgrading all cookies to secure only, etc.
* The `force_ssl` controller method redirects HTTP requests to certain
  controllers to HTTPS.

As a consultant, I've seen many applications with misconfigured HTTPS
setups due to developers adding `force_ssl` to `ApplicationController`
and not enabling `config.force_ssl`. With this configuration, many
application requests can be served over HTTP such as assets, requests
that hit mounted engines, etc. In addition, because cookies are not
upgraded to secure only in this configuration and HSTS headers are not
set, it's possible for cookies that are meant to be secure to be sent
over HTTP.

The confusion between these two methods of forcing HTTPS is compounded
by the fact that they share an identical name. This makes finding
documentation on the "right" method confusing.

HTTPS throughout is quickly becomming table stakes for all web sites.
Sites are expected to operate over HTTPS for all communication,
sensitive or otherwise. Let's encourage use of the broader-reaching
`ActionDispatch::SSL` middleware and elminate this source of user
confusion. If, for some reason, applications need to expose certain
endpoints over HTTP they can do so by properly configuring
`config.ssl_options`.

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"