For a while, I have felt that the following is the correct way to improve the mass assignment problem without increasing the burden on new users. Now that the problem with the Rails default has been brought up again, it's a good time to revisit it.
When creating a form with form_for
, include a signed token including all of the fields that were created at form creation time. Only these fields are allowed.
To allow new known fields to be added via JS, we could add:
<%= f.allowed_fields "foo", "bar", "baz" %>
The first strategy will not full satisfy apps that support a lot of HTTP requests that do not come from forms generated by Rails.
Because accessible fields is usually a function of authorization, and is not global, we should move allowed fields into the controller. The basic idea is:
class PostsController < ApplicationController
# attributes can be marked accessible for the entire controller
attr_accessible :foo, :bar
def create
# mass assignment can also be done on an instance basis
# this can be used to override the class defaults
attr_accessible(true) if user.admin?
...
end
end
I would imagine that Rails authorization frameworks like CanCan could add sugar to make this even easier in common cases.
The core problem with Rails mass assignment is that attribute protection is an authorization concern. By moving it to the controller, we can have smart defaults (like signed fields in form_for
) and in more advanced cases, make it easier to decide what fields are allowed on a per-user basis.
By moving it into the correct place, we will probably find other nice abstractions that we can use over time to make nice defaults for users without compromising security.
I'm not arguing against putting "magic" in views, although I'd have to say that most existing Rails view helpers can be fairly easily duplicated with Ember views and Handlebars helpers without passing any extra information (like the allowed fields) back and forth in the JSON. What I'm arguing is that we shouldn't be putting "unnecessary magic" in the views when the logic belongs to the controllers and/or the models. The app on the server, whether the logic is in the model or the controller (or both), is more than capable of deciding what is and isn't allowed without some magic hidden form field or extra data being passed back and forth in the JSON.
The code is already there and has been for years, it's just a matter of people actually using it. It should be turned on by default. To use a real-world analogy here, how many carjackings have been prevented since the major auto manufacturers started making new cars do one simple thing that drivers have been unwilling to do on their own for years even though they had to know it was a security concern (locking the doors while the car is in gear)? The power door locks have been there for a long time, it's just a matter of wiring them up so they lock by default.
While mass-assignment is definitely a security issue, it's not the same kind of issue as XSRF where you actually do need some magic value to be passed back and forth (and even then, it's not sent to the client in the JSON, there's a meta tag for that.. but the same solution wouldn't work here).
With Rails 3.1+ scoped attr_accessible, I honestly don't see why the logic even needs to be moved from models to controllers.. with the code as it is now you're more than capable of handling mass-assignment at the authorization level in your controllers while keeping attr_accessible in the model (and keeping it DRY, which I thought was supposed to be a Rails convention)... but even moving it to the controllers makes a lot more sense to me than adding unnecessary magic to the views that does nothing but complicate things for anyone who needs to send data to the server in any way other than through a form generated with the form_for helper. Let's not just think about Javascript frontends here.. what if you want to build an app with an API? Are you going to expect everyone who consumes your API to handle this magic value that's really of no use to them (which means at least one extra API call just to get the magic value before they can submit any data.. which means more load on your server and theirs) when your model or your controller can and should be taking care of it? So what about those APIs? Do we just turn this off, like people typically do with the XSRF protection in that situation? I think turning this off for API calls without some other layer of protection the model and/or controller would be much more dangerous than turning off the XSRF protection is in the same situation.. and again, if the model or the controller can do it already why do we need this magic (and if they can't, then this magic is clearly the wrong answer anyway)?
What I really think needs to be done, and it actually needed to be done a long time ago (how long has it been known now that mass-assignment is a security issue? since before Rails 1?) is to actually enforce whitelisting by default, which a recent Rails commit at least takes a step toward doing even if I don't think they went far enough with it.