1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00

Merge branch 'master' of github.com:rails/rails

This commit is contained in:
David Heinemeier Hansson 2011-04-24 20:33:33 -05:00
commit b306502286
14 changed files with 312 additions and 75 deletions

View file

@ -24,10 +24,7 @@ module ActiveModel
# include ActiveModel::MassAssignmentSecurity
#
# attr_accessible :first_name, :last_name
#
# def self.admin_accessible_attributes
# accessible_attributes + [ :plan_id ]
# end
# attr_accessible :first_name, :last_name, :plan_id, :as => :admin
#
# def update
# ...
@ -38,18 +35,17 @@ module ActiveModel
# protected
#
# def account_params
# sanitize_for_mass_assignment(params[:account])
# end
#
# def mass_assignment_authorizer
# admin ? admin_accessible_attributes : super
# scope = admin ? :admin : :default
# sanitize_for_mass_assignment(params[:account], scope)
# end
#
# end
#
module ClassMethods
# Attributes named in this macro are protected from mass-assignment
# whenever attributes are sanitized before assignment.
# whenever attributes are sanitized before assignment. A scope for the
# attributes is optional, if no scope is provided then :default is used.
# A scope can be defined by using the :as option.
#
# Mass-assignment to these attributes will simply be ignored, to assign
# to them you can use direct writer methods. This is meant to protect
@ -60,36 +56,58 @@ module ActiveModel
# include ActiveModel::MassAssignmentSecurity
#
# attr_accessor :name, :credit_rating
# attr_protected :credit_rating
#
# def attributes=(values)
# sanitize_for_mass_assignment(values).each do |k, v|
# attr_protected :credit_rating, :last_login
# attr_protected :last_login, :as => :admin
#
# def assign_attributes(values, options = {})
# sanitize_for_mass_assignment(values, options[:as]).each do |k, v|
# send("#{k}=", v)
# end
# end
# end
#
# When using a :default scope :
#
# customer = Customer.new
# customer.attributes = { "name" => "David", "credit_rating" => "Excellent" }
# customer.assign_attributes({ "name" => "David", "credit_rating" => "Excellent", :last_login => 1.day.ago }, :as => :default)
# customer.name # => "David"
# customer.credit_rating # => nil
# customer.last_login # => nil
#
# customer.credit_rating = "Average"
# customer.credit_rating # => "Average"
#
# And using the :admin scope :
#
# customer = Customer.new
# customer.assign_attributes({ "name" => "David", "credit_rating" => "Excellent", :last_login => 1.day.ago }, :as => :admin)
# customer.name # => "David"
# customer.credit_rating # => "Excellent"
# customer.last_login # => nil
#
# 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
def attr_protected(*args)
options = args.extract_options!
scope = options[:as] || :default
self._protected_attributes = protected_attributes_configs.dup
self._protected_attributes[scope] = self.protected_attributes(scope) + args
self._active_authorizer = self._protected_attributes
end
# Specifies a white list of model attributes that can be set via
# mass-assignment.
#
# Like +attr_protected+, a scope for the attributes is optional,
# if no scope is provided then :default is used. A scope can be defined by
# using the :as option.
#
# 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
@ -102,57 +120,90 @@ module ActiveModel
# include ActiveModel::MassAssignmentSecurity
#
# attr_accessor :name, :credit_rating
# attr_accessible :name
#
# def attributes=(values)
# sanitize_for_mass_assignment(values).each do |k, v|
# attr_accessible :name
# attr_accessible :name, :credit_rating, :as => :admin
#
# def assign_attributes(values, options = {})
# sanitize_for_mass_assignment(values, options[:as]).each do |k, v|
# send("#{k}=", v)
# end
# end
# end
#
# When using a :default scope :
#
# customer = Customer.new
# customer.attributes = { :name => "David", :credit_rating => "Excellent" }
# customer.assign_attributes({ "name" => "David", "credit_rating" => "Excellent", :last_login => 1.day.ago }, :as => :default)
# customer.name # => "David"
# customer.credit_rating # => nil
#
# customer.credit_rating = "Average"
# customer.credit_rating # => "Average"
#
# And using the :admin scope :
#
# customer = Customer.new
# customer.assign_attributes({ "name" => "David", "credit_rating" => "Excellent", :last_login => 1.day.ago }, :as => :admin)
# customer.name # => "David"
# customer.credit_rating # => "Excellent"
#
# 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
def attr_accessible(*args)
options = args.extract_options!
scope = options[:as] || :default
self._accessible_attributes = accessible_attributes_configs.dup
self._accessible_attributes[scope] = self.accessible_attributes(scope) + args
self._active_authorizer = self._accessible_attributes
end
def protected_attributes
self._protected_attributes ||= BlackList.new(attributes_protected_by_default).tap do |w|
w.logger = self.logger if self.respond_to?(:logger)
end
def protected_attributes(scope = :default)
protected_attributes_configs[scope]
end
def accessible_attributes
self._accessible_attributes ||= WhiteList.new.tap { |w| w.logger = self.logger if self.respond_to?(:logger) }
def accessible_attributes(scope = :default)
accessible_attributes_configs[scope]
end
def active_authorizer
self._active_authorizer ||= protected_attributes
def active_authorizers
self._active_authorizer ||= protected_attributes_configs
end
alias active_authorizer active_authorizers
def attributes_protected_by_default
[]
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)
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)
end
end
end
protected
def sanitize_for_mass_assignment(attributes)
mass_assignment_authorizer.sanitize(attributes)
def sanitize_for_mass_assignment(attributes, scope = :default)
mass_assignment_authorizer(scope).sanitize(attributes)
end
def mass_assignment_authorizer
self.class.active_authorizer
def mass_assignment_authorizer(scope = :default)
self.class.active_authorizer[scope]
end
end
end

