I implemented this as a plugin. Let me know what you think: http://github.com/ryanb/trusted-params/tree/master
Regards, Ryan On Jun 1, 8:09 am, Ryan Bates <[email protected]> wrote: > Hi Luca, > > Excellent! I agree with the majority of what you propose. The only > thing is I don't think assign_attributes is an adequate solution. Many > times what one needs to assign is in the params hash, and this needs > to happen conditionally. I suppose one can use slice/except like > this... > > def create > @comment = @commentable.comments.build(params[:reply].except > (:spam, :important, :troll)) > @comment.assign_attributes(params[:reply].slice > (:spam, :important, :troll)) if admin? > end > > This has some duplication and seems a bit more convoluted than it > needs to be. I prefer my original proposal of defining if the params > hash is trusted. > > def create > params[:reply].trusted! if admin? > @comment = @commentable.comments.build(params[:reply]) > end > > Much cleaner. But other than that I agree with everything else. > Thanks! > > Ryan > > On Jun 1, 4:23 am, Luca Guidi <[email protected]> wrote: > > > > > Assumptions: > > - attr_accessible/attr_protected aren't used because coders often > > forget about those macros, and also because they are annoying to work > > with. The current API doesn't encourage them. > > - attr_protected is dangerous (rails-spikes.com) > > - Protection should be at class level > > - Controllers should decide which attributes should be passed > > - The API should be pessimistic by default > > - A warning isn't enough, since security issues are involved, the > > communication's semantic should be stronger (ie an exception) > > - Ease the assignment of non-accessible attributes > > > Design decisions: > > - attr_protected remotion > > - All the attributes are unaccessible by default, this encourage the > > usage of attr_accessible > > - Add the attr_accessible :all facility > > - Add a global flag (ActiveRecord::Base.protect_from_mass_assigment, > > true by default) for backward compatibility and as context facility. > > As instance, when you hack with IRb or the application console, > > probably you don't want to be annoyed by security concerns. > > - Raise an exception when try to mass assign non-accessible > > attributes. This should make the code more robust, and provide a > > stronger pointer to the problem. > > - Introduction of ActiveRecord::Base.assign_attributes as non- > > accessible attributes assignment facility. > > Example: > > before: > > def create > > @comment = @commentable.comments.build(params[:reply]) > > @comment.user = current_user > > @comment.request = request > > > # ... > > end > > > after: > > def create > > @comment = @commentable.comments.build(params[:reply]) > > @comment.assign_attributes :user => current_user, :request => > > request > > > # ... > > end > > > It shouldn't be hard to migrate legacy code: > > - Existing code with attr_protected, should be changed, with the > > opposite semantic. > > - Existing code with attr_accessible, is already OK. > > - Existing code without any protection, should introduce > > attr_accessible in each class. > > - Existing code with a large codebase or more complex examples can > > turn off the protection in first instance, then re-enable when the > > migration is done. > > - Optional: coders can use #assign_attributes, as replacement of > > manual assignment (@comment.user = current_user) > > > Hope this make sense. > > > Cheers, > > Luca. > > > -- > > blog:www.lucaguidi.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 [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 -~----------~----~----~----~------~----~------~--~---
