CODEWITHSUNDEEP
ruby-on-railssecuritymass-assignment
dpb
dpb

Reputation: 1205

rails: mass-assignment security concern with belongs_to relationships

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:

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.

Upvotes: 5

Views: 1034

Answers (2)

dpb
dpb

Reputation: 1205

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

Upvotes: 0

pjammer
pjammer

Reputation: 9577

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

Upvotes: 5

Related Questions