View file

@ -10,10 +10,27 @@ class MassAssignmentSecurityTest < ActiveModel::TestCase
assert_equal expected, sanitized
end
def test_only_moderator_scope_attribute_accessible
user = SpecialUser.new
expected = { "name" => "John Smith", "email" => "john@smith.com" }
sanitized = user.sanitize_for_mass_assignment(expected.merge("admin" => true), :moderator)
assert_equal expected, sanitized
sanitized = user.sanitize_for_mass_assignment({ "name" => "John Smith", "email" => "john@smith.com", "admin" => true })
assert_equal({}, sanitized)
end
def test_attributes_accessible
user = Person.new
expected = { "name" => "John Smith", "email" => "john@smith.com" }
sanitized = user.sanitize_for_mass_assignment(expected.merge("super_powers" => true))
sanitized = user.sanitize_for_mass_assignment(expected.merge("admin" => true))
assert_equal expected, sanitized
end
def test_admin_scoped_attributes_accessible
user = Person.new
expected = { "name" => "John Smith", "email" => "john@smith.com", "admin" => true }
sanitized = user.sanitize_for_mass_assignment(expected.merge("super_powers" => true), :admin)
assert_equal expected, sanitized
end
@ -26,20 +43,30 @@ class MassAssignmentSecurityTest < ActiveModel::TestCase
def test_mass_assignment_protection_inheritance
assert_blank LoosePerson.accessible_attributes
assert_equal Set.new([ 'credit_rating', 'administrator']), LoosePerson.protected_attributes
assert_equal Set.new(['credit_rating', 'administrator']), LoosePerson.protected_attributes
assert_blank LoosePerson.accessible_attributes
assert_equal Set.new(['credit_rating']), LoosePerson.protected_attributes(:admin)
assert_blank LooseDescendant.accessible_attributes
assert_equal Set.new([ 'credit_rating', 'administrator', 'phone_number']), LooseDescendant.protected_attributes
assert_equal Set.new(['credit_rating', 'administrator', 'phone_number']), LooseDescendant.protected_attributes
assert_blank LooseDescendantSecond.accessible_attributes
assert_equal Set.new([ 'credit_rating', 'administrator', 'phone_number', 'name']), LooseDescendantSecond.protected_attributes,
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_blank TightPerson.protected_attributes - TightPerson.attributes_protected_by_default
assert_equal Set.new([ 'name', 'address' ]), TightPerson.accessible_attributes
assert_equal Set.new(['name', 'address']), TightPerson.accessible_attributes
assert_blank TightPerson.protected_attributes(:admin) - TightPerson.attributes_protected_by_default
assert_equal Set.new(['name', 'address', 'admin']), TightPerson.accessible_attributes(:admin)
assert_blank TightDescendant.protected_attributes - TightDescendant.attributes_protected_by_default
assert_equal Set.new([ 'name', 'address', 'phone_number' ]), TightDescendant.accessible_attributes
assert_equal Set.new(['name', 'address', 'phone_number']), TightDescendant.accessible_attributes
assert_blank TightDescendant.protected_attributes(:admin) - TightDescendant.attributes_protected_by_default
assert_equal Set.new(['name', 'address', 'admin', 'super_powers']), TightDescendant.accessible_attributes(:admin)
end
def test_mass_assignment_multiparameter_protector

