From b6d33bd6bdc9280f9fd227eee3e02a5348b87966 Mon Sep 17 00:00:00 2001 From: Adam Milligan Date: Sat, 4 Oct 2008 16:11:15 -0700 Subject: [PATCH] Methods called on association proxies now respect access control --- .../associations/association_proxy.rb | 22 ++++++++++++++++++++ .../associations/has_one_association.rb | 2 +- .../associations/belongs_to_associations_test.rb | 10 +++++++++ .../associations/has_one_associations_test.rb | 10 +++++++++ .../has_one_through_associations_test.rb | 10 +++++++++ activerecord/test/models/club.rb | 6 +++++ activerecord/test/models/company.rb | 13 ++++++++++- 7 files changed, 71 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb index b617147..17dbae7 100644 --- a/activerecord/lib/active_record/associations/association_proxy.rb +++ b/activerecord/lib/active_record/associations/association_proxy.rb @@ -193,10 +193,26 @@ module ActiveRecord @reflection.klass.send :with_scope, *args, &block end + # #method_missing will raise a NoMethodError exception if the target + # has declared the specified method as private. However, callers + # should still be able to call private methods on the target class via + # #send. This decoration forwards private methods to the target, thus + # skipping the privacy check in #method_missing. + def send_with_privacy_leniency(method, *args) + if loaded? && !self.respond_to?(method) && @target.private_methods.include?(method.to_s) + @target.send(method, *args) + else + send_without_privacy_leniency(method, *args) + end + end + alias_method_chain :send, :privacy_leniency + private # Forwards any missing method call to the \target. def method_missing(method, *args) if load_target + protect_if_private(method, args) + if block_given? @target.send(method, *args) { |*block_args| yield(*block_args) } else @@ -205,6 +221,12 @@ module ActiveRecord end end + def protect_if_private(method, args) + if @target.private_methods.include?(method.to_s) + raise NoMethodError.new("Attempt to call private method", method, args) + end + end + # Loads the \target if needed and returns it. # # This method is abstract in the sense that it relies on +find_target+, diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index c92ef5c..9603230 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -57,7 +57,7 @@ module ActiveRecord protected def owner_quoted_id if @reflection.options[:primary_key] - quote_value(@owner.send(@reflection.options[:primary_key])) + @owner.class.quote_value(@owner.send(@reflection.options[:primary_key])) else @owner.quoted_id end diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index 37b6836..90783e6 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -441,4 +441,14 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert log.valid? assert log.save end + + def test_belongs_to_proxy_should_not_respond_to_private_methods + assert_raises(NoMethodError) { companies(:first_firm).private_method } + assert_raises(NoMethodError) { companies(:second_client).firm.private_method } + end + + def test_belongs_to_proxy_should_respond_to_private_methods_via_send + companies(:first_firm).send(:private_method) + companies(:second_client).firm.send(:private_method) + end end diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index ec06be5..14032a6 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -349,4 +349,14 @@ class HasOneAssociationsTest < ActiveRecord::TestCase assert companies(:first_firm).readonly_account.readonly? end + def test_has_one_proxy_should_not_respond_to_private_methods + assert_raises(NoMethodError) { accounts(:signals37).private_method } + assert_raises(NoMethodError) { companies(:first_firm).account.private_method } + end + + def test_has_one_proxy_should_respond_to_private_methods_via_send + accounts(:signals37).send(:private_method) + companies(:first_firm).account.send(:private_method) + end + end diff --git a/activerecord/test/cases/associations/has_one_through_associations_test.rb b/activerecord/test/cases/associations/has_one_through_associations_test.rb index 77e3cb1..ff4021f 100644 --- a/activerecord/test/cases/associations/has_one_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_through_associations_test.rb @@ -110,4 +110,14 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase new_member.club = new_club = Club.create(:name => "LRUG") assert_equal new_club, new_member.club.target end + + def test_has_one_through_proxy_should_not_respond_to_private_methods + assert_raises(NoMethodError) { clubs(:moustache_club).private_method } + assert_raises(NoMethodError) { @member.club.private_method } + end + + def test_has_one_through_proxy_should_respond_to_private_methods_via_send + clubs(:moustache_club).send(:private_method) + @member.club.send(:private_method) + end end diff --git a/activerecord/test/models/club.rb b/activerecord/test/models/club.rb index 3ddb691..6e7cdd6 100644 --- a/activerecord/test/models/club.rb +++ b/activerecord/test/models/club.rb @@ -4,4 +4,10 @@ class Club < ActiveRecord::Base has_many :current_memberships has_one :sponsor has_one :sponsored_member, :through => :sponsor, :source => :sponsorable, :source_type => "Member" + + private + + def private_method + "I'm sorry sir, this is a *private* club, not a *pirate* club" + end end \ No newline at end of file diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index 0eb8ae0..62d2086 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -13,6 +13,12 @@ class Company < AbstractCompany def arbitrary_method "I am Jack's profound disappointment" end + + private + + def private_method + "I am Jack's innermost fears and aspirations" + end end module Namespaced @@ -129,9 +135,14 @@ class Account < ActiveRecord::Base true end - protected def validate errors.add_on_empty "credit_limit" end + + private + + def private_method + "Sir, yes sir!" + end end -- 1.6.0