Ensure methods called on association proxies respect access control. [#1083 state:resolved] [Adam Milligan, Pratik]

7 files changed, 60 insertions, 2 deletions

diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb
index b617147f67..d1a79df6e6 100644
--- a/activerecord/lib/active_record/associations/association_proxy.rb
+++ b/activerecord/lib/active_record/associations/association_proxy.rb
@@ -140,6 +140,15 @@ module ActiveRecord
         @target.inspect
       end
 
+      def send(method, *args)
+        if proxy_respond_to?(method)
+          super
+        else
+          load_target
+          @target.send(method, *args)
+        end
+      end
+
       protected
         # Does the association have a <tt>:dependent</tt> option?
         def dependent?
@@ -197,6 +206,8 @@ module ActiveRecord
         # Forwards any missing method call to the \target.
         def method_missing(method, *args)
           if load_target
+            raise NoMethodError unless @target.respond_to?(method)
+
             if block_given?
               @target.send(method, *args)  { |*block_args| yield(*block_args) }
             else
diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb
index c92ef5c2c9..960323004d 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 9c718c4fef..40a8503980 100644
--- a/activerecord/test/cases/associations/belongs_to_associations_test.rb
+++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb
@@ -428,4 +428,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 ec06be5eba..14032a67c0 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 77e3cb1776..ff4021fe02 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 3ddb691dfb..6e7cdd643a 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 0eb8ae0a15..62d20861f3 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