diff options
Diffstat (limited to 'activemodel')
10 files changed, 109 insertions, 49 deletions
diff --git a/activemodel/CHANGELOG b/activemodel/CHANGELOG index be4de2e53c..b1ad315c46 100644 --- a/activemodel/CHANGELOG +++ b/activemodel/CHANGELOG @@ -1,3 +1,5 @@ +* Provide mass_assignment_sanitizer as an easy API to replace the sanitizer behavior. Also support both :logger (default) and :strict sanitizer behavior [Bogdan Gusiev] + *Rails 3.1.0 (unreleased)* * attr_accessible and friends now accepts :as as option to specify a role [Josh Kalderimis] diff --git a/activemodel/lib/active_model/mass_assignment_security.rb b/activemodel/lib/active_model/mass_assignment_security.rb index 483b577681..a7b4706906 100644 --- a/activemodel/lib/active_model/mass_assignment_security.rb +++ b/activemodel/lib/active_model/mass_assignment_security.rb @@ -1,5 +1,7 @@ -require 'active_support/core_ext/class/attribute.rb' +require 'active_support/core_ext/class/attribute' +require 'active_support/core_ext/string/inflections' require 'active_model/mass_assignment_security/permission_set' +require 'active_model/mass_assignment_security/sanitizer' module ActiveModel # = Active Model Mass-Assignment Security @@ -10,6 +12,9 @@ module ActiveModel class_attribute :_accessible_attributes class_attribute :_protected_attributes class_attribute :_active_authorizer + + class_attribute :_mass_assignment_sanitizer + self.mass_assignment_sanitizer = :logger end # Mass assignment security provides an interface for protecting attributes @@ -41,6 +46,16 @@ module ActiveModel # # end # + # = Configuration options + # + # * <tt>mass_assignment_sanitizer</tt> - Defines sanitize method. Possible values are: + # * <tt>:logger</tt> (default) - writes filtered attributes to logger + # * <tt>:strict</tt> - raise <tt>ActiveModel::MassAssignmentSecurity::Error</tt> on any protected attribute update + # + # You can specify your own sanitizer object eg. MySanitizer.new. + # See <tt>ActiveModel::MassAssignmentSecurity::LoggerSanitizer</tt> for example implementation. + # + # module ClassMethods # Attributes named in this macro are protected from mass-assignment # whenever attributes are sanitized before assignment. A role for the @@ -154,7 +169,7 @@ module ActiveModel options = args.extract_options! role = options[:as] || :default - self._accessible_attributes = accessible_attributes_configs.dup + self._accessible_attributes = accessible_attributes_configs.dup self._accessible_attributes[role] = self.accessible_attributes(role) + args self._active_authorizer = self._accessible_attributes @@ -177,21 +192,25 @@ module ActiveModel [] end + def mass_assignment_sanitizer=(value) + self._mass_assignment_sanitizer = if value.is_a?(Symbol) + const_get(:"#{value.to_s.camelize}Sanitizer").new(self) + else + value + end + end + private def protected_attributes_configs self._protected_attributes ||= begin - default_black_list = BlackList.new(attributes_protected_by_default).tap do |w| - w.logger = self.logger if self.respond_to?(:logger) - end - Hash.new(default_black_list) + Hash.new { |h,k| h[k] = BlackList.new(attributes_protected_by_default) } end end def accessible_attributes_configs self._accessible_attributes ||= begin - default_white_list = WhiteList.new.tap { |w| w.logger = self.logger if self.respond_to?(:logger) } - Hash.new(default_white_list) + Hash.new { |h,k| h[k] = WhiteList.new } end end end @@ -199,7 +218,7 @@ module ActiveModel protected def sanitize_for_mass_assignment(attributes, role = :default) - mass_assignment_authorizer(role).sanitize(attributes) + _mass_assignment_sanitizer.sanitize(attributes, mass_assignment_authorizer(role)) end def mass_assignment_authorizer(role = :default) diff --git a/activemodel/lib/active_model/mass_assignment_security/permission_set.rb b/activemodel/lib/active_model/mass_assignment_security/permission_set.rb index 9fcb94d48a..a1fcdf1a38 100644 --- a/activemodel/lib/active_model/mass_assignment_security/permission_set.rb +++ b/activemodel/lib/active_model/mass_assignment_security/permission_set.rb @@ -1,10 +1,8 @@ require 'set' -require 'active_model/mass_assignment_security/sanitizer' module ActiveModel module MassAssignmentSecurity class PermissionSet < Set - attr_accessor :logger def +(values) super(values.map(&:to_s)) @@ -14,6 +12,10 @@ module ActiveModel super(remove_multiparameter_id(key)) end + def deny?(key) + raise NotImplementedError, "#deny?(key) suppose to be overwritten" + end + protected def remove_multiparameter_id(key) @@ -22,7 +24,6 @@ module ActiveModel end class WhiteList < PermissionSet - include Sanitizer def deny?(key) !include?(key) @@ -30,7 +31,6 @@ module ActiveModel end class BlackList < PermissionSet - include Sanitizer def deny?(key) include?(key) diff --git a/activemodel/lib/active_model/mass_assignment_security/sanitizer.rb b/activemodel/lib/active_model/mass_assignment_security/sanitizer.rb index 150beb1ff2..ee43a6694f 100644 --- a/activemodel/lib/active_model/mass_assignment_security/sanitizer.rb +++ b/activemodel/lib/active_model/mass_assignment_security/sanitizer.rb @@ -1,9 +1,14 @@ +require 'active_support/core_ext/module/delegation' + module ActiveModel module MassAssignmentSecurity - module Sanitizer + class Sanitizer + def initialize(target=nil) + end + # Returns all attributes not denied by the authorizer. - def sanitize(attributes) - sanitized_attributes = attributes.reject { |key, value| deny?(key) } + def sanitize(attributes, authorizer) + sanitized_attributes = attributes.reject { |key, value| authorizer.deny?(key) } debug_protected_attribute_removal(attributes, sanitized_attributes) sanitized_attributes end @@ -12,12 +17,38 @@ module ActiveModel def debug_protected_attribute_removal(attributes, sanitized_attributes) removed_keys = attributes.keys - sanitized_attributes.keys - warn!(removed_keys) if removed_keys.any? + process_removed_attributes(removed_keys) if removed_keys.any? + end + + def process_removed_attributes(attrs) + raise NotImplementedError, "#process_removed_attributes(attrs) suppose to be overwritten" + end + end + + class LoggerSanitizer < Sanitizer + delegate :logger, :to => :@target + + def initialize(target) + @target = target + super end - def warn!(attrs) - self.logger.debug "WARNING: Can't mass-assign protected attributes: #{attrs.join(', ')}" if self.logger + def logger? + @target.respond_to?(:logger) && @target.logger end + + def process_removed_attributes(attrs) + logger.debug "WARNING: Can't mass-assign protected attributes: #{attrs.join(', ')}" if logger? + end + end + + class StrictSanitizer < Sanitizer + def process_removed_attributes(attrs) + raise ActiveModel::MassAssignmentSecurity::Error, "Can't mass-assign protected attributes: #{attrs.join(', ')}" + end + end + + class Error < StandardError end end end diff --git a/activemodel/lib/active_model/validations/exclusion.rb b/activemodel/lib/active_model/validations/exclusion.rb index ab4de3c459..d3b8d31502 100644 --- a/activemodel/lib/active_model/validations/exclusion.rb +++ b/activemodel/lib/active_model/validations/exclusion.rb @@ -1,4 +1,4 @@ -require 'active_support/core_ext/range.rb' +require 'active_support/core_ext/range' module ActiveModel diff --git a/activemodel/lib/active_model/validations/inclusion.rb b/activemodel/lib/active_model/validations/inclusion.rb index bfb65d2f3f..9a9270d615 100644 --- a/activemodel/lib/active_model/validations/inclusion.rb +++ b/activemodel/lib/active_model/validations/inclusion.rb @@ -1,4 +1,4 @@ -require 'active_support/core_ext/range.rb' +require 'active_support/core_ext/range' module ActiveModel diff --git a/activemodel/test/cases/mass_assignment_security/black_list_test.rb b/activemodel/test/cases/mass_assignment_security/black_list_test.rb index ed168bc016..0ec7f8719c 100644 --- a/activemodel/test/cases/mass_assignment_security/black_list_test.rb +++ b/activemodel/test/cases/mass_assignment_security/black_list_test.rb @@ -16,13 +16,5 @@ class BlackListTest < ActiveModel::TestCase 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/activemodel/test/cases/mass_assignment_security/sanitizer_test.rb b/activemodel/test/cases/mass_assignment_security/sanitizer_test.rb index 9a73a5ad91..62a6ec9c9b 100644 --- a/activemodel/test/cases/mass_assignment_security/sanitizer_test.rb +++ b/activemodel/test/cases/mass_assignment_security/sanitizer_test.rb @@ -3,36 +3,41 @@ require 'logger' require 'active_support/core_ext/object/inclusion' class SanitizerTest < ActiveModel::TestCase + attr_accessor :logger - class SanitizingAuthorizer - include ActiveModel::MassAssignmentSecurity::Sanitizer - - attr_accessor :logger - + class Authorizer < ActiveModel::MassAssignmentSecurity::PermissionSet def deny?(key) key.in?(['admin']) end - end def setup - @sanitizer = SanitizingAuthorizer.new + @logger_sanitizer = ActiveModel::MassAssignmentSecurity::LoggerSanitizer.new(self) + @strict_sanitizer = ActiveModel::MassAssignmentSecurity::StrictSanitizer.new(self) + @authorizer = Authorizer.new end test "sanitize attributes" do original_attributes = { 'first_name' => 'allowed', 'admin' => 'denied' } - attributes = @sanitizer.sanitize(original_attributes) + attributes = @logger_sanitizer.sanitize(original_attributes, @authorizer) 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 + test "debug mass assignment removal with LoggerSanitizer" do original_attributes = { 'first_name' => 'allowed', 'admin' => 'denied' } log = StringIO.new - @sanitizer.logger = Logger.new(log) - @sanitizer.sanitize(original_attributes) + self.logger = Logger.new(log) + @logger_sanitizer.sanitize(original_attributes, @authorizer) assert_match(/admin/, log.string, "Should log removed attributes: #{log.string}") end + test "debug mass assignment removal with StrictSanitizer" do + original_attributes = { 'first_name' => 'allowed', 'admin' => 'denied' } + assert_raise ActiveModel::MassAssignmentSecurity::Error do + @strict_sanitizer.sanitize(original_attributes, @authorizer) + end + end + end diff --git a/activemodel/test/cases/mass_assignment_security/white_list_test.rb b/activemodel/test/cases/mass_assignment_security/white_list_test.rb index aa3596ad2a..737b55492a 100644 --- a/activemodel/test/cases/mass_assignment_security/white_list_test.rb +++ b/activemodel/test/cases/mass_assignment_security/white_list_test.rb @@ -16,13 +16,4 @@ class WhiteListTest < ActiveModel::TestCase 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/activemodel/test/cases/mass_assignment_security_test.rb b/activemodel/test/cases/mass_assignment_security_test.rb index 43a12eed61..a778240827 100644 --- a/activemodel/test/cases/mass_assignment_security_test.rb +++ b/activemodel/test/cases/mass_assignment_security_test.rb @@ -1,6 +1,15 @@ require "cases/helper" require 'models/mass_assignment_specific' + +class CustomSanitizer < ActiveModel::MassAssignmentSecurity::Sanitizer + + def process_removed_attributes(attrs) + raise StandardError + end + +end + class MassAssignmentSecurityTest < ActiveModel::TestCase def test_attribute_protection @@ -76,4 +85,15 @@ class MassAssignmentSecurityTest < ActiveModel::TestCase assert_equal sanitized, { } end + def test_custom_sanitizer + user = User.new + User.mass_assignment_sanitizer = CustomSanitizer.new + assert_raise StandardError do + user.sanitize_for_mass_assignment("admin" => true) + end + ensure + User.mass_assignment_sanitizer = nil + + end + end |