aboutsummaryrefslogtreecommitdiffstats
path: root/actionpack
diff options
context:
space:
mode:
Diffstat (limited to 'actionpack')
-rw-r--r--actionpack/lib/action_controller.rb2
-rw-r--r--actionpack/lib/action_controller/base.rb1
-rw-r--r--actionpack/lib/action_controller/metal/params_wrapper.rb9
-rw-r--r--actionpack/lib/action_controller/metal/strong_parameters.rb125
-rw-r--r--actionpack/lib/action_controller/railtie.rb4
-rw-r--r--actionpack/test/controller/parameters/nested_parameters_test.rb113
-rw-r--r--actionpack/test/controller/parameters/parameters_permit_test.rb73
-rw-r--r--actionpack/test/controller/parameters/parameters_require_test.rb10
-rw-r--r--actionpack/test/controller/params_wrapper_test.rb40
-rw-r--r--actionpack/test/controller/permitted_params_test.rb25
-rw-r--r--actionpack/test/controller/required_params_test.rb30
-rw-r--r--actionpack/test/fixtures/company.rb1
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
generated by cgit v1.2.3 (git 2.25.1) at 2025-01-09 07:19:24 +0000