Merge pull request #7251 from rails/integrate-strong_parameters

Integrate strong_parameters in Rails 4

14 files changed, 74 insertions, 171 deletions

diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb
index 9f47e7e631..495f0cde59 100644
--- a/activerecord/lib/active_record/associations/association.rb
+++ b/activerecord/lib/active_record/associations/association.rb
@@ -233,10 +233,10 @@ module ActiveRecord
         def stale_state
         end
 
-        def build_record(attributes, options)
-          reflection.build_association(attributes, options) do |record|
+        def build_record(attributes)
+          reflection.build_association(attributes) do |record|
             attributes = create_scope.except(*(record.changed - [reflection.foreign_key]))
-            record.assign_attributes(attributes, :without_protection => true)
+            record.assign_attributes(attributes)
           end
         end
     end
diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
index 424c1a5b79..fe3e5b00f7 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -95,22 +95,22 @@ module ActiveRecord
         first_or_last(:last, *args)
       end
 
-      def build(attributes = {}, options = {}, &block)
+      def build(attributes = {}, &block)
         if attributes.is_a?(Array)
-          attributes.collect { |attr| build(attr, options, &block) }
+          attributes.collect { |attr| build(attr, &block) }
         else
-          add_to_target(build_record(attributes, options)) do |record|
+          add_to_target(build_record(attributes)) do |record|
             yield(record) if block_given?
           end
         end
       end
 
-      def create(attributes = {}, options = {}, &block)
-        create_record(attributes, options, &block)
+      def create(attributes = {}, &block)
+        create_record(attributes, &block)
       end
 
-      def create!(attributes = {}, options = {}, &block)
-        create_record(attributes, options, true, &block)
+      def create!(attributes = {}, &block)
+        create_record(attributes, true, &block)
       end
 
       # Add +records+ to this association. Returns +self+ so method calls may
@@ -425,16 +425,16 @@ module ActiveRecord
           persisted + memory
         end
 
-        def create_record(attributes, options, raise = false, &block)
+        def create_record(attributes, raise = false, &block)
           unless owner.persisted?
             raise ActiveRecord::RecordNotSaved, "You cannot call create unless the parent is saved"
           end
 
           if attributes.is_a?(Array)
-            attributes.collect { |attr| create_record(attr, options, raise, &block) }
+            attributes.collect { |attr| create_record(attr, raise, &block) }
           else
             transaction do
-              add_to_target(build_record(attributes, options)) do |record|
+              add_to_target(build_record(attributes)) do |record|
                 yield(record) if block_given?
                 insert_record(record, true, raise)
               end
diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb
index 66132b7260..c113957faa 100644
--- a/activerecord/lib/active_record/associations/collection_proxy.rb
+++ b/activerecord/lib/active_record/associations/collection_proxy.rb
@@ -222,8 +222,8 @@ module ActiveRecord
       #
       #   person.pets.size  # => 5 # size of the collection
       #   person.pets.count # => 0 # count from database
-      def build(attributes = {}, options = {}, &block)
-        @association.build(attributes, options, &block)
+      def build(attributes = {}, &block)
+        @association.build(attributes, &block)
       end
 
       ##
@@ -253,8 +253,8 @@ module ActiveRecord
       #   #       #<Pet id: 2, name: "Spook", person_id: 1>,
       #   #       #<Pet id: 3, name: "Choo-Choo", person_id: 1>
       #   #    ]
-      def create(attributes = {}, options = {}, &block)
-        @association.create(attributes, options, &block)
+      def create(attributes = {}, &block)
+        @association.create(attributes, &block)
       end
 
       ##
@@ -265,14 +265,13 @@ module ActiveRecord
       #   end
       #
       #   class Pet
-      #     attr_accessible :name
       #     validates :name, presence: true
       #   end
       #
       #   person.pets.create!(name: nil)
       #   # => ActiveRecord::RecordInvalid: Validation failed: Name can't be blank
