mass_assignment_security moved from AR to AMo, and minor test cleanup

Signed-off-by: José Valim <jose.valim@gmail.com>

activemodel/lib/active_model.rb

@@ -38,6 +38,7 @@ module ActiveModel
   autoload :EachValidator, 'active_model/validator'
   autoload :Errors
   autoload :Lint
+  autoload :MassAssignmentSecurity
   autoload :Name, 'active_model/naming'
   autoload :Naming
   autoload :Observer, 'active_model/observing'

activemodel/lib/active_model/mass_assignment_security.rb

@@ -1,6 +1,7 @@
-require 'active_record/mass_assignment_security/permission_set'
+require 'active_support/core_ext/class/attribute.rb'
+require 'active_model/mass_assignment_security/permission_set'
 
-module ActiveRecord
+module ActiveModel
   # = Active Record Mass-Assignment Security
   module MassAssignmentSecurity
     extend ActiveSupport::Concern
@@ -112,11 +113,13 @@ module ActiveRecord
       end
 
       def protected_attributes
-        self._protected_attributes ||= BlackList.new(attributes_protected_by_default).tap { |w| w.logger = logger }
+        self._protected_attributes ||= BlackList.new(attributes_protected_by_default).tap do |w|
+          w.logger = self.logger if self.respond_to?(:logger)
+        end
       end
 
       def accessible_attributes
-        self._accessible_attributes ||= WhiteList.new.tap { |w| w.logger = logger }
+        self._accessible_attributes ||= WhiteList.new.tap { |w| w.logger = self.logger if self.respond_to?(:logger) }
       end
 
       def active_authorizer

activemodel/lib/active_model/mass_assignment_security/permission_set.rb

@@ -1,6 +1,6 @@
-require 'active_record/mass_assignment_security/sanitizer'
+require 'active_model/mass_assignment_security/sanitizer'
 
-module ActiveRecord
+module ActiveModel
   module MassAssignmentSecurity
 
     class PermissionSet < Set

activemodel/lib/active_model/mass_assignment_security/sanitizer.rb

@@ -1,4 +1,4 @@
-module ActiveRecord
+module ActiveModel
   module MassAssignmentSecurity
     module Sanitizer
 
@@ -17,11 +17,11 @@ module ActiveRecord
         end
 
         def debug?
-          logger.present?
+          self.logger.present?
         end
 
         def warn!(attrs)
-          logger.debug "WARNING: Can't mass-assign protected attributes: #{attrs.join(', ')}"
+          self.logger.debug "WARNING: Can't mass-assign protected attributes: #{attrs.join(', ')}"
         end
 
     end

activemodel/test/cases/mass_assignment_security/black_list_test.rb

@@ -1,9 +1,9 @@
 require "cases/helper"
 
-class BlackListTest < ActiveRecord::TestCase
+class BlackListTest < ActiveModel::TestCase
 
   def setup
-    @black_list   = ActiveRecord::MassAssignmentSecurity::BlackList.new
+    @black_list   = ActiveModel::MassAssignmentSecurity::BlackList.new
     @included_key = 'admin'
     @black_list  += [ @included_key ]
   end

activemodel/test/cases/mass_assignment_security/permission_set_test.rb

@@ -1,9 +1,9 @@
 require "cases/helper"
 
-class PermissionSetTest < ActiveRecord::TestCase
+class PermissionSetTest < ActiveModel::TestCase
 
   def setup
-    @permission_list = ActiveRecord::MassAssignmentSecurity::PermissionSet.new
+    @permission_list = ActiveModel::MassAssignmentSecurity::PermissionSet.new
   end
 
   test "+ stringifies added collection values" do

activemodel/test/cases/mass_assignment_security/sanitizer_test.rb

@@ -1,9 +1,10 @@
 require "cases/helper"
+require 'logger'
 
-class SanitizerTest < ActiveRecord::TestCase
+class SanitizerTest < ActiveModel::TestCase
 
   class SanitizingAuthorizer
-    include ActiveRecord::MassAssignmentSecurity::Sanitizer
+    include ActiveModel::MassAssignmentSecurity::Sanitizer
 
     attr_accessor :logger
 

activemodel/test/cases/mass_assignment_security/white_list_test.rb

@@ -1,9 +1,9 @@
 require "cases/helper"
 
-class WhiteListTest < ActiveRecord::TestCase
+class WhiteListTest < ActiveModel::TestCase
 
   def setup
-    @white_list   = ActiveRecord::MassAssignmentSecurity::WhiteList.new
+    @white_list   = ActiveModel::MassAssignmentSecurity::WhiteList.new
     @included_key = 'first_name'
     @white_list  += [ @included_key ]
   end

activemodel/test/cases/mass_assignment_security_test.rb

