Merge pull request #6343 from Matt-Yorkley/api-controllers

API controllers: strong paramaters

app/controllers/admin/enterprise_fees_controller.rb

@@ -27,7 +27,7 @@ module Admin
     end
 
     def bulk_update
-      @enterprise_fee_set = EnterpriseFeeSet.new(params[:enterprise_fee_set])
+      @enterprise_fee_set = EnterpriseFeeSet.new(enterprise_fee_bulk_params)
 
       if @enterprise_fee_set.save
         redirect_to redirect_path, notice: I18n.t(:enterprise_fees_update_notice)
@@ -78,5 +78,15 @@ module Admin
 
       main_app.admin_enterprise_fees_path
     end
+
+    def enterprise_fee_bulk_params
+      params.require(:enterprise_fee_set).permit(
+        collection_attributes: [
+          :id, :enterprise_id, :fee_type, :name, :tax_category_id,
+          :inherits_tax_category, :calculator_type,
+          { calculator_attributes: PermittedAttributes::Calculator.attributes }
+        ]
+      )
+    end
   end
 end

app/controllers/admin/order_cycles_controller.rb

@@ -223,7 +223,7 @@ module Admin
     end
 
     def order_cycle_set
-      @order_cycle_set ||= OrderCycleSet.new(@order_cycles, params[:order_cycle_set])
+      @order_cycle_set ||= OrderCycleSet.new(@order_cycles, order_cycle_bulk_params)
     end
 
     def require_order_cycle_set_params
@@ -240,5 +240,11 @@ module Admin
     def order_cycle_params
       PermittedAttributes::OrderCycle.new(params).call
     end
+
+    def order_cycle_bulk_params
+      params.require(:order_cycle_set).permit(
+        collection_attributes: [:id] + PermittedAttributes::OrderCycle.basic_attributes
+      )
+    end
   end
 end

app/controllers/api/base_controller.rb

@@ -4,6 +4,7 @@ require "spree/core/controller_helpers/ssl"
 
 module Api
   class BaseController < ActionController::Metal
+    include ActionController::StrongParameters
     include Spree::Api::ControllerSetup
     include Spree::Core::ControllerHelpers::SSL
     include ::ActionController::Head

app/controllers/api/customers_controller.rb

@@ -11,11 +11,15 @@ module Api
       @customer = Customer.find(params[:id])
       authorize! :update, @customer
 
-      if @customer.update(params[:customer])
+      if @customer.update(customer_params)
         render json: @customer, serializer: CustomerSerializer, status: :ok
       else
         invalid_resource!(@customer)
       end
     end
+
+    def customer_params
+      params.require(:customer).permit(:code, :email, :enterprise_id, :allow_charges)
+    end
   end
 end

app/controllers/api/enterprises_controller.rb

@@ -12,7 +12,7 @@ module Api
       # params[:user_ids] breaks the enterprise creation
       # We remove them from params and save them after creating the enterprise
       user_ids = params[:enterprise].delete(:user_ids)
-      @enterprise = Enterprise.new(params[:enterprise])
+      @enterprise = Enterprise.new(enterprise_params)
       if @enterprise.save
         @enterprise.user_ids = user_ids
         render text: @enterprise.id, status: :created
@@ -25,7 +25,7 @@ module Api
       @enterprise = Enterprise.find_by(permalink: params[:id]) || Enterprise.find(params[:id])
       authorize! :update, @enterprise
 
-      if @enterprise.update(params[:enterprise])
+      if @enterprise.update(enterprise_params)
         render text: @enterprise.id, status: :ok
       else
         invalid_resource!(@enterprise)
@@ -69,5 +69,9 @@ module Api
     def override_visible
       params[:enterprise][:visible] = false
     end
+
+    def enterprise_params
+      PermittedAttributes::Enterprise.new(params).call
+    end
   end
 end

app/controllers/api/products_controller.rb

@@ -17,7 +17,7 @@ module Api
     def create
       authorize! :create, Spree::Product
       params[:product][:available_on] ||= Time.zone.now
