Skip to content
This repository

specify a role for identifying accessible attributes when wrapping params #4445

Merged
merged 1 commit into from almost 2 years ago

3 participants

Nick Ragaz José Valim Isaac Sanders
Nick Ragaz

This is an addition to #3900

In addition to using attr_accessible to identify which params to wrap, this also allows you to specify a role using an :as option on wrap_parameters. Then the wrapped params will only be those accessible to that role.

José Valim
Owner

Thanks, but this won't work properly know because in the case the user doesn't pass :as, it will be set to nil and it won't work as expected:

https://github.com/rails/rails/blob/master/activemodel/lib/active_model/mass_assignment_security.rb

The default for :as needs to be :default.

Nick Ragaz
José Valim
Owner
Nick Ragaz
Isaac Sanders

Is this still an issue?

Nick Ragaz
Isaac Sanders

Is it worth keeping the issue open for rails?

Nick Ragaz
José Valim
Owner

Yes, this needs to be fixed. Could you please rebase your pull request, then push --force it to the same branch and ping me again? Thanks.

Nick Ragaz
nragaz commented May 01, 2012

@josevalim -- would be happy to, but I don't understand how you want me to rebase it. I never use rebasing and don't want to screw it up. Could you clarify?

Isaac Sanders

Rebasing: git rebase <branch-name>

Nick Ragaz Add a role option to wrap_parameters.
The role option identifies which parameters are accessible and should be wrapped. The default role is :default.
bfb25f9
Nick Ragaz
nragaz commented May 04, 2012

@josevalim -- rebased. Thanks!

José Valim josevalim merged commit efb054b into from May 04, 2012
José Valim josevalim closed this May 04, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

May 04, 2012
Nick Ragaz Add a role option to wrap_parameters.
The role option identifies which parameters are accessible and should be wrapped. The default role is :default.
bfb25f9
This page is out of date. Refresh to see the latest.
5  actionpack/lib/action_controller/metal/params_wrapper.rb
@@ -167,8 +167,9 @@ def _set_wrapper_defaults(options, model=nil)
167 167
 
168 168
         unless options[:include] || options[:exclude]
169 169
           model ||= _default_wrap_model
170  
-          if model.respond_to?(:accessible_attributes) && model.accessible_attributes.present?
171  
-            options[:include] = model.accessible_attributes.to_a
  170
+          role = options.has_key?(:as) ? options[:as] : :default
  171
+          if model.respond_to?(:accessible_attributes) && model.accessible_attributes(role).present?
  172
+            options[:include] = model.accessible_attributes(role).to_a
172 173
           elsif model.respond_to?(:attribute_names) && model.attribute_names.present?
173 174
             options[:include] = model.attribute_names
174 175
           end
17  actionpack/test/controller/params_wrapper_test.rb
@@ -174,7 +174,7 @@ def test_derived_wrapped_keys_from_specified_model
174 174
   
175 175
   def test_accessible_wrapped_keys_from_matching_model
176 176
     User.expects(:respond_to?).with(:accessible_attributes).returns(true)
177  
-    User.expects(:accessible_attributes).twice.returns(["username"])
  177
+    User.expects(:accessible_attributes).with(:default).twice.returns(["username"])
178 178
     
179 179
     with_default_wrapper_options do
180 180
       @request.env['CONTENT_TYPE'] = 'application/json'
@@ -186,7 +186,7 @@ def test_accessible_wrapped_keys_from_matching_model
186 186
   def test_accessible_wrapped_keys_from_specified_model
187 187
     with_default_wrapper_options do
188 188
       Person.expects(:respond_to?).with(:accessible_attributes).returns(true)
189  
-      Person.expects(:accessible_attributes).twice.returns(["username"])
  189
+      Person.expects(:accessible_attributes).with(:default).twice.returns(["username"])
190 190
 
191 191
       UsersController.wrap_parameters Person
192 192
 
@@ -195,6 +195,19 @@ def test_accessible_wrapped_keys_from_specified_model
195 195
       assert_parameters({ 'username' => 'sikachu', 'title' => 'Developer', 'person' => { 'username' => 'sikachu' }})
196 196
     end
197 197
   end
  198
+  
  199
+  def test_accessible_wrapped_keys_with_role_from_specified_model
  200
+    with_default_wrapper_options do
  201
+      Person.expects(:respond_to?).with(:accessible_attributes).returns(true)
  202
+      Person.expects(:accessible_attributes).with(:admin).twice.returns(["username"])
  203
+
  204
+      UsersController.wrap_parameters Person, :as => :admin
  205
+
  206
+      @request.env['CONTENT_TYPE'] = 'application/json'
  207
+      post :parse, { 'username' => 'sikachu', 'title' => 'Developer' }
  208
+      assert_parameters({ 'username' => 'sikachu', 'title' => 'Developer', 'person' => { 'username' => 'sikachu' }})
  209
+    end
  210
+  end
198 211
 
199 212
   def test_not_wrapping_abstract_model
200 213
     User.expects(:respond_to?).with(:accessible_attributes).returns(false)
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.