From 7c86e8e21ba6a1f88226ddd0cf012a563f234d06 Mon Sep 17 00:00:00 2001 From: Josh Kalderimis Date: Wed, 7 Jul 2010 17:05:42 +0200 Subject: minor changes to mass assignment security patch to bring it in line with rails standards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- activerecord/lib/active_record/base.rb | 4 +- .../lib/active_record/mass_assignment_security.rb | 198 ++++++++++----------- .../mass_assignment_security/permission_set.rb | 7 +- .../mass_assignment_security/sanitizer.rb | 8 +- 4 files changed, 99 insertions(+), 118 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index affb1fee3c..56b4ba8260 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1480,7 +1480,7 @@ MSG attributes = new_attributes.stringify_keys multi_parameter_attributes = [] - attributes = remove_attributes_protected_from_mass_assignment(attributes) if guard_protected_attributes + attributes = sanitize_for_mass_assignment(attributes) if guard_protected_attributes attributes.each do |k, v| if k.include?("(") @@ -1797,7 +1797,7 @@ MSG include AttributeMethods::PrimaryKey include AttributeMethods::TimeZoneConversion include AttributeMethods::Dirty - extend MassAssignmentSecurity + include 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 index 07beb6405e..8f4d6e1c74 100644 --- a/activerecord/lib/active_record/mass_assignment_security.rb +++ b/activerecord/lib/active_record/mass_assignment_security.rb @@ -1,7 +1,16 @@ 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, @@ -11,7 +20,7 @@ module ActiveRecord # on their role: # # class AccountsController < ApplicationController - # extend ActiveRecord::MassAssignmentSecurity + # include ActiveRecord::MassAssignmentSecurity # # attr_accessible :first_name, :last_name # @@ -28,7 +37,7 @@ module ActiveRecord # protected # # def account_params - # remove_attributes_protected_from_mass_assignment(params[:account]) + # sanitize_for_mass_assignment(params[:account]) # end # # def mass_assignment_authorizer @@ -37,123 +46,96 @@ module ActiveRecord # # end # - def self.extended(base) - base.send(:include, InstanceMethods) - end - - module InstanceMethods - - protected - - def remove_attributes_protected_from_mass_assignment(attributes) - mass_assignment_authorizer.sanitize(attributes) - end - - def mass_assignment_authorizer - self.class.mass_assignment_authorizer - end + module ClassMethods + # 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+. + # + # Note that using Hash#except or Hash#slice 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 - 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" + # + # Note that using Hash#except or Hash#slice 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 - # 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+. - # - # Note that using Hash#except or Hash#slice in place of +attr_protected+ - # to sanitize attributes won't provide sufficient protection. - def attr_protected(*keys) - use_authorizer(:protected_attributes) - protected_attributes.merge(keys) - end + def protected_attributes + self._protected_attributes ||= BlackList.new(attributes_protected_by_default).tap { |w| w.logger = logger } + 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" - # - # Note that using Hash#except or Hash#slice in place of +attr_accessible+ - # to sanitize attributes won't provide sufficient protection. - def attr_accessible(*keys) - use_authorizer(:accessible_attributes) - accessible_attributes.merge(keys) - end + def accessible_attributes + self._accessible_attributes ||= WhiteList.new.tap { |w| w.logger = logger } + end - # Returns an array of all the attributes that have been protected from mass-assignment. - def protected_attributes - read_inheritable_attribute(:protected_attributes) || begin - authorizer = BlackList.new - authorizer += attributes_protected_by_default - authorizer.logger = logger - write_inheritable_attribute(:protected_attributes, authorizer) + def active_authorizer + self._active_authorizer ||= protected_attributes end - end - # Returns an array of all the attributes that have been made accessible to mass-assignment. - def accessible_attributes - read_inheritable_attribute(:accessible_attributes) || begin - authorizer = WhiteList.new - authorizer.logger = logger - write_inheritable_attribute(:accessible_attributes, authorizer) + def attributes_protected_by_default + [] end end - def mass_assignment_authorizer - protected_attributes - end + protected - private + def sanitize_for_mass_assignment(attributes) + mass_assignment_authorizer.sanitize(attributes) + end - # Sets the active authorizer, (attr_protected or attr_accessible). Subsequent calls - # will raise an exception when using a different authorizer_id. - def use_authorizer(authorizer_id) # :nodoc: - if active_authorizer_id = read_inheritable_attribute(:active_authorizer_id) - unless authorizer_id == active_authorizer_id - raise("Already using #{active_authorizer_id}, cannot use #{authorizer_id}") - end - else - write_inheritable_attribute(:active_authorizer_id, authorizer_id) - end + def mass_assignment_authorizer + self.class.active_authorizer 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 index 1d34dce02e..8446a4103b 100644 --- a/activerecord/lib/active_record/mass_assignment_security/permission_set.rb +++ b/activerecord/lib/active_record/mass_assignment_security/permission_set.rb @@ -2,11 +2,11 @@ require 'active_record/mass_assignment_security/sanitizer' module ActiveRecord module MassAssignmentSecurity - class PermissionSet < Set + class PermissionSet < Set attr_accessor :logger - def merge(values) + def +(values) super(values.map(&:to_s)) end @@ -19,7 +19,6 @@ module ActiveRecord def remove_multiparameter_id(key) key.gsub(/\(.+/, '') end - end class WhiteList < PermissionSet @@ -28,7 +27,6 @@ module ActiveRecord def deny?(key) !include?(key) end - end class BlackList < PermissionSet @@ -37,7 +35,6 @@ module ActiveRecord def deny?(key) include?(key) 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 index 4a099a147c..11de35f9d6 100644 --- a/activerecord/lib/active_record/mass_assignment_security/sanitizer.rb +++ b/activerecord/lib/active_record/mass_assignment_security/sanitizer.rb @@ -13,15 +13,17 @@ module ActiveRecord 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 + 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 -- cgit v1.2.3