Good practice for protecting methods

Being a newbie ROR developer I've been thinking of ways of protecting certain methods to make sure the correct user is updating their own content. Here is an example of my approach.

Would you recommend a cleaner way or better way of doing such tasks?

# Example Controller 
class Owner::PropertiesController < Owner::BaseController

  def index
  end

  etc.....

  def update
    @property = Property.find(params[:id])

    # Check correct owner 
    check_owner(:owner_id => @property.owner_id)

    if @property.update_attributes(params[:property])
      redirect_to([:owner, @property], :notice => 'Property was successfully updated.')
    else
      render :action => "edit"
    end

  end

  def destroy
    @property = Property.find(params[:id])

    # Check c开发者_开发问答orrect owner 
    check_owner(:owner_id => @property.owner_id)

    @property.destroy
    redirect_to(owner_properties_url)
  end

  private

  def check_owner p = {}
    if p[:owner_id] != session[:owner_id]
      redirect_to([:owner, @property], :notice => "Property not found.")
    end
  end

It's one way of doing it, albeit a little clunky IMO. I tend to take the following approach in those situations:

class FoosController < ApplicationController
  before_filter :find_user

  def create
    @foo = @user.foos.build
  end

  def update
    @foo = @user.foos.find(params[:id])
  end

  private

  def find_user
    @user = User.find(session[:current_user_id])
  end
end

It's a lot cleaner, and the intent is obvious: you're only interested in trying to find a Foo that belongs to @user. One of the downsides to this approach is that if ownership rules change, there's a bit of work involved to change it, but I've found it serves me reasonably well.

You could use a gem like declarative_authorization to do this as well. If you want to do it yourself I would recommend simply DRYing up your code a little bit:

class Owner::PropertiesController < Owner::BaseController
  before_filter :check_owner, :only => [:update, :destroy]

  def update
    if @property.update_attributes(params[:property])
      redirect_to([:owner, @property], :notice => 'Property was successfully updated.')
    else
      render :action => "edit"
    end
  end

  def destroy
    @property.destroy
    redirect_to(owner_properties_url)
  end

  private

  def check_owner
    @property = Property.find(params[:id]

    if @property.owner_id != session[:owner_id]
      redirect_to([:owner, @property], :notice => "Property not found.") and return
    end
  end
end

Additionally, you can filter your properties by an owner to ensure that a user who is not the owner can not interact with properties that aren't his/hers. For example:

def update
  @owner = Owner.find(session[:owner_id])
  @property = @owner.properties.find(params[:id])
  redirect_to unauthorized_page and return if @property.nil?
end

This forces the properties that you are searching to be the ones that belong to the session[:owner_id] instead of the entire universe of properties. This means that properties that the session[:owner_id] does not own will not even be considered. You can then put this code into a before_filter as well so that it's reusable in multiple actions.

Consider using the technique discussed here.

Add an association within your user model and query properties only through that association.

property = Property.find(params[:id])
# vs
property = current_user.properties.find(params[:id])