-      def create!(attributes = {}, options = {}, &block)
-        @association.create!(attributes, options, &block)
+      def create!(attributes = {}, &block)
+        @association.create!(attributes, &block)
       end
 
       ##
diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb
index d8f14c896c..c7d8a84a7e 100644
--- a/activerecord/lib/active_record/associations/has_many_through_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -96,10 +96,10 @@ module ActiveRecord
           @through_records.delete(record.object_id)
         end
 
-        def build_record(attributes, options = {})
+        def build_record(attributes)
           ensure_not_nested
 
-          record = super(attributes, options)
+          record = super(attributes)
 
           inverse = source_reflection.inverse_of
           if inverse
diff --git a/activerecord/lib/active_record/associations/singular_association.rb b/activerecord/lib/active_record/associations/singular_association.rb
index b84cb4922d..32f4557c28 100644
--- a/activerecord/lib/active_record/associations/singular_association.rb
+++ b/activerecord/lib/active_record/associations/singular_association.rb
@@ -17,16 +17,16 @@ module ActiveRecord
         replace(record)
       end
 
-      def create(attributes = {}, options = {}, &block)
-        create_record(attributes, options, &block)
+      def create(attributes = {}, &block)
+        create_record(attributes, &block)
       end
 
-      def create!(attributes = {}, options = {}, &block)
-        create_record(attributes, options, true, &block)
+      def create!(attributes = {}, &block)
+        create_record(attributes, true, &block)
       end
 
-      def build(attributes = {}, options = {})
-        record = build_record(attributes, options)
+      def build(attributes = {})
+        record = build_record(attributes)
         yield(record) if block_given?
         set_new_record(record)
         record
@@ -51,8 +51,8 @@ module ActiveRecord
           replace(record)
         end
 
-        def create_record(attributes, options, raise_error = false)
-          record = build_record(attributes, options)
+        def create_record(attributes, raise_error = false)
+          record = build_record(attributes)
           yield(record) if block_given?
           saved = record.save
           set_new_record(record)
diff --git a/activerecord/lib/active_record/attribute_assignment.rb b/activerecord/lib/active_record/attribute_assignment.rb
index d9989274c8..ccc61ec5d3 100644
--- a/activerecord/lib/active_record/attribute_assignment.rb
+++ b/activerecord/lib/active_record/attribute_assignment.rb
@@ -1,98 +1,32 @@
 
 module ActiveRecord
-  ActiveSupport.on_load(:active_record_config) do
-    mattr_accessor :whitelist_attributes,      instance_accessor: false
-    mattr_accessor :mass_assignment_sanitizer, instance_accessor: false
-  end
-
   module AttributeAssignment
     extend ActiveSupport::Concern
-    include ActiveModel::MassAssignmentSecurity
-
-    included do
-      initialize_mass_assignment_sanitizer
-    end
-
-    module ClassMethods
-      def inherited(child) # :nodoc:
-        child.send :initialize_mass_assignment_sanitizer if self == Base
-        super
-      end
-
-      private
-
-      # The primary key and inheritance column can never be set by mass-assignment for security reasons.
-      def attributes_protected_by_default
-        default = [ primary_key, inheritance_column ]
-        default << 'id' unless primary_key.eql? 'id'
-        default
-      end
-
-      def initialize_mass_assignment_sanitizer
-        attr_accessible(nil) if Model.whitelist_attributes
-        self.mass_assignment_sanitizer = Model.mass_assignment_sanitizer if Model.mass_assignment_sanitizer
-      end
-    end
+    include ActiveModel::DeprecatedMassAssignmentSecurity
+    include ActiveModel::ForbiddenAttributesProtection
 
     # Allows you to set all the attributes at once by passing in a hash with keys
     # matching the attribute names (which again matches the column names).
     #
