Merge pull request #7251 from rails/integrate-strong_parameters

Integrate strong_parameters in Rails 4

7 files changed, 251 insertions, 41 deletions

diff --git a/actionpack/test/controller/parameters/nested_parameters_test.rb b/actionpack/test/controller/parameters/nested_parameters_test.rb
new file mode 100644
index 0000000000..ea67126cc5
--- /dev/null
+++ b/actionpack/test/controller/parameters/nested_parameters_test.rb
@@ -0,0 +1,113 @@
+require 'abstract_unit'
+require 'action_controller/metal/strong_parameters'
+
+class NestedParametersTest < ActiveSupport::TestCase
+  test "permitted nested parameters" do
+    params = ActionController::Parameters.new({
+      book: {
+        title: "Romeo and Juliet",
+        authors: [{
+          name: "William Shakespeare",
+          born: "1564-04-26"
+        }, {
+          name: "Christopher Marlowe"
+        }],
+        details: {
+          pages: 200,
+          genre: "Tragedy"
+        }
+      },
+      magazine: "Mjallo!"
+    })
+
+    permitted = params.permit book: [ :title, { authors: [ :name ] }, { details: :pages } ]
+
+    assert permitted.permitted?
+    assert_equal "Romeo and Juliet", permitted[:book][:title]
+    assert_equal "William Shakespeare", permitted[:book][:authors][0][:name]
+    assert_equal "Christopher Marlowe", permitted[:book][:authors][1][:name]
+    assert_equal 200, permitted[:book][:details][:pages]
+    assert_nil permitted[:book][:details][:genre]
+    assert_nil permitted[:book][:authors][1][:born]
+    assert_nil permitted[:magazine]
+  end
+
+  test "nested arrays with strings" do
+    params = ActionController::Parameters.new({
+      :book => {
+        :genres => ["Tragedy"]
+      }
+    })
+
+    permitted = params.permit :book => :genres
+    assert_equal ["Tragedy"], permitted[:book][:genres]
+  end
+
+  test "permit may specify symbols or strings" do
+    params = ActionController::Parameters.new({
+      :book => {
+        :title => "Romeo and Juliet",
+        :author => "William Shakespeare"
+      },
+      :magazine => "Shakespeare Today"
+    })
+
+    permitted = params.permit({:book => ["title", :author]}, "magazine")
+    assert_equal "Romeo and Juliet", permitted[:book][:title]
+    assert_equal "William Shakespeare", permitted[:book][:author]
+    assert_equal "Shakespeare Today", permitted[:magazine]
+  end
+
+  test "nested array with strings that should be hashes" do
+    params = ActionController::Parameters.new({
+      book: {
+        genres: ["Tragedy"]
+      }
+    })
+
+    permitted = params.permit book: { genres: :type }
+    assert_empty permitted[:book][:genres]
+  end
+
+  test "nested array with strings that should be hashes and additional values" do
+    params = ActionController::Parameters.new({
+      book: {
+        title: "Romeo and Juliet",
+        genres: ["Tragedy"]
+      }
+    })
+
+    permitted = params.permit book: [ :title, { genres: :type } ]
+    assert_equal "Romeo and Juliet", permitted[:book][:title]
+    assert_empty permitted[:book][:genres]
+  end
+
+  test "nested string that should be a hash" do
+    params = ActionController::Parameters.new({
+      book: {
+        genre: "Tragedy"
+      }
+    })
+
+    permitted = params.permit book: { genre: :type }
+    assert_nil permitted[:book][:genre]
+  end
+
+  test "fields_for-style nested params" do
+    params = ActionController::Parameters.new({
+      book: {
+        authors_attributes: {
+          :'0' => { name: 'William Shakespeare', age_of_death: '52' },
+          :'-1' => { name: 'Unattributed Assistant' }
+        }
+      }
+    })
+    permitted = params.permit book: { authors_attributes: [ :name ] }
+
+    assert_not_nil permitted[:book][:authors_attributes]['0']
+    assert_not_nil permitted[:book][:authors_attributes]['-1']
+    assert_nil permitted[:book][:authors_attributes]['0'][:age_of_death]
+    assert_equal 'William Shakespeare', permitted[:book][:authors_attributes]['0'][:name]
+    assert_equal 'Unattributed Assistant', permitted[:book][:authors_attributes]['-1'][:name]
+  end
+end
diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb
new file mode 100644
index 0000000000..7fe8e6051b
--- /dev/null
+++ b/actionpack/test/controller/parameters/parameters_permit_test.rb
@@ -0,0 +1,73 @@
+require 'abstract_unit'
+require 'action_controller/metal/strong_parameters'
+
+class ParametersPermitTest < ActiveSupport::TestCase
+  setup do
+    @params = ActionController::Parameters.new({ person: { 
+      age: "32", name: { first: "David", last: "Heinemeier Hansson" }
+    }})
+  end
+
+  test "fetch raises ParameterMissing exception" do
+    e = assert_raises(ActionController::ParameterMissing) do
+      @params.fetch :foo
+    end
+    assert_equal :foo, e.param
+  end
+
+  test "fetch doesnt raise ParameterMissing exception if there is a default" do
+    assert_equal "monkey", @params.fetch(:foo, "monkey")
+    assert_equal "monkey", @params.fetch(:foo) { "monkey" }
+  end
+
+  test "permitted is sticky on accessors" do
+    assert !@params.slice(:person).permitted?
+    assert !@params[:person][:name].permitted?
+
+    @params.each { |key, value| assert(value.permitted?) if key == :person }
+
+    assert !@params.fetch(:person).permitted?
+
+    assert !@params.values_at(:person).first.permitted?
+  end
+
+  test "permitted is sticky on mutators" do
+    assert !@params.delete_if { |k| k == :person }.permitted?
+    assert !@params.keep_if { |k,v| k == :person }.permitted?
+  end
+
+  test "permitted is sticky beyond merges" do
+    assert !@params.merge(a: "b").permitted?
+  end
+
+  test "modifying the parameters" do
+    @params[:person][:hometown] = "Chicago"
+    @params[:person][:family] = { brother: "Jonas" }
+
+    assert_equal "Chicago", @params[:person][:hometown]
+    assert_equal "Jonas", @params[:person][:family][:brother]
+  end
+
+  test "permitting parameters that are not there should not include the keys" do
+    assert !@params.permit(:person, :funky).has_key?(:funky)
+  end
+
+  test "permit state is kept on a dup" do
+    @params.permit!
+    assert_equal @params.permitted?, @params.dup.permitted?
+  end
+
+  test "permitted takes a default value when Parameters.permit_all_parameters is set" do
+    begin
+      ActionController::Parameters.permit_all_parameters = true
+      params = ActionController::Parameters.new({ person: {
+        age: "32", name: { first: "David", last: "Heinemeier Hansson" }
+      }})
+
+      assert params.slice(:person).permitted?
+      assert params[:person][:name].permitted?
+    ensure
+      ActionController::Parameters.permit_all_parameters = false
+    end
+  end
+end
diff --git a/actionpack/test/controller/parameters/parameters_require_test.rb b/actionpack/test/controller/parameters/parameters_require_test.rb
new file mode 100644
index 0000000000..bdaba8d2d8
--- /dev/null
+++ b/actionpack/test/controller/parameters/parameters_require_test.rb
@@ -0,0 +1,10 @@
+require 'abstract_unit'
+require 'action_controller/metal/strong_parameters'
+
+class ParametersRequireTest < ActiveSupport::TestCase
+  test "required parameters must be present not merely not nil" do
+    assert_raises(ActionController::ParameterMissing) do
+      ActionController::Parameters.new(person: {}).require(:person)
+    end
+  end
+end
diff --git a/actionpack/test/controller/params_wrapper_test.rb b/actionpack/test/controller/params_wrapper_test.rb
index 5b05f77045..209f021cf7 100644
--- a/actionpack/test/controller/params_wrapper_test.rb
+++ b/actionpack/test/controller/params_wrapper_test.rb
@@ -155,7 +155,6 @@ class ParamsWrapperTest < ActionController::TestCase
   end
 
   def test_derived_wrapped_keys_from_matching_model
