Skip to content
Browse files

Added mass-assignment security :as and :without_protection support to…

… AR.new and AR.create
  • Loading branch information...
1 parent b8ccd05 commit 7c5ae0a88fc9406857ee362c827c57eb23fd5f95 @joshk joshk committed May 1, 2011
Showing with 142 additions and 29 deletions.
  1. +28 −5 activerecord/lib/active_record/base.rb
  2. +114 −24 activerecord/test/cases/mass_assignment_security_test.rb
View
33 activerecord/lib/active_record/base.rb
@@ -475,10 +475,19 @@ def find_by_sql(sql, binds = [])
# The +attributes+ parameter can be either be a Hash or an Array of Hashes. These Hashes describe the
# attributes on the objects that are to be created.
#
+ # +create+ respects mass-assignment security and accepts either +:as+ or +:without_protection+ options
+ # in the +options+ parameter.
+ #
# ==== Examples
# # Create a single new object
# User.create(:first_name => 'Jamie')
#
+ # # Create a single new object using the :admin mass-assignment security scope
+ # User.create({ :first_name => 'Jamie', :is_admin => true }, :as => :admin)
+ #
+ # # Create a single new object bypassing mass-assignment security
+ # User.create({ :first_name => 'Jamie', :is_admin => true }, :without_protection => true)
+ #
# # Create an Array of new objects
# User.create([{ :first_name => 'Jamie' }, { :first_name => 'Jeremy' }])
#
@@ -491,11 +500,11 @@ def find_by_sql(sql, binds = [])
# User.create([{ :first_name => 'Jamie' }, { :first_name => 'Jeremy' }]) do |u|
# u.is_admin = false
# end
- def create(attributes = nil, &block)
+ def create(attributes = nil, options = {}, &block)
@pixeltrix
Ruby on Rails member
pixeltrix added a note May 12, 2011
@joshk
joshk added a note May 12, 2011

I'll get this fixed up right now, thanks for spotting that Andrew!

@joshk
joshk added a note May 12, 2011

Thanks @pixeltrix

#524

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
if attributes.is_a?(Array)
- attributes.collect { |attr| create(attr, &block) }
+ attributes.collect { |attr| create(attr, options, &block) }
else
- object = new(attributes)
+ object = new(attributes, options)
yield(object) if block_given?
object.save
object
@@ -1484,7 +1493,20 @@ def encode_quoted_value(value) #:nodoc:
# attributes but not yet saved (pass a hash with key names matching the associated table column names).
# In both instances, valid attribute keys are determined by the column names of the associated table --
# hence you can't have attributes that aren't part of the table columns.
- def initialize(attributes = nil)
+ #
+ # +initialize+ respects mass-assignment security and accepts either +:as+ or +:without_protection+ options
+ # in the +options+ parameter.
+ #
+ # ==== Examples
+ # # Instantiates a single new object
+ # User.new(:first_name => 'Jamie')
+ #
+ # # Instantiates a single new object using the :admin mass-assignment security scope
+ # User.new({ :first_name => 'Jamie', :is_admin => true }, :as => :admin)
+ #
+ # # Instantiates a single new object bypassing mass-assignment security
+ # User.new({ :first_name => 'Jamie', :is_admin => true }, :without_protection => true)
+ def initialize(attributes = nil, options = {})
@pixeltrix
Ruby on Rails member
pixeltrix added a note May 12, 2011

This is going to break everybody who has overridden initialize in their models and there isn't even a mention of it in the CHANGELOG.

@joshk
joshk added a note May 12, 2011

