Fix AC::Parameters not being sanitized for query methods.

4 files changed, 64 insertions, 7 deletions

diff --git a/activemodel/lib/active_model/forbidden_attributes_protection.rb b/activemodel/lib/active_model/forbidden_attributes_protection.rb
index b4fa378601..d2c6a89cc2 100644
--- a/activemodel/lib/active_model/forbidden_attributes_protection.rb
+++ b/activemodel/lib/active_model/forbidden_attributes_protection.rb
@@ -17,8 +17,9 @@ module ActiveModel
   module ForbiddenAttributesProtection # :nodoc:
     protected
       def sanitize_for_mass_assignment(attributes)
-        if attributes.respond_to?(:permitted?) && !attributes.permitted?
-          raise ActiveModel::ForbiddenAttributesError
+        if attributes.respond_to?(:permitted?)
+          raise ActiveModel::ForbiddenAttributesError if !attributes.permitted?
+          attributes.to_h
         else
           attributes
         end
diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
index eb53a18f0f..ccb0ab18ae 100644
--- a/activerecord/lib/active_record/relation/query_methods.rb
+++ b/activerecord/lib/active_record/relation/query_methods.rb
@@ -558,11 +558,8 @@ module ActiveRecord
     end
 
     def where!(opts, *rest) # :nodoc:
-      if Hash === opts
-        opts = sanitize_forbidden_attributes(opts)
-        references!(PredicateBuilder.references(opts))
-      end
-
+      opts = sanitize_forbidden_attributes(opts)
+      references!(PredicateBuilder.references(opts)) if Hash === opts
       self.where_clause += where_clause_factory.build(opts, rest)
       self
     end
@@ -619,6 +616,7 @@ module ActiveRecord
     end
 
     def having!(opts, *rest) # :nodoc:
+      opts = sanitize_forbidden_attributes(opts)
       references!(PredicateBuilder.references(opts)) if Hash === opts
 
       self.having_clause += having_clause_factory.build(opts, rest)
diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb
index aa10817527..d904b802fa 100644
--- a/activerecord/test/cases/calculations_test.rb
+++ b/activerecord/test/cases/calculations_test.rb
@@ -681,4 +681,36 @@ class CalculationsTest < ActiveRecord::TestCase
     end
     assert block_called
   end
+
+  def test_having_with_strong_parameters
+    protected_params = Class.new do
+      attr_reader :permitted
+      alias :permitted? :permitted
+
+      def initialize(parameters)
+        @parameters = parameters
+        @permitted = false
+      end
+
+      def to_h
+        @parameters
+      end
+
+      def permit!
+        @permitted = true
+        self
+      end
+    end
+
+    params = protected_params.new(credit_limit: '50')
+
+    assert_raises(ActiveModel::ForbiddenAttributesError) do
+      Account.group(:id).having(params)
+    end
+
+    result = Account.group(:id).having(params.permit!)
+    assert_equal 50, result[0].credit_limit
+    assert_equal 50, result[1].credit_limit
+    assert_equal 50, result[2].credit_limit
+  end
 end
diff --git a/activerecord/test/cases/relation/where_test.rb b/activerecord/test/cases/relation/where_test.rb
index 6af31017d6..c3a1471205 100644
--- a/activerecord/test/cases/relation/where_test.rb
+++ b/activerecord/test/cases/relation/where_test.rb
@@ -276,5 +276,31 @@ module ActiveRecord
 
       assert_equal essays(:david_modest_proposal), essay
     end
+
+    def test_where_with_strong_parameters
+      protected_params = Class.new do
+        attr_reader :permitted
+        alias :permitted? :permitted
+
+        def initialize(parameters)
+          @parameters = parameters
+          @permitted = false
+        end
+
+        def to_h
+          @parameters
+        end
+
+        def permit!
+          @permitted = true
+          self
+        end
+      end
+
+      author = authors(:david)
+      params = protected_params.new(name: author.name)
+      assert_raises(ActiveModel::ForbiddenAttributesError) { Author.where(params) }
+      assert_equal author, Author.where(params.permit!).first
+    end
   end
 end