From 34bcd2591f5a23aeba495c7743575f0c3bd51ed0 Mon Sep 17 00:00:00 2001 From: Eric Chapweske Date: Wed, 9 Sep 2009 13:33:31 -0700 Subject: [PATCH] Mass assignment security refactoring --- activerecord/lib/active_record.rb | 1 + activerecord/lib/active_record/base.rb | 139 ++------------------ .../lib/active_record/mass_assignment_security.rb | 95 +++++++++++++ .../mass_assignment_security/authorizor_list.rb | 27 ++++ .../authorizor_list/conflict_detection.rb | 26 ++++ .../mass_assignment_security/permission_set.rb | 39 ++++++ .../mass_assignment_security/sanitizer.rb | 44 ++++++ .../sanitizer/authorizors.rb | 43 ++++++ activerecord/test/cases/base_test.rb | 19 ++-- .../authorizor_list_test.rb | 54 ++++++++ .../mass_assignment_security/authorizors_test.rb | 50 +++++++ .../mass_assignment_security/black_list_test.rb | 24 ++++ .../permission_set_test.rb | 37 +++++ .../mass_assignment_security/sanitizer_test.rb | 24 ++++ .../mass_assignment_security/white_list_test.rb | 24 ++++ 15 files changed, 506 insertions(+), 140 deletions(-) create mode 100644 activerecord/lib/active_record/mass_assignment_security.rb create mode 100644 activerecord/lib/active_record/mass_assignment_security/authorizor_list.rb create mode 100644 activerecord/lib/active_record/mass_assignment_security/authorizor_list/conflict_detection.rb create mode 100644 activerecord/lib/active_record/mass_assignment_security/permission_set.rb create mode 100644 activerecord/lib/active_record/mass_assignment_security/sanitizer.rb create mode 100644 activerecord/lib/active_record/mass_assignment_security/sanitizer/authorizors.rb create mode 100644 activerecord/test/cases/mass_assignment_security/authorizor_list_test.rb create mode 100644 activerecord/test/cases/mass_assignment_security/authorizors_test.rb create mode 100644 activerecord/test/cases/mass_assignment_security/black_list_test.rb create mode 100644 activerecord/test/cases/mass_assignment_security/permission_set_test.rb create mode 100644 activerecord/test/cases/mass_assignment_security/sanitizer_test.rb create mode 100644 activerecord/test/cases/mass_assignment_security/white_list_test.rb diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index 2f1e757..4f4ac2a 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -54,6 +54,7 @@ module ActiveRecord autoload :Callbacks, 'active_record/callbacks' autoload :DynamicFinderMatch, 'active_record/dynamic_finder_match' autoload :DynamicScopeMatch, 'active_record/dynamic_scope_match' + autoload :MassAssignmentSecurity, 'active_record/mass_assignment_security' autoload :Migration, 'active_record/migration' autoload :Migrator, 'active_record/migration' autoload :NamedScope, 'active_record/named_scope' diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 72742cb..72d6536 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1032,102 +1032,6 @@ module ActiveRecord #:nodoc: update_counters(id, counter_name => -1) end - # Attributes named in this macro are protected from mass-assignment, - # such as new(attributes), - # update_attributes(attributes), or - # attributes=(attributes). - # - # 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+. - # - # If the access logic of your application is richer you can use Hash#except - # or Hash#slice to sanitize the hash of parameters before they are - # passed to Active Record. - # - # For example, it could be the case that the list of protected attributes - # for a given model depends on the role of the user: - # - # # Assumes plan_id is not protected because it depends on the role. - # params[:account] = params[:account].except(:plan_id) unless admin? - # @account.update_attributes(params[:account]) - # - # Note that +attr_protected+ is still applied to the received hash. Thus, - # with this technique you can at most _extend_ the list of protected - # attributes for a particular mass-assignment call. - def attr_protected(*attributes) - write_inheritable_attribute(:attr_protected, Set.new(attributes.map {|a| a.to_s}) + (protected_attributes || [])) - end - - # Returns an array of all the attributes that have been protected from mass-assignment. - def protected_attributes # :nodoc: - read_inheritable_attribute(:attr_protected) - end - - # Specifies a white list of model attributes that can be set via - # mass-assignment, such as new(attributes), - # update_attributes(attributes), or - # attributes=(attributes) - # - # 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" - # - # If the access logic of your application is richer you can use Hash#except - # or Hash#slice to sanitize the hash of parameters before they are - # passed to Active Record. - # - # For example, it could be the case that the list of accessible attributes - # for a given model depends on the role of the user: - # - # # Assumes plan_id is accessible because it depends on the role. - # params[:account] = params[:account].except(:plan_id) unless admin? - # @account.update_attributes(params[:account]) - # - # Note that +attr_accessible+ is still applied to the received hash. Thus, - # with this technique you can at most _narrow_ the list of accessible - # attributes for a particular mass-assignment call. - def attr_accessible(*attributes) - write_inheritable_attribute(:attr_accessible, Set.new(attributes.map(&:to_s)) + (accessible_attributes || [])) - end - - # Returns an array of all the attributes that have been made accessible to mass-assignment. - def accessible_attributes # :nodoc: - read_inheritable_attribute(:attr_accessible) - end - # Attributes listed as readonly can be set for a new record, but will be ignored in database updates afterwards. def attr_readonly(*attributes) write_inheritable_attribute(:attr_readonly, Set.new(attributes.map(&:to_s)) + (readonly_attributes || [])) @@ -2845,7 +2749,12 @@ module ActiveRecord #:nodoc: }.compact.join(", ") "#<#{self.class} #{attributes_as_nice_string}>" end - + # The primary key and inheritance column can never be set by mass-assignment for security reasons. + def self.attributes_protected_by_default + default = [ primary_key, inheritance_column ] + default << 'id' unless primary_key.eql? 'id' + default + end private def create_or_update raise ReadOnlyRecord if readonly? @@ -2912,27 +2821,6 @@ module ActiveRecord #:nodoc: end end - def remove_attributes_protected_from_mass_assignment(attributes) - safe_attributes = - if self.class.accessible_attributes.nil? && self.class.protected_attributes.nil? - attributes.reject { |key, value| attributes_protected_by_default.include?(key.gsub(/\(.+/, "")) } - elsif self.class.protected_attributes.nil? - attributes.reject { |key, value| !self.class.accessible_attributes.include?(key.gsub(/\(.+/, "")) || attributes_protected_by_default.include?(key.gsub(/\(.+/, "")) } - elsif self.class.accessible_attributes.nil? - attributes.reject { |key, value| self.class.protected_attributes.include?(key.gsub(/\(.+/,"")) || attributes_protected_by_default.include?(key.gsub(/\(.+/, "")) } - else - raise "Declare either attr_protected or attr_accessible for #{self.class}, but not both." - end - - removed_attributes = attributes.keys - safe_attributes.keys - - if removed_attributes.any? - log_protected_attribute_removal(removed_attributes) - end - - safe_attributes - end - # Removes attributes which have been marked as readonly. def remove_readonly_attributes(attributes) unless self.class.readonly_attributes.nil? @@ -2942,18 +2830,8 @@ module ActiveRecord #:nodoc: end end - def log_protected_attribute_removal(*attributes) - if logger - logger.debug "WARNING: Can't mass-assign these protected attributes: #{attributes.join(', ')}" - end - end - - # The primary key and inheritance column can never be set by mass-assignment for security reasons. - def attributes_protected_by_default - default = [ self.class.primary_key, self.class.inheritance_column ] - default << 'id' unless self.class.primary_key.eql? 'id' - default - end + + # Returns a copy of the attributes hash where all the values have been safely quoted for use in # an SQL statement. @@ -3125,6 +3003,7 @@ module ActiveRecord #:nodoc: end Base.class_eval do + extend MassAssignmentSecurity extend ActiveModel::Naming extend QueryCache::ClassMethods include Validations diff --git a/activerecord/lib/active_record/mass_assignment_security.rb b/activerecord/lib/active_record/mass_assignment_security.rb new file mode 100644 index 0000000..e50ece5 --- /dev/null +++ b/activerecord/lib/active_record/mass_assignment_security.rb @@ -0,0 +1,95 @@ +require 'active_record/mass_assignment_security/sanitizer' + +module ActiveRecord + module MassAssignmentSecurity + extend ActiveSupport::Concern + + def self.extended(base) + base.send(:include, InstanceMethods) + end + + module InstanceMethods + + def remove_attributes_protected_from_mass_assignment(attributes) + self.class.mass_assignment_sanitizer.sanitize(attributes) + end + + end + + # Attributes named in this macro are protected from mass-assignment, + # such as new(attributes), + # update_attributes(attributes), or + # attributes=(attributes). + # + # 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+. + def attr_protected(*keys) + mass_assignment_sanitizer.deny(keys) + end + + # Specifies a white list of model attributes that can be set via + # mass-assignment, such as new(attributes), + # update_attributes(attributes), or + # attributes=(attributes) + # + # 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" + def attr_accessible(*keys) + mass_assignment_sanitizer.allow(keys) + end + + # Returns an array of all the attributes that have been protected from mass-assignment. + def protected_attributes + mass_assignment_sanitizer.denied_keys + end + + # Returns an array of all the attributes that have been made accessible to mass-assignment. + def accessible_attributes + mass_assignment_sanitizer.allowed_keys + end + + def mass_assignment_sanitizer + read_inheritable_attribute(:mass_assignment_sanitizer) || begin + sanitizer = Sanitizer.new + sanitizer.logger = logger + sanitizer.always_deny(attributes_protected_by_default) + write_inheritable_attribute(:mass_assignment_sanitizer, sanitizer) + end + end + + end +end \ No newline at end of file diff --git a/activerecord/lib/active_record/mass_assignment_security/authorizor_list.rb b/activerecord/lib/active_record/mass_assignment_security/authorizor_list.rb new file mode 100644 index 0000000..ddfb936 --- /dev/null +++ b/activerecord/lib/active_record/mass_assignment_security/authorizor_list.rb @@ -0,0 +1,27 @@ +require 'active_record/mass_assignment_security/authorizor_list/conflict_detection' + +module ActiveRecord + module MassAssignmentSecurity + class AuthorizorList < Hash + include ConflictDetection + + def deny?(key) + active_authorizor.deny?(key) + end + + protected + + def active_authorizor + self[active_authorizor_key] + end + + private + + def active_authorizor_key + @active_authorizor_key ||= keys.detect { |key| self[key].present? } + end + + end + end +end + \ No newline at end of file diff --git a/activerecord/lib/active_record/mass_assignment_security/authorizor_list/conflict_detection.rb b/activerecord/lib/active_record/mass_assignment_security/authorizor_list/conflict_detection.rb new file mode 100644 index 0000000..6687ce8 --- /dev/null +++ b/activerecord/lib/active_record/mass_assignment_security/authorizor_list/conflict_detection.rb @@ -0,0 +1,26 @@ +module ActiveRecord + module MassAssignmentSecurity + class AuthorizorList < Hash + module ConflictDetection + + def []=(key, value) + if conflict?(key) + raise "Already using #{index(active_authorizor)}, cannot use #{key} for #{value.inspect}" + end + super + end + + protected + + def conflict?(key) + active_authorizor != default && inactive?(key) + end + + def inactive?(key) + key != index(active_authorizor) + end + + end + end + end +end \ No newline at end of file diff --git a/activerecord/lib/active_record/mass_assignment_security/permission_set.rb b/activerecord/lib/active_record/mass_assignment_security/permission_set.rb new file mode 100644 index 0000000..f236a58 --- /dev/null +++ b/activerecord/lib/active_record/mass_assignment_security/permission_set.rb @@ -0,0 +1,39 @@ +module ActiveRecord + module MassAssignmentSecurity + + class PermissionSet < Set + + def +(values) + super(values.map(&:to_s)) + end + + def include?(key) + super(remove_multiparameter_position_id(key)) + end + + protected + + def remove_multiparameter_position_id(key) + key.gsub(/\(.+/, '') + end + + end + + class WhiteList < PermissionSet + + def deny?(key) + !include?(key) + end + + end + + class BlackList < PermissionSet + + def deny?(key) + include?(key) + end + + end + + end +end diff --git a/activerecord/lib/active_record/mass_assignment_security/sanitizer.rb b/activerecord/lib/active_record/mass_assignment_security/sanitizer.rb new file mode 100644 index 0000000..1a0ce23 --- /dev/null +++ b/activerecord/lib/active_record/mass_assignment_security/sanitizer.rb @@ -0,0 +1,44 @@ +require 'active_record/mass_assignment_security/sanitizer/authorizors' + +module ActiveRecord + module MassAssignmentSecurity + class Sanitizer + include Authorizors + + attr_accessor :logger + + def sanitize(attributes) + sanitized_attributes = attributes.reject { |key, value| deny?(key) } + debug_protected_attribute_removal(attributes, sanitized_attributes) if debug? + sanitized_attributes + end + + # :nodoc Allows autonomous changes to sanitizer's authorizor when inheriting. + def dup + new_sanitizer = super + new_sanitizer.authorizor = authorizor.dup + new_sanitizer + end + + protected + + attr_accessor :authorizor, :always_deny_authorizor + + def deny?(key) + authorizor.deny?(key) || always_deny_authorizor.deny?(key) + end + + def debug_protected_attribute_removal(attributes, sanitized_attributes) + removed_keys = attributes.keys - sanitized_attributes.keys + if removed_keys.any? + logger.debug "WARNING: Can't mass-assign protected attributes: #{removed_keys.join(', ')}" + end + end + + def debug? + logger.present? + end + + end + end +end diff --git a/activerecord/lib/active_record/mass_assignment_security/sanitizer/authorizors.rb b/activerecord/lib/active_record/mass_assignment_security/sanitizer/authorizors.rb new file mode 100644 index 0000000..aa67692 --- /dev/null +++ b/activerecord/lib/active_record/mass_assignment_security/sanitizer/authorizors.rb @@ -0,0 +1,43 @@ +require 'active_record/mass_assignment_security/authorizor_list' +require 'active_record/mass_assignment_security/permission_set' + +module ActiveRecord + module MassAssignmentSecurity + module Authorizors + + def initialize(*args) + super + self.always_deny_authorizor = BlackList.new + self.authorizor = AuthorizorList.new + authorizor[:allow] = WhiteList.new + authorizor[:deny] = BlackList.new + authorizor.default = BlackList.new + end + + def allow(keys) + authorizor[:allow] += keys + end + + def allowed_keys + authorizor[:allow] + end + + def deny(keys) + authorizor[:deny] += keys + end + + def denied_keys + authorizor[:deny] + end + + def always_deny(keys) + self.always_deny_authorizor += keys + end + + def always_denied_keys + always_deny_authorizor + end + + end + end +end \ No newline at end of file diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 8421a8f..2b3383f 100755 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -71,9 +71,8 @@ class Task < ActiveRecord::Base attr_protected :starting end -class TopicWithProtectedContentAndAccessibleAuthorName < ActiveRecord::Base +class TopicWithProtectedContent < ActiveRecord::Base self.table_name = 'topics' - attr_accessible :author_name attr_protected :content end @@ -967,9 +966,9 @@ class BasicsTest < ActiveRecord::TestCase end def test_mass_assignment_should_raise_exception_if_accessible_and_protected_attribute_writers_are_both_used - topic = TopicWithProtectedContentAndAccessibleAuthorName.new - assert_raise(RuntimeError) { topic.attributes = { "author_name" => "me" } } - assert_raise(RuntimeError) { topic.attributes = { "content" => "stuff" } } + assert_raise(RuntimeError) do + TopicWithProtectedContent.attr_accessible :author_name + end end def test_mass_assignment_protection @@ -1032,19 +1031,19 @@ class BasicsTest < ActiveRecord::TestCase end def test_mass_assignment_protection_inheritance - assert_nil LoosePerson.accessible_attributes + assert LoosePerson.accessible_attributes.blank? assert_equal Set.new([ 'credit_rating', 'administrator' ]), LoosePerson.protected_attributes - assert_nil LooseDescendant.accessible_attributes + assert LooseDescendant.accessible_attributes.blank? assert_equal Set.new([ 'credit_rating', 'administrator', 'phone_number' ]), LooseDescendant.protected_attributes - assert_nil LooseDescendantSecond.accessible_attributes + assert LooseDescendantSecond.accessible_attributes.blank? assert_equal Set.new([ 'credit_rating', 'administrator', 'phone_number', 'name' ]), LooseDescendantSecond.protected_attributes, 'Running attr_protected twice in one class should merge the protections' - assert_nil TightPerson.protected_attributes + assert TightPerson.protected_attributes.blank? assert_equal Set.new([ 'name', 'address' ]), TightPerson.accessible_attributes - assert_nil TightDescendant.protected_attributes + assert TightDescendant.protected_attributes.blank? assert_equal Set.new([ 'name', 'address', 'phone_number' ]), TightDescendant.accessible_attributes end diff --git a/activerecord/test/cases/mass_assignment_security/authorizor_list_test.rb b/activerecord/test/cases/mass_assignment_security/authorizor_list_test.rb new file mode 100644 index 0000000..b478a85 --- /dev/null +++ b/activerecord/test/cases/mass_assignment_security/authorizor_list_test.rb @@ -0,0 +1,54 @@ +require "cases/helper" + +class AuthorizorListTest < ActiveRecord::TestCase + include ActiveRecord::MassAssignmentSecurity + + def setup + @authorizor_list = AuthorizorList.new + @authorizor_list.default = BlackList.new + end + + test "deny? is false when active authorizor allows key" do + active_authorizor = mock(:deny? => false) + @authorizor_list.stubs(:active_authorizor).returns(active_authorizor) + + assert_equal false, @authorizor_list.deny?('admin') + end + + test "deny? is true when active authorizor denies key" do + active_authorizor = mock(:deny? => true) + @authorizor_list.stubs(:active_authorizor).returns(active_authorizor) + + assert_equal true, @authorizor_list.deny?('admin') + end + + test "active authorizor is default when no populated authorizor is found" do + @authorizor_list[:empty] = [] + + assert_equal @authorizor_list.default, @authorizor_list.send(:active_authorizor) + end + + test "active authorizor is a populated authorizor" do + @authorizor_list[:empty] = [] + @authorizor_list[:populated] = [ true ] + + assert_equal @authorizor_list[:populated], @authorizor_list.send(:active_authorizor) + end + + test "do not memoize default authorizor" do + @authorizor_list.deny?('admin') + assert !@authorizor_list.instance_variable_get(:@active_authorizor_key), "Memoized default authorizor" + end + + test "memoize active authorizor" do + @authorizor_list[:deny] = BlackList.new + @authorizor_list[:deny] += [ 'admin' ] + @authorizor_list.deny?('admin') + + assert_equal :deny, @authorizor_list.instance_variable_get(:@active_authorizor_key) + + @authorizor_list[:deny] += [ 'price' ] + assert @authorizor_list.deny?('price'), "Authorizor not up-to-date" + end + +end diff --git a/activerecord/test/cases/mass_assignment_security/authorizors_test.rb b/activerecord/test/cases/mass_assignment_security/authorizors_test.rb new file mode 100644 index 0000000..314e3dd --- /dev/null +++ b/activerecord/test/cases/mass_assignment_security/authorizors_test.rb @@ -0,0 +1,50 @@ +require "cases/helper" + +class AuthorizorsTest < ActiveRecord::TestCase + include ActiveRecord::MassAssignmentSecurity + + def setup + @sanitizer = Sanitizer.new + end + + test "allow adds values to allowed_keys" do + @sanitizer.allow testable_keys + + assert_equal testable_keys, @sanitizer.allowed_keys.to_a + end + + test "deny adds values to denied_keys" do + @sanitizer.deny testable_keys + + assert_equal testable_keys, @sanitizer.denied_keys.to_a + end + + test "always_deny adds values to always_denied_keys" do + @sanitizer.always_deny testable_keys + + assert_equal testable_keys, @sanitizer.always_denied_keys.to_a + end + + test "deny by default keys is a blacklist" do + assert @sanitizer.always_denied_keys.is_a?(BlackList) + end + + test "denied keys is a blacklist" do + assert @sanitizer.denied_keys.is_a?(BlackList) + end + + test "allowed keys is a whitelist" do + assert @sanitizer.allowed_keys.is_a?(WhiteList) + end + + test "authorizor is an authorizor list" do + assert @sanitizer.__send__(:authorizor).is_a?(AuthorizorList) + end + + protected + + def testable_keys + [ 'first_name', 'last_name' ] + end + +end diff --git a/activerecord/test/cases/mass_assignment_security/black_list_test.rb b/activerecord/test/cases/mass_assignment_security/black_list_test.rb new file mode 100644 index 0000000..0ffe9ad --- /dev/null +++ b/activerecord/test/cases/mass_assignment_security/black_list_test.rb @@ -0,0 +1,24 @@ +require "cases/helper" + +class BlackListTest < ActiveRecord::TestCase + include ActiveRecord::MassAssignmentSecurity + + def setup + @black_list = 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 "is a permission set" do + assert @black_list.is_a?(PermissionSet) + 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 new file mode 100644 index 0000000..8bc0184 --- /dev/null +++ b/activerecord/test/cases/mass_assignment_security/permission_set_test.rb @@ -0,0 +1,37 @@ +require "cases/helper" + +class PermissionSetTest < ActiveRecord::TestCase + include ActiveRecord::MassAssignmentSecurity + + def setup + @permission_set = PermissionSet.new + end + + test "+ stringifies added collection values" do + key_collection = [ :admin ] + string_collection = Set[ 'admin' ] + @permission_set += key_collection + + assert_equal string_collection, @permission_set + end + + test "is a set" do + assert @permission_set.is_a?(Set) + end + + test "include? normalizes multi-parameter keys" do + multi_param_key = 'admin(1)' + @permission_set << 'admin' + + assert_equal true, @permission_set.include?(multi_param_key) + end + + test "include? non-multi-parameter keys" do + non_multi_param_key = 'admin' + @permission_set << non_multi_param_key + + assert_equal true, @permission_set.include?(non_multi_param_key) + assert_equal false, @permission_set.include?('not_included(1)') + end + +end diff --git a/activerecord/test/cases/mass_assignment_security/sanitizer_test.rb b/activerecord/test/cases/mass_assignment_security/sanitizer_test.rb new file mode 100644 index 0000000..32f08ad --- /dev/null +++ b/activerecord/test/cases/mass_assignment_security/sanitizer_test.rb @@ -0,0 +1,24 @@ +require "cases/helper" + +class SanitizerTest < ActiveRecord::TestCase + include ActiveRecord::MassAssignmentSecurity + + def setup + @sanitizer = Sanitizer.new + @sanitizer.logger = Logger.new(StringIO.new) + end + + test "sanitize attributes" do + authorizor = BlackList.new + authorizor += [ :admin ] + @sanitizer.stubs(:authorizor).returns(authorizor) + original_attributes = { 'first_name' => 'allowed', 'admin' => 'denied', 'admin(1)' => '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" + assert !attributes.key?('admin(1)'), "Multi-parameter key should be detected" + end + +end + \ No newline at end of file diff --git a/activerecord/test/cases/mass_assignment_security/white_list_test.rb b/activerecord/test/cases/mass_assignment_security/white_list_test.rb new file mode 100644 index 0000000..b25a660 --- /dev/null +++ b/activerecord/test/cases/mass_assignment_security/white_list_test.rb @@ -0,0 +1,24 @@ +require "cases/helper" + +class WhiteListTest < ActiveRecord::TestCase + include ActiveRecord::MassAssignmentSecurity + + def setup + @white_list = 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 "is a permission set" do + assert @white_list.is_a?(PermissionSet) + end + +end \ No newline at end of file -- 1.5.6.4