View file

@ -45,13 +45,14 @@ class SecurePasswordTest < ActiveModel::TestCase
end
test "visitor#password_digest should be protected against mass assignment" do
assert Visitor.active_authorizer.kind_of?(ActiveModel::MassAssignmentSecurity::BlackList)
assert Visitor.active_authorizer.include?(:password_digest)
assert Visitor.active_authorizers[:default].kind_of?(ActiveModel::MassAssignmentSecurity::BlackList)
assert Visitor.active_authorizers[:default].include?(:password_digest)
end
test "Administrator's mass_assignment_authorizer should be WhiteList" do
assert Administrator.active_authorizer.kind_of?(ActiveModel::MassAssignmentSecurity::WhiteList)
assert !Administrator.active_authorizer.include?(:password_digest)
assert Administrator.active_authorizer.include?(:name)
active_authorizer = Administrator.active_authorizers[:default]
assert active_authorizer.kind_of?(ActiveModel::MassAssignmentSecurity::WhiteList)
assert !active_authorizer.include?(:password_digest)
assert active_authorizer.include?(:name)
end
end

View file

@ -5,9 +5,17 @@ class User
public :sanitize_for_mass_assignment
end
class SpecialUser
include ActiveModel::MassAssignmentSecurity
attr_accessible :name, :email, :as => :moderator
public :sanitize_for_mass_assignment
end
class Person
include ActiveModel::MassAssignmentSecurity
attr_accessible :name, :email
attr_accessible :name, :email, :admin, :as => :admin
public :sanitize_for_mass_assignment
end
@ -32,6 +40,7 @@ end
class LoosePerson
include ActiveModel::MassAssignmentSecurity
attr_protected :credit_rating, :administrator
attr_protected :credit_rating, :as => :admin
end
class LooseDescendant < LoosePerson
@ -46,6 +55,7 @@ end
class TightPerson
include ActiveModel::MassAssignmentSecurity
attr_accessible :name, :address
attr_accessible :name, :address, :admin, :as => :admin
def self.attributes_protected_by_default
["mobile_number"]
@ -54,4 +64,5 @@ end
class TightDescendant < TightPerson
attr_accessible :phone_number
attr_accessible :super_powers, :as => :admin
end

View file

@ -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?("(")

View file

@ -50,6 +50,9 @@ module ActiveRecord
initializer "active_record.set_configs" do |app|
ActiveSupport.on_load(:active_record) do
if app.config.active_record.delete(:whitelist_attributes)
attr_accessible(nil)
end
app.config.active_record.each do |k,v|
send "#{k}=", v
end

View file

@ -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'

View file

@ -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

View file

@ -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'

View file

@ -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

View file

@ -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

View file

