From a08d04bedfd01cc0a517ccedf74f2ceac70eb28d Mon Sep 17 00:00:00 2001 From: Josh Kalderimis Date: Sat, 23 Apr 2011 15:00:24 +0200 Subject: Added assign_attributes to Active Record which accepts a mass-assignment security scope using the :as option, while also allowing mass-assignment security to be bypassed using :with_protected --- activerecord/lib/active_record/base.rb | 41 ++++++++++++- activerecord/test/cases/base_test.rb | 2 +- .../test/cases/mass_assignment_security_test.rb | 71 ++++++++++++++++++++++ activerecord/test/cases/persistence_test.rb | 2 +- activerecord/test/models/loose_person.rb | 24 -------- activerecord/test/models/person.rb | 19 ++++++ 6 files changed, 132 insertions(+), 27 deletions(-) delete mode 100644 activerecord/test/models/loose_person.rb (limited to 'activerecord') diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 9a01d793f9..4512e8c8ad 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1640,10 +1640,49 @@ end # user.is_admin? # => true def attributes=(new_attributes, guard_protected_attributes = true) return unless new_attributes.is_a?(Hash) + if guard_protected_attributes + assign_attributes(new_attributes) + else + assign_attributes(new_attributes, :without_protection => true) + end + end + + # Allows you to set all the attributes for a particular mass-assignment + # security scope by passing in a hash of attributes with keys matching + # the attribute names (which again matches the column names) and the scope + # name using the :as option. + # + # To bypass mass-assignment security you can use the :without_protection => true + # option. + # + # class User < ActiveRecord::Base + # attr_accessible :name + # attr_accessible :name, :is_admin, :as => :admin + # end + # + # user = User.new + # user.assign_attributes({ :name => 'Josh', :is_admin => true }) + # user.name # => "Josh" + # user.is_admin? # => false + # + # user = User.new + # user.assign_attributes({ :name => 'Josh', :is_admin => true }, :as => :admin) + # user.name # => "Josh" + # user.is_admin? # => true + # + # user = User.new + # user.assign_attributes({ :name => 'Josh', :is_admin => true }, :without_protection => true) + # user.name # => "Josh" + # user.is_admin? # => true + def assign_attributes(new_attributes, options = {}) attributes = new_attributes.stringify_keys + scope = options[:as] || :default multi_parameter_attributes = [] - attributes = sanitize_for_mass_assignment(attributes) if guard_protected_attributes + + unless options[:without_protection] + attributes = sanitize_for_mass_assignment(attributes, scope) + end attributes.each do |k, v| if k.include?("(") diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 815ff7b825..ef833857ce 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -18,7 +18,7 @@ require 'models/comment' require 'models/minimalistic' require 'models/warehouse_thing' require 'models/parrot' -require 'models/loose_person' +require 'models/person' require 'models/edge' require 'models/joke' require 'rexml/document' diff --git a/activerecord/test/cases/mass_assignment_security_test.rb b/activerecord/test/cases/mass_assignment_security_test.rb index 025ec1d3fa..43016df479 100644 --- a/activerecord/test/cases/mass_assignment_security_test.rb +++ b/activerecord/test/cases/mass_assignment_security_test.rb @@ -3,6 +3,7 @@ require 'models/company' require 'models/subscriber' require 'models/keyboard' require 'models/task' +require 'models/person' class MassAssignmentSecurityTest < ActiveRecord::TestCase @@ -30,6 +31,66 @@ class MassAssignmentSecurityTest < ActiveRecord::TestCase end end + def test_assign_attributes_uses_default_scope_when_no_scope_is_provided + p = LoosePerson.new + p.assign_attributes(attributes_hash) + + assert_equal nil, p.id + assert_equal 'Josh', p.first_name + assert_equal 'male', p.gender + assert_equal nil, p.comments + end + + def test_assign_attributes_skips_mass_assignment_security_protection_when_without_protection_is_used + p = LoosePerson.new + p.assign_attributes(attributes_hash, :without_protection => true) + + assert_equal 5, p.id + assert_equal 'Josh', p.first_name + assert_equal 'male', p.gender + assert_equal 'rides a sweet bike', p.comments + end + + def test_assign_attributes_with_default_scope_and_attr_protected_attributes + p = LoosePerson.new + p.assign_attributes(attributes_hash, :as => :default) + + assert_equal nil, p.id + assert_equal 'Josh', p.first_name + assert_equal 'male', p.gender + assert_equal nil, p.comments + end + + def test_assign_attributes_with_admin_scope_and_attr_protected_attributes + p = LoosePerson.new + p.assign_attributes(attributes_hash, :as => :admin) + + assert_equal nil, p.id + assert_equal 'Josh', p.first_name + assert_equal 'male', p.gender + assert_equal 'rides a sweet bike', p.comments + end + + def test_assign_attributes_with_default_scope_and_attr_accessible_attributes + p = TightPerson.new + p.assign_attributes(attributes_hash, :as => :default) + + assert_equal nil, p.id + assert_equal 'Josh', p.first_name + assert_equal 'male', p.gender + assert_equal nil, p.comments + end + + def test_assign_attributes_with_admin_scope_and_attr_accessible_attributes + p = TightPerson.new + p.assign_attributes(attributes_hash, :as => :admin) + + assert_equal nil, p.id + assert_equal 'Josh', p.first_name + assert_equal 'male', p.gender + assert_equal 'rides a sweet bike', p.comments + end + def test_protection_against_class_attribute_writers [:logger, :configurations, :primary_key_prefix_type, :table_name_prefix, :table_name_suffix, :pluralize_table_names, :default_timezone, :schema_format, :lock_optimistically, :record_timestamps].each do |method| @@ -40,4 +101,14 @@ class MassAssignmentSecurityTest < ActiveRecord::TestCase end end + private + + def attributes_hash + { + :id => 5, + :first_name => 'Josh', + :gender => 'male', + :comments => 'rides a sweet bike' + } + end end \ No newline at end of file diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb index 9aa13f04cd..3683e3430c 100644 --- a/activerecord/test/cases/persistence_test.rb +++ b/activerecord/test/cases/persistence_test.rb @@ -12,7 +12,7 @@ require 'models/minimalistic' require 'models/warehouse_thing' require 'models/parrot' require 'models/minivan' -require 'models/loose_person' +require 'models/person' require 'rexml/document' require 'active_support/core_ext/exception' diff --git a/activerecord/test/models/loose_person.rb b/activerecord/test/models/loose_person.rb deleted file mode 100644 index 256c281d0d..0000000000 --- a/activerecord/test/models/loose_person.rb +++ /dev/null @@ -1,24 +0,0 @@ -class LoosePerson < ActiveRecord::Base - self.table_name = 'people' - self.abstract_class = true - - attr_protected :credit_rating, :administrator -end - -class LooseDescendant < LoosePerson - attr_protected :phone_number -end - -class LooseDescendantSecond< LoosePerson - attr_protected :phone_number - attr_protected :name -end - -class TightPerson < ActiveRecord::Base - self.table_name = 'people' - attr_accessible :name, :address -end - -class TightDescendant < TightPerson - attr_accessible :phone_number -end \ No newline at end of file diff --git a/activerecord/test/models/person.rb b/activerecord/test/models/person.rb index ad59d12672..9c4794902d 100644 --- a/activerecord/test/models/person.rb +++ b/activerecord/test/models/person.rb @@ -48,3 +48,22 @@ class PersonWithDependentNullifyJobs < ActiveRecord::Base has_many :references, :foreign_key => :person_id has_many :jobs, :source => :job, :through => :references, :dependent => :nullify end + + +class LoosePerson < ActiveRecord::Base + self.table_name = 'people' + self.abstract_class = true + + attr_protected :comments + attr_protected :as => :admin +end + +class LooseDescendant < LoosePerson; end + +class TightPerson < ActiveRecord::Base + self.table_name = 'people' + attr_accessible :first_name, :gender + attr_accessible :first_name, :gender, :comments, :as => :admin +end + +class TightDescendant < TightPerson; end \ No newline at end of file -- cgit v1.2.3