Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Use composition to figure out the forgery protection strategy

  • Loading branch information...
commit 765006ded8e8368a85f908748f9c96786ff851a0 1 parent b4051ed
@spastorino spastorino authored
View
36 actionpack/lib/action_controller/metal/request_forgery_protection.rb
@@ -50,6 +50,10 @@ module RequestForgeryProtection
config_accessor :request_forgery_protection_token
self.request_forgery_protection_token ||= :authenticity_token
+ # Holds the class which implements the request forgery protection.
+ config_accessor :forgery_protection_strategy

hi! I've came across this commit by running gems-status software (https://github.com/jordimassaguerpla/gems-status). I assume forgery protection has security implications. Thus, my question is, is this only refactoring or those it has anything to do with new security issues? Should I worry / update? Thanks.

@rafaelfranca Owner

It is only a refactoring.

@spastorino Owner

@jordimassaguerpla basically to allow gems like Devise to properly hook handle_unverified_request method

thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ self.forgery_protection_strategy = nil
+
# Controls whether request forgery protection is turned on or not. Turned off by default only in test mode.
config_accessor :allow_forgery_protection
self.allow_forgery_protection = true if allow_forgery_protection.nil?
@@ -82,14 +86,14 @@ module ClassMethods
# * <tt>:reset_session</tt> - Resets the session.
# * <tt>:null_session</tt> - Provides an empty session during request but doesn't reset it completely. Used as default if <tt>:with</tt> option is not specified.
def protect_from_forgery(options = {})
- include protection_method_module(options[:with] || :null_session)
+ self.forgery_protection_strategy = protection_method_class(options[:with] || :null_session)
self.request_forgery_protection_token ||= :authenticity_token
prepend_before_action :verify_authenticity_token, options
end
private
- def protection_method_module(name)
+ def protection_method_class(name)
ActionController::RequestForgeryProtection::ProtectionMethods.const_get(name.to_s.classify)
rescue NameError
raise ArgumentError, 'Invalid request forgery protection method, use :null_session, :exception, or :reset_session'
@@ -97,17 +101,22 @@ def protection_method_module(name)
end
module ProtectionMethods
- module NullSession
- protected
+ class NullSession
+ def initialize(controller)
+ @controller = controller
+ end
# This is the method that defines the application behavior when a request is found to be unverified.
def handle_unverified_request
+ request = @controller.request
request.session = NullSessionHash.new(request.env)
request.env['action_dispatch.request.flash_hash'] = nil
request.env['rack.session.options'] = { skip: true }
request.env['action_dispatch.cookies'] = NullCookieJar.build(request)
end
+ protected
+
class NullSessionHash < Rack::Session::Abstract::SessionHash #:nodoc:
def initialize(env)
super(nil, env)
@@ -135,16 +144,20 @@ def write(*)
end
end
- module ResetSession
- protected
+ class ResetSession
+ def initialize(controller)
+ @controller = controller
+ end
def handle_unverified_request
- reset_session
+ @controller.reset_session
end
end
- module Exception
- protected
+ class Exception
+ def initialize(controller)
+ @controller = controller
+ end
def handle_unverified_request
raise ActionController::InvalidAuthenticityToken
@@ -153,6 +166,11 @@ def handle_unverified_request
end
protected
+ def handle_unverified_request
+ @forgery_protection ||= forgery_protection_strategy.new(self)
+ @forgery_protection.handle_unverified_request
+ end
+
# The actual before_action that is used. Modify this to change how you handle unverified requests.
def verify_authenticity_token
unless verified_request?
@jordimassaguerpla

hi! I've came across this commit by running gems-status software (https://github.com/jordimassaguerpla/gems-status). I assume forgery protection has security implications. Thus, my question is, is this only refactoring or those it has anything to do with new security issues? Should I worry / update? Thanks.

@spastorino

@jordimassaguerpla basically to allow gems like Devise to properly hook handle_unverified_request method

Please sign in to comment.
Something went wrong with that request. Please try again.