I like it. Sent from my Verizon Wireless BlackBerry -----Original Message----- From: cainlevy <cainl...@gmail.com>
Date: Fri, 22 May 2009 19:42:03 To: <rubyonrails-core@googlegroups.com> Subject: [Rails-core] redesigning mass attribute assignment (activerecord 3.0?) Hello! I'd like to refactor the code for mass assignment to support a new security paradigm, but figured that I might save some time getting feedback from the gatekeepers of the code. :-) We're all familiar with attr_accessible and attr_protected. These methods provide rudimentary support from malicious mass assignment ... when people remember to use them. I usually remember to set them myself, except that I really wouldn't be surprised if someone auditing my code found one or two spots that I've forgotten. But this isn't about just changing the default to a hardcore blacklist instead of an all-encompassing whitelist. This is about when attr_accessible and attr_protected aren't enough. They're not enough when user roles enter into the picture. Just to take the simplest possible example, let's suppose that an application has normal registered users and it has staff. The staff, very trustable sorts (crossed fingers), are allowed to change attributes that normal users are not. It's a problem that attr_accessible and attr_protected, being very static lists, are not prepared to handle. But let's not stop there. We've all coded spots where we wanted to directly assign a couple of attributes ourselves to trusted values, but we couldn't use mass assignment because we (the devs) have essentially told the code to not even trust ourselves. And yet ... it's such a pretty syntax. Who wants to give that up? I do! Well, just a little. My idea (most likely not original) is to let the controller specify which attributes may be assigned. In its briefest form, my proposal is be to remove attr_accessible/attr_protected and change the mass assignment API as follows: * User.new(params[:user]) becomes User.new(params[:user], whitelist) * @user.attributes = params[:user] becomes @user.assign(params[:user], whitelist) Overall I think that this makes things a whole lot cleaner! Which may or may not be the same thing as fewer LOC. Consider: * The whitelist would be optional, and if absent, would default to most every column. Which makes it usable for normal coding, and doesn't get in the way when you're just getting started. * The whitelist makes it very obvious how the code works. Obvious is better than hidden. I normally don't realize that my attr_accessible isn't configured right until I happen to scan logs and see that "can't mass assign" message. * The whitelist can be a nested hash of any depth, offering excellent support for the nascent nested forms functionality. * The whitelist can be different in a controller that enforces broader security and requires admin users in the first place. * It moves security where it belongs, into the context of a user. Security is business logic, in my mind, and I do like that in my (still-skinny) controllers. * The whitelist can be generated on-demand according to the current user. Which would offer an actual extension point for permission-management plugins and their User.current class/thread variable hacks! * I like the assign() syntax better than the attributes= syntax. But that could be just me. I think that Rails 3.0 would be the right time for an API change like this. It would provide a much better extension point for permission management plugins, and, well, there are no holy cows, right? So ... should I put in the time and code it up? What do you all think? -Lance -- rails blog: http://codelevy.com --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