@@ -0,0 +1,52 @@
+require "cases/helper"
+require 'models/mass_assignment_specific'
+
+class MassAssignmentSecurityTest < ActiveModel::TestCase
+
+  def test_attribute_protection
+    user = User.new
+    expected = { "name" => "John Smith", "email" => "john@smith.com" }
+    sanitized = user.sanitize_for_mass_assignment(expected.merge("admin" => true))
+    assert_equal expected, sanitized
+  end
+
+  def test_attributes_accessible
+    user = Person.new
+    expected = { "name" => "John Smith", "email" => "john@smith.com" }
+    sanitized = user.sanitize_for_mass_assignment(expected.merge("super_powers" => true))
+    assert_equal expected, sanitized
+  end
+
+  def test_attributes_protected_by_default
+    firm = Firm.new
+    expected = { }
+    sanitized = firm.sanitize_for_mass_assignment({ "type" => "Client" })
+    assert_equal expected, sanitized
+  end
+
+  def test_mass_assignment_protection_inheritance
+    assert LoosePerson.accessible_attributes.blank?
+    assert_equal Set.new([ 'credit_rating', 'administrator']), LoosePerson.protected_attributes
+
+    assert LooseDescendant.accessible_attributes.blank?
+    assert_equal Set.new([ 'credit_rating', 'administrator', 'phone_number']), LooseDescendant.protected_attributes
+
+    assert LooseDescendantSecond.accessible_attributes.blank?
+    assert_equal Set.new([ 'credit_rating', 'administrator', 'phone_number', 'name']), LooseDescendantSecond.protected_attributes,
+      'Running attr_protected twice in one class should merge the protections'
+
+    assert (TightPerson.protected_attributes - TightPerson.attributes_protected_by_default).blank?
+    assert_equal Set.new([ 'name', 'address' ]), TightPerson.accessible_attributes
+
+    assert (TightDescendant.protected_attributes - TightDescendant.attributes_protected_by_default).blank?
+    assert_equal Set.new([ 'name', 'address', 'phone_number' ]), TightDescendant.accessible_attributes
+  end
+
+  def test_mass_assignment_multiparameter_protector
+    task = Task.new
+    attributes = { "starting(1i)" => "2004", "starting(2i)" => "6", "starting(3i)" => "24" }
+    sanitized = task.sanitize_for_mass_assignment(attributes)
+    assert_equal sanitized, { }
+  end
+
+end

activemodel/test/models/mass_assignment_specific.rb

@@ -0,0 +1,57 @@
+class User
+  include ActiveModel::MassAssignmentSecurity
+  attr_protected :admin
+
+  public :sanitize_for_mass_assignment
+end
+
+class Person
+  include ActiveModel::MassAssignmentSecurity
+  attr_accessible :name, :email
+
+  public :sanitize_for_mass_assignment
+end
+
+class Firm
+  include ActiveModel::MassAssignmentSecurity
+
+  public :sanitize_for_mass_assignment
+
+  def self.attributes_protected_by_default
+    ["type"]
+  end
+end
+
+class Task
+  include ActiveModel::MassAssignmentSecurity
+  attr_protected :starting
+
+  public :sanitize_for_mass_assignment
+end
+
+class LoosePerson
+  include ActiveModel::MassAssignmentSecurity
+  attr_protected :credit_rating, :administrator
+end
+
+class LooseDescendant < LoosePerson
+  attr_protected :phone_number
+end
+
+class LooseDescendantSecond< LoosePerson
+  attr_protected :phone_number
+  attr_protected :name
+end
+
+class TightPerson
+  include ActiveModel::MassAssignmentSecurity
+  attr_accessible :name, :address
+
+  def self.attributes_protected_by_default
+    ["mobile_number"]
+  end
+end
+
+class TightDescendant < TightPerson
+  attr_accessible :phone_number
+end

activerecord/lib/active_record.rb

@@ -64,7 +64,6 @@ module ActiveRecord
     autoload :CounterCache
     autoload :DynamicFinderMatch
     autoload :DynamicScopeMatch
-    autoload :MassAssignmentSecurity
     autoload :Migration
     autoload :Migrator, 'active_record/migration'
     autoload :NamedScope

activerecord/lib/active_record/base.rb

@@ -1797,7 +1797,7 @@ MSG
     include AttributeMethods::PrimaryKey
     include AttributeMethods::TimeZoneConversion
     include AttributeMethods::Dirty
-    include MassAssignmentSecurity
+    include ActiveModel::MassAssignmentSecurity
     include Callbacks, ActiveModel::Observing, Timestamp
     include Associations, AssociationPreload, NamedScope
 

activerecord/test/cases/base_test.rb

@@ -17,7 +17,7 @@ require 'models/comment'
 require 'models/minimalistic'
 require 'models/warehouse_thing'
 require 'models/parrot'
