Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strong Parameters and Rails 3 #60

Closed
leonelgalan opened this issue Jun 5, 2013 · 8 comments
Closed

Strong Parameters and Rails 3 #60

leonelgalan opened this issue Jun 5, 2013 · 8 comments

Comments

@leonelgalan
Copy link

After setting config.whitelist_attributes = false in application.rb, the model that is acting as a tree raises ActiveModel::MassAssignmentSecurity::Error. This is, because closure_tree is adding attr_accessible :parent (https://github.com/mceachen/closure_tree/blob/32da850126336fec6036ad8154a9b1fa738cb30e/lib/closure_tree/model.rb#L17)

https://github.com/mceachen/closure_tree/blob/32da850126336fec6036ad8154a9b1fa738cb30e/lib/closure_tree/support.rb#L23

    def use_attr_accessible?
      ActiveRecord::VERSION::MAJOR == 3 &&
        defined?(ActiveModel::MassAssignmentSecurity) &&
        model_class.ancestors.include?(ActiveModel::MassAssignmentSecurity)
    end

I beleive that this happens because of the last condition of use_attr_accessible?, which returns true even when using strong parameters. A better condition will be model_class.accessible_attributes.empty?, which returns true only when attr_accessible has been used in the model.

@mceachen
Copy link
Collaborator

mceachen commented Jun 6, 2013

https://travis-ci.org/mceachen/closure_tree/builds/7831942 is building e72b0a1 which has your suggestion. Thanks!

@leonelgalan
Copy link
Author

Thanks for trying, based on the failed builds (4 is passing because of the frist condition), I just noticed I missed a ! (NOT) before the suggested condition. If it's not empty, it means the user is already using attr_accessible with some fields and its safe to use it in closure_tree.

@mceachen
Copy link
Collaborator

mceachen commented Jun 8, 2013

OK—we'll see how 4379712 goes.

@mceachen
Copy link
Collaborator

mceachen commented Jun 8, 2013

HURRAY https://travis-ci.org/mceachen/closure_tree/builds/7897320 I'll get this into the next version that I'm about to release.

@mceachen mceachen closed this as completed Jun 8, 2013
@samnang
Copy link

samnang commented Feb 17, 2014

@mceachen I'm still getting an issue that @leonelgalan described above in my rails 3 app that is using strong parameter:

>> CaseType.accessible_attributes.nil?
=> false
>> CaseType.accessible_attributes
=> #<ActiveModel::MassAssignmentSecurity::WhiteList: {}>
>> CaseType.accessible_attributes.empty?
=> true

So I think the correct way it should check for empty instead of checking nil.

@doxavore
Copy link

doxavore commented Jun 5, 2014

I'm seeing the same thing as @samnang in Rails 3.2.18 with strong_parameters enabled. Patching https://github.com/mceachen/closure_tree/blob/ff8331eca84300330aad6455837d3872bd9ce519/lib/closure_tree/support_flags.rb#L7 to use .empty? instead of .nil? fixed it for me as well.

@mceachen
Copy link
Collaborator

mceachen commented Jun 5, 2014

Ok, I'll try to get that into master soon. Thanks for the comment.
On Jun 4, 2014 6:28 PM, "Doug Mayer" notifications@github.com wrote:

I'm seeing the same thing as @samnang https://github.com/samnang in
Rails 3.2.18 with strong_parameters enabled. Patching
https://github.com/mceachen/closure_tree/blob/ff8331eca84300330aad6455837d3872bd9ce519/lib/closure_tree/support_flags.rb#L7
to use .empty? instead of .nil? fixed it for me as well.


Reply to this email directly or view it on GitHub
#60 (comment)
.

mceachen added a commit that referenced this issue Jun 15, 2014
@mceachen
Copy link
Collaborator

ok, it's in v4.6.1. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants