Merge pull request #3900 from jfturcot/accessible_wrap_params

ParamsWrapper only wrap the accessible attributes when they were set

actionpack/CHANGELOG.md

@@ -106,6 +106,11 @@
     persistent between requests so if you need to manipulate the environment
     for your test you need to do it before the cookie jar is created.
 
+*   ActionController::ParamsWrapper on ActiveRecord models now only wrap
+    attr_accessible attributes if they were set, if not, only the attributes
+    returned by the class method attribute_names will be wrapped. This fixes
+    the wrapping of nested attributes by adding them to attr_accessible.
+
 ## Rails 3.1.4 (unreleased) ##
 
 *   Allow to use asset_path on named_routes aliasing RailsHelper's

actionpack/lib/action_controller/metal/params_wrapper.rb

@@ -43,6 +43,11 @@ module ActionController
   #       wrap_parameters :person, :include => [:username, :password]
   #     end
   #
+  # On ActiveRecord models with no +:include+ or +:exclude+ option set, 
+  # if attr_accessible is set on that model, it will only wrap the accessible
+  # parameters, else it will only wrap the parameters returned by the class 
+  # method attribute_names.
+  #
   # If you're going to pass the parameters to an +ActiveModel+ object (such as
   # +User.new(params[:user])+), you might consider passing the model class to
   # the method instead. The +ParamsWrapper+ will actually try to determine the
@@ -162,7 +167,9 @@ def _set_wrapper_defaults(options, model=nil)
 
         unless options[:include] || options[:exclude]
           model ||= _default_wrap_model
-          if model.respond_to?(:attribute_names) && model.attribute_names.present?
+          if model.respond_to?(:accessible_attributes) && model.accessible_attributes.present?
+            options[:include] = model.accessible_attributes.to_a
+          elsif model.respond_to?(:attribute_names) && model.attribute_names.present?
             options[:include] = model.attribute_names
           end
         end

actionpack/test/controller/params_wrapper_test.rb

@@ -26,7 +26,7 @@ def parse
       self.class.last_parameters = request.params.except(:controller, :action)
       head :ok
     end
-  end
+end
 
   class User; end
   class Person; end
@@ -147,6 +147,7 @@ def test_nested_params
   end
 
   def test_derived_wrapped_keys_from_matching_model
+    User.expects(:respond_to?).with(:accessible_attributes).returns(false)
     User.expects(:respond_to?).with(:attribute_names).returns(true)
     User.expects(:attribute_names).twice.returns(["username"])
 
@@ -159,6 +160,7 @@ def test_derived_wrapped_keys_from_matching_model
 
   def test_derived_wrapped_keys_from_specified_model
     with_default_wrapper_options do
+      Person.expects(:respond_to?).with(:accessible_attributes).returns(false)
       Person.expects(:respond_to?).with(:attribute_names).returns(true)
       Person.expects(:attribute_names).twice.returns(["username"])
 
@@ -169,8 +171,33 @@ def test_derived_wrapped_keys_from_specified_model
       assert_parameters({ 'username' => 'sikachu', 'title' => 'Developer', 'person' => { 'username' => 'sikachu' }})
     end
   end
+  
+  def test_accessible_wrapped_keys_from_matching_model
+    User.expects(:respond_to?).with(:accessible_attributes).returns(true)
+    User.expects(:accessible_attributes).twice.returns(["username"])
+    
+    with_default_wrapper_options do
+      @request.env['CONTENT_TYPE'] = 'application/json'
+      post :parse, { 'username' => 'sikachu', 'title' => 'Developer' }
+      assert_parameters({ 'username' => 'sikachu', 'title' => 'Developer', 'user' => { 'username' => 'sikachu' }})
+    end
+  end
+  
+  def test_accessible_wrapped_keys_from_specified_model
+    with_default_wrapper_options do
+      Person.expects(:respond_to?).with(:accessible_attributes).returns(true)
+      Person.expects(:accessible_attributes).twice.returns(["username"])
+
+      UsersController.wrap_parameters Person
+
+      @request.env['CONTENT_TYPE'] = 'application/json'
+      post :parse, { 'username' => 'sikachu', 'title' => 'Developer' }
+      assert_parameters({ 'username' => 'sikachu', 'title' => 'Developer', 'person' => { 'username' => 'sikachu' }})
+    end
+  end
 
   def test_not_wrapping_abstract_model
+    User.expects(:respond_to?).with(:accessible_attributes).returns(false)
     User.expects(:respond_to?).with(:attribute_names).returns(true)
     User.expects(:attribute_names).returns([])