Permalink
Browse files

Merge pull request #7251 from rails/integrate-strong_parameters

Integrate strong_parameters in Rails 4
  • Loading branch information...
dhh committed Sep 18, 2012
2 parents ade7010 + 3919fcd commit c49d959e9d40101f1712a452004695f4ce27d84c
Showing with 662 additions and 2,232 deletions.
  1. +2 −0 actionpack/lib/action_controller.rb
  2. +1 −0 actionpack/lib/action_controller/base.rb
  3. +2 −7 actionpack/lib/action_controller/metal/params_wrapper.rb
  4. +125 −0 actionpack/lib/action_controller/metal/strong_parameters.rb
  5. +4 −0 actionpack/lib/action_controller/railtie.rb
  6. +113 −0 actionpack/test/controller/parameters/nested_parameters_test.rb
  7. +73 −0 actionpack/test/controller/parameters/parameters_permit_test.rb
  8. +10 −0 actionpack/test/controller/parameters/parameters_require_test.rb
  9. +0 −40 actionpack/test/controller/params_wrapper_test.rb
  10. +25 −0 actionpack/test/controller/permitted_params_test.rb
  11. +30 −0 actionpack/test/controller/required_params_test.rb
  12. +0 −1 actionpack/test/fixtures/company.rb
  13. +2 −1 activemodel/lib/active_model.rb
  14. +19 −0 activemodel/lib/active_model/deprecated_mass_assignment_security.rb
  15. +14 −0 activemodel/lib/active_model/forbidden_attributes_protection.rb
  16. +0 −350 activemodel/lib/active_model/mass_assignment_security.rb
  17. +0 −40 activemodel/lib/active_model/mass_assignment_security/permission_set.rb
  18. +0 −74 activemodel/lib/active_model/mass_assignment_security/sanitizer.rb
  19. +16 −0 activemodel/test/cases/deprecated_mass_assignment_security_test.rb
  20. +36 −0 activemodel/test/cases/forbidden_attributes_protection_test.rb
  21. +0 −20 activemodel/test/cases/mass_assignment_security/black_list_test.rb
  22. +0 −36 activemodel/test/cases/mass_assignment_security/permission_set_test.rb
  23. +0 −50 activemodel/test/cases/mass_assignment_security/sanitizer_test.rb
  24. +0 −19 activemodel/test/cases/mass_assignment_security/white_list_test.rb
  25. +0 −118 activemodel/test/cases/mass_assignment_security_test.rb
  26. +0 −12 activemodel/test/cases/secure_password_test.rb
  27. +5 −0 activemodel/test/models/account.rb
  28. +1 −3 activemodel/test/models/administrator.rb
  29. +0 −76 activemodel/test/models/mass_assignment_specific.rb
  30. +3 −0 activemodel/test/models/project.rb
  31. +1 −2 activemodel/test/models/visitor.rb
  32. +3 −3 activerecord/lib/active_record/associations/association.rb
  33. +10 −10 activerecord/lib/active_record/associations/collection_association.rb
  34. +6 −7 activerecord/lib/active_record/associations/collection_proxy.rb
  35. +2 −2 activerecord/lib/active_record/associations/has_many_through_association.rb
  36. +8 −8 activerecord/lib/active_record/associations/singular_association.rb
  37. +9 −87 activerecord/lib/active_record/attribute_assignment.rb
  38. +2 −3 activerecord/lib/active_record/attribute_methods/primary_key.rb
  39. +3 −3 activerecord/lib/active_record/core.rb
  40. +11 −27 activerecord/lib/active_record/nested_attributes.rb
  41. +9 −9 activerecord/lib/active_record/persistence.rb
  42. +2 −2 activerecord/lib/active_record/reflection.rb
  43. +6 −6 activerecord/lib/active_record/relation.rb
  44. +0 −1 activerecord/lib/active_record/schema_migration.rb
  45. +3 −3 activerecord/lib/active_record/validations.rb
  46. +0 −5 activerecord/lib/rails/generators/active_record/model/templates/model.rb
  47. +0 −35 activerecord/test/cases/associations/has_many_associations_test.rb
  48. +0 −15 activerecord/test/cases/associations/has_many_through_associations_test.rb
  49. +0 −32 activerecord/test/cases/associations/has_one_associations_test.rb
  50. +0 −4 activerecord/test/cases/base_test.rb
  51. +6 −22 activerecord/test/cases/deprecated_dynamic_methods_test.rb
  52. +1 −1 activerecord/test/cases/dup_test.rb
  53. +49 −0 activerecord/test/cases/forbidden_attributes_protection_test.rb
  54. +0 −966 activerecord/test/cases/mass_assignment_security_test.rb
  55. +0 −40 activerecord/test/cases/persistence_test.rb
  56. +0 −6 activerecord/test/cases/validations_test.rb
  57. +3 −5 activerecord/test/models/bulb.rb
  58. +0 −1 activerecord/test/models/company.rb
  59. +0 −1 activerecord/test/models/company_in_module.rb
  60. +0 −12 activerecord/test/models/person.rb
  61. +0 −2 activerecord/test/models/reader.rb
  62. +0 −2 activerecord/test/models/reply.rb
  63. +0 −5 railties/lib/rails/generators/rails/app/templates/config/application.rb
  64. +0 −3 railties/lib/rails/generators/rails/app/templates/config/environments/development.rb.tt
  65. +0 −5 railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt
  66. +2 −0 railties/lib/rails/generators/rails/scaffold_controller/scaffold_controller_generator.rb
  67. +14 −2 railties/lib/rails/generators/rails/scaffold_controller/templates/controller.rb
  68. +22 −13 railties/test/application/configuration_test.rb
  69. +0 −1 railties/test/application/loading_test.rb
  70. +0 −15 railties/test/generators/app_generator_test.rb
  71. +0 −10 railties/test/generators/model_generator_test.rb
  72. +5 −2 railties/test/generators/scaffold_controller_generator_test.rb
  73. +4 −4 railties/test/generators/scaffold_generator_test.rb
  74. +0 −3 railties/test/isolation/abstract_unit.rb
