Skip to content
This repository

new applications enforce whitelist mode for mass assignment #4062

Closed
wants to merge 1 commit into from

6 participants

Sergey Nartimov José Valim Jeremy Kemper Alexey Muranov Xavier Noria David Heinemeier Hansson
Sergey Nartimov

Previous issues #3453 and #3952, according to discussion it's ok to enable it in 4.0

Sergey Nartimov

@josevalim Could you give comments about it? Can it be in 4.0?

José Valim
Owner

I have asked other Rails Core Teams for feedback. Let's wait. :) /cc @jeremy @dhh @fxn

Sergey Nartimov

Updated description to include both issues with discussion

Jeremy Kemper
Owner

Mixed feelings about this. +1 to secure-by-default, but geez, how did we end up here, having to list out accessible attributes for every model?

Seems like a lot of paperwork.

(If we do turn this on by default, the model generator should include attr_accessible too.)

Alexey Muranov

How about #3157?

Alexey Muranov

I think, during development i would like to not to have to whitelist all needed attributes. In fact, i plan to use mass assignment security only in subclasses, not in the base classes that inherit directly from ActiveRecord::Base.

Xavier Noria
Owner

Same feeling as @jeremy secure by default sounds good, but geez. Not sure I like this as a default. We have SQLite as default to be able to fire up an application quickly and try stuff. People can opt-in... not convinced.

David Heinemeier Hansson
Owner
dhh commented

Yeah, I don't like this idea either. -1.

José Valim josevalim closed this
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.

Dec 20, 2011
Sergey Nartimov new applications enforce whitelist mode for mass assignment 04d5f3e
This page is out of date. Refresh to see the latest.
4  railties/CHANGELOG.md
Source Rendered
... ...
@@ -1,3 +1,7 @@
  1
+## Rails 4.0.0 (unreleased) ##
  2
+*   New applications enforce whitelist mode for mass assignment using `config.active_record.whitelist_attributes`
  3
+    turned on by default in `config/application.rb`. *Sergey Nartimov*
  4
+
1 5
 ## Rails 3.2.0 (unreleased) ##
2 6
 
3 7
 *   Added `config.exceptions_app` to set the exceptions application invoked by the ShowException middleware when an exception happens. Defaults to `ActionDispatch::PublicExceptions.new(Rails.public_path)`. *José Valim*
4  railties/lib/rails/generators/rails/app/templates/config/application.rb
@@ -54,12 +54,14 @@ class Application < Rails::Application
54 54
     # like if you have constraints or database-specific column types
55 55
     # config.active_record.schema_format = :sql
56 56
 
  57
+<% unless options.skip_active_record? -%>
57 58
     # Enforce whitelist mode for mass assignment.
58 59
     # This will create an empty whitelist of attributes available for mass-assignment for all models
59 60
     # in your app. As such, your models will need to explicitly whitelist or blacklist accessible
60 61
     # parameters by using an attr_accessible or attr_protected declaration.
61  
-    # config.active_record.whitelist_attributes = true
  62
+    config.active_record.whitelist_attributes = true
62 63
 
  64
+<% end -%>
63 65
 <% unless options.skip_sprockets? -%>
64 66
     # Enable the asset pipeline
65 67
     config.assets.enabled = true
13  railties/test/application/configuration_test.rb
@@ -283,15 +283,22 @@ def index
283 283
     end
284 284
 
285 285
     test "sets all Active Record models to whitelist all attributes by default" do
  286
+      require "#{app_path}/config/environment"
  287
+
  288
+      assert_equal ActiveModel::MassAssignmentSecurity::WhiteList,
  289
+                   ActiveRecord::Base.active_authorizers[:default].class
  290
+      assert_equal [""], ActiveRecord::Base.active_authorizers[:default].to_a
  291
+    end
  292
+
  293
+    test "sets all Active Record models to blacklist all attributes by default when whitelist is disabled" do
286 294
       add_to_config <<-RUBY
287  
-        config.active_record.whitelist_attributes = true
  295
+        config.active_record.whitelist_attributes = false
288 296
       RUBY
289 297
 
290 298
       require "#{app_path}/config/environment"
291 299
 
292  
-      assert_equal ActiveModel::MassAssignmentSecurity::WhiteList,
  300
+      assert_equal ActiveModel::MassAssignmentSecurity::BlackList,
293 301
                    ActiveRecord::Base.active_authorizers[:default].class
294  
-      assert_equal [""], ActiveRecord::Base.active_authorizers[:default].to_a
295 302
     end
296 303
 
297 304
     test "registers interceptors with ActionMailer" do
1  railties/test/application/loading_test.rb
@@ -19,6 +19,7 @@ def app
19 19
   test "constants in app are autoloaded" do
20 20
     app_file "app/models/post.rb", <<-MODEL
21 21
       class Post < ActiveRecord::Base
  22
+        attr_accessible :title
22 23
         validates_acceptance_of :title, :accept => "omg"
23 24
       end
24 25
     MODEL
10  railties/test/generators/app_generator_test.rb
@@ -202,7 +202,10 @@ def test_config_jdbc_database_when_no_option_given
202 202
   def test_generator_if_skip_active_record_is_given
203 203
     run_generator [destination_root, "--skip-active-record"]
204 204
     assert_no_file "config/database.yml"
205  
-    assert_file "config/application.rb", /#\s+require\s+["']active_record\/railtie["']/
  205
+    assert_file "config/application.rb" do |content|
  206
+      assert_match(/#\s+require\s+["']active_record\/railtie["']/, content)
  207
+      assert_no_match(/config.active_record.whitelist_attributes = true/, content)
  208
+    end
206 209
     assert_file "test/test_helper.rb" do |helper_content|
207 210
       assert_no_match(/fixtures :all/, helper_content)
208 211
     end
@@ -230,6 +233,11 @@ def test_generator_if_skip_sprockets_is_given
230 233
     assert_file "test/performance/browsing_test.rb"
231 234
   end
232 235
 
  236
+  def test_inclusion_of_activerecord_whitelist_attributes
  237
+    run_generator([destination_root])
  238
+    assert_file "config/application.rb", /config.active_record.whitelist_attributes = true/
  239
+  end
  240
+
233 241
   def test_inclusion_of_therubyrhino_under_jruby
234 242
     run_generator([destination_root])
235 243
     if defined?(JRUBY_VERSION)
1  railties/test/isolation/abstract_unit.rb
@@ -261,6 +261,7 @@ def use_frameworks(arr)
261 261
                     :activerecord,
262 262
                     :activeresource] - arr
263 263
       remove_from_config "config.active_record.identity_map = true" if to_remove.include? :activerecord
  264
+      remove_from_config "config.active_record.whitelist_attributes = true" if to_remove.include? :activerecord
264 265
       $:.reject! {|path| path =~ %r'/(#{to_remove.join('|')})/' }
265 266
     end
266 267
 
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.