From 6fef87f17fa6fde7c15668faa43b563eebc0a918 Mon Sep 17 00:00:00 2001 From: blackst0ne Date: Thu, 21 Jun 2018 21:44:31 +1100 Subject: [Rails5] Force the `protect_from_forgery` callback run first Since Rails 5.0 the `protect_from_forgery` callback doesn't run first by default anymore. [1] Instead it gets inserted into callbacks chain where callbacks get called in order. This commit forces the callback to run first. [1]: https://github.com/rails/rails/commit/39794037817703575c35a75f1961b01b83791191 --- app/controllers/application_controller.rb | 2 +- app/controllers/health_controller.rb | 2 +- app/controllers/metrics_controller.rb | 2 +- app/controllers/omniauth_callbacks_controller.rb | 2 +- ...blackst0ne-fix-protect-from-forgery-in-application-controller.yml | 5 +++++ lib/gitlab/request_forgery_protection.rb | 2 +- 6 files changed, 10 insertions(+), 5 deletions(-) create mode 100644 changelogs/unreleased/blackst0ne-fix-protect-from-forgery-in-application-controller.yml diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 56312f801fb..21cc6dfdd16 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -27,7 +27,7 @@ class ApplicationController < ActionController::Base after_action :set_page_title_header, if: -> { request.format == :json } - protect_from_forgery with: :exception + protect_from_forgery with: :exception, prepend: true helper_method :can? helper_method :import_sources_enabled?, :github_import_enabled?, :gitea_import_enabled?, :github_import_configured?, :gitlab_import_enabled?, :gitlab_import_configured?, :bitbucket_import_enabled?, :bitbucket_import_configured?, :google_code_import_enabled?, :fogbugz_import_enabled?, :git_import_enabled?, :gitlab_project_import_enabled? diff --git a/app/controllers/health_controller.rb b/app/controllers/health_controller.rb index 16abf7bab7e..e54f372344d 100644 --- a/app/controllers/health_controller.rb +++ b/app/controllers/health_controller.rb @@ -1,5 +1,5 @@ class HealthController < ActionController::Base - protect_from_forgery with: :exception, except: :storage_check + protect_from_forgery with: :exception, except: :storage_check, prepend: true include RequiresWhitelistedMonitoringClient CHECKS = [ diff --git a/app/controllers/metrics_controller.rb b/app/controllers/metrics_controller.rb index 33b682d2859..0400ffcfee5 100644 --- a/app/controllers/metrics_controller.rb +++ b/app/controllers/metrics_controller.rb @@ -1,7 +1,7 @@ class MetricsController < ActionController::Base include RequiresWhitelistedMonitoringClient - protect_from_forgery with: :exception + protect_from_forgery with: :exception, prepend: true def index response = if Gitlab::Metrics.prometheus_metrics_enabled? diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index 27fd5f7ba37..ba62d2d5142 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -2,7 +2,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController include AuthenticatesWithTwoFactor include Devise::Controllers::Rememberable - protect_from_forgery except: [:kerberos, :saml, :cas3] + protect_from_forgery except: [:kerberos, :saml, :cas3], prepend: true def handle_omniauth omniauth_flow(Gitlab::Auth::OAuth) diff --git a/changelogs/unreleased/blackst0ne-fix-protect-from-forgery-in-application-controller.yml b/changelogs/unreleased/blackst0ne-fix-protect-from-forgery-in-application-controller.yml new file mode 100644 index 00000000000..da75ea8b09e --- /dev/null +++ b/changelogs/unreleased/blackst0ne-fix-protect-from-forgery-in-application-controller.yml @@ -0,0 +1,5 @@ +--- +title: "[Rails5] Force the callback run first" +merge_request: 20055 +author: "@blackst0ne" +type: fixed diff --git a/lib/gitlab/request_forgery_protection.rb b/lib/gitlab/request_forgery_protection.rb index ccfe0d6bed3..a502ad8a541 100644 --- a/lib/gitlab/request_forgery_protection.rb +++ b/lib/gitlab/request_forgery_protection.rb @@ -5,7 +5,7 @@ module Gitlab module RequestForgeryProtection class Controller < ActionController::Base - protect_from_forgery with: :exception + protect_from_forgery with: :exception, prepend: true rescue_from ActionController::InvalidAuthenticityToken do |e| logger.warn "This CSRF token verification failure is handled internally by `GitLab::RequestForgeryProtection`" -- cgit v1.2.1