From 86d7ed33754f80690395309dd307c6d9ecc0022f Mon Sep 17 00:00:00 2001
From: Josh Kalderimis <josh.kalderimis@gmail.com>
Date: Sun, 1 May 2011 23:30:07 +0200
Subject: [PATCH] singular and collection relations in AR can now specify
 mass-assignment security options (:as and :without_protection) in build,
 create and create! methods.

---
 .../associations/collection_association.rb    |  20 +-
 .../has_many_through_association.rb           |   4 +-
 .../associations/singular_association.rb      |  16 +-
 .../cases/mass_assignment_security_test.rb    | 340 ++++++++++++++++--
 activerecord/test/models/person.rb            |  23 +-
 activerecord/test/schema/schema.rb            |   2 +
 6 files changed, 346 insertions(+), 59 deletions(-)

diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
index 33a184d48d..6cdec8c487 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -93,20 +93,20 @@ module ActiveRecord
         first_or_last(:last, *args)
       end
 
-      def build(attributes = {}, &block)
-        build_or_create(attributes, :build, &block)
+      def build(attributes = {}, options = {}, &block)
+        build_or_create(:build, attributes, options, &block)
       end
 
-      def create(attributes = {}, &block)
+      def create(attributes = {}, options = {}, &block)
         unless owner.persisted?
           raise ActiveRecord::RecordNotSaved, "You cannot call create unless the parent is saved"
         end
 
-        build_or_create(attributes, :create, &block)
+        build_or_create(:create, attributes, options, &block)
       end
 
-      def create!(attrs = {}, &block)
-        record = create(attrs, &block)
+      def create!(attrs = {}, options = {}, &block)
+        record = create(attrs, options, &block)
         Array.wrap(record).each(&:save!)
         record
       end
@@ -403,9 +403,9 @@ module ActiveRecord
           end + existing
         end
 
-        def build_or_create(attributes, method)
+        def build_or_create(method, attributes, options)
           records = Array.wrap(attributes).map do |attrs|
-            record = build_record(attrs)
+            record = build_record(attrs, options)
 
             add_to_target(record) do
               yield(record) if block_given?
@@ -421,8 +421,8 @@ module ActiveRecord
           raise NotImplementedError
         end
 
-        def build_record(attributes)
-          reflection.build_association(scoped.scope_for_create.merge(attributes))
+        def build_record(attributes, options)
+          reflection.build_association(scoped.scope_for_create.merge(attributes), options)
         end
 
         def delete_or_destroy(records, method)
diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb
index 9d2b29685b..7708228d23 100644
--- a/activerecord/lib/active_record/associations/has_many_through_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -60,10 +60,10 @@ module ActiveRecord
           through_record
         end
 
-        def build_record(attributes)
+        def build_record(attributes, options = {})
           ensure_not_nested
 
-          record = super(attributes)
+          record = super(attributes, options)
 
           inverse = source_reflection.inverse_of
           if inverse
diff --git a/activerecord/lib/active_record/associations/singular_association.rb b/activerecord/lib/active_record/associations/singular_association.rb
index 4edbe216be..ea4d73d414 100644
--- a/activerecord/lib/active_record/associations/singular_association.rb
+++ b/activerecord/lib/active_record/associations/singular_association.rb
@@ -17,16 +17,16 @@ module ActiveRecord
         replace(record)
       end
 
-      def create(attributes = {})
-        new_record(:create, attributes)
+      def create(attributes = {}, options = {})
+        new_record(:create, attributes, options)
       end
 
-      def create!(attributes = {})
-        build(attributes).tap { |record| record.save! }
+      def create!(attributes = {}, options = {})
+        build(attributes, options).tap { |record| record.save! }
       end
 
-      def build(attributes = {})
-        new_record(:build, attributes)
+      def build(attributes = {}, options = {})
+        new_record(:build, attributes, options)
       end
 
       private
