How to prevent mass assignments on associations the right way?

Question

Let's say i have a model Warehouse, a model Car, and a model Dealer.

model Car is like:

attr_accessible :make, :year
belongs_to :warehouse
belongs_to :dealer

controller Cars is like:

def create
  car = current_dealer.find(params[:car][:warehouse_id]).cars.new(params[:car])
  car.save!
end

the view of Cars#new is like:

<%= semantic_form_for @car do |f| %>
  <%= f.inputs do %>
    <%= f.input :warehouse, :include_blank => false %>
    <%= f.input :make %>
    <%= f.input :year %>
  <% end %>
<% end %>

Dealers can choose a warehouse when adding a car, The above code is protected against mass assignments (a.k.a. dealers adding cars to warehouses they don't own), But it raises an exception saying :warehouse_id cannot be mass assigned, That's because it's brought with the parameters too as params[:car][:warehouse_id].

How to get rid of that error without manually assigning attributes? And is that a good method anyways?

P.S. i tried params[:car].delete(:warehouse_id) but that doesn't look like the right way to do this.

I'm confused. Do you want a Car's `warehouse` to be mass assignable or not? — varatis, Jul 01 '12 at 19:42
@varatis No, that shouldn't be. Only :make and :year can be mass assigned. — CodeOverload, Jul 01 '12 at 19:43
Can you post the view associated with this create action? It seems like you're trying to assign it from the form, which is antithetical to disallowing mass assignment. — varatis, Jul 01 '12 at 19:47
@varatis, the view is pretty standard formtastic, like: `<%= semantic_form_for @car do |f| %><%= f.inputs do %><%= f.input :warehouse, :include_blank => false %>` ... — CodeOverload, Jul 01 '12 at 19:51
Yeah, see the problem -- you're saying `form_for @car`, and trying to assign one of its attributes (warehouse). That's mass-assignment. — varatis, Jul 01 '12 at 19:54

tybro0103 · Answer 1 · 2012-07-01T20:03:06.410

2

Since :warehouse_id is not a mass-assignable attribute of car, you can't post it from a form as an attribute of car. Rails will raise the mass-assignment error if you name your params in such a manner even if you do nothing with them in the controller.

Rather than doing (non formtastic specific):

<%= f.hidden_field :warehouse_id %>

do:

<%= hidden_field_tag :warehouse_id, @car.warehouse_id %>

I'm not too familiar with formtastic, but I would think the above line should work.

In the controller:

def create
  @car = current_dealer.find(params[:warehouse_id]).cars.new(params[:car])
  @warehouse = Warehouse.find(params[:warehouse_id])
  @car.warehouse = @warehouse
  @car.save!
end

It's a little more tedious, I know. Unfortunately, securing your code requires more effort.

Conclusion: params[:car][:warehouse_id] = mass assignment

edited Jul 01 '12 at 20:03

answered Jul 01 '12 at 19:47

tybro0103

48,327
33
144
170

But how am i supposed to let dealers choose which one of their warehouses they want to add the car to? – CodeOverload Jul 01 '12 at 19:49
Well Formtastic sends `warehouse_id` as `params[:car][:warehouse_id]` so that's the issue. – CodeOverload Jul 01 '12 at 19:53
That is indeed the issue. I'm not too familiar with that particular plugin, but you'll have to change how that variable is passed :( – tybro0103 Jul 01 '12 at 19:55
@tybro0103 is correct. If you need the warehouse selectable, just change the `hidden_field_tag` into a `select_tag` with the appropriate values. The key is that you need a stand-alone tag and can't use your form builder, since that is for the car model. – Thilo Aug 18 '12 at 07:38

score 1 · Answer 2 · answered Aug 18 '12 at 06:37

1

Stop using attr_accessible, use strong_parameters

Remove attr_accessible :make, :year

Add gem 'strong_parameters' to your Gemfile.

Add include ActiveModel::ForbiddenAttributesProtection to your each of your models.

Then, replace:

car = current_dealer.find(params[:car][:warehouse_id]).cars.new(params[:car])

with:

car = current_dealer.find(params[:car][:warehouse_id]).cars.new(params.require(:car).permit([:make, :year]))

answered Aug 18 '12 at 06:37

ronalchn

12,225
10
51
61

Sounds like this will be baked into Rails 4. There's RailsCast on it: http://railscasts.com/episodes/371-strong-parameters I probably wouldn't completely remove attr_accessible just yet, though. – tybro0103 Aug 20 '12 at 15:49

score 0 · Answer 3 · answered Jul 02 '12 at 13:50

Mass Assignment determines only if a field can be set or not through a form (or API), it does not control what can be set. The problem is you are trying to use mass assignment to control validation or restriction of data. In your case you want to be able to save a warehouse_id through a user selected pick list therefore it must be available for mass assignment to use the standard params approach to creating a car.

A suggested solution would be to first use the Formtastic select input to restrict the values that a dealer may select. Although I don't know how you currently restrict warehouses the general version is:

f.input :warehouse, :as => :select, :collection => "Whatever your selection rule is"

This should ensure that only the list of Warehouses that are available to a dealer are actually show in the form.

This doesn't solve the issue a person with enough technical knowledge could change the warehouse_id in the URL and still post to a warehouse that they do not own. So you should also add a validation on the Car model which would ensure that only warehouses belonging to the dealer were valid selections.

This should solve your issue, leave the mass assignment in place for warehouse_id, which you need, while still ensuring that a dealer can only choose a warehouse that belongs to them.

How to prevent mass assignments on associations the right way?

3 Answers3