diff options
Diffstat (limited to 'activerecord')
-rw-r--r-- | activerecord/lib/active_record.rb | 1 | ||||
-rw-r--r-- | activerecord/lib/active_record/base.rb | 2 | ||||
-rw-r--r-- | activerecord/lib/active_record/mass_assignment_security.rb | 142 | ||||
-rw-r--r-- | activerecord/lib/active_record/mass_assignment_security/permission_set.rb | 41 | ||||
-rw-r--r-- | activerecord/lib/active_record/mass_assignment_security/sanitizer.rb | 29 | ||||
-rw-r--r-- | activerecord/test/cases/base_test.rb | 2 | ||||
-rw-r--r-- | activerecord/test/cases/mass_assignment_security/black_list_test.rb | 28 | ||||
-rw-r--r-- | activerecord/test/cases/mass_assignment_security/permission_set_test.rb | 30 | ||||
-rw-r--r-- | activerecord/test/cases/mass_assignment_security/sanitizer_test.rb | 36 | ||||
-rw-r--r-- | activerecord/test/cases/mass_assignment_security/white_list_test.rb | 28 | ||||
-rw-r--r-- | activerecord/test/cases/mass_assignment_security_test.rb | 71 | ||||
-rw-r--r-- | activerecord/test/models/loose_person.rb (renamed from activerecord/test/models/mass_assignment_specific.rb) | 10 |
12 files changed, 12 insertions, 408 deletions
diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index 9ab05a0548..e2f2508ae8 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -64,7 +64,6 @@ module ActiveRecord autoload :CounterCache autoload :DynamicFinderMatch autoload :DynamicScopeMatch - autoload :MassAssignmentSecurity autoload :Migration autoload :Migrator, 'active_record/migration' autoload :NamedScope diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 56b4ba8260..f22a9de7b1 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1797,7 +1797,7 @@ MSG include AttributeMethods::PrimaryKey include AttributeMethods::TimeZoneConversion include AttributeMethods::Dirty - include MassAssignmentSecurity + include ActiveModel::MassAssignmentSecurity include Callbacks, ActiveModel::Observing, Timestamp include Associations, AssociationPreload, NamedScope diff --git a/activerecord/lib/active_record/mass_assignment_security.rb b/activerecord/lib/active_record/mass_assignment_security.rb deleted file mode 100644 index 8f4d6e1c74..0000000000 --- a/activerecord/lib/active_record/mass_assignment_security.rb +++ /dev/null @@ -1,142 +0,0 @@ -require 'active_record/mass_assignment_security/permission_set' - -module ActiveRecord - # = Active Record Mass-Assignment Security - module MassAssignmentSecurity - extend ActiveSupport::Concern - - included do - class_attribute :_accessible_attributes - class_attribute :_protected_attributes - class_attribute :_active_authorizer - end - - # Mass assignment security provides an interface for protecting attributes - # from end-user assignment. For more complex permissions, mass assignment security - # may be handled outside the model by extending a non-ActiveRecord class, - # such as a controller, with this behavior. - # - # For example, a logged in user may need to assign additional attributes depending - # on their role: - # - # class AccountsController < ApplicationController - # include ActiveRecord::MassAssignmentSecurity - # - # attr_accessible :first_name, :last_name - # - # def self.admin_accessible_attributes - # accessible_attributes + [ :plan_id ] - # end - # - # def update - # ... - # @account.update_attributes(account_params) - # ... - # end - # - # protected - # - # def account_params - # sanitize_for_mass_assignment(params[:account]) - # end - # - # def mass_assignment_authorizer - # admin ? admin_accessible_attributes : super - # end - # - # end - # - module ClassMethods - # Attributes named in this macro are protected from mass-assignment, - # such as <tt>new(attributes)</tt>, - # <tt>update_attributes(attributes)</tt>, or - # <tt>attributes=(attributes)</tt>. - # - # Mass-assignment to these attributes will simply be ignored, to assign - # to them you can use direct writer methods. This is meant to protect - # sensitive attributes from being overwritten by malicious users - # tampering with URLs or forms. - # - # class Customer < ActiveRecord::Base - # attr_protected :credit_rating - # end - # - # customer = Customer.new("name" => David, "credit_rating" => "Excellent") - # customer.credit_rating # => nil - # customer.attributes = { "description" => "Jolly fellow", "credit_rating" => "Superb" } - # customer.credit_rating # => nil - # - # customer.credit_rating = "Average" - # customer.credit_rating # => "Average" - # - # To start from an all-closed default and enable attributes as needed, - # have a look at +attr_accessible+. - # - # Note that using <tt>Hash#except</tt> or <tt>Hash#slice</tt> in place of +attr_protected+ - # to sanitize attributes won't provide sufficient protection. - def attr_protected(*names) - self._protected_attributes = self.protected_attributes + names - self._active_authorizer = self._protected_attributes - end - - # Specifies a white list of model attributes that can be set via - # mass-assignment, such as <tt>new(attributes)</tt>, - # <tt>update_attributes(attributes)</tt>, or - # <tt>attributes=(attributes)</tt> - # - # This is the opposite of the +attr_protected+ macro: Mass-assignment - # will only set attributes in this list, to assign to the rest of - # attributes you can use direct writer methods. This is meant to protect - # sensitive attributes from being overwritten by malicious users - # tampering with URLs or forms. If you'd rather start from an all-open - # default and restrict attributes as needed, have a look at - # +attr_protected+. - # - # class Customer < ActiveRecord::Base - # attr_accessible :name, :nickname - # end - # - # customer = Customer.new(:name => "David", :nickname => "Dave", :credit_rating => "Excellent") - # customer.credit_rating # => nil - # customer.attributes = { :name => "Jolly fellow", :credit_rating => "Superb" } - # customer.credit_rating # => nil - # - # customer.credit_rating = "Average" - # customer.credit_rating # => "Average" - # - # Note that using <tt>Hash#except</tt> or <tt>Hash#slice</tt> in place of +attr_accessible+ - # to sanitize attributes won't provide sufficient protection. - def attr_accessible(*names) - self._accessible_attributes = self.accessible_attributes + names - self._active_authorizer = self._accessible_attributes - end - - def protected_attributes - self._protected_attributes ||= BlackList.new(attributes_protected_by_default).tap { |w| w.logger = logger } - end - - def accessible_attributes - self._accessible_attributes ||= WhiteList.new.tap { |w| w.logger = logger } - end - - def active_authorizer - self._active_authorizer ||= protected_attributes - end - - def attributes_protected_by_default - [] - end - end - - protected - - def sanitize_for_mass_assignment(attributes) - mass_assignment_authorizer.sanitize(attributes) - end - - def mass_assignment_authorizer - self.class.active_authorizer - end - - end -end diff --git a/activerecord/lib/active_record/mass_assignment_security/permission_set.rb b/activerecord/lib/active_record/mass_assignment_security/permission_set.rb deleted file mode 100644 index 8446a4103b..0000000000 --- a/activerecord/lib/active_record/mass_assignment_security/permission_set.rb +++ /dev/null @@ -1,41 +0,0 @@ -require 'active_record/mass_assignment_security/sanitizer' - -module ActiveRecord - module MassAssignmentSecurity - - class PermissionSet < Set - attr_accessor :logger - - def +(values) - super(values.map(&:to_s)) - end - - def include?(key) - super(remove_multiparameter_id(key)) - end - - protected - - def remove_multiparameter_id(key) - key.gsub(/\(.+/, '') - end - end - - class WhiteList < PermissionSet - include Sanitizer - - def deny?(key) - !include?(key) - end - end - - class BlackList < PermissionSet - include Sanitizer - - def deny?(key) - include?(key) - end - end - - end -end
\ No newline at end of file diff --git a/activerecord/lib/active_record/mass_assignment_security/sanitizer.rb b/activerecord/lib/active_record/mass_assignment_security/sanitizer.rb deleted file mode 100644 index 11de35f9d6..0000000000 --- a/activerecord/lib/active_record/mass_assignment_security/sanitizer.rb +++ /dev/null @@ -1,29 +0,0 @@ -module ActiveRecord - module MassAssignmentSecurity - module Sanitizer - - # Returns all attributes not denied by the authorizer. - def sanitize(attributes) - sanitized_attributes = attributes.reject { |key, value| deny?(key) } - debug_protected_attribute_removal(attributes, sanitized_attributes) if debug? - sanitized_attributes - end - - protected - - def debug_protected_attribute_removal(attributes, sanitized_attributes) - removed_keys = attributes.keys - sanitized_attributes.keys - warn!(removed_keys) if removed_keys.any? - end - - def debug? - logger.present? - end - - def warn!(attrs) - logger.debug "WARNING: Can't mass-assign protected attributes: #{attrs.join(', ')}" - end - - end - end -end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 2eecb6e344..5a72e9c6e0 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -17,7 +17,7 @@ require 'models/comment' require 'models/minimalistic' require 'models/warehouse_thing' require 'models/parrot' -require 'models/mass_assignment_specific' +require 'models/loose_person' require 'rexml/document' require 'active_support/core_ext/exception' diff --git a/activerecord/test/cases/mass_assignment_security/black_list_test.rb b/activerecord/test/cases/mass_assignment_security/black_list_test.rb deleted file mode 100644 index 8b7f48a5f6..0000000000 --- a/activerecord/test/cases/mass_assignment_security/black_list_test.rb +++ /dev/null @@ -1,28 +0,0 @@ -require "cases/helper" - -class BlackListTest < ActiveRecord::TestCase - - def setup - @black_list = ActiveRecord::MassAssignmentSecurity::BlackList.new - @included_key = 'admin' - @black_list += [ @included_key ] - end - - test "deny? is true for included items" do - assert_equal true, @black_list.deny?(@included_key) - end - - test "deny? is false for non-included items" do - assert_equal false, @black_list.deny?('first_name') - end - - test "sanitize attributes" do - original_attributes = { 'first_name' => 'allowed', 'admin' => 'denied', 'admin(1)' => 'denied' } - attributes = @black_list.sanitize(original_attributes) - - assert attributes.key?('first_name'), "Allowed key shouldn't be rejected" - assert !attributes.key?('admin'), "Denied key should be rejected" - assert !attributes.key?('admin(1)'), "Multi-parameter key should be detected" - end - -end diff --git a/activerecord/test/cases/mass_assignment_security/permission_set_test.rb b/activerecord/test/cases/mass_assignment_security/permission_set_test.rb deleted file mode 100644 index ca8985042a..0000000000 --- a/activerecord/test/cases/mass_assignment_security/permission_set_test.rb +++ /dev/null @@ -1,30 +0,0 @@ -require "cases/helper" - -class PermissionSetTest < ActiveRecord::TestCase - - def setup - @permission_list = ActiveRecord::MassAssignmentSecurity::PermissionSet.new - end - - test "+ stringifies added collection values" do - symbol_collection = [ :admin ] - new_list = @permission_list += symbol_collection - - assert new_list.include?('admin'), "did not add collection to #{@permission_list.inspect}}" - end - - test "include? normalizes multi-parameter keys" do - multi_param_key = 'admin(1)' - new_list = @permission_list += [ 'admin' ] - - assert new_list.include?(multi_param_key), "#{multi_param_key} not found in #{@permission_list.inspect}" - end - - test "include? normal keys" do - normal_key = 'admin' - new_list = @permission_list += [ normal_key ] - - assert new_list.include?(normal_key), "#{normal_key} not found in #{@permission_list.inspect}" - end - -end diff --git a/activerecord/test/cases/mass_assignment_security/sanitizer_test.rb b/activerecord/test/cases/mass_assignment_security/sanitizer_test.rb deleted file mode 100644 index 122bc7e114..0000000000 --- a/activerecord/test/cases/mass_assignment_security/sanitizer_test.rb +++ /dev/null @@ -1,36 +0,0 @@ -require "cases/helper" - -class SanitizerTest < ActiveRecord::TestCase - - class SanitizingAuthorizer - include ActiveRecord::MassAssignmentSecurity::Sanitizer - - attr_accessor :logger - - def deny?(key) - [ 'admin' ].include?(key) - end - - end - - def setup - @sanitizer = SanitizingAuthorizer.new - end - - test "sanitize attributes" do - original_attributes = { 'first_name' => 'allowed', 'admin' => 'denied' } - attributes = @sanitizer.sanitize(original_attributes) - - assert attributes.key?('first_name'), "Allowed key shouldn't be rejected" - assert !attributes.key?('admin'), "Denied key should be rejected" - end - - test "debug mass assignment removal" do - original_attributes = { 'first_name' => 'allowed', 'admin' => 'denied' } - log = StringIO.new - @sanitizer.logger = Logger.new(log) - @sanitizer.sanitize(original_attributes) - assert (log.string =~ /admin/), "Should log removed attributes: #{log.string}" - end - -end diff --git a/activerecord/test/cases/mass_assignment_security/white_list_test.rb b/activerecord/test/cases/mass_assignment_security/white_list_test.rb deleted file mode 100644 index 4601263437..0000000000 --- a/activerecord/test/cases/mass_assignment_security/white_list_test.rb +++ /dev/null @@ -1,28 +0,0 @@ -require "cases/helper" - -class WhiteListTest < ActiveRecord::TestCase - - def setup - @white_list = ActiveRecord::MassAssignmentSecurity::WhiteList.new - @included_key = 'first_name' - @white_list += [ @included_key ] - end - - test "deny? is false for included items" do - assert_equal false, @white_list.deny?(@included_key) - end - - test "deny? is true for non-included items" do - assert_equal true, @white_list.deny?('admin') - end - - test "sanitize attributes" do - original_attributes = { 'first_name' => 'allowed', 'admin' => 'denied', 'admin(1)' => 'denied' } - attributes = @white_list.sanitize(original_attributes) - - assert attributes.key?('first_name'), "Allowed key shouldn't be rejected" - assert !attributes.key?('admin'), "Denied key should be rejected" - assert !attributes.key?('admin(1)'), "Multi-parameter key should be detected" - end - -end diff --git a/activerecord/test/cases/mass_assignment_security_test.rb b/activerecord/test/cases/mass_assignment_security_test.rb index 07154da93b..025ec1d3fa 100644 --- a/activerecord/test/cases/mass_assignment_security_test.rb +++ b/activerecord/test/cases/mass_assignment_security_test.rb @@ -1,28 +1,11 @@ require "cases/helper" -require 'models/reply' require 'models/company' require 'models/subscriber' require 'models/keyboard' -require 'models/mass_assignment_specific' +require 'models/task' class MassAssignmentSecurityTest < ActiveRecord::TestCase - def test_mass_assignment_protection - firm = Firm.new - firm.attributes = { "name" => "Next Angle", "rating" => 5 } - assert_equal 1, firm.rating - end - - def test_mass_assignment_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| - assert_respond_to Task, method - assert_respond_to Task, "#{method}=" - assert_respond_to Task.new, method - assert !Task.new.respond_to?("#{method}=") - end - end - def test_customized_primary_key_remains_protected subscriber = Subscriber.new(:nick => 'webster123', :name => 'nice try') assert_nil subscriber.id @@ -47,50 +30,14 @@ class MassAssignmentSecurityTest < ActiveRecord::TestCase end end - def test_mass_assignment_protection_on_defaults - firm = Firm.new - firm.attributes = { "id" => 5, "type" => "Client" } - assert_nil firm.id - assert_equal "Firm", firm[:type] - end - - def test_mass_assignment_accessible - reply = Reply.new("title" => "hello", "content" => "world", "approved" => true) - reply.save - - assert reply.approved? - - reply.approved = false - reply.save - - assert !reply.approved? - end - - def test_mass_assignment_protection_inheritance - assert LoosePerson.accessible_attributes.blank? - assert_equal Set.new([ 'credit_rating', 'administrator', *LoosePerson.attributes_protected_by_default ]), LoosePerson.protected_attributes - - assert LooseDescendant.accessible_attributes.blank? - assert_equal Set.new([ 'credit_rating', 'administrator', 'phone_number', *LoosePerson.attributes_protected_by_default ]), LooseDescendant.protected_attributes - - assert LooseDescendantSecond.accessible_attributes.blank? - assert_equal Set.new([ 'credit_rating', 'administrator', 'phone_number', 'name', *LoosePerson.attributes_protected_by_default ]), - LooseDescendantSecond.protected_attributes, 'Running attr_protected twice in one class should merge the protections' - - assert (TightPerson.protected_attributes - TightPerson.attributes_protected_by_default).blank? - assert_equal Set.new([ 'name', 'address' ]), TightPerson.accessible_attributes - - assert (TightDescendant.protected_attributes - TightDescendant.attributes_protected_by_default).blank? - assert_equal Set.new([ 'name', 'address', 'phone_number' ]), TightDescendant.accessible_attributes - end - - def test_mass_assignment_multiparameter_protector - task = Task.new - time = Time.mktime(2000, 1, 1, 1) - task.starting = time - attributes = { "starting(1i)" => "2004", "starting(2i)" => "6", "starting(3i)" => "24" } - task.attributes = attributes - assert_equal time, task.starting + 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| + assert_respond_to Task, method + assert_respond_to Task, "#{method}=" + assert_respond_to Task.new, method + assert !Task.new.respond_to?("#{method}=") + end end end
\ No newline at end of file diff --git a/activerecord/test/models/mass_assignment_specific.rb b/activerecord/test/models/loose_person.rb index 13a80e0197..256c281d0d 100644 --- a/activerecord/test/models/mass_assignment_specific.rb +++ b/activerecord/test/models/loose_person.rb @@ -1,6 +1,7 @@ class LoosePerson < ActiveRecord::Base self.table_name = 'people' self.abstract_class = true + attr_protected :credit_rating, :administrator end @@ -20,13 +21,4 @@ end class TightDescendant < TightPerson attr_accessible :phone_number -end - -class Task < ActiveRecord::Base - attr_protected :starting -end - -class TopicWithProtectedContent < ActiveRecord::Base - self.table_name = 'topics' - attr_protected :content end
\ No newline at end of file |