Changed ActiveRecord attributes to respect access control.

Signed-off-by: Michael Koziarski <michael@koziarski.com>
[#1084 state:committed]

2 files changed, 54 insertions, 5 deletions

diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb
index 020da01871..e5486738f0 100644
--- a/activerecord/lib/active_record/attribute_methods.rb
+++ b/activerecord/lib/active_record/attribute_methods.rb
@@ -232,6 +232,10 @@ module ActiveRecord
     def method_missing(method_id, *args, &block)
       method_name = method_id.to_s
 
+      if self.class.private_method_defined?(method_name)
+        raise NoMethodError("Attempt to call private method", method_name, args)
+      end
+
       # If we haven't generated any methods yet, generate them, then
       # see if we've created the method we're looking for.
       if !self.class.generated_methods?
@@ -334,10 +338,12 @@ module ActiveRecord
     # <tt>person.respond_to?(:name=)</tt>, and <tt>person.respond_to?(:name?)</tt>
     # which will all return +true+.
     alias :respond_to_without_attributes? :respond_to?
-    def respond_to?(method, include_priv = false)
+    def respond_to?(method, include_private_methods = false)
       method_name = method.to_s
       if super
         return true
+      elsif self.private_methods.include?(method_name) && !include_private_methods
+        return false
       elsif !self.class.generated_methods?
         self.class.define_attribute_methods
         if self.class.generated_methods.include?(method_name)
diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb
index ce293a469e..160716f944 100644
--- a/activerecord/test/cases/attribute_methods_test.rb
+++ b/activerecord/test/cases/attribute_methods_test.rb
@@ -58,19 +58,19 @@ class AttributeMethodsTest < ActiveRecord::TestCase
 
   def test_kernel_methods_not_implemented_in_activerecord
     %w(test name display y).each do |method|
-      assert_equal false, ActiveRecord::Base.instance_method_already_implemented?(method), "##{method} is defined"
+      assert !ActiveRecord::Base.instance_method_already_implemented?(method), "##{method} is defined"
     end
   end
 
   def test_primary_key_implemented
-    assert_equal true, Class.new(ActiveRecord::Base).instance_method_already_implemented?('id')
+    assert Class.new(ActiveRecord::Base).instance_method_already_implemented?('id')
   end
 
   def test_defined_kernel_methods_implemented_in_model
     %w(test name display y).each do |method|
       klass = Class.new ActiveRecord::Base
       klass.class_eval "def #{method}() 'defined #{method}' end"
-      assert_equal true, klass.instance_method_already_implemented?(method), "##{method} is not defined"
+      assert klass.instance_method_already_implemented?(method), "##{method} is not defined"
     end
   end
 
@@ -80,7 +80,7 @@ class AttributeMethodsTest < ActiveRecord::TestCase
       abstract.class_eval "def #{method}() 'defined #{method}' end"
       abstract.abstract_class = true
       klass = Class.new abstract
-      assert_equal true, klass.instance_method_already_implemented?(method), "##{method} is not defined"
+      assert klass.instance_method_already_implemented?(method), "##{method} is not defined"
     end
   end
 
@@ -228,6 +228,40 @@ class AttributeMethodsTest < ActiveRecord::TestCase
     assert_equal [:field_b], Minimalistic.skip_time_zone_conversion_for_attributes 
   end
 
+  def test_read_attributes_respect_access_control
+    privatize("title")
+
+    topic = @target.new(:title => "The pros and cons of programming naked.")
+    assert !topic.respond_to?(:title)
+    assert_raise(NoMethodError) { topic.title }
+    topic.send(:title)
+  end
+
+  def test_write_attributes_respect_access_control
+    privatize("title=(value)")
+
+    topic = @target.new
+    assert !topic.respond_to?(:title=)
+    assert_raise(NoMethodError) { topic.title = "Pants"}
+    topic.send(:title=, "Very large pants")
+  end
+
+  def test_question_attributes_respect_access_control
+    privatize("title?")
+
+    topic = @target.new(:title => "Isaac Newton's pants")
+    assert !topic.respond_to?(:title?)
+    assert_raise(NoMethodError) { topic.title? }
+    assert topic.send(:title?)
+  end
+
+  def test_bulk_update_respects_access_control
+    privatize("title=(value)")
+
+    assert_raise(ActiveRecord::UnknownAttributeError) { topic = @target.new(:title => "Rants about pants") }
+    assert_raise(ActiveRecord::UnknownAttributeError) { @target.new.attributes = { :title => "Ants in pants" } }
+  end
+
   private
   def time_related_columns_on_topic
     Topic.columns.select{|c| [:time, :date, :datetime, :timestamp].include?(c.type)}.map(&:name)
@@ -244,4 +278,13 @@ class AttributeMethodsTest < ActiveRecord::TestCase
     Time.zone = old_zone
     ActiveRecord::Base.time_zone_aware_attributes = old_tz
   end
+
+  def privatize(method_signature)
+    @target.class_eval <<-private_method
+      private
+      def #{method_signature}
+        "I'm private"
+      end
+    private_method
+  end
 end