-      @product = Spree::Product.new(params[:product])
+      @product = Spree::Product.new(product_params)
       begin
         if @product.save
           render json: @product, serializer: Api::Admin::ProductSerializer, status: :created
@@ -33,7 +33,7 @@ module Api
     def update
       authorize! :update, Spree::Product
       @product = find_product(params[:id])
-      if @product.update(params[:product])
+      if @product.update(product_params)
         render json: @product, serializer: Api::Admin::ProductSerializer, status: :ok
       else
         invalid_resource!(@product)
@@ -156,5 +156,9 @@ module Api
         per_page: (params[:per_page] || DEFAULT_PER_PAGE).to_i
       }
     end
+
+    def product_params
+      params.require(:product).permit PermittedAttributes::Product.attributes
+    end
   end
 end

app/controllers/api/shipments_controller.rb

@@ -30,7 +30,7 @@ module Api
         @shipment.adjustment.open
       end
 
-      @shipment.update(params[:shipment])
+      @shipment.update(shipment_params[:shipment])
 
       if unlock == 'yes'
         @shipment.adjustment.close
@@ -88,7 +88,7 @@ module Api
 
     def find_and_update_shipment
       @shipment = @order.shipments.find_by!(number: params[:id])
-      @shipment.update(params[:shipment])
+      @shipment.update(shipment_params[:shipment]) if shipment_params[:shipment].present?
       @shipment.reload
     end
 
@@ -101,5 +101,12 @@ module Api
     def get_or_create_shipment(stock_location_id)
       @order.shipment || @order.shipments.create(stock_location_id: stock_location_id)
     end
+
+    def shipment_params
+      params.permit(
+        [:id, :order_id, :variant_id, :quantity,
+         { shipment: [:tracking, :selected_shipping_rate_id] }]
+      )
+    end
   end
 end

app/controllers/api/taxons_controller.rb

@@ -22,7 +22,7 @@ module Api
 
     def create
       authorize! :create, Spree::Taxon
-      @taxon = Spree::Taxon.new(params[:taxon])
+      @taxon = Spree::Taxon.new(taxon_params)
       @taxon.taxonomy_id = params[:taxonomy_id]
       taxonomy = Spree::Taxonomy.find_by(id: params[:taxonomy_id])
 
@@ -42,7 +42,7 @@ module Api
 
     def update
       authorize! :update, Spree::Taxon
-      if taxon.update(params[:taxon])
+      if taxon.update(taxon_params)
         render json: taxon, serializer: Api::TaxonSerializer, status: :ok
       else
         invalid_resource!(taxon)
@@ -66,5 +66,11 @@ module Api
     def taxon
       @taxon ||= taxonomy.taxons.find(params[:id])
     end
+
+    def taxon_params
+      return if params[:taxon].blank?
+
+      params.require(:taxon).permit([:name, :parent_id])
+    end
   end
 end

app/controllers/api/variants_controller.rb

@@ -17,7 +17,7 @@ module Api
 
     def create
       authorize! :create, Spree::Variant
-      @variant = scope.new(params[:variant])
+      @variant = scope.new(variant_params)
       if @variant.save
         render json: @variant, serializer: Api::VariantSerializer, status: :created
       else
@@ -28,7 +28,7 @@ module Api
     def update
       authorize! :update, Spree::Variant
       @variant = scope.find(params[:id])
-      if @variant.update(params[:variant])
+      if @variant.update(variant_params)
         render json: @variant, serializer: Api::VariantSerializer, status: :ok
       else
         invalid_resource!(@product)
@@ -69,5 +69,9 @@ module Api
       end
       variants
     end
+
+    def variant_params
+      params.require(:variant).permit(PermittedAttributes::Variant.attributes)
+    end
   end
 end

app/controllers/spree/admin/payment_methods_controller.rb

@@ -15,7 +15,7 @@ module Spree
         @payment_method = params[:payment_method].
           delete(:type).
           constantize.
-          new(payment_method_params)
+          new(PermittedAttributes::PaymentMethod.new(params[:payment_method]).call)
         @object = @payment_method
 
         invoke_callbacks(:create, :before)
