From 86d09ff21e606b0db4eaa07ae8d5cd6e4528772d Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 22 Feb 2020 17:41:45 +0000 Subject: [PATCH 1/5] Bring strong parameters code from spree to payment_methods_controller This code comes from spree commit https://github.com/openfoodfoundation/spree/commit/fbc2d150f640399d73baab5295416da7131b95e7 --- .../spree/admin/payment_methods_controller.rb | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/app/controllers/spree/admin/payment_methods_controller.rb b/app/controllers/spree/admin/payment_methods_controller.rb index 4665417bcf..a1303eed0f 100644 --- a/app/controllers/spree/admin/payment_methods_controller.rb +++ b/app/controllers/spree/admin/payment_methods_controller.rb @@ -15,7 +15,7 @@ module Spree @payment_method = params[:payment_method]. delete(:type). constantize. - new(params[:payment_method]) + new(payment_method_params) @object = @payment_method invoke_callbacks(:create, :before) @@ -40,8 +40,8 @@ module Spree @payment_method = PaymentMethod.find(params[:id]) end - payment_method_params = params[ActiveModel::Naming.param_key(@payment_method)] || {} - attributes = params[:payment_method].merge(payment_method_params) + update_params = params[ActiveModel::Naming.param_key(@payment_method)] || {} + attributes = payment_method_params.merge(update_params) attributes.each do |k, _v| if k.include?("password") && attributes[k].blank? attributes.delete(k) @@ -100,6 +100,10 @@ module Spree private + def payment_method_params + params.require(:payment_method).permit! + end + def force_environment params[:payment_method][:environment] = Rails.env unless spree_current_user.admin? end From 38849f55894bc6b4c6faeced397b55b8c8b55030 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 25 Feb 2020 14:48:03 +0000 Subject: [PATCH 2/5] Extract method in payments_method_controller to make it readable --- .../spree/admin/payment_methods_controller.rb | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/app/controllers/spree/admin/payment_methods_controller.rb b/app/controllers/spree/admin/payment_methods_controller.rb index a1303eed0f..c4c317abed 100644 --- a/app/controllers/spree/admin/payment_methods_controller.rb +++ b/app/controllers/spree/admin/payment_methods_controller.rb @@ -40,15 +40,7 @@ module Spree @payment_method = PaymentMethod.find(params[:id]) end - update_params = params[ActiveModel::Naming.param_key(@payment_method)] || {} - attributes = payment_method_params.merge(update_params) - attributes.each do |k, _v| - if k.include?("password") && attributes[k].blank? - attributes.delete(k) - end - end - - if @payment_method.update_attributes(attributes) + if @payment_method.update_attributes(params_for_update) invoke_callbacks(:update, :after) flash[:success] = Spree.t(:successfully_updated, resource: Spree.t(:payment_method)) redirect_to edit_admin_payment_method_path(@payment_method) @@ -160,6 +152,21 @@ module Spree def stripe_provider?(provider) provider.name.ends_with?("StripeConnect", "StripeSCA") end + + # Merge payment method params with gateway params like :gateway_stripe_connect + # Also, remove password if present and blank + def params_for_update + gateway_params = params[ActiveModel::Naming.param_key(@payment_method)] || {} + params_for_update = payment_method_params.merge(gateway_params) + + params_for_update.each do |key, _value| + if key.include?("password") && params_for_update[key].blank? + params_for_update.delete(key) + end + end + + params_for_update + end end end end From e5ebf457656391b8457699032a56c321e2bc4ecb Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 25 Feb 2020 15:05:04 +0000 Subject: [PATCH 3/5] Improve strong params implementation on payment methods controller by specifying specific list of permitted attributes --- app/controllers/spree/admin/payment_methods_controller.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/controllers/spree/admin/payment_methods_controller.rb b/app/controllers/spree/admin/payment_methods_controller.rb index c4c317abed..31efc89b05 100644 --- a/app/controllers/spree/admin/payment_methods_controller.rb +++ b/app/controllers/spree/admin/payment_methods_controller.rb @@ -93,7 +93,10 @@ module Spree private def payment_method_params - params.require(:payment_method).permit! + params.require(:payment_method).permit( + :name, :type, :environment, :preferred_password, :preferred_enterprise_id, + :distributor_ids => [] + ) end def force_environment From eac0da9812de6d00bda71e2bdb7526ebf19f973a Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 25 Feb 2020 15:18:27 +0000 Subject: [PATCH 4/5] Fix payment method controllers by removing unnecessary param that only exists in stripe connect payment method preferred_enterprise_id --- .../spree/admin/payment_methods_controller_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/controllers/spree/admin/payment_methods_controller_spec.rb b/spec/controllers/spree/admin/payment_methods_controller_spec.rb index 9373986e6f..ff23298c44 100644 --- a/spec/controllers/spree/admin/payment_methods_controller_spec.rb +++ b/spec/controllers/spree/admin/payment_methods_controller_spec.rb @@ -8,7 +8,7 @@ module Spree describe Admin::PaymentMethodsController, type: :controller do describe "#create and #update" do let!(:enterprise) { create(:distributor_enterprise, owner: user) } - let(:payment_method) { GatewayWithPassword.create!(name: "Bogus", preferred_password: "haxme", distributor_ids: [enterprise.id], preferred_enterprise_id: enterprise.id) } + let(:payment_method) { GatewayWithPassword.create!(name: "Bogus", preferred_password: "haxme", distributor_ids: [enterprise.id]) } let!(:user) { create(:user) } before { allow(controller).to receive(:spree_current_user) { user } } @@ -32,7 +32,7 @@ module Spree it "can create a payment method of a valid type" do expect { - spree_post :create, payment_method: { name: "Test Method", type: "Spree::Gateway::Bogus", distributor_ids: [enterprise.id], preferred_enterprise_id: enterprise.id } + spree_post :create, payment_method: { name: "Test Method", type: "Spree::Gateway::Bogus", distributor_ids: [enterprise.id] } }.to change(Spree::PaymentMethod, :count).by(1) expect(response).to be_redirect @@ -41,7 +41,7 @@ module Spree it "can not create a payment method of an invalid type" do expect { - spree_post :create, payment_method: { name: "Invalid Payment Method", type: "Spree::InvalidType", distributor_ids: [enterprise.id], preferred_enterprise_id: enterprise.id } + spree_post :create, payment_method: { name: "Invalid Payment Method", type: "Spree::InvalidType", distributor_ids: [enterprise.id] } }.to change(Spree::PaymentMethod, :count).by(0) expect(response).to be_redirect From df799340dfa830bb6d3d899e9c67a9665c8d3013 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 7 Mar 2020 20:30:22 +0000 Subject: [PATCH 5/5] Add missing permitted attributes to payment_methods controller --- app/controllers/spree/admin/payment_methods_controller.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/controllers/spree/admin/payment_methods_controller.rb b/app/controllers/spree/admin/payment_methods_controller.rb index 31efc89b05..a142eb6eb1 100644 --- a/app/controllers/spree/admin/payment_methods_controller.rb +++ b/app/controllers/spree/admin/payment_methods_controller.rb @@ -94,8 +94,12 @@ module Spree def payment_method_params params.require(:payment_method).permit( - :name, :type, :environment, :preferred_password, :preferred_enterprise_id, - :distributor_ids => [] + :name, :description, :type, :active, + :environment, :display_on, :tag_list, + :preferred_enterprise_id, :preferred_server, :preferred_login, :preferred_password, + :calculator_type, + :preferred_signature, :preferred_solution, :preferred_landing_page, :preferred_logourl, + :preferred_test_mode, distributor_ids: [] ) end