From adb7563ccb003f41ab0da4562379c84e6e73cadf Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Thu, 16 Oct 2025 13:11:41 +1100 Subject: [PATCH 1/4] Fix possible code injection It will fix this security issue : https://github.com/openfoodfoundation/openfoodnetwork/security/code-scanning/247 --- .../spree/admin/payment_methods_controller.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/app/controllers/spree/admin/payment_methods_controller.rb b/app/controllers/spree/admin/payment_methods_controller.rb index 204de6936f..998f48932d 100644 --- a/app/controllers/spree/admin/payment_methods_controller.rb +++ b/app/controllers/spree/admin/payment_methods_controller.rb @@ -89,8 +89,13 @@ module Spree @payment_method = PaymentMethod.find(params[:pm_id]) end else - @payment_method = params[:provider_type].constantize.new + @payment_method = if allowed_payment_methods.include?(params[:provider_type]) + params[:provider_type].constantize.new + else + PaymentMethod.new + end end + render partial: 'provider_settings' end @@ -202,6 +207,10 @@ module Spree def add_type_to_calculator_attributes(hash) hash["calculator_attributes"]["type"] = hash["calculator_type"] end + + def allowed_payment_methods + %w{Spree::PaymentMethod::Check Spree::Gateway::PayPalExpress Spree::Gateway::StripeSCA} + end end end end From 1f0e5417439d5cf39a4a51ac245bd418c36f22f1 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 21 Oct 2025 15:11:10 +1100 Subject: [PATCH 2/4] Update spec description wording --- .../spree/admin/payment_methods_controller_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/controllers/spree/admin/payment_methods_controller_spec.rb b/spec/controllers/spree/admin/payment_methods_controller_spec.rb index e826174e8b..398389bf24 100644 --- a/spec/controllers/spree/admin/payment_methods_controller_spec.rb +++ b/spec/controllers/spree/admin/payment_methods_controller_spec.rb @@ -230,7 +230,7 @@ RSpec.describe Spree::Admin::PaymentMethodsController do end end - context "Requesting provider preference fields" do + describe "#show_provider_preferences" do let(:enterprise) { create(:distributor_enterprise) } let(:user) do new_user = create(:user, email: 'enterprise@hub.com', password: 'blahblah', @@ -244,7 +244,7 @@ RSpec.describe Spree::Admin::PaymentMethodsController do allow(controller).to receive_messages spree_current_user: user end - context "on an existing payment method" do + context "with an existing payment method" do let(:payment_method) { create(:payment_method) } context "where I have permission" do @@ -273,7 +273,7 @@ RSpec.describe Spree::Admin::PaymentMethodsController do end end - context "where I do not have permission" do + context "when I do not have permission" do before do payment_method.distributors = [] end @@ -288,7 +288,7 @@ RSpec.describe Spree::Admin::PaymentMethodsController do end end - context "on a new payment method" do + context "with a new payment method" do it "renders provider settings with a new payment method of type" do spree_get :show_provider_preferences, pm_id: "", From 8d4a1ff32011a325d6f83785daf833c38c7baa83 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 21 Oct 2025 15:11:45 +1100 Subject: [PATCH 3/4] Update spec to cover new code path --- .../spree/admin/payment_methods_controller_spec.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/spec/controllers/spree/admin/payment_methods_controller_spec.rb b/spec/controllers/spree/admin/payment_methods_controller_spec.rb index 398389bf24..7827705283 100644 --- a/spec/controllers/spree/admin/payment_methods_controller_spec.rb +++ b/spec/controllers/spree/admin/payment_methods_controller_spec.rb @@ -296,6 +296,17 @@ RSpec.describe Spree::Admin::PaymentMethodsController do expect(assigns(:payment_method)).to be_a_new Spree::Gateway::PayPalExpress expect(response).to render_template partial: '_provider_settings' end + + context "with a non valid payment method" do + it "renders provider settings with a new generic payment method" do + spree_get :show_provider_preferences, + pm_id: "", + provider_type: "Spree::Gateway::Hacked" + + expect(assigns(:payment_method)).to be_a_new Spree::PaymentMethod + expect(response).to render_template partial: '_provider_settings' + end + end end end end From bd0db57768e1abdaf3442d805d414d4fc9c35be2 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Mon, 3 Nov 2025 15:58:27 +1100 Subject: [PATCH 4/4] Per review, more concise code --- .../spree/admin/payment_methods_controller.rb | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/app/controllers/spree/admin/payment_methods_controller.rb b/app/controllers/spree/admin/payment_methods_controller.rb index 998f48932d..97f77420fd 100644 --- a/app/controllers/spree/admin/payment_methods_controller.rb +++ b/app/controllers/spree/admin/payment_methods_controller.rb @@ -10,6 +10,12 @@ module Spree respond_to :html + PAYMENT_METHODS = %w{ + Spree::PaymentMethod::Check + Spree::Gateway::PayPalExpress + Spree::Gateway::StripeSCA + }.index_with(&:constantize).freeze + def create force_environment @@ -89,11 +95,7 @@ module Spree @payment_method = PaymentMethod.find(params[:pm_id]) end else - @payment_method = if allowed_payment_methods.include?(params[:provider_type]) - params[:provider_type].constantize.new - else - PaymentMethod.new - end + @payment_method = PAYMENT_METHODS.fetch(params[:provider_type], PaymentMethod).new end render partial: 'provider_settings' @@ -207,10 +209,6 @@ module Spree def add_type_to_calculator_attributes(hash) hash["calculator_attributes"]["type"] = hash["calculator_type"] end - - def allowed_payment_methods - %w{Spree::PaymentMethod::Check Spree::Gateway::PayPalExpress Spree::Gateway::StripeSCA} - end end end end