@@ -92,17 +92,6 @@ module Spree
 
       private
 
-      def payment_method_params
-        params.require(:payment_method).permit(
-          :name, :description, :type, :active,
-          :environment, :display_on, :tag_list,
-          :preferred_enterprise_id, :preferred_server, :preferred_login, :preferred_password,
-          :calculator_type, :preferred_api_key,
-          :preferred_signature, :preferred_solution, :preferred_landing_page, :preferred_logourl,
-          :preferred_test_mode, distributor_ids: []
-        )
-      end
-
       def force_environment
         params[:payment_method][:environment] = Rails.env unless spree_current_user.admin?
       end
@@ -164,7 +153,7 @@ module Spree
       # 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 = params[:payment_method].merge(gateway_params)
 
         params_for_update.each do |key, _value|
           if key.include?("password") && params_for_update[key].blank?
@@ -172,7 +161,7 @@ module Spree
           end
         end
 
-        params_for_update
+        PermittedAttributes::PaymentMethod.new(params_for_update).call
       end
     end
   end

app/controllers/spree/admin/shipping_methods_controller.rb

@@ -86,12 +86,7 @@ module Spree
         params.require(:shipping_method).permit(
           :name, :description, :display_on, :require_ship_address, :tag_list, :calculator_type,
           distributor_ids: [],
-          calculator_attributes: [
-            :id, :preferred_currency, :preferred_amount, :preferred_unit_from_list,
-            :preferred_per_unit, :preferred_flat_percent, :preferred_first_item,
-            :preferred_additional_item, :preferred_max_items, :preferred_minimal_amount,
-            :preferred_normal_amount, :preferred_discount_amount
-          ]
+          calculator_attributes: PermittedAttributes::Calculator.attributes
         )
       end
     end

app/services/permitted_attributes/calculator.rb

@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+module PermittedAttributes
+  class Calculator
+    def self.attributes
+      [
+        :id, :preferred_currency, :preferred_amount, :preferred_flat_percent,
+        :preferred_minimal_amount, :preferred_normal_amount, :preferred_discount_amount,
+        :preferred_unit_from_list, :preferred_per_unit, :preferred_first_item,
+        :preferred_additional_item, :preferred_max_items
+      ]
+    end
+  end
+end

app/services/permitted_attributes/order_cycle.rb

@@ -9,17 +9,24 @@ module PermittedAttributes
     def call
       return @params[:order_cycle] if @params[:order_cycle].blank?
 
-      @params.require(:order_cycle).permit(
+      @params.require(:order_cycle).permit(attributes)
+    end
+
+    def self.basic_attributes
+      [
         :name, :orders_open_at, :orders_close_at, :coordinator_id,
         :preferred_product_selection_from_coordinator_inventory_only,
-        incoming_exchanges: permitted_exchange_attributes,
-        outgoing_exchanges: permitted_exchange_attributes,
         schedule_ids: [], coordinator_fee_ids: []
-      )
+      ]
     end
 
     private
 
+    def attributes
+      self.class.basic_attributes + [incoming_exchanges: permitted_exchange_attributes,
+                                     outgoing_exchanges: permitted_exchange_attributes]
+    end
+
     def permitted_exchange_attributes
       [
         :id, :sender_id, :receiver_id, :enterprise_id, :incoming, :active,

app/services/permitted_attributes/payment_method.rb

@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+module PermittedAttributes
+  class PaymentMethod
+    def initialize(params)
+      @params = params
+    end
+
+    def call
+      @params.permit(
+        [:name, :description, :type, :active,
+         :environment, :display_on, :tag_list,
+         :preferred_enterprise_id, :preferred_server, :preferred_login, :preferred_password,
+         :calculator_type, :preferred_api_key,
+         :preferred_signature, :preferred_solution, :preferred_landing_page, :preferred_logourl,
+         :preferred_test_mode, :calculator_type, { distributor_ids: [] },
+         { calculator_attributes: PermittedAttributes::Calculator.attributes }]
+      )
+    end
+  end
+end