mass_assignment_security moved from AR to AMo, and minor test cleanup

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

10 files changed, 448 insertions, 0 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/activemodel/lib/active_model/mass_assignment_security.rb b/activemodel/lib/active_model/mass_assignment_security.rb
new file mode 100644
index 0000000000..c0549ba6c0
--- /dev/null
+++ b/activemodel/lib/active_model/mass_assignment_security.rb
@@ -0,0 +1,145 @@
+require 'active_support/core_ext/class/attribute.rb'
+require 'active_model/mass_assignment_security/permission_set'
+
+module ActiveModel
+  # = Active Record Mass-Assignment Security
+  module MassAssignmentSecurity
+    extend ActiveSupport::Concern
+
+    included do
+      class_attribute :_accessible_attributes
+      class_attribute :_protected_attributes
+      class_attribute :_active_authorizer
+    end
+
+    # 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 ActiveRecord::MassAssignmentSecurity
+    #
+    #   attr_accessible :first_name, :last_name
+    #
+    #   def self.admin_accessible_attributes
+    #     accessible_attributes + [ :plan_id ]
+    #   end
+    #
+    #   def update
+    #     ...
+    #     @account.update_attributes(account_params)
+    #     ...
+    #   end
+    #
+    #   protected
+    #
+    #   def account_params
+    #     sanitize_for_mass_assignment(params[:account])
+    #   end
+    #
+    #   def mass_assignment_authorizer
+    #     admin ? admin_accessible_attributes : super
+    #   end
+    #
+    # end
+    #
+    module ClassMethods
+      # Attributes named in this macro are protected from mass-assignment,
+      # such as <tt>new(attributes)</tt>,
+      # <tt>update_attributes(attributes)</tt>, or
+      # <tt>attributes=(attributes)</tt>.
+      #
+      # 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 < ActiveRecord::Base
+      #     attr_protected :credit_rating
+      #   end
+      #
+      #   customer = Customer.new("name" => David, "credit_rating" => "Excellent")
+      #   customer.credit_rating # => nil
+      #   customer.attributes = { "description" => "Jolly fellow", "credit_rating" => "Superb" }
+      #   customer.credit_rating # => nil
+      #
+      #   customer.credit_rating = "Average"
+      #   customer.credit_rating # => "Average"
+      #
+      # To start from an all-closed default and enable attributes as needed,
+      # have a look at +attr_accessible+.
+      #
+      # Note that using <tt>Hash#except</tt> or <tt>Hash#slice</tt> in place of +attr_protected+
+      # to sanitize attributes won't provide sufficient protection.
+      def attr_protected(*names)
+        self._protected_attributes = self.protected_attributes + names
+        self._active_authorizer = self._protected_attributes
+      end
+
+      # Specifies a white list of model attributes that can be set via
+      # mass-assignment, such as <tt>new(attributes)</tt>,
+      # <tt>update_attributes(attributes)</tt>, or
+      # <tt>attributes=(attributes)</tt>
+      #
+      # 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 < ActiveRecord::Base
+      #     attr_accessible :name, :nickname
+      #   end
+      #
+      #   customer = Customer.new(:name => "David", :nickname => "Dave", :credit_rating => "Excellent")
+      #   customer.credit_rating # => nil
+      #   customer.attributes = { :name => "Jolly fellow", :credit_rating => "Superb" }
+      #   customer.credit_rating # => nil
+      #
+      #   customer.credit_rating = "Average"
+      #   customer.credit_rating # => "Average"
+      #
+      # Note that using <tt>Hash#except</tt> or <tt>Hash#slice</tt> in place of +attr_accessible+
+      # to sanitize attributes won't provide sufficient protection.
+      def attr_accessible(*names)
+        self._accessible_attributes = self.accessible_attributes + names
+        self._active_authorizer = self._accessible_attributes
+      end
+
+      def protected_attributes
+        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 = self.logger if self.respond_to?(:logger) }
+      end
+
+      def active_authorizer
+        self._active_authorizer ||= protected_attributes
+      end
+
+      def attributes_protected_by_default
+        []
+      end
+    end
+
+    protected
+
+      def sanitize_for_mass_assignment(attributes)
+        mass_assignment_authorizer.sanitize(attributes)
+      end
+
+      def mass_assignment_authorizer
+        self.class.active_authorizer
+      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
new file mode 100644
index 0000000000..978da493d7
--- /dev/null
+++ b/activemodel/lib/active_model/mass_assignment_security/permission_set.rb
@@ -0,0 +1,41 @@
+require 'active_model/mass_assignment_security/sanitizer'
+
+module ActiveModel
+  module MassAssignmentSecurity
+
+    class PermissionSet < Set
+      attr_accessor :logger
+
+      def +(values)
+        super(values.map(&:to_s))
+      end
+
+      def include?(key)
+        super(remove_multiparameter_id(key))
+      end
+
+      protected
+
+        def remove_multiparameter_id(key)
+          key.gsub(/\(.+/, '')
+        end
+    end
+
+    class WhiteList < PermissionSet
+      include Sanitizer
+
+      def deny?(key)
+        !include?(key)
+      end
+    end
+
+    class BlackList < PermissionSet
+      include Sanitizer
+
+      def deny?(key)
+        include?(key)
+      end
+    end
+
+  end
+end
\ No newline at end of file
diff --git a/activemodel/lib/active_model/mass_assignment_security/sanitizer.rb b/activemodel/lib/active_model/mass_assignment_security/sanitizer.rb
new file mode 100644
index 0000000000..275e481fb8
--- /dev/null
+++ b/activemodel/lib/active_model/mass_assignment_security/sanitizer.rb
@@ -0,0 +1,29 @@
+module ActiveModel
+  module MassAssignmentSecurity
+    module Sanitizer
+
+      # Returns all attributes not denied by the authorizer.
+      def sanitize(attributes)
+        sanitized_attributes = attributes.reject { |key, value| deny?(key) }
+        debug_protected_attribute_removal(attributes, sanitized_attributes) if debug?
+        sanitized_attributes
+      end
+
+      protected
+
+        def debug_protected_attribute_removal(attributes, sanitized_attributes)
+          removed_keys = attributes.keys - sanitized_attributes.keys
+          warn!(removed_keys) if removed_keys.any?
+        end
+
+        def debug?
+          self.logger.present?
+        end
+
+        def warn!(attrs)
+          self.logger.debug "WARNING: Can't mass-assign protected attributes: #{attrs.join(', ')}"
+        end
+
+    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
new file mode 100644
index 0000000000..ed168bc016
--- /dev/null
+++ b/activemodel/test/cases/mass_assignment_security/black_list_test.rb
@@ -0,0 +1,28 @@
+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
+
+  test "sanitize attributes" do
+    original_attributes = { 'first_name' => 'allowed', 'admin' => 'denied', 'admin(1)' => 'denied' }
+    attributes = @black_list.sanitize(original_attributes)
+
+    assert attributes.key?('first_name'), "Allowed key shouldn't be rejected"
+    assert !attributes.key?('admin'),     "Denied key should be rejected"
+    assert !attributes.key?('admin(1)'),  "Multi-parameter key should be detected"
+  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
new file mode 100644
index 0000000000..d005b638e4
--- /dev/null
+++ b/activemodel/test/cases/mass_assignment_security/permission_set_test.rb
@@ -0,0 +1,30 @@
+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 "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
new file mode 100644
index 0000000000..367207aab3
--- /dev/null
+++ b/activemodel/test/cases/mass_assignment_security/sanitizer_test.rb
@@ -0,0 +1,37 @@
+require "cases/helper"
+require 'logger'
+
+class SanitizerTest < ActiveModel::TestCase
+
+  class SanitizingAuthorizer
+    include ActiveModel::MassAssignmentSecurity::Sanitizer
+
+    attr_accessor :logger
+
+    def deny?(key)
+       [ 'admin' ].include?(key)
+    end
+
+  end
+
+  def setup
+    @sanitizer = SanitizingAuthorizer.new
+  end
+
+  test "sanitize attributes" do
+    original_attributes = { 'first_name' => 'allowed', 'admin' => 'denied' }
+    attributes = @sanitizer.sanitize(original_attributes)
+
+    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" do
+    original_attributes = { 'first_name' => 'allowed', 'admin' => 'denied' }
+    log = StringIO.new
+    @sanitizer.logger = Logger.new(log)
+    @sanitizer.sanitize(original_attributes)
+    assert (log.string =~ /admin/), "Should log removed attributes: #{log.string}"
+  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
new file mode 100644
index 0000000000..aa3596ad2a
--- /dev/null
+++ b/activemodel/test/cases/mass_assignment_security/white_list_test.rb
@@ -0,0 +1,28 @@
+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
+
+  test "sanitize attributes" do
+    original_attributes = { 'first_name' => 'allowed', 'admin' => 'denied', 'admin(1)' => 'denied' }
+    attributes = @white_list.sanitize(original_attributes)
+
+    assert attributes.key?('first_name'), "Allowed key shouldn't be rejected"
+    assert !attributes.key?('admin'),     "Denied key should be rejected"
+    assert !attributes.key?('admin(1)'),  "Multi-parameter key should be detected"
+  end
+
+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