expose accessible_attributes directly, deprecate attr_accessible and attr_protected?

188 views
Skip to first unread message

Gary Weaver

unread,
Sep 21, 2012, 12:47:43 PM9/21/12
to rubyonra...@googlegroups.com
Would be nice to allow redefinition of accessible_attributes via self.accessible_attributes in models.

The problem with attr_accessible is that it is cumulative/additive, but it could be interpreted by the developer as redefinition, and that could lead to nasty security issues.

I think that attr_accessible and attr_protected should go the way of set_primary_key and set_table_name (which both got deprecated and changed to self.primary_key= and self.table_name=), such that self.accessible_attributes could be manipulated in a more well-defined way.

The following would be equivalent to attr_accessible :name, :status:

self.accessible_attributes[:default] += :name, :status

The following would be equivalent to attr_protected :name, :status:

self.accessible_attributes[:default] -= :name, :status

The following would be redefining the whitelist, similar to what can be done with self._accessible_attributes[:default] = :name, :status currently (even if you shouldn't be messing with internals):

self.accessible_attributes[:default] = :name, :status

It's too bad that += can't be defined on the self.accessible_attributes Hash instance, because it would be nice not to have to specify the role if it is :default. I know mass assignment security is somewhat being taken off of the stove with strong_parameters being integrated, but it's still there.

What do you think?

Gary Weaver

unread,
Sep 21, 2012, 12:50:33 PM9/21/12
to rubyonra...@googlegroups.com
On Friday, September 21, 2012 12:47:44 PM UTC-4, Gary Weaver wrote:
self.accessible_attributes[:default] += :name, :status


Sorry I left out the []'s on these, because originally was thinking there had to be a way to redefine += so could take *args, and use options = args.extract_options!, args becomes the array, etc.

Rafael Mendonça França

unread,
Sep 21, 2012, 1:16:47 PM9/21/12
to rubyonra...@googlegroups.com
Please see https://github.com/rails/rails/pull/7251

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To view this discussion on the web visit https://groups.google.com/d/msg/rubyonrails-core/-/7mLfon7YnQoJ.

To post to this group, send email to rubyonra...@googlegroups.com.
To unsubscribe from this group, send email to rubyonrails-co...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.

Gary Weaver

unread,
Sep 21, 2012, 4:21:03 PM9/21/12
to rubyonra...@googlegroups.com
On Friday, September 21, 2012 1:17:41 PM UTC-4, Rafael Mendonça França wrote:
Please see https://github.com/rails/rails/pull/7251

Thanks! That is good to have a link to.

Looks like that gem isn't even currently in: https://github.com/rails/rails/blob/integrate-strong_parameters/Gemfile

I guess that means mass assignment security is not only deprecated now, it is deprecated and going to be removed completely in Rails 4. (Did I get that right?) Maybe I'd already read that. If so, sorry for mentioning here again.

Would this be a good request to move to the https://github.com/rails/protected_attributes gem, or is mass assignment so effectively dead that it won't have any new behavior added that isn't required, and that gem is the nursing home for attr_accessible/protected as it waits to die?

Rafael Mendonça França

unread,
Sep 21, 2012, 4:33:24 PM9/21/12
to rubyonra...@googlegroups.com
Yes it was removed and if you want to use attr_accessible and friends you have to add the protected_attributes in your Gemfile.

We will support protected_attributes until the 4.1 release and after that we will drop support. So I don't think we would add/change features in that gem.

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To view this discussion on the web visit https://groups.google.com/d/msg/rubyonrails-core/-/2xlkSfOkxbUJ.
Reply all
Reply to author
Forward
0 new messages