Strong Parameters and Find in Rails 4

Question

I am having trouble with strong parameters in my custom controller. I understand how to use strong parameters in a new or update action. However, I can't seem to figure out if this is an unsecure use of the params[] hash or not in my custom action.

My view redirects to a controller with an order id and an operation number:

link_to 'Confirm', confirmpayment_confirm_path(order: order, operacion: order.operacion), :data => { confirm: 'Are you sure?' }

My confirmpayment controller is as follows:

class ConfirmpaymentController < ApplicationController
  before_action :authenticate_user!

  def lookup
    authorize! :lookup, :confirmpayment
    @orders=Order.where(:status => 'PENDING')
  end

  def confirm
    authorize! :confirm, :confirmpayment
    @order=Order.find(params[:order])
    @order.payment_id = params[:operacion]
    @order.confirm_payment_date = DateTime.now()
    @order.save
  end

  def order_params
    params.require(:order).permit(:order, :operacion) 
  end

end  

The question is:

I am not using order_params anywhere in my confirm action, since this is not a new order. I am using the parameter to find the proper order and confirm it. Is this secure? Or am I missing something?

Answer 1

So to clear a few things up.

Strong Parameters is responsible for the allowed parameters which are passed to your database. It should prevent the users to modify attributes in the database which they aren't allowed to modify.

For example:

You have the following table with columns:

User:
 - firstname
 - lastname
 - email
 - password_digest
 - role (user, admin)

You probably want to prevent normal users to change their role. But if you pass a parameters hash as it is to the database, he could also add a role key with value to the hash. Your application would accept it. Strong parameters checks the hash and prevent the change.

In your example above, Strong Parameters brings no advantages. You assign the values directly to the appropriate table columns. It isn't possible to modify any other attributes in the database for the user. If you don't have any other methods in your controller, you could remove the entire #order_params. Strong Parameters just raises an exception if you would try to pass a hash directly through.

However I would recommend you to search for the payment in the database before you assign it. If you assign payment_id directly you have no guarantee that payment exists. So first check for the payment and if you found it assign it to the order.

Here is how I would have done it:

class PaymentConfirmationController < ApplicationController
  before_action :authenticate_user!
  before_action :authorize_user!   # To DRY up your code

  def lookup
    @orders = Order.where(:status => 'PENDING')
  end

  def confirm
    @order = Order.find(params[:order_id])
    @payment = Payment.find(params[:operation_id])
    # You should catch the exceptions if an order or payment wasn't found

    @order.payment = @payment
    @order.payment_confirmation_date = Time.now()
    @order.save
  end

  private

  def authorize_user!
    authorize! :confirm, :confirmpayment
  end
end

I haven't tested the code but it should work.

Here are the Docs of Strong Parameters. They describe everything in more detail.

I hope this helps!

Happy coding :)