Merge pull request #29742 from lugray/default_protect_from_forgery

Default protect from forgery

actionpack/CHANGELOG.md

+*   Protect from forgery by default
+
+    Rather than protecting from forgery in the generated ApplicationController,
+    add it to ActionController::Base depending on
+    `config.action_controller.default_protect_from_forgery`. This configuration
+    defaults to false to support older versions which have removed it from their
+    ApplicationController, but is set to true for Rails 5.2.
+
+    *Lisa Ugray*
+
 *   Fallback `ActionController::Parameters#to_s` to `Hash#to_s`.
 
     *Kir Shatrov*

actionpack/lib/action_controller/metal/request_forgery_protection.rb

@@ -85,6 +85,10 @@ module RequestForgeryProtection
       config_accessor :per_form_csrf_tokens
       self.per_form_csrf_tokens = false
 
+      # Controls whether forgery protection is enabled by default.
+      config_accessor :default_protect_from_forgery
+      self.default_protect_from_forgery = false
+
       helper_method :form_authenticity_token
       helper_method :protect_against_forgery?
     end
@@ -128,6 +132,15 @@ def protect_from_forgery(options = {})
         append_after_action :verify_same_origin_request
       end
 
+      # Turn off request forgery protection. This is a wrapper for:
+      #
+      #   skip_before_action :verify_authenticity_token
+      #
+      # See +skip_before_action+ for allowed options.
+      def skip_forgery_protection(options = {})
+        skip_before_action :verify_authenticity_token, options
+      end
+
       private
 
         def protection_method_class(name)

actionpack/lib/action_controller/railtie.rb

@@ -69,5 +69,13 @@ class Railtie < Rails::Railtie #:nodoc:
         config.compile_methods! if config.respond_to?(:compile_methods!)
       end
     end
+
+    initializer "action_controller.request_forgery_protection" do |app|
+      ActiveSupport.on_load(:action_controller_base) do
+        if app.config.action_controller.default_protect_from_forgery
+          protect_from_forgery with: :exception
+        end
+      end
+    end
   end
 end

actionpack/test/controller/request_forgery_protection_test.rb

@@ -163,6 +163,13 @@ def post_two
   end
 end
 
+class SkipProtectionController < ActionController::Base
+  include RequestForgeryProtectionActions
+  protect_from_forgery with: :exception
+  skip_forgery_protection if: :skip_requested
+  attr_accessor :skip_requested
+end
+
 # common test methods
 module RequestForgeryProtectionTests
   def setup
@@ -964,3 +971,26 @@ def assert_matches_session_token_on_server(form_token, method = "post")
       assert_equal expected, actual
     end
 end
+
+class SkipProtectionControllerTest < ActionController::TestCase
+  def test_should_not_allow_post_without_token_when_not_skipping
+    @controller.skip_requested = false
+    assert_blocked { post :index }
+  end
+
+  def test_should_allow_post_without_token_when_skipping
+    @controller.skip_requested = true
+    assert_not_blocked { post :index }
+  end
+
+  def assert_blocked
+    assert_raises(ActionController::InvalidAuthenticityToken) do
+      yield
+    end
+  end
+
+  def assert_not_blocked
+    assert_nothing_raised { yield }
+    assert_response :success
+  end
+end

guides/source/configuring.md

@@ -401,6 +401,8 @@ The schema dumper adds one additional configuration option:
 
 * `config.action_controller.per_form_csrf_tokens` configures whether CSRF tokens are only valid for the method/action they were generated for.
 
+* `config.action_controller.default_protect_from_forgery` determines whether forgery protection is added on `ActionController:Base`.  This is false by default, but enabled when loading defaults for Rails 5.2.
+
 * `config.action_controller.relative_url_root` can be used to tell Rails that you are [deploying to a subdirectory](configuring.html#deploy-to-a-subdirectory-relative-url-root). The default is `ENV['RAILS_RELATIVE_URL_ROOT']`.
 
 * `config.action_controller.permit_all_parameters` sets all the parameters for mass assignment to be permitted by default. The default value is `false`.

railties/lib/rails/application/configuration.rb

@@ -96,6 +96,10 @@ def load_defaults(target_version)
             active_support.use_authenticated_message_encryption = true
           end
 
+          if respond_to?(:action_controller)
+            action_controller.default_protect_from_forgery = true
+          end
+
         else
           raise "Unknown version #{target_version.to_s.inspect}"
         end

railties/lib/rails/generators/rails/app/templates/app/controllers/application_controller.rb.tt

 class ApplicationController < ActionController::<%= options[:api] ? "API" : "Base" %>
-<%- unless options[:api] -%>
-  protect_from_forgery with: :exception
-<%- end -%>
 end

railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_5_2.rb.tt

@@ -17,3 +17,7 @@
 # Use AES-256-GCM authenticated encryption as default cipher for encrypting messages
 # instead of AES-256-CBC, when use_authenticated_message_encryption is set to true.
 # Rails.application.config.active_support.use_authenticated_message_encryption = true
+
+# Add default protection from forgery to ActionController::Base instead of in
+# ApplicationController.
+# Rails.applocation.config.action_controller.default_protect_from_forgery = true

railties/test/application/configuration_test.rb

@@ -1209,6 +1209,13 @@ def create
       assert_equal false, ActionController::Parameters.action_on_unpermitted_parameters
     end
 
+    test "config.action_controller.default_protect_from_forgery is true by default" do
+      app "development"
+
+      assert_equal true, ActionController::Base.default_protect_from_forgery
+      assert_includes ActionController::Base.__callbacks[:process_action].map(&:filter), :verify_authenticity_token
+    end
+
     test "config.action_controller.permit_all_parameters can be configured in an initializer" do
       app_file "config/initializers/permit_all_parameters.rb", <<-RUBY
         Rails.application.config.action_controller.permit_all_parameters = true

railties/test/application/rake_test.rb

@@ -117,7 +117,7 @@ def test_should_not_eager_load_model_for_rake
     end
 
     def test_code_statistics_sanity
-      assert_match "Code LOC: 26     Test LOC: 0     Code to Test Ratio: 1:0.0",
+      assert_match "Code LOC: 25     Test LOC: 0     Code to Test Ratio: 1:0.0",
         Dir.chdir(app_path) { `bin/rails stats` }
     end