Turn on mass assignment security by default

[knewter]

So apps using combustion won't have their tests show mass assignment failures by default. This hit me on http://github.com/isotope11/xrono and http://github.com/isotope11/wacky in the past two days. I did a post mortem and figured out why, I'll paste that email in at the bottom for more detail if you want it - it's very stream-of-consciousness, where I'll be more explicit here.

Combustion should add config.active_record.whitelist_attributes = true to the application.rb file so that its dummy rails app acts more like a default stock rails app in that regard. Otherwise all your tests pass, you pull the engine into a new rails app, and none of the create/update calls work.

I'd have sent a pull request in, but I know @parndt is in the middle of extracting combustion-rails, which is where this would need to go. Just wanted to keep a record of it. When you've got that gem ready I'll gladly send in a PR @parndt and @pat, k?

Thanks :) Thoughts?

----- postmortem email ----- So I ran into the exact same problems with the wiki engine I was working on yesterday. I would definitely say, first off, that adding attr_accessible to everything is not the way to go to resolve the issue since Rails4 is going to stop using that as a means of managing mass assignment by default.

I'm confused why combustion isn't bringing in the latest rails. If you look here, you can see that it's very lax with what it requests: https://github.com/pat/combustion/blob/v0.3.1/combustion.gemspec

I looked into the wiki, and it did in fact pull in rails 3.2.9. This suggests that the version of rails isn't the problem. Maybe the combustion app has assignment security turned off. Looking into that, I see this as the application.rb that combustion loads:

https://github.com/pat/combustion/blob/v0.3.1/lib/combustion/application.rb

In that, the whitelist_attributes config option isn't set at all. So now it's down to whatever the rails default is, I guess? I'd have assumed the default was to do the same thing the config set up, but it turns out it's not:

https://github.com/rails/rails/blob/v3.2.9/activerecord/lib/active_record/railtie.rb#L62-L64

If you don't set a config option for that at all, it doesn't run either attr_accessible or attr_protected on ActiveRecord::Base. This is because ActiveModel is where mass assignment security lives, and it doesn't make sense for that to be a default for normal activemodel objects - would be confusing.

So yeah, I think that combustion should add that configuration option. I'll send a pull request in. That explains why it's behaving that way.

[pat]

Hi Josh

Your suggestion sounds spot on to me - much better to have Combustion enforce the stricter approach, and doubly so if that's normally how Rails is configured. I'm not sure when @parndt was going to have a stab at combustion-rails, but if it's just a line or two of code, feel free to submit a pull request sooner rather than later, and we'll get it merged in.

Thanks!

Pat

[parndt]

I'll probably leave combustion-rails to next year ;-)