Ok, primary ticket: https://rails.lighthouseapp.com/projects/8994/tickets/2704-allow-calltime-list-of-allowed-attributes-for-mass-assignment
And the bonus round: https://rails.lighthouseapp.com/projects/8994/tickets/2705-deprecate-attr_accessible-attr_protected On May 23, 5:40 pm, cainlevy <[email protected]> wrote: > Which points to an interesting question -- should the model or the > controller be responsible for filtering the attributes? That is, > should the burden be on the model to only assign allowed parameters, > or the controller to only pass allowed parameters? It certainly seems > simple to do it from the controller using something like your > Hash#pick method, but I think it's safer to do it from the model. For > example, if the model is responsible for filtering assignable > attributes, it may create an intelligent default blacklist for cases > where the developer has paid no attention. > > I've just about finished a patch to implement AR::Base#assign > (attributes, allowed_attributes). In the process I've realized that > allowed_attributes can simply be an override to attr_accessible/ > attr_protected, which makes for an easily backwards compatible API > update. So that'll be my first ticket. > > I'd really prefer to remove attr_accessible/attr_protected altogether > as I believe they are in all ways inferior to the new approach and > would only serve to clutter the API in the name of backwards > compatibility. But that's a secondary concern, and will be in a second > ticket that may be evaluated independently. > > On May 23, 4:40 pm, Damian Janowski <[email protected]> wrote: > > > On Fri, May 22, 2009 at 11:42 PM, cainlevy <[email protected]> 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 [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
