diff options
Diffstat (limited to 'actionpack')
| -rw-r--r-- | actionpack/lib/action_controller.rb | 2 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/base.rb | 1 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/metal/params_wrapper.rb | 9 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/metal/strong_parameters.rb | 125 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/railtie.rb | 4 | ||||
| -rw-r--r-- | actionpack/test/controller/parameters/nested_parameters_test.rb | 113 | ||||
| -rw-r--r-- | actionpack/test/controller/parameters/parameters_permit_test.rb | 73 | ||||
| -rw-r--r-- | actionpack/test/controller/parameters/parameters_require_test.rb | 10 | ||||
| -rw-r--r-- | actionpack/test/controller/params_wrapper_test.rb | 40 | ||||
| -rw-r--r-- | actionpack/test/controller/permitted_params_test.rb | 25 | ||||
| -rw-r--r-- | actionpack/test/controller/required_params_test.rb | 30 | ||||
| -rw-r--r-- | actionpack/test/fixtures/company.rb | 1 | 
12 files changed, 385 insertions, 48 deletions
diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index 8c1f548e47..1a13d7af29 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -2,6 +2,7 @@ require 'active_support/rails'  require 'abstract_controller'  require 'action_dispatch'  require 'action_controller/metal/live' +require 'action_controller/metal/strong_parameters'  module ActionController    extend ActiveSupport::Autoload @@ -34,6 +35,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 84e8c2a698..6b8d9384d4 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -214,6 +214,7 @@ module ActionController        Caching,        MimeResponds,        ImplicitRender, +      StrongParameters,        Cookies,        Flash, diff --git a/actionpack/lib/action_controller/metal/params_wrapper.rb b/actionpack/lib/action_controller/metal/params_wrapper.rb index 2736948ce0..bbb711b49d 100644 --- a/actionpack/lib/action_controller/metal/params_wrapper.rb +++ b/actionpack/lib/action_controller/metal/params_wrapper.rb @@ -42,9 +42,7 @@ module ActionController    #     end    #    # On ActiveRecord models with no +:include+ or +:exclude+ option set, -  # if attr_accessible is set on that model, it will only wrap the accessible -  # parameters, else it will only wrap the parameters returned by the class -  # method attribute_names. +  # it will only wrap the parameters returned by the class method attribute_names.    #    # If you're going to pass the parameters to an +ActiveModel+ object (such as    # <tt>User.new(params[:user])</tt>), you might consider passing the model class to @@ -165,10 +163,7 @@ module ActionController          unless options[:include] || options[:exclude]            model ||= _default_wrap_model -          role = options.fetch(:as, :default) -          if model.respond_to?(:accessible_attributes) && model.accessible_attributes(role).present? -            options[:include] = model.accessible_attributes(role).to_a -          elsif model.respond_to?(:attribute_names) && model.attribute_names.present? +          if model.respond_to?(:attribute_names) && model.attribute_names.present?              options[:include] = model.attribute_names            end          end 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..c3c1921706 --- /dev/null +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -0,0 +1,125 @@ +require 'active_support/concern' +require 'active_support/core_ext/hash/indifferent_access' +require 'active_support/rescuable' + +module ActionController +  class ParameterMissing < KeyError +    attr_reader :param + +    def initialize(param) +      @param = param +      super("key not found: #{param}") +    end +  end + +  class Parameters < ActiveSupport::HashWithIndifferentAccess +    cattr_accessor :permit_all_parameters, instance_accessor: false +    attr_accessor :permitted +    alias :permitted? :permitted + +    def initialize(attributes = nil) +      super(attributes) +      @permitted = self.class.permit_all_parameters +    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 +        elsif object.is_a?(Hash) && object.keys.all? { |k| k =~ /\A-?\d+\z/ } +          hash = object.class.new +          object.each { |k,v| hash[k] = yield v } +          hash +        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/lib/action_controller/railtie.rb b/actionpack/lib/action_controller/railtie.rb index 3ecc105e22..d7e8194bf6 100644 --- a/actionpack/lib/action_controller/railtie.rb +++ b/actionpack/lib/action_controller/railtie.rb @@ -19,6 +19,10 @@ module ActionController        ActionController::Helpers.helpers_path = app.helpers_paths      end +    initializer "action_controller.parameters_config" do |app| +      ActionController::Parameters.permit_all_parameters = app.config.action_controller.delete(:permit_all_parameters) +    end +      initializer "action_controller.set_configs" do |app|        paths   = app.config.paths        options = app.config.action_controller 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  | 
