views:

149

answers:

2

I've been reading up on rails security concerns and the one that makes me the most concerned is mass assignment. My application is making use of attr_accessible, however I'm not sure if I quite know what the best way to handle the exposed relationships is. Let's assume that we have a basic content creation/ownership website. A user can have create blog posts, and have one category associated with that blog post.

So I have three models:

  • user
  • post: belongs to a user and a category
  • category: belongs to user

I allow mass-assignment on the category_id, so the user could nil it out, change it to one of their categories, or through mass-assignment, I suppose they could change it to someone else's category. That is where I'm kind of unsure about what the best way to proceed would be.

The resources I have investigated (particularly railscast #178 and a resource that was provided from that railscast), both mention that the association should not be mass-assignable, which makes sense. I'm just not sure how else to allow the user to change what the category of the post would be in a railsy way.

Any ideas on how best to solve this? Am I looking at it the wrong way?

UPDATE: Hopefully clarifying my concern a bit more.

Let's say I'm in Post, do I need something like the following:

def create
  @post = Post.new(params[:category])

  @post.user_id = current_user.id

  # CHECK HERE IF REQUESTED CATEGORY_ID IS OWNED BY USER

  # continue on as normal here
end

That seems like a lot of work? I would need to check that on every controller in both the update and create action. Keep in mind that there is more than just one belongs_to relationship.

+2  A: 

Your user can change it through an edit form of some kind, i presume.

Based on that, Mass Assignment is really for nefarious types who seek to mess with your app through things like curl. I call them curl kiddies.

All that to say, if you use attr_protected - (here you put the fields you Do Not want them to change) or the kid's favourite attr_accessible(the fields that are OK to change).

You'll hear arguments for both, but if you use attr_protected :user_id in your model, and then in your CategoryController#create action you can do something like

def create
  @category = Category.new(params[:category])

  @category.user_id = current_user.id
  respond_to do |format|
....#continue on as normal here
end
pjammer
Thanks @pjammer. I'm particularly interested in the example above. What you have written is correct, but I'm more interested in the belongs_to from Post to Category. I already have something like this (though I have it in the model) for the current_user on Post and Category.
dpb
A: 

OK, so searched around a bit, and finally came up with something workable for me. I like keeping logic out of the controllers where possible, so this solution is a model-based solution:

# Post.rb
validates_each :asset_category_id do |record, attr, value|
  self.validates_associated_permission(record, attr, value)
end

# This can obviously be put in a base class/utility class of some sort.
def self.validates_associated_permission(record, attr, value)
  return if value.blank?
  class_string = attr.to_s.gsub(/_id$/, '')
  klass = class_string.camelize.constantize

  # Check here that the associated record is the users
  # I'm leaving this part as pseudo code as everyone's auth code is
  # unique.
  if klass.find_by_id(value).can_write(current_user)
    record.errors.add attr, 'cannot be found.'
  end
end

I also found that rails 3.0 will have a better way to specify this instead of the 3 lines required for the ultra generic validates_each.

http://ryandaigle.com/articles/2009/8/11/what-s-new-in-edge-rails-independent-model-validators

dpb
Other answer was very helpful, but this is what actually solved my problem, thus I'm accepting it.
dpb