From 8688c86e991daf178769266da2d8178810862a6f Mon Sep 17 00:00:00 2001 From: nick evans Date: Sat, 9 Nov 2024 12:29:08 -0500 Subject: [PATCH 1/4] Add rails 7.2 and 8.0 to the build matrix Also, edge rails requires ruby 3.2 now, so it's dropped from ruby 3.1. --- .github/workflows/tests.yml | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index b5afd34..1055f0d 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -73,7 +73,7 @@ jobs: - ruby: "3.1" rails: ~> 7.1.0 - ruby: "3.1" - rails: edge + rails: ~> 7.2.0 - ruby: "3.2" rails: ~> 6.0.0 @@ -83,6 +83,10 @@ jobs: rails: ~> 7.0.0 - ruby: "3.2" rails: ~> 7.1.0 + - ruby: "3.2" + rails: ~> 7.2.0 + - ruby: "3.2" + rails: ~> 8.0.0 - ruby: "3.2" rails: edge @@ -94,6 +98,10 @@ jobs: rails: ~> 7.0.0 - ruby: "3.3" rails: ~> 7.1.0 + - ruby: "3.3" + rails: ~> 7.2.0 + - ruby: "3.3" + rails: ~> 8.0.0 - ruby: "3.3" rails: edge From 16d5ec623050fe50b4928bc7a20e4c3838349afc Mon Sep 17 00:00:00 2001 From: nick evans Date: Tue, 24 Sep 2024 15:38:25 -0400 Subject: [PATCH 2/4] Inherit config from ActionController::Base By using an inheritable_copy of the config (rather than just redefining the methods) we gain the ability to _override_ the config locally. --- .../rails_csrf_protection/token_verifier.rb | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/omniauth/rails_csrf_protection/token_verifier.rb b/lib/omniauth/rails_csrf_protection/token_verifier.rb index 30adfa5..559709d 100644 --- a/lib/omniauth/rails_csrf_protection/token_verifier.rb +++ b/lib/omniauth/rails_csrf_protection/token_verifier.rb @@ -13,19 +13,20 @@ module RailsCsrfProtection # authenticity token, you can find the source code at # https://github.com/rails/rails/blob/v5.2.2/actionpack/lib/action_controller/metal/request_forgery_protection.rb#L217-L240. class TokenVerifier + # Inherits config from ActionController::Base. + # Define _before_ including ActionController::RequestForgeryProtection. + def self.config + @_config ||= ActionController::Base.config.inheritable_copy + end + include ActiveSupport::Configurable include ActionController::RequestForgeryProtection # `ActionController::RequestForgeryProtection` contains a few # configurable options. As we want to make sure that our configuration is # the same as what being set in `ActionController::Base`, we should make - # all out configuration methods to delegate to `ActionController::Base`. - config.each_key do |configuration_name| - undef_method configuration_name - define_method configuration_name do - ActionController::Base.config[configuration_name] - end - end + # our configuration delegate to `ActionController::Base`. + config.each_key do |key| config.delete(key) end def call(env) dup._call(env) From dcdf698627954db11edd0b00e24ad775d0aa959b Mon Sep 17 00:00:00 2001 From: nick evans Date: Tue, 24 Sep 2024 15:46:09 -0400 Subject: [PATCH 3/4] Use verify_authenticity_token directly By adding a logger and setting the protection_strategy to raise an exception, we can use verify_authenticity_token directly. The main benefit of this is that we will get a more helpful error message attached to the exception. --- .../rails_csrf_protection/token_verifier.rb | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/omniauth/rails_csrf_protection/token_verifier.rb b/lib/omniauth/rails_csrf_protection/token_verifier.rb index 559709d..0732e10 100644 --- a/lib/omniauth/rails_csrf_protection/token_verifier.rb +++ b/lib/omniauth/rails_csrf_protection/token_verifier.rb @@ -28,6 +28,13 @@ def self.config # our configuration delegate to `ActionController::Base`. config.each_key do |key| config.delete(key) end + # OmniAuth expects us to raise an exception on auth failure. + self.forgery_protection_strategy = protection_method_class(:exception) + + # Logging from ActionController::RequestForgeryProtection is redundant. + # OmniAuth logs basically the same message (from the exception). + self.log_warning_on_csrf_failure = false + def call(env) dup._call(env) end @@ -35,15 +42,15 @@ def call(env) def _call(env) @request = ActionDispatch::Request.new(env.dup) - unless verified_request? - raise ActionController::InvalidAuthenticityToken - end + verify_authenticity_token end private attr_reader :request delegate :params, :session, to: :request + + delegate :logger, to: OmniAuth end end end From 68b4ab21f7a2a7089c78a32b26944346100751f7 Mon Sep 17 00:00:00 2001 From: nick evans Date: Tue, 1 Oct 2024 10:00:18 -0400 Subject: [PATCH 4/4] Wrap exception with OmniAuth::AuthenticityError This allows the exception to be handled by the appropriate OmniAuth error handler. The original exception will still be available from the wrapping exceptions's `#cause`, for error reporting and diagnostics. --- lib/omniauth/rails_csrf_protection/token_verifier.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/omniauth/rails_csrf_protection/token_verifier.rb b/lib/omniauth/rails_csrf_protection/token_verifier.rb index 0732e10..f76b699 100644 --- a/lib/omniauth/rails_csrf_protection/token_verifier.rb +++ b/lib/omniauth/rails_csrf_protection/token_verifier.rb @@ -41,8 +41,13 @@ def call(env) def _call(env) @request = ActionDispatch::Request.new(env.dup) - verify_authenticity_token + rescue ActionController::ActionControllerError => ex + logger.warn "Attack prevented by #{self.class}" + # wrapped exception: + # * rescued and handled by OmniAuth::Strategy#request_call + # * contains #cause with original exception + raise OmniAuth::AuthenticityError, "[#{ex.class}] #{ex}" end private