That is a good point, I will add better notes to the AR changelog describing this change better. Thanks for spotting this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@attributes = attributes_from_column_definition
@association_cache = {}
@aggregation_cache = {}
@@ -1500,7 +1522,8 @@ def initialize(attributes = nil)
set_serialized_attributes
populate_with_current_scope_attributes
- self.attributes = attributes unless attributes.nil?
+
+ assign_attributes(attributes, options) if attributes
result = yield self if block_given?
run_callbacks :initialize
View
138 activerecord/test/cases/mass_assignment_security_test.rb
@@ -7,6 +7,12 @@
class MassAssignmentSecurityTest < ActiveRecord::TestCase
+ def setup
+ # another AR test modifies the columns which causes issues with create calls
+ TightPerson.reset_column_information
+ LoosePerson.reset_column_information
+ end
+
def test_customized_primary_key_remains_protected
subscriber = Subscriber.new(:nick => 'webster123', :name => 'nice try')
assert_nil subscriber.id
@@ -35,60 +41,114 @@ 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 'm', p.gender
- assert_equal nil, p.comments
+ assert_default_attributes(p)
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 'm', p.gender
- assert_equal 'rides a sweet bike', p.comments
+ assert_all_attributes(p)
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 'm', p.gender
- assert_equal nil, p.comments
+ assert_default_attributes(p)
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 'm', p.gender
- assert_equal 'rides a sweet bike', p.comments
+ assert_admin_attributes(p)
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 'm', p.gender
- assert_equal nil, p.comments
+ assert_default_attributes(p)
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 'm', p.gender
- assert_equal 'rides a sweet bike', p.comments
+ assert_admin_attributes(p)
+ end
+
+ def test_new_with_attr_accessible_attributes
+ p = TightPerson.new(attributes_hash)
+
+ assert_default_attributes(p)
+ end
+
+ def test_new_with_attr_protected_attributes
+ p = LoosePerson.new(attributes_hash)
+
+ assert_default_attributes(p)
+ end
+
+ def test_create_with_attr_accessible_attributes
+ p = TightPerson.create(attributes_hash)
+
+ assert_default_attributes(p, true)
+ end
+
+ def test_create_with_attr_protected_attributes
+ p = LoosePerson.create(attributes_hash)
+
+ assert_default_attributes(p, true)
+ end
+
+ def test_new_with_admin_scope_with_attr_accessible_attributes
+ p = TightPerson.new(attributes_hash, :as => :admin)
+
+ assert_admin_attributes(p)
+ end
+
+ def test_new_with_admin_scope_with_attr_protected_attributes
+ p = LoosePerson.new(attributes_hash, :as => :admin)
+
+ assert_admin_attributes(p)
+ end
+
+ def test_create_with_admin_scope_with_attr_accessible_attributes
+ p = TightPerson.create(attributes_hash, :as => :admin)
+
+ assert_admin_attributes(p, true)
+ end
+
+ def test_create_with_admin_scope_with_attr_protected_attributes
+ p = LoosePerson.create(attributes_hash, :as => :admin)
+
+ assert_admin_attributes(p, true)
+ end
+
+ def test_new_with_without_protection_with_attr_accessible_attributes
+ p = TightPerson.new(attributes_hash, :without_protection => true)
+
+ assert_all_attributes(p)
+ end
+
+ def test_new_with_without_protection_with_attr_protected_attributes
+ p = LoosePerson.new(attributes_hash, :without_protection => true)
+
+ assert_all_attributes(p)
+ end
+
+ def test_create_with_without_protection_with_attr_accessible_attributes
+ p = TightPerson.create(attributes_hash, :without_protection => true)
+
+ assert_all_attributes(p)
+ end
+
+ def test_create_with_without_protection_with_attr_protected_attributes
+ p = LoosePerson.create(attributes_hash, :without_protection => true)
+
+ assert_all_attributes(p)
end
def test_protection_against_class_attribute_writers
@@ -111,4 +171,34 @@ def attributes_hash
:comments => 'rides a sweet bike'
}
end
+
+ def assert_default_attributes(person, create = false)
+ unless create
+ assert_nil person.id
+ else
+ assert !!person.id
+ end
+ assert_equal 'Josh', person.first_name
+ assert_equal 'm', person.gender
+ assert_nil person.comments
+ end
+
+ def assert_admin_attributes(person, create = false)
+ unless create
+ assert_nil person.id
+ else
+ assert !!person.id
+ end
+ assert_equal 'Josh', person.first_name
+ assert_equal 'm', person.gender
+ assert_equal 'rides a sweet bike', person.comments
+ end
+
+ def assert_all_attributes(person)
+ assert_equal 5, person.id
+ assert_equal 'Josh', person.first_name
+ assert_equal 'm', person.gender
+ assert_equal 'rides a sweet bike', person.comments
+ end
+
end

0 comments on commit 7c5ae0a

Please sign in to comment.
Something went wrong with that request. Please try again.