diff options
Diffstat (limited to 'actionpack')
8 files changed, 340 insertions, 0 deletions
diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index 8c1f548e47..e76dc954d9 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -34,6 +34,7 @@ module ActionController autoload :Rescue autoload :Responder autoload :Streaming + autoload :StrongParameters autoload :Testing autoload :UrlFor end diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index f829f5e8a2..d799107a14 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -196,6 +196,7 @@ module ActionController Caching, MimeResponds, ImplicitRender, + StrongParameters, Cookies, Flash, diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb new file mode 100644 index 0000000000..b027901f28 --- /dev/null +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -0,0 +1,120 @@ +require 'active_support/concern' +require 'active_support/core_ext/hash/indifferent_access' +require 'active_support/rescuable' + +module ActionController + class ParameterMissing < IndexError + attr_reader :param + + def initialize(param) + @param = param + super("key not found: #{param}") + end + end + + class Parameters < ActiveSupport::HashWithIndifferentAccess + attr_accessor :permitted + alias :permitted? :permitted + + def initialize(attributes = nil) + super(attributes) + @permitted = false + end + + def permit! + @permitted = true + self + end + + def require(key) + self[key].presence || raise(ParameterMissing.new(key)) + end + + alias :required :require + + def permit(*filters) + params = self.class.new + + filters.each do |filter| + case filter + when Symbol, String then + params[filter] = self[filter] if has_key?(filter) + when Hash then + self.slice(*filter.keys).each do |key, value| + return unless value + + key = key.to_sym + + params[key] = each_element(value) do |value| + # filters are a Hash, so we expect value to be a Hash too + next if filter.is_a?(Hash) && !value.is_a?(Hash) + + value = self.class.new(value) if !value.respond_to?(:permit) + + value.permit(*Array.wrap(filter[key])) + end + end + end + end + + params.permit! + end + + def [](key) + convert_hashes_to_parameters(key, super) + end + + def fetch(key, *args) + convert_hashes_to_parameters(key, super) + rescue KeyError + raise ActionController::ParameterMissing.new(key) + end + + def slice(*keys) + self.class.new(super) + end + + def dup + super.tap do |duplicate| + duplicate.instance_variable_set :@permitted, @permitted + end + end + + private + def convert_hashes_to_parameters(key, value) + if value.is_a?(Parameters) || !value.is_a?(Hash) + value + else + # Convert to Parameters on first access + self[key] = self.class.new(value) + end + end + + def each_element(object) + if object.is_a?(Array) + object.map { |el| yield el }.compact + else + yield object + end + end + end + + module StrongParameters + extend ActiveSupport::Concern + include ActiveSupport::Rescuable + + included do + rescue_from(ActionController::ParameterMissing) do |parameter_missing_exception| + render text: "Required parameter missing: #{parameter_missing_exception.param}", status: :bad_request + end + end + + def params + @_params ||= Parameters.new(request.parameters) + end + + def params=(val) + @_params = val.is_a?(Hash) ? Parameters.new(val) : val + end + end +end 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..00a682a273 --- /dev/null +++ b/actionpack/test/controller/parameters/nested_parameters_test.rb @@ -0,0 +1,94 @@ +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 +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..5545f60d49 --- /dev/null +++ b/actionpack/test/controller/parameters/parameters_require_test.rb @@ -0,0 +1,9 @@ +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/parameters/parameters_taint_test.rb b/actionpack/test/controller/parameters/parameters_taint_test.rb new file mode 100644 index 0000000000..45fc368683 --- /dev/null +++ b/actionpack/test/controller/parameters/parameters_taint_test.rb @@ -0,0 +1,60 @@ +require 'action_controller/metal/strong_parameters' + +class ParametersTaintTest < 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_nothing_raised do + assert_equal "monkey", @params.fetch(:foo, "monkey") + assert_equal "monkey", @params.fetch(:foo) { "monkey" } + end + 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 +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/controller/tainted_params_test.rb b/actionpack/test/controller/tainted_params_test.rb new file mode 100644 index 0000000000..881b9d40fa --- /dev/null +++ b/actionpack/test/controller/tainted_params_test.rb @@ -0,0 +1,25 @@ +require 'abstract_unit' + +class PeopleController < ActionController::Base + def create + render text: params[:person].permitted? ? "untainted" : "tainted" + end + + def create_with_permit + render text: params[:person].permit(:name).permitted? ? "untainted" : "tainted" + end +end + +class ActionControllerTaintedParamsTest < ActionController::TestCase + tests PeopleController + + test "parameters are tainted" do + post :create, { person: { name: "Mjallo!" } } + assert_equal "tainted", response.body + end + + test "parameters can be permitted and are then not tainted" do + post :create_with_permit, { person: { name: "Mjallo!" } } + assert_equal "untainted", response.body + end +end |