undefined | Better HN

0 pointsScriptor14y ago0 comments

The issue seems to be a result of an entire hash of the parameters passed in the request being sent to the new method for models.

Is this really common practice in Rails code? Sure you can specify in the model that certain attributes can't be changed. But shouldn't this stuff be checked when validating form input? Normally I'd have a hash of filters, with the field/column name mapped to the appropriate set of rules for that field. Anything that's not specified in the filters doesn't go to the model's new method.

In this case, it's like the equivalent of PHP code where you santizie the data in $_POST and just send that whole variable to the database.

Choosing which fields are accepted shouldn't just be a model security issue, it's a form validation issue. This makes choosing whether an admin can change a field or not trivial as well. If a request is made as an admin, (maybe through a form that's only accessible by someone with an admin role) then you just apply the validation rules for an admin. Otherwise you apply the rules for a user.

0 comments

cdcarter14y ago

It's common in tutorials and quickstart guides, usually with a note saying that you really should be protecting things and not using mass-assignment. It just makes the blog in 5 minutes videos cleaner.

ScriptorOP14y ago

Reminds me of all the PHP tutorials floating around that only treat security as a side note. Essentially, the default configuration with Rails seems to be that users can set the data for any database column. It's almost as bad as register_globals...

winstonm14y ago

Yes, mass assignment with Model.new(params) and model.attributes=params is a best practice for professional production Rails websites. Business and security rules for field updates are coded in the model (attr_accessible/attr_protected/validates).

That's how it's been since Rails 1. Which is cool. But it's error-prone for newbies, especially when Rails's model and controller generators make all attrs writeable by default, with nary a generated comment about how or why to lock things down. In a culture of convention over configuration, attrs should be locked down by default: "config.active_record.whitelist_attributes=true" for new apps, and throw a helpful message when I mass assign to a model that has no accessible attrs configured yet.

j / k navigate · click thread line to collapse