Rails - May 2006 - good practice or waste of time?

I have what I hope is a simple question regarding a security practice  
I''ve been using in my first Rails app. I want to know if it''s
worthwhile or if the extra typing isn''t worth it.

I have 3 models that are related to each other.

class User < AR:Base
has_one :library
end

class Library < AR:Base
belongs_to :user
has_many :items
end

class Item < AR:Base
belongs_to :library
end

In my library_controller, most of the actions should only be  
available to a logged in user. Using the acts_as_authenticated plugin  
I have setup the appropriate pieces and use a before_filter to  
require the session to be that of a logged in user. So far, so good.

Here''s where things (finally) get interesting. When my controller  
needs to edit an item, I typically do this:

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

def find_library
   find_user
   @library = @user.library
end

def edit_item
   find_library
   @item = @user.library.items.find(params[:id]) <--- necessary?
   # other processing
end

It''s that second line in my #edit_item action that I''m curious
about.
It works just fine if I do:

def edit_item
   @item = Item.find(params[:id])
   # other stuff
end

However, that second method looks insecure to me since I suppose it  
is possible for a logged_in user to get malicious and try  
substituting in some different :id values to see what gets returned.  
The second action let''s that user essentially have access to the  
entire table whereas the first action constrains their possible set  
of choices.

Am I right? (Be gentle, I''m a rails nuby.)

cr

You are correct.  The first way would limit the find to only the items
within that user''s library.  Another way would be to use the
:conditions param in your second example to restrict the query, such
as (untested):

Item.find(params[:id], :conditions => [ "library_id = ?",
@user.library.id ])

Depending on your implementation, you could just keep track of the
"current" library id somewhere such as in the session.

But your first way seems fine.

Tom

On 5/22/06, cremes.devlist@mac.com <cremes.devlist@mac.com>
wrote:
> I have what I hope is a simple question regarding a security practice
> I''ve been using in my first Rails app. I want to know if
it''s
> worthwhile or if the extra typing isn''t worth it.
>
> I have 3 models that are related to each other.
>
> class User < AR:Base
> has_one :library
> end
>
> class Library < AR:Base
> belongs_to :user
> has_many :items
> end
>
> class Item < AR:Base
> belongs_to :library
> end
>
> In my library_controller, most of the actions should only be
> available to a logged in user. Using the acts_as_authenticated plugin
> I have setup the appropriate pieces and use a before_filter to
> require the session to be that of a logged in user. So far, so good.
>
> Here''s where things (finally) get interesting. When my controller
> needs to edit an item, I typically do this:
>
> def find_user
>    @user = User.find_by_login(session[:user])
> end
>
> def find_library
>    find_user
>    @library = @user.library
> end
>
> def edit_item
>    find_library
>    @item = @user.library.items.find(params[:id]) <--- necessary?
>    # other processing
> end
>
> It''s that second line in my #edit_item action that I''m
curious about.
> It works just fine if I do:
>
> def edit_item
>    @item = Item.find(params[:id])
>    # other stuff
> end
>
> However, that second method looks insecure to me since I suppose it
> is possible for a logged_in user to get malicious and try
> substituting in some different :id values to see what gets returned.
> The second action let''s that user essentially have access to the
> entire table whereas the first action constrains their possible set
> of choices.
>
> Am I right? (Be gentle, I''m a rails nuby.)
>
> cr
> _______________________________________________
> Rails mailing list
> Rails@lists.rubyonrails.org
> http://lists.rubyonrails.org/mailman/listinfo/rails
>

-- 
Tom Davies

http://blog.atomgiant.com
http://gifthat.com

> I suppose it
> is possible for a logged_in user to get malicious and try
> substituting in some different :id values to see what gets returned.
I wouldn''t have phrased it quite that way.  It''s not possible,
it''s certain.

Users will attack your system in every way possible, both by design and by
accident.  If you''re exposed to customers who don''t sit down
the hall from
you, assume that they a. despise you and everything you stand for, and b.
are either complete fools or evil geniuses, whichever is more dangerous.
(At least when you''re thinking about security, perhaps not so much when
designing your UI...)

The interface you provide is things like your controllers.  The HTML is
merely a gentle hint about the way you would like things to happen. 
Don''t
ever think that users are constrained by the HTML that you use.

 - James Moore