-    # If any attributes are protected by either +attr_protected+ or
-    # +attr_accessible+ then only settable attributes will be assigned.
-    #
-    #   class User < ActiveRecord::Base
-    #     attr_protected :is_admin
-    #   end
-    #
-    #   user = User.new
-    #   user.attributes = { :username => 'Phusion', :is_admin => true }
-    #   user.username   # => "Phusion"
-    #   user.is_admin?  # => false
+    # If the passed hash responds to permitted? method and the return value
+    # of this method is false an ActiveModel::ForbiddenAttributesError exception
+    # is raised.
     def attributes=(new_attributes)
       return unless new_attributes.is_a?(Hash)
 
       assign_attributes(new_attributes)
     end
 
-    # Allows you to set all the attributes for a particular mass-assignment
-    # security role by passing in a hash of attributes with keys matching
-    # the attribute names (which again matches the column names) and the role
-    # name using the :as option.
-    #
-    # To bypass mass-assignment security you can use the :without_protection => true
-    # option.
-    #
-    #   class User < ActiveRecord::Base
-    #     attr_accessible :name
-    #     attr_accessible :name, :is_admin, :as => :admin
-    #   end
-    #
-    #   user = User.new
-    #   user.assign_attributes({ :name => 'Josh', :is_admin => true })
-    #   user.name       # => "Josh"
-    #   user.is_admin?  # => false
-    #
-    #   user = User.new
-    #   user.assign_attributes({ :name => 'Josh', :is_admin => true }, :as => :admin)
-    #   user.name       # => "Josh"
-    #   user.is_admin?  # => true
-    #
-    #   user = User.new
-    #   user.assign_attributes({ :name => 'Josh', :is_admin => true }, :without_protection => true)
-    #   user.name       # => "Josh"
-    #   user.is_admin?  # => true
-    def assign_attributes(new_attributes, options = {})
+    # Allows you to set all the attributes by passing in a hash of attributes with
+    # keys matching the attribute names (which again matches the column names)
+    def assign_attributes(new_attributes)
       return if new_attributes.blank?
 
       attributes                  = new_attributes.stringify_keys
       multi_parameter_attributes  = []
       nested_parameter_attributes = []
-      previous_options            = @mass_assignment_options
-      @mass_assignment_options    = options
 
-      unless options[:without_protection]
-        attributes = sanitize_for_mass_assignment(attributes, mass_assignment_role)
-      end
+      attributes = sanitize_for_mass_assignment(attributes)
 
       attributes.each do |k, v|
         if k.include?("(")
@@ -106,18 +40,6 @@ module ActiveRecord
 
       assign_nested_parameter_attributes(nested_parameter_attributes) unless nested_parameter_attributes.empty?
       assign_multiparameter_attributes(multi_parameter_attributes) unless multi_parameter_attributes.empty?
-    ensure
-      @mass_assignment_options = previous_options
-    end
-
-    protected
-
-    def mass_assignment_options
-      @mass_assignment_options ||= {}
-    end
-
-    def mass_assignment_role
-      mass_assignment_options[:as] || :default
     end
 
     private
diff --git a/activerecord/lib/active_record/attribute_methods/primary_key.rb b/activerecord/lib/active_record/attribute_methods/primary_key.rb
index 7b7811a706..aa6704d5c9 100644
--- a/activerecord/lib/active_record/attribute_methods/primary_key.rb
+++ b/activerecord/lib/active_record/attribute_methods/primary_key.rb
@@ -18,7 +18,7 @@ module ActiveRecord
 
       # Sets the primary key value
       def id=(value)
-        write_attribute(self.class.primary_key, value)
+        write_attribute(self.class.primary_key, value) if self.class.primary_key
       end
 
       # Queries the primary key value
@@ -53,8 +53,7 @@ module ActiveRecord
         end
 
         # Defines the primary key field -- can be overridden in subclasses. Overwriting will negate any effect of the
-        # primary_key_prefix_type setting, though. Since primary keys are usually protected from mass assignment,
-        # remember to let your database generate them or include the key in +attr_accessible+.
+        # primary_key_prefix_type setting, though.
         def primary_key
           @primary_key = reset_primary_key unless defined? @primary_key
           @primary_key
diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb
index 43ffca0227..8f798830d4 100644
--- a/activerecord/lib/active_record/core.rb
+++ b/activerecord/lib/active_record/core.rb
@@ -172,7 +172,7 @@ module ActiveRecord
     #
     #   # Instantiates a single new object bypassing mass-assignment security
     #   User.new({ :first_name => 'Jamie', :is_admin => true }, :without_protection => true)
-    def initialize(attributes = nil, options = {})
+    def initialize(attributes = nil)
       defaults = self.class.column_defaults.dup
       defaults.each { |k, v| defaults[k] = v.dup if v.duplicable? }
 
@@ -183,7 +183,7 @@ module ActiveRecord
       ensure_proper_type
       populate_with_current_scope_attributes
 
-      assign_attributes(attributes, options) if attributes
+      assign_attributes(attributes) if attributes
 
       yield self if block_given?
       run_callbacks :initialize unless _initialize_callbacks.empty?
@@ -386,7 +386,7 @@ module ActiveRecord
       @destroyed               = false
       @marked_for_destruction  = false
       @new_record              = true
-      @mass_assignment_options = nil
+      @txn                     = nil
       @_start_transaction_state = {}
     end
   end
diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb
index 3005dc042c..a33285b16f 100644
--- a/activerecord/lib/active_record/nested_attributes.rb
+++ b/activerecord/lib/active_record/nested_attributes.rb
@@ -194,18 +194,6 @@ module ActiveRecord
     # the parent model is saved. This happens inside the transaction initiated
     # by the parents save method. See ActiveRecord::AutosaveAssociation.
     #
-    # === Using with attr_accessible
-    #
-    # The use of <tt>attr_accessible</tt> can interfere with nested attributes
-    # if you're not careful. For example, if the <tt>Member</tt> model above
-    # was using <tt>attr_accessible</tt> like this:
-    #
-    #   attr_accessible :name
-    #
-    # You would need to modify it to look like this:
-    #
-    #   attr_accessible :name, :posts_attributes
-    #
     # === Validating the presence of a parent model
     #
     # If you want to validate that a child record is associated with a parent
@@ -224,9 +212,7 @@ module ActiveRecord
     module ClassMethods
       REJECT_ALL_BLANK_PROC = proc { |attributes| attributes.all? { |key, value| key == '_destroy' || value.blank? } }
 
-      # Defines an attributes writer for the specified association(s). If you
-      # are using <tt>attr_protected</tt> or <tt>attr_accessible</tt>, then you
-      # will need to add the attribute writer to the allowed list.
+      # Defines an attributes writer for the specified association(s).
       #
       # Supported options:
       # [:allow_destroy]
