CODEWITHSUNDEEP
ruby-on-railsrubyruby-on-rails-3model-view-controller
user2158382
user2158382

Reputation: 4510

Rails: How to prevent unauthorized access of controller update action

Hi I am new to rails and I am trying to figure out how to prevent unauthorized access to the update action of a controller.

I know I could have a before_filer that kicks out people that arent logged in and I have redirect_to in the edit action, but I want a way to stop a user from editing an object that does not belong to them.

Ex: A authorized user can simply change a job object in my app, by directly sending a PUT request with any job.id as a parameter and change any field they want.

Here is my controller:

  def update
    @job = Job.find(params[:id])


    @job.update_attributes(params[:job])
    redirect_to jobs_path
  end

To try and fix this problem I tried to check in the update action if it the user was authorized and if they werent, i would redirect them to the index page.

  def update
    @job = Job.find(params[:id])

    if @job.user.id != current_login
      redirect_to jobs_path
    end

    @job.update_attributes(params[:job])
    redirect_to jobs_path
  end

But when I try to do this, rails gives me an error saying I can only have one redirect in an action.

Upvotes: 0

Views: 3364

Answers (5)

gregates
gregates

Reputation: 6714

Well, the straightforward fix to your immediate problem is to use flow control to make sure that only one redirect_to is ever reached on a single request, as many others have suggested.

However, that's not really how I'd solve your larger problem.

First, there are a lot of existing solutions for managing authorization, such as cancan or rolify. I'd look into those.

Second, I'd use a before_filter to block access, as you suggest. Something like:

before_filter :load_job, :only => [:show, :edit, :update, :delete]
before_filter :require_authorization, :only => [:edit, :update, :delete]

def load_job
  @job = Job.find(params[:id])
end

def require_authorization
  redirect_to jobs_path unless current_user.can_edit?(@job) # or whatever you want to check
end

The before filters will execute in order, so you'll already have the user & the job available when you check permissions, and can check permissions for that specific job.

Upvotes: 2

Todd A. Jacobs
Todd A. Jacobs

Reputation: 84343

Use an Else Clause

The problem is that the redirect_to method doesn't end the current method; it just tells the controller to set some headers. In order to prevent this problem, you need to make sure that control doesn't "fall through" to the second redirect. One way to do this would be to put your alternative path into an else clause. For example:

def update
  @job = Job.find(params[:id])

  if @job.user.id != current_login
    redirect_to jobs_path
  else
    @job.update_attributes(params[:job])
    redirect_to jobs_path
  end
end

Upvotes: 0

Ayonix
Ayonix

Reputation: 2002

This is probably because after the first redirect the second one could still be executed. Thus putting the update_attributes and the second redirect into the else path like this should solve the problem:

    def update
      @job = Job.find(params[:id])

      if @job.user.id != current_login
        redirect_to jobs_path
      else
        @job.update_attributes(params[:job])
        redirect_to jobs_path
      end
    end

Upvotes: 1

vee
vee

Reputation: 38645

You can either do redirect_to jobs_path and return or return redirect_to jobs_path.

Try the following:

def update
    @job = Job.find(params[:id])

    if @job.user.id != current_login
      redirect_to jobs_path and return
    end

    @job.update_attributes(params[:job])
    redirect_to jobs_path and return
  end

Upvotes: 0

Simon Repp
Simon Repp

Reputation: 548

def update
    @job = Job.find(params[:id])

    @job.update_attributes(params[:job]) unless @job.user.id != current_login

    redirect_to jobs_path
end

:)

Upvotes: 1

Related Questions