From 86d7ed33754f80690395309dd307c6d9ecc0022f Mon Sep 17 00:00:00 2001 From: Josh Kalderimis Date: Sun, 1 May 2011 23:30:07 +0200 Subject: 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 +- .../associations/has_many_through_association.rb | 4 +- .../associations/singular_association.rb | 16 +- .../test/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 + + 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 \ No newline at end of file +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 -- cgit v1.2.3