From 3b8311230cd8ec208a89c526b03cd04bea1818f1 Mon Sep 17 00:00:00 2001 From: Alberto Vena Date: Wed, 18 Jul 2018 13:33:13 +0200 Subject: [PATCH 1/3] Set dummy app forgery protection to true This is how new (Rails 5.2+) apps are generated by default starting from rails/rails@ec4a836 commit. This change makes api specs fail since json POST requests are now under forgery protection but we have a token based authentication in place that should replace this check. This will be handled in a next commit. We are applying this changes only when rails version is >= 5.2 --- .../spree/dummy/templates/rails/test.rb | 3 --- core/lib/spree/testing_support/dummy_app.rb | 15 +++++++++++++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/core/lib/generators/spree/dummy/templates/rails/test.rb b/core/lib/generators/spree/dummy/templates/rails/test.rb index 5dab451bee8..ab8d005dab6 100644 --- a/core/lib/generators/spree/dummy/templates/rails/test.rb +++ b/core/lib/generators/spree/dummy/templates/rails/test.rb @@ -20,9 +20,6 @@ # Raise exceptions instead of rendering exception templates config.action_dispatch.show_exceptions = false - # Disable request forgery protection in test environment - config.action_controller.allow_forgery_protection = false - # Tell Action Mailer not to deliver emails to the real world. # The :test delivery method accumulates sent emails in the # ActionMailer::Base.deliveries array. diff --git a/core/lib/spree/testing_support/dummy_app.rb b/core/lib/spree/testing_support/dummy_app.rb index 6d5651c7525..ffb4d327935 100644 --- a/core/lib/spree/testing_support/dummy_app.rb +++ b/core/lib/spree/testing_support/dummy_app.rb @@ -12,9 +12,16 @@ require 'solidus_core' +# @private +def forgery_protected_by_default? + Gem::Version.new(Rails.version) >= Gem::Version.new('5.2') +end + # @private class ApplicationController < ActionController::Base - protect_from_forgery with: :exception + if !forgery_protected_by_default? + protect_from_forgery with: :exception + end end # @private @@ -50,14 +57,18 @@ class Application < ::Rails::Application config.public_file_server.headers = { 'Cache-Control' => 'public, max-age=3600' } config.whiny_nils = true config.consider_all_requests_local = true + config.action_controller.allow_forgery_protection = true config.action_controller.perform_caching = false config.action_dispatch.show_exceptions = false config.active_support.deprecation = :stderr config.action_mailer.delivery_method = :test - config.action_controller.allow_forgery_protection = false config.active_support.deprecation = :stderr config.secret_key_base = 'SECRET_TOKEN' + if forgery_protected_by_default? + config.action_controller.default_protect_from_forgery = true + end + if config.active_record.sqlite3 # Rails >= 5.2 config.active_record.sqlite3.represent_boolean_as_integer = true From c78b8c66c515dc86cf5699b14a583af8ba59188f Mon Sep 17 00:00:00 2001 From: Alberto Vena Date: Wed, 18 Jul 2018 17:41:31 +0200 Subject: [PATCH 2/3] Use js in specs that require some javascript rails magic link_to with `method: :put` uses rails-ujs to append the csfr token to the request taking that form a specific tag in the . These specs was only passing since forgery protection was previously disabled in test mode. --- backend/spec/features/admin/orders/payments_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/spec/features/admin/orders/payments_spec.rb b/backend/spec/features/admin/orders/payments_spec.rb index 039afa3d664..5e5a931ab4c 100644 --- a/backend/spec/features/admin/orders/payments_spec.rb +++ b/backend/spec/features/admin/orders/payments_spec.rb @@ -22,7 +22,7 @@ end # Regression tests for https://github.com/spree/spree/issues/1453 - context 'with a check payment' do + context 'with a check payment', js: true do let(:order) { create(:completed_order_with_totals, number: 'R100') } let!(:payment) do create(:payment, @@ -205,7 +205,7 @@ visit spree.admin_order_payments_path(order.reload) end - it "can successfully be created and captured" do + it "can successfully be created and captured", js: true do click_on 'Update' expect(page).to have_content("Payment has been successfully created!") click_icon(:capture) From 6f3f17262c49da898b0d63157a87029f4e1f27aa Mon Sep 17 00:00:00 2001 From: Alberto Vena Date: Wed, 18 Jul 2018 17:45:24 +0200 Subject: [PATCH 3/3] Skip forgery protection in api controllers Rails is now enabling forgery protection by default so we need to explicitly disable it for api requests, as described here: http://api.rubyonrails.org/classes/ActionController/RequestForgeryProtection.html --- api/app/controllers/spree/api/base_controller.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/api/app/controllers/spree/api/base_controller.rb b/api/app/controllers/spree/api/base_controller.rb index 2f1c937ddb2..d390a89c732 100644 --- a/api/app/controllers/spree/api/base_controller.rb +++ b/api/app/controllers/spree/api/base_controller.rb @@ -7,6 +7,7 @@ module Api class BaseController < ActionController::Base self.responder = Spree::Api::Responders::AppResponder respond_to :json + protect_from_forgery unless: -> { request.format.json? } include CanCan::ControllerAdditions include Spree::Core::ControllerHelpers::Store