-    User.expects(:respond_to?).with(:accessible_attributes).returns(false)
     User.expects(:respond_to?).with(:attribute_names).returns(true)
     User.expects(:attribute_names).twice.returns(["username"])
 
@@ -168,7 +167,6 @@ class ParamsWrapperTest < ActionController::TestCase
 
   def test_derived_wrapped_keys_from_specified_model
     with_default_wrapper_options do
-      Person.expects(:respond_to?).with(:accessible_attributes).returns(false)
       Person.expects(:respond_to?).with(:attribute_names).returns(true)
       Person.expects(:attribute_names).twice.returns(["username"])
 
@@ -179,46 +177,8 @@ class ParamsWrapperTest < ActionController::TestCase
       assert_parameters({ 'username' => 'sikachu', 'title' => 'Developer', 'person' => { 'username' => 'sikachu' }})
     end
   end
-  
-  def test_accessible_wrapped_keys_from_matching_model
-    User.expects(:respond_to?).with(:accessible_attributes).returns(true)
-    User.expects(:accessible_attributes).with(:default).twice.returns(["username"])
-    
-    with_default_wrapper_options do
-      @request.env['CONTENT_TYPE'] = 'application/json'
-      post :parse, { 'username' => 'sikachu', 'title' => 'Developer' }
-      assert_parameters({ 'username' => 'sikachu', 'title' => 'Developer', 'user' => { 'username' => 'sikachu' }})
-    end
-  end
-  
-  def test_accessible_wrapped_keys_from_specified_model
-    with_default_wrapper_options do
-      Person.expects(:respond_to?).with(:accessible_attributes).returns(true)
-      Person.expects(:accessible_attributes).with(:default).twice.returns(["username"])
-
-      UsersController.wrap_parameters Person
-
-      @request.env['CONTENT_TYPE'] = 'application/json'
-      post :parse, { 'username' => 'sikachu', 'title' => 'Developer' }
-      assert_parameters({ 'username' => 'sikachu', 'title' => 'Developer', 'person' => { 'username' => 'sikachu' }})
-    end
-  end
-  
-  def test_accessible_wrapped_keys_with_role_from_specified_model
-    with_default_wrapper_options do
-      Person.expects(:respond_to?).with(:accessible_attributes).returns(true)
-      Person.expects(:accessible_attributes).with(:admin).twice.returns(["username"])
-
-      UsersController.wrap_parameters Person, :as => :admin
-
-      @request.env['CONTENT_TYPE'] = 'application/json'
-      post :parse, { 'username' => 'sikachu', 'title' => 'Developer' }
-      assert_parameters({ 'username' => 'sikachu', 'title' => 'Developer', 'person' => { 'username' => 'sikachu' }})
-    end
-  end
 
   def test_not_wrapping_abstract_model