@@ -2,6 +2,7 @@
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
@@ -214,6 +214,7 @@ def self.without_modules(*modules)
Caching,
MimeResponds,
ImplicitRender,
StrongParameters,
Cookies,
Flash,
@@ -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 @@ def _set_wrapper_defaults(options, model=nil)
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
@@ -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

This comment has been minimized.

Show comment
Hide comment
@schneems

schneems Sep 18, 2012

Member

Can we add some docs for StrongParameters module and Parameters class? Seems trivial, but when I find myself code spelunking in a new file while trying to add a bugfix or feature having the intent of classes in comments in the file makes the experience much easier.

@schneems

schneems Sep 18, 2012

Member

Can we add some docs for StrongParameters module and Parameters class? Seems trivial, but when I find myself code spelunking in a new file while trying to add a bugfix or feature having the intent of classes in comments in the file makes the experience much easier.

This comment has been minimized.

Show comment
Hide comment
@frodsan

frodsan Sep 19, 2012

Contributor

I'm currently working on that #7692 :)

@frodsan

frodsan Sep 19, 2012

Contributor

I'm currently working on that #7692 :)

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
@@ -19,6 +19,10 @@ class Railtie < Rails::Railtie #:nodoc:
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
@@ -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]

This comment has been minimized.

Show comment
Hide comment
@RKushnir

RKushnir Sep 18, 2012

Contributor

Shouldn't it be 0 instead of 1? The authors[1] doesn't have a born attribute in the original params, so it would be nil anyway.

@RKushnir

RKushnir Sep 18, 2012

Contributor

Shouldn't it be 0 instead of 1? The authors[1] doesn't have a born attribute in the original params, so it would be nil anyway.

This comment has been minimized.

Show comment
Hide comment
@radar

radar Sep 19, 2012

Contributor

Fixed with #7693.

@radar

radar Sep 19, 2012

Contributor

Fixed with #7693.

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
@@ -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
@@ -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
Oops, something went wrong.

7 comments on commit c49d959

@radar

This comment has been minimized.

Show comment
Hide comment
@radar

radar Sep 19, 2012

Contributor

RIP attr_accessible 😢

Contributor

radar replied Sep 19, 2012

RIP attr_accessible 😢

@Fivell

This comment has been minimized.

Show comment
Hide comment
@Fivell

Fivell Sep 20, 2012

Contributor

good work!

Contributor

Fivell replied Sep 20, 2012

good work!

@sj26

This comment has been minimized.

Show comment
Hide comment
@sj26

sj26 Sep 20, 2012

Contributor

👍

Contributor

sj26 replied Sep 20, 2012

👍

@pda

This comment has been minimized.

Show comment
Hide comment
@pda

pda Sep 21, 2012

Contributor

Awesome!
Can we pretend attr_accessible never happened?

Contributor

pda replied Sep 21, 2012

Awesome!
Can we pretend attr_accessible never happened?

@radar

This comment has been minimized.

Show comment
Hide comment
@radar

radar Sep 21, 2012

Contributor
Contributor

radar replied Sep 21, 2012

@robin850

This comment has been minimized.

Show comment
Hide comment
@robin850

robin850 Sep 22, 2012

Member

Nice, thanks @guilleiguaran and @dhh ! 👍

Member

robin850 replied Sep 22, 2012

Nice, thanks @guilleiguaran and @dhh ! 👍

@joneslee85

This comment has been minimized.

Show comment
Hide comment
@joneslee85

joneslee85 Sep 23, 2012

Contributor

I will miss you, attr_accessible...NOT

Contributor

joneslee85 replied Sep 23, 2012

I will miss you, attr_accessible...NOT

Please sign in to comment.