Good practice or waste of time?


#1

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


#2

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, removed_email_address@domain.invalid removed_email_address@domain.invalid 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
removed_email_address@domain.invalid
http://lists.rubyonrails.org/mailman/listinfo/rails


Tom D.

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


#3

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.