Protect id attribute from mass assigment even when the primary key is set to something else. Closes #2438.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@2541 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

3 files changed, 25 insertions, 3 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index a7d354a366..40b8b2c3c8 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Protect id attribute from mass assigment even when the primary key is set to something else. #2438. [Blair Zajac <blair@orcaware.com>]
+
 * Misc doc fixes (typos/grammar/etc.). #2430. [coffee2code]
 
 * Add test coverage for content_columns. #2432. [coffee2code]
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index dd0fe15b3f..16d9579ed0 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -508,7 +508,7 @@ module ActiveRecord #:nodoc:
       #   customer.credit_rating = "Average"
       #   customer.credit_rating # => "Average"
       def attr_protected(*attributes)
-        write_inheritable_array("attr_protected", attributes)
+        write_inheritable_array("attr_protected", attributes - (protected_attributes || []))
       end
 
       # Returns an array of all the attributes that have been protected from mass-assignment.
@@ -521,7 +521,7 @@ module ActiveRecord #:nodoc:
       # protection. If you'd rather start from an all-open default and restrict attributes as needed, have a look at
       # attr_protected.
       def attr_accessible(*attributes)
-        write_inheritable_array("attr_accessible", attributes)
+        write_inheritable_array("attr_accessible", attributes - (accessible_attributes || []))
       end
 
       # Returns an array of all the attributes that have been made accessible to mass-assignment.
@@ -1450,7 +1450,9 @@ module ActiveRecord #:nodoc:
 
       # The primary key and inheritance column can never be set by mass-assignment for security reasons.
       def attributes_protected_by_default
-        [ self.class.primary_key, self.class.inheritance_column ]
+        default = [ self.class.primary_key, self.class.inheritance_column ]
+        default << 'id' unless self.class.primary_key.eql? 'id'
+        default
       end
 
       # Returns copy of the attributes hash where all the values have been safely quoted for use in
diff --git a/activerecord/test/base_test.rb b/activerecord/test/base_test.rb
index 4f591e2cac..c7e795c0e6 100755
--- a/activerecord/test/base_test.rb
+++ b/activerecord/test/base_test.rb
@@ -7,6 +7,8 @@ require 'fixtures/project'
 require 'fixtures/default'
 require 'fixtures/auto_id'
 require 'fixtures/column_name'
+require 'fixtures/subscriber'
+require 'fixtures/keyboard'
 
 class Category < ActiveRecord::Base; end
 class Smarts < ActiveRecord::Base; end
@@ -526,6 +528,22 @@ class BasicsTest < Test::Unit::TestCase
     firm.attributes = { "name" => "Next Angle", "rating" => 5 }
     assert_equal 1, firm.rating
   end
+
+  def test_customized_primary_key_remains_protected
+    subscriber = Subscriber.new(:nick => 'webster123', :name => 'nice try')
+    assert_nil subscriber.id
+
+    keyboard = Keyboard.new(:key_number => 9, :name => 'nice try')
+    assert_nil keyboard.id
+  end
+
+  def test_customized_primary_key_remains_protected_when_refered_to_as_id
+    subscriber = Subscriber.new(:id => 'webster123', :name => 'nice try')
+    assert_nil subscriber.id
+
+    keyboard = Keyboard.new(:id => 9, :name => 'nice try')
+    assert_nil keyboard.id
+  end
   
   def test_mass_assignment_protection_on_defaults
     firm = Firm.new