On Fri, May 22, 2009 at 11:42 PM, cainlevy <cainl...@gmail.com> wrote: > 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.
I completely agree with you. Which attributes are allowed to be mass-assigned is almost always context-sensitive, and thus the whitelist doesn't belong in the class level. > 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) Agreed. How about a simple extension to Hash (somebody let me know if this exists already) that allows you to do: user.attributes = params[:user].pick(:first_name, :last_name) (Naive implementation here: http://gist.github.com/116880) The RHS could be extracted to a separate method in the relevant controller, which would do the context checking. Regardless of the syntax I think the use of attr_accessible at the class level should not be encouraged. I'm +1 on this. --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