@@ -44,9 +44,9 @@ module ActiveRecord
           replace(record)
         end
 
-        def new_record(method, attributes)
+        def new_record(method, attributes, options)
           attributes = scoped.scope_for_create.merge(attributes || {})
-          record = reflection.send("#{method}_association", attributes)
+          record = reflection.send("#{method}_association", attributes, options)
           set_new_record(record)
           record
         end
diff --git a/activerecord/test/cases/mass_assignment_security_test.rb b/activerecord/test/cases/mass_assignment_security_test.rb
index 67950c8068..fbbae99e8b 100644
--- a/activerecord/test/cases/mass_assignment_security_test.rb
+++ b/activerecord/test/cases/mass_assignment_security_test.rb
@@ -5,14 +5,64 @@ require 'models/keyboard'
 require 'models/task'
 require 'models/person'
 
-class MassAssignmentSecurityTest < ActiveRecord::TestCase
 
+module MassAssignmentTestHelpers
   def setup
     # another AR test modifies the columns which causes issues with create calls
     TightPerson.reset_column_information
     LoosePerson.reset_column_information
   end
 
+  def attributes_hash
+    {
+      :id => 5,
+      :first_name => 'Josh',
+      :gender   => 'm',
+      :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
+
+module MassAssignmentRelationTestHelpers
+  def setup
+    super
+    @person = LoosePerson.create(attributes_hash)
+  end
+end
+
+
+class MassAssignmentSecurityTest < ActiveRecord::TestCase
+  include MassAssignmentTestHelpers
+
   def test_customized_primary_key_remains_protected
     subscriber = Subscriber.new(:nick => 'webster123', :name => 'nice try')
     assert_nil subscriber.id
@@ -161,44 +211,268 @@ class MassAssignmentSecurityTest < ActiveRecord::TestCase
     end
   end
 
-  private
+end
 
-  def attributes_hash
-    {
-      :id => 5,
-      :first_name => 'Josh',
-      :gender   => 'm',
-      :comments => 'rides a sweet bike'
-    }
+
+class MassAssignmentSecurityHasOneRelationsTest < ActiveRecord::TestCase
+  include MassAssignmentTestHelpers
+  include MassAssignmentRelationTestHelpers
+
+  # build
+
+  def test_has_one_build_with_attr_protected_attributes
+    best_friend = @person.build_best_friend(attributes_hash)
+    assert_default_attributes(best_friend)
   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
+  def test_has_one_build_with_attr_accessible_attributes
+    best_friend = @person.build_best_friend(attributes_hash)
+    assert_default_attributes(best_friend)
   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
+  def test_has_one_build_with_admin_scope_with_attr_protected_attributes
+    best_friend = @person.build_best_friend(attributes_hash, :as => :admin)
+    assert_admin_attributes(best_friend)
   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
+  def test_has_one_build_with_admin_scope_with_attr_accessible_attributes
+    best_friend = @person.build_best_friend(attributes_hash, :as => :admin)
+    assert_admin_attributes(best_friend)
   end
 
-end
\ No newline at end of file
+  def test_has_one_build_without_protection
+    best_friend = @person.build_best_friend(attributes_hash, :without_protection => true)
+    assert_all_attributes(best_friend)
+  end
+
+  # create
+
+  def test_has_one_create_with_attr_protected_attributes
+    best_friend = @person.create_best_friend(attributes_hash)
+    assert_default_attributes(best_friend, true)
+  end
+
+  def test_has_one_create_with_attr_accessible_attributes
+    best_friend = @person.create_best_friend(attributes_hash)
+    assert_default_attributes(best_friend, true)
+  end
+
+  def test_has_one_create_with_admin_scope_with_attr_protected_attributes
+    best_friend = @person.create_best_friend(attributes_hash, :as => :admin)
+    assert_admin_attributes(best_friend, true)
+  end
+
+  def test_has_one_create_with_admin_scope_with_attr_accessible_attributes
+    best_friend = @person.create_best_friend(attributes_hash, :as => :admin)
+    assert_admin_attributes(best_friend, true)
+  end
+
+  def test_has_one_create_without_protection
+    best_friend = @person.create_best_friend(attributes_hash, :without_protection => true)
+    assert_all_attributes(best_friend)
+  end
+
+  # create!
+
+  def test_has_one_create_with_bang_with_attr_protected_attributes
+    best_friend = @person.create_best_friend!(attributes_hash)
+    assert_default_attributes(best_friend, true)
+  end
+
+  def test_has_one_create_with_bang_with_attr_accessible_attributes
+    best_friend = @person.create_best_friend!(attributes_hash)
+    assert_default_attributes(best_friend, true)
+  end
+
+  def test_has_one_create_with_bang_with_admin_scope_with_attr_protected_attributes
+    best_friend = @person.create_best_friend!(attributes_hash, :as => :admin)
+    assert_admin_attributes(best_friend, true)
+  end
+
+  def test_has_one_create_with_bang_with_admin_scope_with_attr_accessible_attributes
+    best_friend = @person.create_best_friend!(attributes_hash, :as => :admin)
+    assert_admin_attributes(best_friend, true)
+  end
+
+  def test_has_one_create_with_bang_without_protection
+    best_friend = @person.create_best_friend!(attributes_hash, :without_protection => true)
+    assert_all_attributes(best_friend)
+  end
+
+end
+
+
+class MassAssignmentSecurityBelongsToRelationsTest < ActiveRecord::TestCase
+  include MassAssignmentTestHelpers
+  include MassAssignmentRelationTestHelpers
+
+  # build
+
+  def test_has_one_build_with_attr_protected_attributes
+    best_friend = @person.build_best_friend_of(attributes_hash)
+    assert_default_attributes(best_friend)
+  end
+
+  def test_has_one_build_with_attr_accessible_attributes
+    best_friend = @person.build_best_friend_of(attributes_hash)
+    assert_default_attributes(best_friend)
+  end
+
+  def test_has_one_build_with_admin_scope_with_attr_protected_attributes
+    best_friend = @person.build_best_friend_of(attributes_hash, :as => :admin)
+    assert_admin_attributes(best_friend)
+  end
+
+  def test_has_one_build_with_admin_scope_with_attr_accessible_attributes
+    best_friend = @person.build_best_friend_of(attributes_hash, :as => :admin)
+    assert_admin_attributes(best_friend)
+  end
+
+  def test_has_one_build_without_protection
+    best_friend = @person.build_best_friend_of(attributes_hash, :without_protection => true)
+    assert_all_attributes(best_friend)
+  end
+
+  # create
+
+  def test_has_one_create_with_attr_protected_attributes
+    best_friend = @person.create_best_friend_of(attributes_hash)
+    assert_default_attributes(best_friend, true)
+  end
+
+  def test_has_one_create_with_attr_accessible_attributes
+    best_friend = @person.create_best_friend_of(attributes_hash)
+    assert_default_attributes(best_friend, true)
+  end
+
+  def test_has_one_create_with_admin_scope_with_attr_protected_attributes
+    best_friend = @person.create_best_friend_of(attributes_hash, :as => :admin)
+    assert_admin_attributes(best_friend, true)
+  end
+
+  def test_has_one_create_with_admin_scope_with_attr_accessible_attributes
+    best_friend = @person.create_best_friend_of(attributes_hash, :as => :admin)
+    assert_admin_attributes(best_friend, true)
+  end
+
+  def test_has_one_create_without_protection
+    best_friend = @person.create_best_friend_of(attributes_hash, :without_protection => true)
+    assert_all_attributes(best_friend)
+  end
+
+  # create!
+
+  def test_has_one_create_with_bang_with_attr_protected_attributes
+    best_friend = @person.create_best_friend!(attributes_hash)
+    assert_default_attributes(best_friend, true)
+  end
+
+  def test_has_one_create_with_bang_with_attr_accessible_attributes
+    best_friend = @person.create_best_friend!(attributes_hash)
+    assert_default_attributes(best_friend, true)
+  end
+
+  def test_has_one_create_with_bang_with_admin_scope_with_attr_protected_attributes
+    best_friend = @person.create_best_friend!(attributes_hash, :as => :admin)
+    assert_admin_attributes(best_friend, true)
+  end
+
+  def test_has_one_create_with_bang_with_admin_scope_with_attr_accessible_attributes
+    best_friend = @person.create_best_friend!(attributes_hash, :as => :admin)
+    assert_admin_attributes(best_friend, true)
+  end
+
+  def test_has_one_create_with_bang_without_protection
+    best_friend = @person.create_best_friend!(attributes_hash, :without_protection => true)
+    assert_all_attributes(best_friend)
+  end
+
+end
+
+
+class MassAssignmentSecurityHasManyRelationsTest < ActiveRecord::TestCase
+  include MassAssignmentTestHelpers
+  include MassAssignmentRelationTestHelpers
+
+  # build
+
+  def test_has_one_build_with_attr_protected_attributes
+    best_friend = @person.best_friends.build(attributes_hash)
+    assert_default_attributes(best_friend)
+  end
+
+  def test_has_one_build_with_attr_accessible_attributes
+    best_friend = @person.best_friends.build(attributes_hash)
+    assert_default_attributes(best_friend)
+  end
+
+  def test_has_one_build_with_admin_scope_with_attr_protected_attributes
+    best_friend = @person.best_friends.build(attributes_hash, :as => :admin)
+    assert_admin_attributes(best_friend)
+  end
+
+  def test_has_one_build_with_admin_scope_with_attr_accessible_attributes
+    best_friend = @person.best_friends.build(attributes_hash, :as => :admin)
+    assert_admin_attributes(best_friend)
+  end
+
+  def test_has_one_build_without_protection
+    best_friend = @person.best_friends.build(attributes_hash, :without_protection => true)
+    assert_all_attributes(best_friend)
+  end
+
+  # create
+
+  def test_has_one_create_with_attr_protected_attributes
+    best_friend = @person.best_friends.create(attributes_hash)
+    assert_default_attributes(best_friend, true)
+  end
+
+  def test_has_one_create_with_attr_accessible_attributes
+    best_friend = @person.best_friends.create(attributes_hash)
+    assert_default_attributes(best_friend, true)
+  end
+
+  def test_has_one_create_with_admin_scope_with_attr_protected_attributes
+    best_friend = @person.best_friends.create(attributes_hash, :as => :admin)
+    assert_admin_attributes(best_friend, true)
+  end
+
+  def test_has_one_create_with_admin_scope_with_attr_accessible_attributes
+    best_friend = @person.best_friends.create(attributes_hash, :as => :admin)
+    assert_admin_attributes(best_friend, true)
+  end
+
+  def test_has_one_create_without_protection
+    best_friend = @person.best_friends.create(attributes_hash, :without_protection => true)
+    assert_all_attributes(best_friend)
+  end
+
+  # create!
+
+  def test_has_one_create_with_bang_with_attr_protected_attributes
+    best_friend = @person.best_friends.create!(attributes_hash)
+    assert_default_attributes(best_friend, true)
+  end
+
+  def test_has_one_create_with_bang_with_attr_accessible_attributes
+    best_friend = @person.best_friends.create!(attributes_hash)
+    assert_default_attributes(best_friend, true)
+  end
+
+  def test_has_one_create_with_bang_with_admin_scope_with_attr_protected_attributes
+    best_friend = @person.best_friends.create!(attributes_hash, :as => :admin)
+    assert_admin_attributes(best_friend, true)
+  end
+
+  def test_has_one_create_with_bang_with_admin_scope_with_attr_accessible_attributes
+    best_friend = @person.best_friends.create!(attributes_hash, :as => :admin)
+    assert_admin_attributes(best_friend, true)
+  end
+
+  def test_has_one_create_with_bang_without_protection
+    best_friend = @person.best_friends.create!(attributes_hash, :without_protection => true)
+    assert_all_attributes(best_friend)
+  end
+
+end
diff --git a/activerecord/test/models/person.rb b/activerecord/test/models/person.rb
index 9c4794902d..a58c9bf572 100644
--- a/activerecord/test/models/person.rb
+++ b/activerecord/test/models/person.rb
@@ -1,6 +1,6 @@
 class Person < ActiveRecord::Base
   has_many :readers
-  has_one :reader
+  has_one  :reader
 
   has_many :posts, :through => :readers
   has_many :posts_with_no_comments, :through => :readers, :source => :post, :include => :comments, :conditions => 'comments.id is null'
@@ -8,23 +8,23 @@ class Person < ActiveRecord::Base
   has_many :references
   has_many :bad_references
   has_many :fixed_bad_references, :conditions => { :favourite => true }, :class_name => 'BadReference'
-  has_one :favourite_reference, :class_name => 'Reference', :conditions => ['favourite=?', true]
+  has_one  :favourite_reference, :class_name => 'Reference', :conditions => ['favourite=?', true]
   has_many :posts_with_comments_sorted_by_comment_id, :through => :readers, :source => :post, :include => :comments, :order => 'comments.id'
 
   has_many :jobs, :through => :references
-  has_many :jobs_with_dependent_destroy, :source => :job, :through => :references, :dependent => :destroy
+  has_many :jobs_with_dependent_destroy,    :source => :job, :through => :references, :dependent => :destroy
   has_many :jobs_with_dependent_delete_all, :source => :job, :through => :references, :dependent => :delete_all
-  has_many :jobs_with_dependent_nullify, :source => :job, :through => :references, :dependent => :nullify
+  has_many :jobs_with_dependent_nullify,    :source => :job, :through => :references, :dependent => :nullify
 
   belongs_to :primary_contact, :class_name => 'Person'
   has_many :agents, :class_name => 'Person', :foreign_key => 'primary_contact_id'
   has_many :agents_of_agents, :through => :agents, :source => :agents
   belongs_to :number1_fan, :class_name => 'Person'
 
-  has_many :agents_posts, :through => :agents, :source => :posts
+  has_many :agents_posts,         :through => :agents,       :source => :posts
   has_many :agents_posts_authors, :through => :agents_posts, :source => :author
 
-  scope :males, :conditions => { :gender => 'M' }
+  scope :males,   :conditions => { :gender => 'M' }
   scope :females, :conditions => { :gender => 'F' }
 end
 
@@ -56,14 +56,25 @@ class LoosePerson < ActiveRecord::Base
 
   attr_protected :comments
   attr_protected :as => :admin
+
+  has_one    :best_friend,    :class_name => 'LoosePerson', :foreign_key => :best_friend_id
+  belongs_to :best_friend_of, :class_name => 'LoosePerson', :foreign_key => :best_friend_of_id
+
+  has_many   :best_friends,   :class_name => 'LoosePerson', :foreign_key => :best_friend_id
 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
+
+  has_one    :best_friend,    :class_name => 'TightPerson', :foreign_key => :best_friend_id
+  belongs_to :best_friend_of, :class_name => 'TightPerson', :foreign_key => :best_friend_of_id
+
+  has_many   :best_friends,   :class_name => 'TightPerson', :foreign_key => :best_friend_id
 end
 
 class TightDescendant < TightPerson; end
\ No newline at end of file
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index ceadb05644..9479242e4f 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -438,6 +438,8 @@ ActiveRecord::Schema.define do
     t.references :number1_fan
     t.integer    :lock_version, :null => false, :default => 0
     t.string     :comments
+    t.references :best_friend
+    t.references :best_friend_of
     t.timestamps
   end