From f8c9a4d3e88181cee644f91e1342bfe896ca64c6 Mon Sep 17 00:00:00 2001 From: Guillermo Iguaran Date: Tue, 17 Jul 2012 00:59:31 -0500 Subject: [PATCH] Remove MassAssignmentSecurity from ActiveModel This will be moved out to protected_attributes gem --- activemodel/lib/active_model.rb | 1 - .../forbidden_attributes_protection.rb | 8 +- .../active_model/mass_assignment_security.rb | 350 ------------------ .../permission_set.rb | 40 -- .../mass_assignment_security/sanitizer.rb | 74 ---- .../forbidden_attributes_protection_test.rb | 8 +- .../black_list_test.rb | 20 - .../permission_set_test.rb | 36 -- .../sanitizer_test.rb | 50 --- .../white_list_test.rb | 19 - .../cases/mass_assignment_security_test.rb | 118 ------ .../test/cases/secure_password_test.rb | 12 - activemodel/test/models/account.rb | 5 + activemodel/test/models/administrator.rb | 4 +- .../test/models/mass_assignment_specific.rb | 83 ----- activemodel/test/models/visitor.rb | 3 +- 16 files changed, 15 insertions(+), 816 deletions(-) delete mode 100644 activemodel/lib/active_model/mass_assignment_security.rb delete mode 100644 activemodel/lib/active_model/mass_assignment_security/permission_set.rb delete mode 100644 activemodel/lib/active_model/mass_assignment_security/sanitizer.rb delete mode 100644 activemodel/test/cases/mass_assignment_security/black_list_test.rb delete mode 100644 activemodel/test/cases/mass_assignment_security/permission_set_test.rb delete mode 100644 activemodel/test/cases/mass_assignment_security/sanitizer_test.rb delete mode 100644 activemodel/test/cases/mass_assignment_security/white_list_test.rb delete mode 100644 activemodel/test/cases/mass_assignment_security_test.rb create mode 100644 activemodel/test/models/account.rb delete mode 100644 activemodel/test/models/mass_assignment_specific.rb diff --git a/activemodel/lib/active_model.rb b/activemodel/lib/active_model.rb index f8b2b79906..40c48731a7 100644 --- a/activemodel/lib/active_model.rb +++ b/activemodel/lib/active_model.rb @@ -36,7 +36,6 @@ module ActiveModel autoload :EachValidator, 'active_model/validator' autoload :ForbiddenAttributesProtection autoload :Lint - autoload :MassAssignmentSecurity autoload :Model autoload :Name, 'active_model/naming' autoload :Naming diff --git a/activemodel/lib/active_model/forbidden_attributes_protection.rb b/activemodel/lib/active_model/forbidden_attributes_protection.rb index 39f1a20734..29bc47f151 100644 --- a/activemodel/lib/active_model/forbidden_attributes_protection.rb +++ b/activemodel/lib/active_model/forbidden_attributes_protection.rb @@ -3,11 +3,11 @@ module ActiveModel end module ForbiddenAttributesProtection - def sanitize_for_mass_assignment(new_attributes, options = {}) - if !new_attributes.respond_to?(:permitted?) || (new_attributes.respond_to?(:permitted?) && new_attributes.permitted?) - super - else + def sanitize_for_mass_assignment(attributes, options = {}) + if attributes.respond_to?(:permitted?) && !attributes.permitted? raise ActiveModel::ForbiddenAttributes + else + attributes end end end diff --git a/activemodel/lib/active_model/mass_assignment_security.rb b/activemodel/lib/active_model/mass_assignment_security.rb deleted file mode 100644 index f9841abcb0..0000000000 --- a/activemodel/lib/active_model/mass_assignment_security.rb +++ /dev/null @@ -1,350 +0,0 @@ -require 'active_support/core_ext/string/inflections' -require 'active_model/mass_assignment_security/permission_set' -require 'active_model/mass_assignment_security/sanitizer' - -module ActiveModel - # == Active Model Mass-Assignment Security - # - # Mass assignment security provides an interface for protecting attributes - # from end-user assignment. For more complex permissions, mass assignment - # security may be handled outside the model by extending a non-ActiveRecord - # class, such as a controller, with this behavior. - # - # For example, a logged in user may need to assign additional attributes - # depending on their role: - # - # class AccountsController < ApplicationController - # include ActiveModel::MassAssignmentSecurity - # - # attr_accessible :first_name, :last_name - # attr_accessible :first_name, :last_name, :plan_id, as: :admin - # - # def update - # ... - # @account.update_attributes(account_params) - # ... - # end - # - # protected - # - # def account_params - # role = admin ? :admin : :default - # sanitize_for_mass_assignment(params[:account], role) - # end - # - # end - # - # === Configuration options - # - # * mass_assignment_sanitizer - Defines sanitize method. Possible - # values are: - # * :logger (default) - writes filtered attributes to logger - # * :strict - raise ActiveModel::MassAssignmentSecurity::Error - # on any protected attribute update. - # - # You can specify your own sanitizer object eg. MySanitizer.new. - # See ActiveModel::MassAssignmentSecurity::LoggerSanitizer for - # example implementation. - module MassAssignmentSecurity - extend ActiveSupport::Concern - - included do - class_attribute :_accessible_attributes, instance_writer: false - class_attribute :_protected_attributes, instance_writer: false - class_attribute :_active_authorizer, instance_writer: false - - class_attribute :_mass_assignment_sanitizer, instance_writer: false - self.mass_assignment_sanitizer = :logger - end - - module ClassMethods - # Attributes named in this macro are protected from mass-assignment - # whenever attributes are sanitized before assignment. A role for the - # attributes is optional, if no role is provided then :default - # is used. A role can be defined by using the :as option with a - # symbol or an array of symbols as the value. - # - # Mass-assignment to these attributes will simply be ignored, to assign - # to them you can use direct writer methods. This is meant to protect - # sensitive attributes from being overwritten by malicious users - # tampering with URLs or forms. - # - # class Customer - # include ActiveModel::MassAssignmentSecurity - # - # attr_accessor :name, :email, :logins_count - # - # attr_protected :logins_count - # # Suppose that admin can not change email for customer - # attr_protected :logins_count, :email, as: :admin - # - # def assign_attributes(values, options = {}) - # sanitize_for_mass_assignment(values, options[:as]).each do |k, v| - # send("#{k}=", v) - # end - # end - # end - # - # When using the :default role: - # - # customer = Customer.new - # customer.assign_attributes({ name: 'David', email: 'a@b.com', logins_count: 5 }, as: :default) - # customer.name # => "David" - # customer.email # => "a@b.com" - # customer.logins_count # => nil - # - # And using the :admin role: - # - # customer = Customer.new - # customer.assign_attributes({ name: 'David', email: 'a@b.com', logins_count: 5}, as: :admin) - # customer.name # => "David" - # customer.email # => nil - # customer.logins_count # => nil - # - # customer.email = 'c@d.com' - # customer.email # => "c@d.com" - # - # To start from an all-closed default and enable attributes as needed, - # have a look at +attr_accessible+. - # - # Note that using Hash#except or Hash#slice in place of - # +attr_protected+ to sanitize attributes provides basically the same - # functionality, but it makes a bit tricky to deal with nested attributes. - def attr_protected(*args) - options = args.extract_options! - role = options[:as] || :default - - self._protected_attributes = protected_attributes_configs.dup - - Array(role).each do |name| - self._protected_attributes[name] = self.protected_attributes(name) + args - end - - self._active_authorizer = self._protected_attributes - end - - # Specifies a white list of model attributes that can be set via - # mass-assignment. - # - # Like +attr_protected+, a role for the attributes is optional, - # if no role is provided then :default is used. A role can be - # defined by using the :as option with a symbol or an array of - # symbols as the value. - # - # This is the opposite of the +attr_protected+ macro: Mass-assignment - # will only set attributes in this list, to assign to the rest of - # attributes you can use direct writer methods. This is meant to protect - # sensitive attributes from being overwritten by malicious users - # tampering with URLs or forms. If you'd rather start from an all-open - # default and restrict attributes as needed, have a look at - # +attr_protected+. - # - # class Customer - # include ActiveModel::MassAssignmentSecurity - # - # attr_accessor :name, :credit_rating - # - # # Both admin and default user can change name of a customer - # attr_accessible :name, as: [:admin, :default] - # # Only admin can change credit rating of a customer - # attr_accessible :credit_rating, as: :admin - # - # def assign_attributes(values, options = {}) - # sanitize_for_mass_assignment(values, options[:as]).each do |k, v| - # send("#{k}=", v) - # end - # end - # end - # - # When using the :default role: - # - # customer = Customer.new - # customer.assign_attributes({ name: 'David', credit_rating: 'Excellent', last_login: 1.day.ago }, as: :default) - # customer.name # => "David" - # customer.credit_rating # => nil - # - # customer.credit_rating = 'Average' - # customer.credit_rating # => "Average" - # - # And using the :admin role: - # - # customer = Customer.new - # customer.assign_attributes({ name: 'David', credit_rating: 'Excellent', last_login: 1.day.ago }, as: :admin) - # customer.name # => "David" - # customer.credit_rating # => "Excellent" - # - # Note that using Hash#except or Hash#slice in place of - # +attr_accessible+ to sanitize attributes provides basically the same - # functionality, but it makes a bit tricky to deal with nested attributes. - def attr_accessible(*args) - options = args.extract_options! - role = options[:as] || :default - - self._accessible_attributes = accessible_attributes_configs.dup - - Array(role).each do |name| - self._accessible_attributes[name] = self.accessible_attributes(name) + args - end - - self._active_authorizer = self._accessible_attributes - end - - # Returns an instance of ActiveModel::MassAssignmentSecurity::BlackList - # with the attributes protected by #attr_protected method. If no +role+ - # is provided, then :default is used. - # - # class Customer - # include ActiveModel::MassAssignmentSecurity - # - # attr_accessor :name, :email, :logins_count - # - # attr_protected :logins_count - # attr_protected :logins_count, :email, as: :admin - # end - # - # Customer.protected_attributes - # # => # - # - # Customer.protected_attributes(:default) - # # => # - # - # Customer.protected_attributes(:admin) - # # => # - def protected_attributes(role = :default) - protected_attributes_configs[role] - end - - # Returns an instance of ActiveModel::MassAssignmentSecurity::WhiteList - # with the attributes protected by #attr_accessible method. If no +role+ - # is provided, then :default is used. - # - # class Customer - # include ActiveModel::MassAssignmentSecurity - # - # attr_accessor :name, :credit_rating - # - # attr_accessible :name, as: [:admin, :default] - # attr_accessible :credit_rating, as: :admin - # end - # - # Customer.accessible_attributes - # # => # - # - # Customer.accessible_attributes(:default) - # # => # - # - # Customer.accessible_attributes(:admin) - # # => # - def accessible_attributes(role = :default) - accessible_attributes_configs[role] - end - - # Returns a hash with the protected attributes (by #attr_accessible or - # #attr_protected) per role. - # - # class Customer - # include ActiveModel::MassAssignmentSecurity - # - # attr_accessor :name, :credit_rating - # - # attr_accessible :name, as: [:admin, :default] - # attr_accessible :credit_rating, as: :admin - # end - # - # Customer.active_authorizers - # # => { - # # :admin=> #, - # # :default=># - # #  } - def active_authorizers - self._active_authorizer ||= protected_attributes_configs - end - alias active_authorizer active_authorizers - - # Returns an empty array by default. You can still override this to define - # the default attributes protected by #attr_protected method. - # - # class Customer - # include ActiveModel::MassAssignmentSecurity - # - # def self.attributes_protected_by_default - # [:name] - # end - # end - # - # Customer.protected_attributes - # # => # - def attributes_protected_by_default - [] - end - - # Defines sanitize method. - # - # class Customer - # include ActiveModel::MassAssignmentSecurity - # - # attr_accessor :name - # - # attr_protected :name - # - # def assign_attributes(values) - # sanitize_for_mass_assignment(values).each do |k, v| - # send("#{k}=", v) - # end - # end - # end - # - # # See ActiveModel::MassAssignmentSecurity::StrictSanitizer for more information. - # Customer.mass_assignment_sanitizer = :strict - # - # customer = Customer.new - # customer.assign_attributes(name: 'David') - # # => ActiveModel::MassAssignmentSecurity::Error: Can't mass-assign protected attributes for Customer: name - # - # Also, you can specify your own sanitizer object. - # - # class CustomSanitizer < ActiveModel::MassAssignmentSecurity::Sanitizer - # def process_removed_attributes(klass, attrs) - # raise StandardError - # end - # end - # - # Customer.mass_assignment_sanitizer = CustomSanitizer.new - # - # customer = Customer.new - # customer.assign_attributes(name: 'David') - # # => StandardError: StandardError - def mass_assignment_sanitizer=(value) - self._mass_assignment_sanitizer = if value.is_a?(Symbol) - const_get(:"#{value.to_s.camelize}Sanitizer").new(self) - else - value - end - end - - private - - def protected_attributes_configs - self._protected_attributes ||= begin - Hash.new { |h,k| h[k] = BlackList.new(attributes_protected_by_default) } - end - end - - def accessible_attributes_configs - self._accessible_attributes ||= begin - Hash.new { |h,k| h[k] = WhiteList.new } - end - end - end - - protected - - def sanitize_for_mass_assignment(attributes, role = nil) #:nodoc: - _mass_assignment_sanitizer.sanitize(self.class, attributes, mass_assignment_authorizer(role)) - end - - def mass_assignment_authorizer(role) #:nodoc: - self.class.active_authorizer[role || :default] - end - end -end diff --git a/activemodel/lib/active_model/mass_assignment_security/permission_set.rb b/activemodel/lib/active_model/mass_assignment_security/permission_set.rb deleted file mode 100644 index f104d0306c..0000000000 --- a/activemodel/lib/active_model/mass_assignment_security/permission_set.rb +++ /dev/null @@ -1,40 +0,0 @@ -require 'set' - -module ActiveModel - module MassAssignmentSecurity - class PermissionSet < Set #:nodoc: - - def +(values) - super(values.compact.map(&:to_s)) - end - - def include?(key) - super(remove_multiparameter_id(key)) - end - - def deny?(key) - raise NotImplementedError, "#deny?(key) supposed to be overwritten" - end - - protected - - def remove_multiparameter_id(key) - key.to_s.gsub(/\(.+/, '') - end - end - - class WhiteList < PermissionSet #:nodoc: - - def deny?(key) - !include?(key) - end - end - - class BlackList < PermissionSet #:nodoc: - - def deny?(key) - include?(key) - end - end - end -end diff --git a/activemodel/lib/active_model/mass_assignment_security/sanitizer.rb b/activemodel/lib/active_model/mass_assignment_security/sanitizer.rb deleted file mode 100644 index dafb7cdff3..0000000000 --- a/activemodel/lib/active_model/mass_assignment_security/sanitizer.rb +++ /dev/null @@ -1,74 +0,0 @@ -module ActiveModel - module MassAssignmentSecurity - class Sanitizer #:nodoc: - # Returns all attributes not denied by the authorizer. - def sanitize(klass, attributes, authorizer) - rejected = [] - sanitized_attributes = attributes.reject do |key, value| - rejected << key if authorizer.deny?(key) - end - process_removed_attributes(klass, rejected) unless rejected.empty? - sanitized_attributes - end - - protected - - def process_removed_attributes(klass, attrs) - raise NotImplementedError, "#process_removed_attributes(attrs) suppose to be overwritten" - end - end - - class LoggerSanitizer < Sanitizer #:nodoc: - def initialize(target) - @target = target - super() - end - - def logger - @target.logger - end - - def logger? - @target.respond_to?(:logger) && @target.logger - end - - def backtrace - if defined? Rails - Rails.backtrace_cleaner.clean(caller) - else - caller - end - end - - def process_removed_attributes(klass, attrs) - if logger? - logger.warn do - "WARNING: Can't mass-assign protected attributes for #{klass.name}: #{attrs.join(', ')}\n" + - backtrace.map { |trace| "\t#{trace}" }.join("\n") - end - end - end - end - - class StrictSanitizer < Sanitizer #:nodoc: - def initialize(target = nil) - super() - end - - def process_removed_attributes(klass, attrs) - return if (attrs - insensitive_attributes).empty? - raise ActiveModel::MassAssignmentSecurity::Error.new(klass, attrs) - end - - def insensitive_attributes - ['id'] - end - end - - class Error < StandardError #:nodoc: - def initialize(klass, attrs) - super("Can't mass-assign protected attributes for #{klass.name}: #{attrs.join(', ')}") - end - end - end -end diff --git a/activemodel/test/cases/forbidden_attributes_protection_test.rb b/activemodel/test/cases/forbidden_attributes_protection_test.rb index d3088b4fc2..680f222a30 100644 --- a/activemodel/test/cases/forbidden_attributes_protection_test.rb +++ b/activemodel/test/cases/forbidden_attributes_protection_test.rb @@ -1,5 +1,5 @@ require 'cases/helper' -require 'models/mass_assignment_specific' +require 'models/account' class ActiveModelMassUpdateProtectionTest < ActiveSupport::TestCase test "forbidden attributes cannot be used for mass updating" do @@ -8,7 +8,7 @@ class ActiveModelMassUpdateProtectionTest < ActiveSupport::TestCase define_method(:permitted?) { false } end assert_raises(ActiveModel::ForbiddenAttributes) do - SpecialPerson.new.sanitize_for_mass_assignment(params) + Account.new.sanitize_for_mass_assignment(params) end end @@ -19,14 +19,14 @@ class ActiveModelMassUpdateProtectionTest < ActiveSupport::TestCase end assert_nothing_raised do assert_equal({ "a" => "b" }, - SpecialPerson.new.sanitize_for_mass_assignment(params)) + Account.new.sanitize_for_mass_assignment(params)) end end test "regular attributes should still be allowed" do assert_nothing_raised do assert_equal({ a: "b" }, - SpecialPerson.new.sanitize_for_mass_assignment(a: "b")) + Account.new.sanitize_for_mass_assignment(a: "b")) end end end diff --git a/activemodel/test/cases/mass_assignment_security/black_list_test.rb b/activemodel/test/cases/mass_assignment_security/black_list_test.rb deleted file mode 100644 index 0ec7f8719c..0000000000 --- a/activemodel/test/cases/mass_assignment_security/black_list_test.rb +++ /dev/null @@ -1,20 +0,0 @@ -require "cases/helper" - -class BlackListTest < ActiveModel::TestCase - - def setup - @black_list = ActiveModel::MassAssignmentSecurity::BlackList.new - @included_key = 'admin' - @black_list += [ @included_key ] - end - - test "deny? is true for included items" do - assert_equal true, @black_list.deny?(@included_key) - end - - test "deny? is false for non-included items" do - assert_equal false, @black_list.deny?('first_name') - end - - -end diff --git a/activemodel/test/cases/mass_assignment_security/permission_set_test.rb b/activemodel/test/cases/mass_assignment_security/permission_set_test.rb deleted file mode 100644 index 8082c49852..0000000000 --- a/activemodel/test/cases/mass_assignment_security/permission_set_test.rb +++ /dev/null @@ -1,36 +0,0 @@ -require "cases/helper" - -class PermissionSetTest < ActiveModel::TestCase - - def setup - @permission_list = ActiveModel::MassAssignmentSecurity::PermissionSet.new - end - - test "+ stringifies added collection values" do - symbol_collection = [ :admin ] - new_list = @permission_list += symbol_collection - - assert new_list.include?('admin'), "did not add collection to #{@permission_list.inspect}}" - end - - test "+ compacts added collection values" do - added_collection = [ nil ] - new_list = @permission_list + added_collection - assert_equal new_list, @permission_list, "did not add collection to #{@permission_list.inspect}}" - end - - test "include? normalizes multi-parameter keys" do - multi_param_key = 'admin(1)' - new_list = @permission_list += [ 'admin' ] - - assert new_list.include?(multi_param_key), "#{multi_param_key} not found in #{@permission_list.inspect}" - end - - test "include? normal keys" do - normal_key = 'admin' - new_list = @permission_list += [ normal_key ] - - assert new_list.include?(normal_key), "#{normal_key} not found in #{@permission_list.inspect}" - end - -end diff --git a/activemodel/test/cases/mass_assignment_security/sanitizer_test.rb b/activemodel/test/cases/mass_assignment_security/sanitizer_test.rb deleted file mode 100644 index b141cec059..0000000000 --- a/activemodel/test/cases/mass_assignment_security/sanitizer_test.rb +++ /dev/null @@ -1,50 +0,0 @@ -require "cases/helper" -require 'active_support/logger' - -class SanitizerTest < ActiveModel::TestCase - attr_accessor :logger - - class Authorizer < ActiveModel::MassAssignmentSecurity::PermissionSet - def deny?(key) - ['admin', 'id'].include?(key) - end - end - - def setup - @logger_sanitizer = ActiveModel::MassAssignmentSecurity::LoggerSanitizer.new(self) - @strict_sanitizer = ActiveModel::MassAssignmentSecurity::StrictSanitizer.new(self) - @authorizer = Authorizer.new - end - - test "sanitize attributes" do - original_attributes = { 'first_name' => 'allowed', 'admin' => 'denied' } - attributes = @logger_sanitizer.sanitize(self.class, original_attributes, @authorizer) - - assert attributes.key?('first_name'), "Allowed key shouldn't be rejected" - assert !attributes.key?('admin'), "Denied key should be rejected" - end - - test "debug mass assignment removal with LoggerSanitizer" do - original_attributes = { 'first_name' => 'allowed', 'admin' => 'denied' } - log = StringIO.new - self.logger = ActiveSupport::Logger.new(log) - @logger_sanitizer.sanitize(self.class, original_attributes, @authorizer) - assert_match(/admin/, log.string, "Should log removed attributes: #{log.string}") - end - - test "debug mass assignment removal with StrictSanitizer" do - original_attributes = { 'first_name' => 'allowed', 'admin' => 'denied' } - assert_raise ActiveModel::MassAssignmentSecurity::Error do - @strict_sanitizer.sanitize(self.class, original_attributes, @authorizer) - end - end - - test "mass assignment insensitive attributes" do - original_attributes = {'id' => 1, 'first_name' => 'allowed'} - - assert_nothing_raised do - @strict_sanitizer.sanitize(self.class, original_attributes, @authorizer) - end - end - -end diff --git a/activemodel/test/cases/mass_assignment_security/white_list_test.rb b/activemodel/test/cases/mass_assignment_security/white_list_test.rb deleted file mode 100644 index 737b55492a..0000000000 --- a/activemodel/test/cases/mass_assignment_security/white_list_test.rb +++ /dev/null @@ -1,19 +0,0 @@ -require "cases/helper" - -class WhiteListTest < ActiveModel::TestCase - - def setup - @white_list = ActiveModel::MassAssignmentSecurity::WhiteList.new - @included_key = 'first_name' - @white_list += [ @included_key ] - end - - test "deny? is false for included items" do - assert_equal false, @white_list.deny?(@included_key) - end - - test "deny? is true for non-included items" do - assert_equal true, @white_list.deny?('admin') - end - -end diff --git a/activemodel/test/cases/mass_assignment_security_test.rb b/activemodel/test/cases/mass_assignment_security_test.rb deleted file mode 100644 index 45757615f5..0000000000 --- a/activemodel/test/cases/mass_assignment_security_test.rb +++ /dev/null @@ -1,118 +0,0 @@ -require "cases/helper" -require 'models/mass_assignment_specific' - - -class CustomSanitizer < ActiveModel::MassAssignmentSecurity::Sanitizer - - def process_removed_attributes(klass, attrs) - raise StandardError - end - -end - -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_attribute_protection_when_role_is_nil - user = User.new - expected = { "name" => "John Smith", "email" => "john@smith.com" } - sanitized = user.sanitize_for_mass_assignment(expected.merge("admin" => true), nil) - assert_equal expected, sanitized - end - - def test_only_moderator_role_attribute_accessible - user = SpecialUser.new - expected = { "name" => "John Smith", "email" => "john@smith.com" } - sanitized = user.sanitize_for_mass_assignment(expected.merge("admin" => true), :moderator) - assert_equal expected, sanitized - - sanitized = user.sanitize_for_mass_assignment({ "name" => "John Smith", "email" => "john@smith.com", "admin" => true }) - assert_equal({}, 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("admin" => true)) - assert_equal expected, sanitized - end - - def test_attributes_accessible_with_admin_role - user = Person.new - expected = { "name" => "John Smith", "email" => "john@smith.com", "admin" => true } - sanitized = user.sanitize_for_mass_assignment(expected.merge("super_powers" => true), :admin) - assert_equal expected, sanitized - end - - def test_attributes_accessible_with_roles_given_as_array - user = Account.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_with_admin_role_when_roles_given_as_array - user = Account.new - expected = { "name" => "John Smith", "email" => "john@smith.com", "admin" => true } - sanitized = user.sanitize_for_mass_assignment(expected.merge("super_powers" => true), :admin) - 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_blank LoosePerson.accessible_attributes - assert_equal Set.new(['credit_rating', 'administrator']), LoosePerson.protected_attributes - - assert_blank LoosePerson.accessible_attributes - assert_equal Set.new(['credit_rating']), LoosePerson.protected_attributes(:admin) - - assert_blank LooseDescendant.accessible_attributes - assert_equal Set.new(['credit_rating', 'administrator', 'phone_number']), LooseDescendant.protected_attributes - - assert_blank LooseDescendantSecond.accessible_attributes - 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_blank TightPerson.protected_attributes - TightPerson.attributes_protected_by_default - assert_equal Set.new(['name', 'address']), TightPerson.accessible_attributes - - assert_blank TightPerson.protected_attributes(:admin) - TightPerson.attributes_protected_by_default - assert_equal Set.new(['name', 'address', 'admin']), TightPerson.accessible_attributes(:admin) - - assert_blank TightDescendant.protected_attributes - TightDescendant.attributes_protected_by_default - assert_equal Set.new(['name', 'address', 'phone_number']), TightDescendant.accessible_attributes - - assert_blank TightDescendant.protected_attributes(:admin) - TightDescendant.attributes_protected_by_default - assert_equal Set.new(['name', 'address', 'admin', 'super_powers']), TightDescendant.accessible_attributes(:admin) - 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 - - def test_custom_sanitizer - old_sanitizer = User._mass_assignment_sanitizer - - user = User.new - User.mass_assignment_sanitizer = CustomSanitizer.new - assert_raise StandardError do - user.sanitize_for_mass_assignment("admin" => true) - end - ensure - User.mass_assignment_sanitizer = old_sanitizer - end -end \ No newline at end of file diff --git a/activemodel/test/cases/secure_password_test.rb b/activemodel/test/cases/secure_password_test.rb index 8650b0e495..19e74d3cc9 100644 --- a/activemodel/test/cases/secure_password_test.rb +++ b/activemodel/test/cases/secure_password_test.rb @@ -54,18 +54,6 @@ class SecurePasswordTest < ActiveModel::TestCase assert @user.authenticate("secret") end - test "visitor#password_digest should be protected against mass assignment" do - assert Visitor.active_authorizers[:default].kind_of?(ActiveModel::MassAssignmentSecurity::BlackList) - assert Visitor.active_authorizers[:default].include?(:password_digest) - end - - test "Administrator's mass_assignment_authorizer should be WhiteList" do - active_authorizer = Administrator.active_authorizers[:default] - assert active_authorizer.kind_of?(ActiveModel::MassAssignmentSecurity::WhiteList) - assert !active_authorizer.include?(:password_digest) - assert active_authorizer.include?(:name) - end - test "User should not be created with blank digest" do assert_raise RuntimeError do @user.run_callbacks :create diff --git a/activemodel/test/models/account.rb b/activemodel/test/models/account.rb new file mode 100644 index 0000000000..eed668d38f --- /dev/null +++ b/activemodel/test/models/account.rb @@ -0,0 +1,5 @@ +class Account + include ActiveModel::ForbiddenAttributesProtection + + public :sanitize_for_mass_assignment +end diff --git a/activemodel/test/models/administrator.rb b/activemodel/test/models/administrator.rb index 2d6d34b3e2..2f3aff290c 100644 --- a/activemodel/test/models/administrator.rb +++ b/activemodel/test/models/administrator.rb @@ -2,12 +2,10 @@ class Administrator extend ActiveModel::Callbacks include ActiveModel::Validations include ActiveModel::SecurePassword - include ActiveModel::MassAssignmentSecurity - + define_model_callbacks :create attr_accessor :name, :password_digest - attr_accessible :name has_secure_password end diff --git a/activemodel/test/models/mass_assignment_specific.rb b/activemodel/test/models/mass_assignment_specific.rb deleted file mode 100644 index 1bf828ad43..0000000000 --- a/activemodel/test/models/mass_assignment_specific.rb +++ /dev/null @@ -1,83 +0,0 @@ -class User - include ActiveModel::MassAssignmentSecurity - attr_protected :admin - - public :sanitize_for_mass_assignment -end - -class SpecialUser - include ActiveModel::MassAssignmentSecurity - attr_accessible :name, :email, :as => :moderator - - public :sanitize_for_mass_assignment -end - -class Person - include ActiveModel::MassAssignmentSecurity - attr_accessible :name, :email - attr_accessible :name, :email, :admin, :as => :admin - - public :sanitize_for_mass_assignment -end - -class SpecialPerson - include ActiveModel::MassAssignmentSecurity - include ActiveModel::ForbiddenAttributesProtection - - public :sanitize_for_mass_assignment -end - -class Account - include ActiveModel::MassAssignmentSecurity - attr_accessible :name, :email, :as => [:default, :admin] - attr_accessible :admin, :as => :admin - - 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 - attr_protected :credit_rating, :as => :admin -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 - attr_accessible :name, :address, :admin, :as => :admin - - def self.attributes_protected_by_default - ["mobile_number"] - end -end - -class TightDescendant < TightPerson - attr_accessible :phone_number - attr_accessible :super_powers, :as => :admin -end diff --git a/activemodel/test/models/visitor.rb b/activemodel/test/models/visitor.rb index d15f448516..4d7f4be097 100644 --- a/activemodel/test/models/visitor.rb +++ b/activemodel/test/models/visitor.rb @@ -2,8 +2,7 @@ class Visitor extend ActiveModel::Callbacks include ActiveModel::Validations include ActiveModel::SecurePassword - include ActiveModel::MassAssignmentSecurity - + define_model_callbacks :create has_secure_password(validations: false)