search

codeforamerica / adopt-a-hydrant

A web application that allows citizens to "adopt" civic infrastructure, such as fire hydrants that need to be shoveled out after it snows.
http://adopt-a-hydrant.herokuapp.com/
BSD 3-Clause "New" or "Revised" License
713 stars 233 forks source link

Can't mass-assign protected attributes: city_id, lng, lat #13

Closed mlevans closed 12 years ago

mlevans commented 12 years ago

I was just installing adopt-a-hydrant locally and came across a mass assignment issue. (Keep in mind that I'm brand new to Rails!)

I ran bundle exec rake db:seed and got the following error: "Can't mass-assign protected attributes: city_id, lng, lat."

So I went into the Thing model (app/models/thing.rb) and changed line 3 to attr_accessible :name, :city_id, :lng, :lat. The seed process now works, and the application is loading the seed data correctly.

Is this a temporary fix or the correct solution?

sferik commented 12 years ago

I enabled attribute whitelisting in 6b2c541f4546f6df34c4f6c63c87778224385f85 as a security measure after a vulnerability in Rails was made public. This patch inadvertently prevented data seeding, which I never noticed, since my local database was seeded long ago (and there are no tests for seeding data). Your workaround will allow data seeding but it should not be committed, lest you open a security hole in Adopt-a-Hydrant. This will be fixed in Rails 4 with strong parameters, which should allow unencumbered seeding of data, since attribute checking will move from the model to the controller.

mlevans commented 12 years ago

Hey Erik,

Thanks for the explanation. So I ran into this same issue when I tried to adopt a hydrant. I used an attr_accessible declaration for the user_id; otherwise the POST request wasn't working. How does the live site get around this issue?

sferik commented 12 years ago

For all I know, it doesn't work in production. I pushed this change in the middle of June, when the site wasn't active for the winter. It may be broken now, but at least it's secure, right? :wink2:

I'll look into this when I get a chance. Maybe I'll add strong parameters, which is available to use in Rails 3.2 apps as a gem. Or maybe you could work on that?

mlevans commented 12 years ago

I'll take a look at strong parameters and update you on my progress.

thorncp commented 12 years ago

FWIW, I just got around the seed data problem by opening up the Thing model in the seeds file, and adding the attr_accessible call there. Here's a commit for reference.

mlevans commented 12 years ago

Closing this after Erik's latest commit: 70049163471eb7d04a035bdbc1689f75f4729826. Thanks for fixing this Erik!