From 4701a50b586a440d118dce29c9a48bfb306ade5c Mon Sep 17 00:00:00 2001
From: Derek Prior <derekprior@gmail.com>
Date: Sat, 17 Mar 2018 12:04:52 -0400
Subject: 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`.
---
 actionpack/test/controller/api/force_ssl_test.rb |  4 ++-
 actionpack/test/controller/force_ssl_test.rb     | 40 +++++++++++++++---------
 2 files changed, 29 insertions(+), 15 deletions(-)

(limited to 'actionpack/test')

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"
-- 
cgit v1.2.3