@ -229,6 +229,8 @@ h4. Configuring Active Record
* +config.active_record.lock_optimistically+ controls whether ActiveRecord will use optimistic locking. By default this is +true+.
* +config.active_record.whitelist_attributes+ will create an empty whitelist of attributes available for mass-assignment security for all models in your app.
The MySQL adapter adds one additional configuration option:
* +ActiveRecord::ConnectionAdapters::MysqlAdapter.emulate_booleans+ controls whether ActiveRecord will consider all +tinyint(1)+ columns in a MySQL database to be booleans. By default this is +true+.

View file

@ -418,10 +418,17 @@ To avoid this, Rails provides two class methods in your Active Record class to c
attr_protected :admin
</ruby>
+attr_protected+ also optionally takes a scope option using :as which allows you to define multiple mass-assignment groupings. If no scope is defined then attributes will be added to the default group.
<ruby>
attr_protected :last_login, :as => :admin
</ruby>
A much better way, because it follows the whitelist-principle, is the +attr_accessible+ method. It is the exact opposite of +attr_protected+, because _(highlight)it takes a list of attributes that will be accessible_. All other attributes will be protected. This way you won't forget to protect attributes when adding new ones in the course of development. Here is an example:
<ruby>
attr_accessible :name
attr_accessible :name, :is_admin, :as => :admin
</ruby>
If you want to set a protected attribute, you will to have to assign it individually:
@ -434,13 +441,31 @@ params[:user] # => {:name => "ow3ned", :admin => true}
@user.admin # => true
</ruby>
A more paranoid technique to protect your whole project would be to enforce that all models whitelist their accessible attributes. This can be easily achieved with a very simple initializer:
When assigning attributes in Active Record using +new+, +attributes=+, or +update_attributes+ the :default scope will be used. To assign attributes using different scopes you should use +assign_attributes+ which accepts an optional :as options parameter. If no :as option is provided then the :default scope will be used. You can also bypass mass-assignment security by using the +:without_protection+ option. Here is an example:
<ruby>
ActiveRecord::Base.send(:attr_accessible, nil)
@user = User.new
@user.assign_attributes({ :name => 'Josh', :is_admin => true })
@user.name # => Josh
@user.is_admin # => false
@user.assign_attributes({ :name => 'Josh', :is_admin => true }, :as => :admin)
@user.name # => Josh
@user.is_admin # => true
@user.assign_attributes({ :name => 'Josh', :is_admin => true }, :without_protection => true)
@user.name # => Josh
@user.is_admin # => true
</ruby>
This will create an empty whitelist of attributes available for mass assignment for all models in your app. As such, your models will need to explicitly whitelist accessible parameters by using an +attr_accessible+ declaration. This technique is best applied at the start of a new project. However, for an existing project with a thorough set of functional tests, it should be straightforward and relatively quick to insert this initializer, run your tests, and expose each attribute (via +attr_accessible+) as dictated by your failing tests.
A more paranoid technique to protect your whole project would be to enforce that all models define their accessible attributes. This can be easily achieved with a very simple application config option of:
<ruby>
config.active_record.whitelist_attributes = true
</ruby>
This will create an empty whitelist of attributes available for mass-assignment for all models in your app. As such, your models will need to explicitly whitelist or blacklist accessible parameters by using an +attr_accessible+ or +attr_protected+ declaration. This technique is best applied at the start of a new project. However, for an existing project with a thorough set of functional tests, it should be straightforward and relatively quick to use this application config option; run your tests, and expose each attribute (via +attr_accessible+ or +attr_protected+) as dictated by your failing tests.
h3. User Management

View file

@ -258,6 +258,18 @@ module ApplicationTests
assert_equal res, last_response.body # value should be unchanged
end
test "sets all Active Record models to whitelist all attributes by default" do
add_to_config <<-RUBY
config.active_record.whitelist_attributes = true
RUBY
require "#{app_path}/config/environment"
assert_equal ActiveModel::MassAssignmentSecurity::WhiteList,
ActiveRecord::Base.active_authorizers[:default].class
assert_equal [""], ActiveRecord::Base.active_authorizers[:default].to_a
end
test "registers interceptors with ActionMailer" do
add_to_config <<-RUBY
config.action_mailer.interceptors = MyMailInterceptor