1

In one of my controllers I have this method:

  def method_name
    if current_user
      @model = Model.find(params[:id])
      if @model.destroy
        flash.alert = 'Model deleted successfully'
        redirect_to models_path
      end
    end
  end

I check if there is a current_user assigned by devise before giving the ability for the @model to be deleted. Is this safe and sufficient in terms of security?

What I really do is just checking if current_user exists. So is there a way that somebody can "trick" the system that current_user does exist and as a result be able to trigger the commands included in the method?

1 Answers1

1

I'm not sure about your application, but what you seem to be missing is that the user has some sort of control over the model (i.e: the ability to delete it).

If your application allows users to be created without any intervention/approval, anyone could create a new user (and thus, have a valid current_user), and remove all objects by just iterating through all your id values:

  • /url/1
  • /url/2
  • ...

Again, not sure what your app does, but you may want to put something like:

if current_user
  @model = current_user.models.find(params[:id])
  if @model.destroy
    flash.alert = 'Model deleted successfully'
    redirect_to models_path
  end
end

This will only allow a user to remove his/her own objects, not every object on the system.

ndrix
  • 3,206
  • 13
  • 17