-    User.expects(:respond_to?).with(:accessible_attributes).returns(false)
     User.expects(:respond_to?).with(:attribute_names).returns(true)
     User.expects(:attribute_names).returns([])
 
diff --git a/actionpack/test/controller/permitted_params_test.rb b/actionpack/test/controller/permitted_params_test.rb
new file mode 100644
index 0000000000..f46249d712
--- /dev/null
+++ b/actionpack/test/controller/permitted_params_test.rb
@@ -0,0 +1,25 @@
+require 'abstract_unit'
+
+class PeopleController < ActionController::Base
+  def create
+    render text: params[:person].permitted? ? "permitted" : "forbidden"
+  end
+
+  def create_with_permit
+    render text: params[:person].permit(:name).permitted? ? "permitted" : "forbidden"
+  end
+end
+
+class ActionControllerPermittedParamsTest < ActionController::TestCase
+  tests PeopleController
+
+  test "parameters are forbidden" do
+    post :create, { person: { name: "Mjallo!" } }
+    assert_equal "forbidden", response.body
+  end
+
+  test "parameters can be permitted and are then not forbidden" do
+    post :create_with_permit, { person: { name: "Mjallo!" } }
+    assert_equal "permitted", response.body
+  end
+end
diff --git a/actionpack/test/controller/required_params_test.rb b/actionpack/test/controller/required_params_test.rb
new file mode 100644
index 0000000000..661bcb3945
--- /dev/null
+++ b/actionpack/test/controller/required_params_test.rb
@@ -0,0 +1,30 @@
+require 'abstract_unit'
+
+class BooksController < ActionController::Base
+  def create
+    params.require(:book).require(:name)
+    head :ok
+  end
+end
+
+class ActionControllerRequiredParamsTest < ActionController::TestCase
+  tests BooksController
+
+  test "missing required parameters will raise exception" do
+    post :create, { magazine: { name: "Mjallo!" } }
+    assert_response :bad_request
+
+    post :create, { book: { title: "Mjallo!" } }
+    assert_response :bad_request
+  end
+
+  test "required parameters that are present will not raise" do
+    post :create, { book: { name: "Mjallo!" } }
+    assert_response :ok
+  end
+
+  test "missing parameters will be mentioned in the return" do
+    post :create, { magazine: { name: "Mjallo!" } }
+    assert_equal "Required parameter missing: book", response.body
+  end
+end
diff --git a/actionpack/test/fixtures/company.rb b/actionpack/test/fixtures/company.rb
index e29978801e..f3ac3642fa 100644
--- a/actionpack/test/fixtures/company.rb
+++ b/actionpack/test/fixtures/company.rb
@@ -1,6 +1,5 @@
 class Company < ActiveRecord::Base
   has_one :mascot
-  attr_protected :rating
   self.sequence_name = :companies_nonstd_seq
 
   validates_presence_of :name