aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJosh Kalderimis <josh.kalderimis@gmail.com>2010-07-08 18:16:36 +0200
committerJosé Valim <jose.valim@gmail.com>2010-07-08 18:28:45 +0200
commit4b66aab00fa0ea6bcc6ec81df19e44de34fd7864 (patch)
treeff870b932c26869d6a27a6a058d37baa6c289e0a
parent7c86e8e21ba6a1f88226ddd0cf012a563f234d06 (diff)
downloadrails-4b66aab00fa0ea6bcc6ec81df19e44de34fd7864.tar.gz
rails-4b66aab00fa0ea6bcc6ec81df19e44de34fd7864.tar.bz2
rails-4b66aab00fa0ea6bcc6ec81df19e44de34fd7864.zip
mass_assignment_security moved from AR to AMo, and minor test cleanup
Signed-off-by: José Valim <jose.valim@gmail.com>
-rw-r--r--activemodel/lib/active_model.rb1
-rw-r--r--activemodel/lib/active_model/mass_assignment_security.rb (renamed from activerecord/lib/active_record/mass_assignment_security.rb)11
-rw-r--r--activemodel/lib/active_model/mass_assignment_security/permission_set.rb (renamed from activerecord/lib/active_record/mass_assignment_security/permission_set.rb)4
-rw-r--r--activemodel/lib/active_model/mass_assignment_security/sanitizer.rb (renamed from activerecord/lib/active_record/mass_assignment_security/sanitizer.rb)6
-rw-r--r--activemodel/test/cases/mass_assignment_security/black_list_test.rb (renamed from activerecord/test/cases/mass_assignment_security/black_list_test.rb)4
-rw-r--r--activemodel/test/cases/mass_assignment_security/permission_set_test.rb (renamed from activerecord/test/cases/mass_assignment_security/permission_set_test.rb)4
-rw-r--r--activemodel/test/cases/mass_assignment_security/sanitizer_test.rb (renamed from activerecord/test/cases/mass_assignment_security/sanitizer_test.rb)5
-rw-r--r--activemodel/test/cases/mass_assignment_security/white_list_test.rb (renamed from activerecord/test/cases/mass_assignment_security/white_list_test.rb)4
-rw-r--r--activemodel/test/cases/mass_assignment_security_test.rb52
-rw-r--r--activemodel/test/models/mass_assignment_specific.rb57
-rw-r--r--activerecord/lib/active_record.rb1
-rw-r--r--activerecord/lib/active_record/base.rb2
-rw-r--r--activerecord/test/cases/base_test.rb2
-rw-r--r--activerecord/test/cases/mass_assignment_security_test.rb71
-rw-r--r--activerecord/test/models/loose_person.rb (renamed from activerecord/test/models/mass_assignment_specific.rb)10
15 files changed, 143 insertions, 91 deletions
diff --git a/activemodel/lib/active_model.rb b/activemodel/lib/active_model.rb
index 026430fee3..5ed21a39c2 100644
--- a/activemodel/lib/active_model.rb
+++ b/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'
diff --git a/activerecord/lib/active_record/mass_assignment_security.rb b/activemodel/lib/active_model/mass_assignment_security.rb
index 8f4d6e1c74..c0549ba6c0 100644
--- a/activerecord/lib/active_record/mass_assignment_security.rb
+++ b/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
diff --git a/activerecord/lib/active_record/mass_assignment_security/permission_set.rb b/activemodel/lib/active_model/mass_assignment_security/permission_set.rb
index 8446a4103b..978da493d7 100644
--- a/activerecord/lib/active_record/mass_assignment_security/permission_set.rb
+++ b/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
diff --git a/activerecord/lib/active_record/mass_assignment_security/sanitizer.rb b/activemodel/lib/active_model/mass_assignment_security/sanitizer.rb
index 11de35f9d6..275e481fb8 100644
--- a/activerecord/lib/active_record/mass_assignment_security/sanitizer.rb
+++ b/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
diff --git a/activerecord/test/cases/mass_assignment_security/black_list_test.rb b/activemodel/test/cases/mass_assignment_security/black_list_test.rb
index 8b7f48a5f6..ed168bc016 100644
--- a/activerecord/test/cases/mass_assignment_security/black_list_test.rb
+++ b/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
diff --git a/activerecord/test/cases/mass_assignment_security/permission_set_test.rb b/activemodel/test/cases/mass_assignment_security/permission_set_test.rb
index ca8985042a..d005b638e4 100644
--- a/activerecord/test/cases/mass_assignment_security/permission_set_test.rb
+++ b/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
diff --git a/activerecord/test/cases/mass_assignment_security/sanitizer_test.rb b/activemodel/test/cases/mass_assignment_security/sanitizer_test.rb
index 122bc7e114..367207aab3 100644
--- a/activerecord/test/cases/mass_assignment_security/sanitizer_test.rb
+++ b/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
diff --git a/activerecord/test/cases/mass_assignment_security/white_list_test.rb b/activemodel/test/cases/mass_assignment_security/white_list_test.rb
index 4601263437..aa3596ad2a 100644
--- a/activerecord/test/cases/mass_assignment_security/white_list_test.rb
+++ b/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
diff --git a/activemodel/test/cases/mass_assignment_security_test.rb b/activemodel/test/cases/mass_assignment_security_test.rb
new file mode 100644
index 0000000000..0f7a38b0bc
--- /dev/null
+++ b/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 \ No newline at end of file
diff --git a/activemodel/test/models/mass_assignment_specific.rb b/activemodel/test/models/mass_assignment_specific.rb
new file mode 100644
index 0000000000..2a8fe170c2
--- /dev/null
+++ b/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 \ No newline at end of file
diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb
index 9ab05a0548..e2f2508ae8 100644
--- a/activerecord/lib/active_record.rb
+++ b/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
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 56b4ba8260..f22a9de7b1 100644
--- a/activerecord/lib/active_record/base.rb
+++ b/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
diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb
index 2eecb6e344..5a72e9c6e0 100644
--- a/activerecord/test/cases/base_test.rb
+++ b/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'
diff --git a/activerecord/test/cases/mass_assignment_security_test.rb b/activerecord/test/cases/mass_assignment_security_test.rb
index 07154da93b..025ec1d3fa 100644
--- a/activerecord/test/cases/mass_assignment_security_test.rb
+++ b/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 \ No newline at end of file
diff --git a/activerecord/test/models/mass_assignment_specific.rb b/activerecord/test/models/loose_person.rb
index 13a80e0197..256c281d0d 100644
--- a/activerecord/test/models/mass_assignment_specific.rb
+++ b/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 \ No newline at end of file