-require 'models/mass_assignment_specific'
+require 'models/loose_person'
 require 'rexml/document'
 require 'active_support/core_ext/exception'
 

activerecord/test/cases/mass_assignment_security_test.rb

@@ -1,28 +1,11 @@
 require "cases/helper"
-require 'models/reply'
 require 'models/company'
 require 'models/subscriber'
 require 'models/keyboard'
-require 'models/mass_assignment_specific'
+require 'models/task'
 
 class MassAssignmentSecurityTest < ActiveRecord::TestCase
 
-  def test_mass_assignment_protection
-    firm = Firm.new
-    firm.attributes = { "name" => "Next Angle", "rating" => 5 }
-    assert_equal 1, firm.rating
-  end
-
-  def test_mass_assignment_protection_against_class_attribute_writers
-    [:logger, :configurations, :primary_key_prefix_type, :table_name_prefix, :table_name_suffix, :pluralize_table_names,
-     :default_timezone, :schema_format, :lock_optimistically, :record_timestamps].each do |method|
-      assert_respond_to  Task, method
-      assert_respond_to  Task, "#{method}="
-      assert_respond_to  Task.new, method
-      assert !Task.new.respond_to?("#{method}=")
-    end
-  end
-
   def test_customized_primary_key_remains_protected
     subscriber = Subscriber.new(:nick => 'webster123', :name => 'nice try')
     assert_nil subscriber.id
@@ -47,50 +30,14 @@ class MassAssignmentSecurityTest < ActiveRecord::TestCase
     end
   end
 
-  def test_mass_assignment_protection_on_defaults
-    firm = Firm.new
-    firm.attributes = { "id" => 5, "type" => "Client" }
-    assert_nil firm.id
-    assert_equal "Firm", firm[:type]
-  end
-
-  def test_mass_assignment_accessible
-    reply = Reply.new("title" => "hello", "content" => "world", "approved" => true)
-    reply.save
-
-    assert reply.approved?
-
-    reply.approved = false
-    reply.save
-
-    assert !reply.approved?
-  end
-
-  def test_mass_assignment_protection_inheritance
-    assert LoosePerson.accessible_attributes.blank?
-    assert_equal Set.new([ 'credit_rating', 'administrator', *LoosePerson.attributes_protected_by_default ]), LoosePerson.protected_attributes
-
-    assert LooseDescendant.accessible_attributes.blank?
-    assert_equal Set.new([ 'credit_rating', 'administrator', 'phone_number', *LoosePerson.attributes_protected_by_default ]), LooseDescendant.protected_attributes
-
-    assert LooseDescendantSecond.accessible_attributes.blank?
-    assert_equal Set.new([ 'credit_rating', 'administrator', 'phone_number', 'name', *LoosePerson.attributes_protected_by_default ]),
-      LooseDescendantSecond.protected_attributes, 'Running attr_protected twice in one class should merge the protections'
-
-    assert (TightPerson.protected_attributes - TightPerson.attributes_protected_by_default).blank?
-    assert_equal Set.new([ 'name', 'address' ]), TightPerson.accessible_attributes
-
-    assert (TightDescendant.protected_attributes - TightDescendant.attributes_protected_by_default).blank?
-    assert_equal Set.new([ 'name', 'address', 'phone_number' ]), TightDescendant.accessible_attributes
-  end
-
-  def test_mass_assignment_multiparameter_protector
-    task = Task.new
-    time = Time.mktime(2000, 1, 1, 1)
-    task.starting = time
-    attributes = { "starting(1i)" => "2004", "starting(2i)" => "6", "starting(3i)" => "24" }
-    task.attributes = attributes
-    assert_equal time, task.starting
+  def test_protection_against_class_attribute_writers
+    [:logger, :configurations, :primary_key_prefix_type, :table_name_prefix, :table_name_suffix, :pluralize_table_names,
+     :default_timezone, :schema_format, :lock_optimistically, :record_timestamps].each do |method|
+      assert_respond_to  Task, method
+      assert_respond_to  Task, "#{method}="
+      assert_respond_to  Task.new, method
+      assert !Task.new.respond_to?("#{method}=")
+    end
   end
 
 end

activerecord/test/models/loose_person.rb

@@ -1,6 +1,7 @@
 class LoosePerson < ActiveRecord::Base
   self.table_name = 'people'
   self.abstract_class = true
+
   attr_protected :credit_rating, :administrator
 end
 
@@ -20,13 +21,4 @@ end
 
 class TightDescendant < TightPerson
   attr_accessible :phone_number
-end
-
-class Task < ActiveRecord::Base
-  attr_protected :starting
-end
-
-class TopicWithProtectedContent < ActiveRecord::Base
-  self.table_name = 'topics'
-  attr_protected  :content
 end