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])
精彩评论