views:

23

answers:

1

Consensus is you shouldn't nest resources deeper than 1 level. So if I have 3 models like this (below is just a hypothetical situation)

User has_many Houses has_many Tenants

and to abide by the above i do

map.resources :users, :has_many => :houses
map.resorces :houses, :has_many => :tenants

Now I want the user to be able edit both their houses and their tenants details but I want to prevent them from trying to edit another users houses and tenants by forging the user_id part of the urls. So I create a before_filter like this

 def prevent_user_acting_as_other_user
        if User.find_by_id(params[:user_id]) != current_user()
            @current_user_session.destroy
            flash[:error] = "Stop screwing around wiseguy"
            redirect_to login_url()
            return
        end
    end

for houses that's easy because the user_id is passed via

edit_user_house_path(@user, @house)

but in the tenents case

tenant house_tenent_path(@house)

no user id is passed. But I can get the user id by doing @house.user.id but then id have to change the code above to this.

    def prevent_user_acting_as_other_user
    if params[:user_id]
        @user = User.find(params[:user_id]
    elsif params[:house_id]
        @user = House.find(params[:house_id]).user
    end
    if @user != current_user()
        #kick em out
    end
end

It does the job, but I'm wondering if there is a more elegant way. Every time I add a new resource that needs protecting from user forgery Ill have to keep adding conditionals. I don't think there will be many cases but would like to know a better approach if one exists.

+1  A: 

Do the following:

class User < ActiveRecord::Base
  has_many :houses
  has_many :tenants
end

class House < ActiveRecord::Base
  belongs_to :user
  has_many :tenants
end

class Tenant < ActiveRecord::Base
  belongs_to :user
  belongs_to :house
end

In your filter do the following:

def kill_session(message)
  @current_user_session.destroy
  flash[:error] = message
   redirect_to login_url()
end

def prevent_user_acting_as_other_user
  if    params[:user_id]  and params[:user_id] != @current_user.id
    kill_session("You don't have access to this page")
  elsif params[:house_id] and !@current_user.houses.exists?(params[:house_id])
    kill_session("You don't have access to this page")
  elsif params[:tenant_id] and !@current_user.tenants.exists?(params[:tanant_id])
    kill_session("You don't have access to this page")
  end
end
KandadaBoggu
Updated the answer to fix the association.
KandadaBoggu
thank you very much for this and for your other answer to recent question.
adam