Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

mass_assignment_security moved from AR to AMo, and minor test cleanup

Signed-off-by: José Valim <jose.valim@gmail.com>
  • Loading branch information...
commit 4b66aab00fa0ea6bcc6ec81df19e44de34fd7864 1 parent 7c86e8e
@joshk joshk authored josevalim committed
View
1  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'
View
11 ...ord/lib/active_record/mass_assignment_security.rb → ...odel/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 @@ def attr_accessible(*names)
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
View
4 ...record/mass_assignment_security/permission_set.rb → ..._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
View
6 ...tive_record/mass_assignment_security/sanitizer.rb → ...ctive_model/mass_assignment_security/sanitizer.rb
@@ -1,4 +1,4 @@
-module ActiveRecord
+module ActiveModel
module MassAssignmentSecurity
module Sanitizer
@@ -17,11 +17,11 @@ def debug_protected_attribute_removal(attributes, sanitized_attributes)
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
View
4 ...cases/mass_assignment_security/black_list_test.rb → ...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
View
4 ...s/mass_assignment_security/permission_set_test.rb → ...s/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
View
5 .../cases/mass_assignment_security/sanitizer_test.rb → .../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
View
4 ...cases/mass_assignment_security/white_list_test.rb → ...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
View
52 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
View
57 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
View
1  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
View
2  activerecord/lib/active_record/base.rb
@@ -1797,7 +1797,7 @@ def object_from_yaml(string)
include AttributeMethods::PrimaryKey
include AttributeMethods::TimeZoneConversion
include AttributeMethods::Dirty
- include MassAssignmentSecurity
+ include ActiveModel::MassAssignmentSecurity
include Callbacks, ActiveModel::Observing, Timestamp
include Associations, AssociationPreload, NamedScope
View
2  activerecord/test/cases/base_test.rb
@@ -17,7 +17,7 @@
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'
View
71 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 @@ def test_mass_assigning_invalid_attribute
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
View
10 activerecord/test/models/mass_assignment_specific.rb → 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 @@ class TightPerson < ActiveRecord::Base
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
Please sign in to comment.
Something went wrong with that request. Please try again.