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
on 2006-05-23 00:49
on 2006-05-23 01:28
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, email@example.com <firstname.lastname@example.org> wrote: > class Library < AR:Base > I have setup the appropriate pieces and use a before_filter to > find_user > It works just fine if I do: > 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 > email@example.com > http://lists.rubyonrails.org/mailman/listinfo/rails > -- Tom D. http://blog.atomgiant.com http://gifthat.com
on 2006-05-23 01:47
> 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 M.