Update and fix forbidden attributes tests

Add AC::Parameters tests for WhereChain#not

4 files changed, 70 insertions, 38 deletions

diff --git a/activerecord/lib/active_record/inheritance.rb b/activerecord/lib/active_record/inheritance.rb
index 589c70db0d..8b719e0bcb 100644
--- a/activerecord/lib/active_record/inheritance.rb
+++ b/activerecord/lib/active_record/inheritance.rb
@@ -198,10 +198,11 @@ module ActiveRecord
       # If this is a StrongParameters hash, and access to inheritance_column is not permitted,
       # this will ignore the inheritance column and return nil
       def subclass_from_attributes?(attrs)
-        attribute_names.include?(inheritance_column) && attrs.is_a?(Hash)
+        attribute_names.include?(inheritance_column) && (attrs.is_a?(Hash) || attrs.respond_to?(:permitted?))
       end
 
       def subclass_from_attributes(attrs)
+        attrs = attrs.to_h if attrs.respond_to?(:permitted?)
         subclass_name = attrs.with_indifferent_access[inheritance_column]
 
         if subclass_name.present?
diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
index 2dc52982c9..7a4bf5338d 100644
--- a/activerecord/lib/active_record/relation/query_methods.rb
+++ b/activerecord/lib/active_record/relation/query_methods.rb
@@ -13,6 +13,8 @@ module ActiveRecord
     # WhereChain objects act as placeholder for queries in which #where does not have any parameter.
     # In this case, #where must be chained with #not to return a new relation.
     class WhereChain
+      include ActiveModel::ForbiddenAttributesProtection
+
       def initialize(scope)
         @scope = scope
       end
@@ -41,6 +43,8 @@ module ActiveRecord
       #    User.where.not(name: "Jon", role: "admin")
       #    # SELECT * FROM users WHERE name != 'Jon' AND role != 'admin'
       def not(opts, *rest)
+        opts = sanitize_forbidden_attributes(opts)
+
         where_clause = @scope.send(:where_clause_factory).build(opts, rest)
 
         @scope.references!(PredicateBuilder.references(opts)) if Hash === opts
diff --git a/activerecord/test/cases/forbidden_attributes_protection_test.rb b/activerecord/test/cases/forbidden_attributes_protection_test.rb
index f4e7646f03..d89045cebf 100644
--- a/activerecord/test/cases/forbidden_attributes_protection_test.rb
+++ b/activerecord/test/cases/forbidden_attributes_protection_test.rb
@@ -3,12 +3,14 @@ require 'active_support/core_ext/hash/indifferent_access'
 require 'models/person'
 require 'models/company'
 
-class ProtectedParams < ActiveSupport::HashWithIndifferentAccess
+class ProtectedParams
   attr_accessor :permitted
   alias :permitted? :permitted
 
+  delegate :keys, :key?, :has_key?, :empty?, to: :@parameters
+
   def initialize(attributes)
-    super(attributes)
+    @parameters = attributes.with_indifferent_access
     @permitted = false
   end
 
@@ -17,6 +19,18 @@ class ProtectedParams < ActiveSupport::HashWithIndifferentAccess
     self
   end
 
+  def [](key)
+    @parameters[key]
+  end
+
+  def to_h
+    @parameters
+  end
+
+  def stringify_keys
+    dup
+  end
+
   def dup
     super.tap do |duplicate|
       duplicate.instance_variable_set :@permitted, @permitted
@@ -75,6 +89,13 @@ class ForbiddenAttributesProtectionTest < ActiveRecord::TestCase
     end
   end
 
+  def test_create_with_works_with_permitted_params
+    params = ProtectedParams.new(first_name: 'Guille').permit!
+
+    person = Person.create_with(params).create!
+    assert_equal 'Guille', person.first_name
+  end
+
   def test_create_with_works_with_params_values
     params = ProtectedParams.new(first_name: 'Guille')
 
@@ -90,10 +111,51 @@ class ForbiddenAttributesProtectionTest < ActiveRecord::TestCase
     end
   end
 
+  def test_where_works_with_permitted_params
+    params = ProtectedParams.new(first_name: 'Guille').permit!
+
+    person = Person.where(params).create!
+    assert_equal 'Guille', person.first_name
+  end
+
   def test_where_works_with_params_values
     params = ProtectedParams.new(first_name: 'Guille')
 
     person = Person.where(first_name: params[:first_name]).create!
     assert_equal 'Guille', person.first_name
   end
+
+  def test_where_not_checks_permitted
+    params = ProtectedParams.new(first_name: 'Guille', gender: 'm')
+
+    assert_raises(ActiveModel::ForbiddenAttributesError) do
+      Person.where().not(params)
+    end
+  end
+
+  def test_where_not_works_with_permitted_params
+    params = ProtectedParams.new(first_name: 'Guille').permit!
+    Person.create!(params)
+    assert_empty Person.where.not(params).select {|p| p.first_name == 'Guille' }
+  end
+
+  def test_strong_params_style_objects_work_with_singular_associations
+    params = ProtectedParams.new( name: "Stern", ship_attributes: ProtectedParams.new(name: "The Black Rock").permit!).permit!
+    part = ShipPart.new(params)
+
+    assert_equal "Stern", part.name
+    assert_equal "The Black Rock", part.ship.name
+  end
+
+  def test_strong_params_style_objects_work_with_collection_associations
+    params = ProtectedParams.new(
+      trinkets_attributes: ProtectedParams.new(
+        "0" => ProtectedParams.new(name: "Necklace").permit!,
+        "1" => ProtectedParams.new(name: "Spoon").permit! ) ).permit!
+    part = ShipPart.new(params)
+
+    assert_equal "Necklace", part.trinkets[0].name
+    assert_equal "Spoon", part.trinkets[1].name
+  end
+
 end
diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb
index 93cb631a04..0b700afcb4 100644
--- a/activerecord/test/cases/nested_attributes_test.rb
+++ b/activerecord/test/cases/nested_attributes_test.rb
@@ -1068,39 +1068,4 @@ class TestHasManyAutosaveAssociationWhichItselfHasAutosaveAssociations < ActiveR
     assert_not part.valid?
     assert_equal ["Ship name can't be blank"], part.errors.full_messages
   end
-
-  class ProtectedParameters
-    def initialize(hash)
-      @hash = hash
-    end
-
-    def permitted?
-      true
-    end
-
-    def [](key)
-      @hash[key]
-    end
-
-    def to_h
-      @hash
-    end
-  end
-
-  test "strong params style objects can be assigned for singular associations" do
-    params = { name: "Stern", ship_attributes:
-               ProtectedParameters.new(name: "The Black Rock") }
-    part = ShipPart.new(params)
-
-    assert_equal "Stern", part.name
-    assert_equal "The Black Rock", part.ship.name
-  end
-
-  test "strong params style objects can be assigned for collection associations" do
-    params = { trinkets_attributes: ProtectedParameters.new("0" => ProtectedParameters.new(name: "Necklace"), "1" => ProtectedParameters.new(name: "Spoon")) }
-    part = ShipPart.new(params)
-
-    assert_equal "Necklace", part.trinkets[0].name
-    assert_equal "Spoon", part.trinkets[1].name
-  end
 end