@@ -296,7 +282,7 @@ module ActiveRecord
                 remove_method(:#{association_name}_attributes=)
               end
               def #{association_name}_attributes=(attributes)
-                assign_nested_attributes_for_#{type}_association(:#{association_name}, attributes, mass_assignment_options)
+                assign_nested_attributes_for_#{type}_association(:#{association_name}, attributes)
               end
             eoruby
           else
@@ -340,15 +326,15 @@ module ActiveRecord
 
       if (options[:update_only] || !attributes['id'].blank?) && (record = send(association_name)) &&
           (options[:update_only] || record.id.to_s == attributes['id'].to_s)
-        assign_to_or_mark_for_destruction(record, attributes, options[:allow_destroy], assignment_opts) unless call_reject_if(association_name, attributes)
+        assign_to_or_mark_for_destruction(record, attributes, options[:allow_destroy]) unless call_reject_if(association_name, attributes)
 
-      elsif attributes['id'].present? && !assignment_opts[:without_protection]
+      elsif attributes['id'].present?
         raise_nested_attributes_record_not_found(association_name, attributes['id'])
 
       elsif !reject_new_record?(association_name, attributes)
         method = "build_#{association_name}"
         if respond_to?(method)
-          send(method, attributes.except(*unassignable_keys(assignment_opts)), assignment_opts)
+          send(method, attributes.except(*UNASSIGNABLE_KEYS))
         else
           raise ArgumentError, "Cannot build association `#{association_name}'. Are you trying to build a polymorphic one-to-one association?"
         end
@@ -382,7 +368,7 @@ module ActiveRecord
     #     { :name => 'John' },
     #     { :id => '2', :_destroy => true }
     #   ])
-    def assign_nested_attributes_for_collection_association(association_name, attributes_collection, assignment_opts = {})
+    def assign_nested_attributes_for_collection_association(association_name, attributes_collection)
       options = self.nested_attributes_options[association_name]
 
       unless attributes_collection.is_a?(Hash) || attributes_collection.is_a?(Array)
@@ -427,7 +413,7 @@ module ActiveRecord
 
         if attributes['id'].blank?
           unless reject_new_record?(association_name, attributes)
-            association.build(attributes.except(*unassignable_keys(assignment_opts)), assignment_opts)
+            association.build(attributes.except(*UNASSIGNABLE_KEYS))
           end
         elsif existing_record = existing_records.detect { |record| record.id.to_s == attributes['id'].to_s }
           unless association.loaded? || call_reject_if(association_name, attributes)
@@ -443,10 +429,8 @@ module ActiveRecord
           end
 
           if !call_reject_if(association_name, attributes)
-            assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy], assignment_opts)
+            assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy])
           end
-        elsif assignment_opts[:without_protection]
-          association.build(attributes.except(*unassignable_keys(assignment_opts)), assignment_opts)
         else
           raise_nested_attributes_record_not_found(association_name, attributes['id'])
         end
@@ -455,8 +439,8 @@ module ActiveRecord
 
     # Updates a record with the +attributes+ or marks it for destruction if
     # +allow_destroy+ is +true+ and has_destroy_flag? returns +true+.
-    def assign_to_or_mark_for_destruction(record, attributes, allow_destroy, assignment_opts)
-      record.assign_attributes(attributes.except(*unassignable_keys(assignment_opts)), assignment_opts)
+    def assign_to_or_mark_for_destruction(record, attributes, allow_destroy)
+      record.assign_attributes(attributes.except(*UNASSIGNABLE_KEYS))
       record.mark_for_destruction if has_destroy_flag?(attributes) && allow_destroy
     end
 
@@ -487,7 +471,7 @@ module ActiveRecord
     end
 
     def unassignable_keys(assignment_opts)
-      assignment_opts[:without_protection] ? UNASSIGNABLE_KEYS - %w[id] : UNASSIGNABLE_KEYS
+      UNASSIGNABLE_KEYS
     end
   end
 end
diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb
index 7bd65c180d..6f38262b86 100644
--- a/activerecord/lib/active_record/persistence.rb
+++ b/activerecord/lib/active_record/persistence.rb
@@ -35,11 +35,11 @@ module ActiveRecord
       #   User.create([{ :first_name => 'Jamie' }, { :first_name => 'Jeremy' }]) do |u|
       #     u.is_admin = false
       #   end
-      def create(attributes = nil, options = {}, &block)
+      def create(attributes = nil, &block)
         if attributes.is_a?(Array)
-          attributes.collect { |attr| create(attr, options, &block) }
+          attributes.collect { |attr| create(attr, &block) }
         else
-          object = new(attributes, options, &block)
+          object = new(attributes, &block)
           object.save
           object
         end
@@ -186,24 +186,24 @@ module ActiveRecord
     #
     # When updating model attributes, mass-assignment security protection is respected.
     # If no +:as+ option is supplied then the +:default+ role will be used.
-    # If you want to bypass the protection given by +attr_protected+ and
-    # +attr_accessible+ then you can do so using the +:without_protection+ option.
-    def update_attributes(attributes, options = {})
+    # If you want to bypass the forbidden attributes protection then you can do so using
+    # the +:without_protection+ option.
+    def update_attributes(attributes)
       # The following transaction covers any possible database side-effects of the
       # attributes assignment. For example, setting the IDs of a child collection.
       with_transaction_returning_status do
-        assign_attributes(attributes, options)
+        assign_attributes(attributes)
         save
       end
     end
 
     # Updates its receiver just like +update_attributes+ but calls <tt>save!</tt> instead
     # of +save+, so an exception is raised if the record is invalid.
-    def update_attributes!(attributes, options = {})
+    def update_attributes!(attributes)
       # The following transaction covers any possible database side-effects of the
       # attributes assignment. For example, setting the IDs of a child collection.
       with_transaction_returning_status do
-        assign_attributes(attributes, options)
+        assign_attributes(attributes)
         save!
       end
     end
diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index d9c65fa170..f322b96f79 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -181,8 +181,8 @@ module ActiveRecord
 
       # Returns a new, unsaved instance of the associated class. +options+ will
       # be passed to the class's constructor.
-      def build_association(*options, &block)
-        klass.new(*options, &block)
+      def build_association(attributes, &block)
+        klass.new(attributes, &block)
       end
 
       def table_name
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index 2d0457636e..ed80422336 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -151,22 +151,22 @@ module ActiveRecord
     #     user.last_name = "O'Hara"
     #   end
     #   # => <User id: 2, first_name: 'Scarlett', last_name: 'Johansson'>
-    def first_or_create(attributes = nil, options = {}, &block)
-      first || create(attributes, options, &block)
+    def first_or_create(attributes = nil, &block)
+      first || create(attributes, &block)
     end
 
     # Like <tt>first_or_create</tt> but calls <tt>create!</tt> so an exception is raised if the created record is invalid.
     #
     # Expects arguments in the same format as <tt>Base.create!</tt>.
-    def first_or_create!(attributes = nil, options = {}, &block)
-      first || create!(attributes, options, &block)
+    def first_or_create!(attributes = nil, &block)
+      first || create!(attributes, &block)
     end
 
     # Like <tt>first_or_create</tt> but calls <tt>new</tt> instead of <tt>create</tt>.
     #
     # Expects arguments in the same format as <tt>Base.new</tt>.
-    def first_or_initialize(attributes = nil, options = {}, &block)
-      first || new(attributes, options, &block)
+    def first_or_initialize(attributes = nil, &block)
+      first || new(attributes, &block)
     end
 
     # Runs EXPLAIN on the query or queries triggered by this relation and
diff --git a/activerecord/lib/active_record/schema_migration.rb b/activerecord/lib/active_record/schema_migration.rb
index ca22154c84..9830abe7d8 100644
--- a/activerecord/lib/active_record/schema_migration.rb
+++ b/activerecord/lib/active_record/schema_migration.rb
@@ -4,7 +4,6 @@ require 'active_record/base'
 
 module ActiveRecord
   class SchemaMigration < ActiveRecord::Base
-    attr_accessible :version
 
     def self.table_name
       "#{Base.table_name_prefix}schema_migrations#{Base.table_name_suffix}"
diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb
index cef2bbd563..ed561bfb3c 100644
--- a/activerecord/lib/active_record/validations.rb
+++ b/activerecord/lib/active_record/validations.rb
@@ -32,11 +32,11 @@ module ActiveRecord
     module ClassMethods
       # Creates an object just like Base.create but calls <tt>save!</tt> instead of +save+
       # so an exception is raised if the record is invalid.
-      def create!(attributes = nil, options = {}, &block)
+      def create!(attributes = nil, &block)
         if attributes.is_a?(Array)
-          attributes.collect { |attr| create!(attr, options, &block) }
+          attributes.collect { |attr| create!(attr, &block) }
         else
-          object = new(attributes, options)
+          object = new(attributes)
           yield(object) if block_given?
           object.save!
           object