From a0000d0ea096dc3c6f811b57d006a80d59057e53 Mon Sep 17 00:00:00 2001 From: Josh Kalderimis Date: Thu, 12 May 2011 09:24:42 +0200 Subject: [PATCH] updated AR#create! to accept an options hash so the mass-assignment security role can be passed in, also updated the Changelog to mention the change to some of the AR method signatures. --- activerecord/CHANGELOG | 28 ++++++++++--------- .../lib/active_record/session_store.rb | 4 +-- activerecord/lib/active_record/validations.rb | 6 ++-- .../cases/mass_assignment_security_test.rb | 24 ++++++++++++++++ 4 files changed, 44 insertions(+), 18 deletions(-) diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 4dc652c08e..32bcf02139 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -2,7 +2,7 @@ * CSV Fixtures are deprecated and support will be removed in Rails 3.2.0 -* AR#new, AR#create and AR#update_attributes all accept a second hash as option that allows you +* AR#new, AR#create, AR#create!, AR#update_attributes and AR#update_attributes! all accept a second hash as option that allows you to specify which role to consider when assigning attributes. This is built on top of ActiveModel's new mass assignment capabilities: @@ -14,7 +14,9 @@ Post.new(params[:post], :as => :admin) assign_attributes() with similar API was also added and attributes=(params, guard) was deprecated. - + + Please note that this changes the method signatures for AR#new, AR#create, AR#create!, AR#update_attributes and AR#update_attributes!. If you have overwritten these methods you should update them accordingly. + [Josh Kalderimis] * default_scope can take a block, lambda, or any other object which responds to `call` for lazy @@ -611,12 +613,12 @@ query. * Add Support for updating deeply nested models from a single form. #1202 [Eloy Duran] - class Book < ActiveRecord::Base - has_one :author - has_many :pages + class Book < ActiveRecord::Base + has_one :author + has_many :pages - accepts_nested_attributes_for :author, :pages - end + accepts_nested_attributes_for :author, :pages + end * Make after_save callbacks fire only if the record was successfully saved. #1735 [Michael Lovitt] @@ -1036,7 +1038,7 @@ so newlines etc are escaped #10385 [Norbert Crombach] "foo.bar" => "`foo`.`bar`" * Complete the assimilation of Sexy Migrations from ErrFree [Chris Wanstrath, PJ Hyett] - http://errtheblog.com/post/2381 + http://errtheblog.com/post/2381 * Qualified column names work in hash conditions, like :conditions => { 'comments.created_at' => ... }. #9733 [Jack Danger Canty] @@ -1152,7 +1154,7 @@ single-table inheritance. #3833, #9886 [Gabriel Gironda, rramdas, François Bea * Improve performance and functionality of the postgresql adapter. Closes #8049 [roderickvd] - For more information see: http://dev.rubyonrails.org/ticket/8049 + For more information see: http://dev.rubyonrails.org/ticket/8049 * Don't clobber includes passed to has_many.count [Jack Danger Canty] @@ -1662,8 +1664,8 @@ during calendar reform. #7649, #7724 [fedot, Geoff Buesing] * Added support for conditions on Base.exists? #5689 [Josh Peek]. Examples: assert (Topic.exists?(:author_name => "David")) - assert (Topic.exists?(:author_name => "Mary", :approved => true)) - assert (Topic.exists?(["parent_id = ?", 1])) + assert (Topic.exists?(:author_name => "Mary", :approved => true)) + assert (Topic.exists?(["parent_id = ?", 1])) * Schema dumper quotes date :default values. [Dave Thomas] @@ -2119,8 +2121,8 @@ during calendar reform. #7649, #7724 [fedot, Geoff Buesing] * Added support for conditions on Base.exists? #5689 [Josh Peek]. Examples: assert (Topic.exists?(:author_name => "David")) - assert (Topic.exists?(:author_name => "Mary", :approved => true)) - assert (Topic.exists?(["parent_id = ?", 1])) + assert (Topic.exists?(:author_name => "Mary", :approved => true)) + assert (Topic.exists?(["parent_id = ?", 1])) * Schema dumper quotes date :default values. [Dave Thomas] diff --git a/activerecord/lib/active_record/session_store.rb b/activerecord/lib/active_record/session_store.rb index 98e21db908..c3e976002e 100644 --- a/activerecord/lib/active_record/session_store.rb +++ b/activerecord/lib/active_record/session_store.rb @@ -40,7 +40,7 @@ module ActiveRecord # You must implement these methods: # # self.find_by_session_id(session_id) - # initialize(hash_of_session_id_and_data) + # initialize(hash_of_session_id_and_data, options_hash = {}) # attr_reader :session_id # attr_accessor :data # save @@ -125,7 +125,7 @@ module ActiveRecord end end - def initialize(attributes = nil) + def initialize(attributes = nil, options = {}) @data = nil super end diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index de36dd20b3..59b6876135 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 save! instead of +save+ # so an exception is raised if the record is invalid. - def create!(attributes = nil, &block) + def create!(attributes = nil, options = {}, &block) if attributes.is_a?(Array) - attributes.collect { |attr| create!(attr, &block) } + attributes.collect { |attr| create!(attr, options, &block) } else - object = new(attributes) + object = new(attributes, options) yield(object) if block_given? object.save! object diff --git a/activerecord/test/cases/mass_assignment_security_test.rb b/activerecord/test/cases/mass_assignment_security_test.rb index 062a642e50..765033852d 100644 --- a/activerecord/test/cases/mass_assignment_security_test.rb +++ b/activerecord/test/cases/mass_assignment_security_test.rb @@ -181,6 +181,18 @@ class MassAssignmentSecurityTest < ActiveRecord::TestCase assert_admin_attributes(p, true) end + def test_create_with_bang_with_admin_role_with_attr_accessible_attributes + p = TightPerson.create!(attributes_hash, :as => :admin) + + assert_admin_attributes(p, true) + end + + def test_create_with_bang_with_admin_role_with_attr_protected_attributes + p = LoosePerson.create!(attributes_hash, :as => :admin) + + assert_admin_attributes(p, true) + end + def test_new_with_without_protection_with_attr_accessible_attributes p = TightPerson.new(attributes_hash, :without_protection => true) @@ -205,6 +217,18 @@ class MassAssignmentSecurityTest < ActiveRecord::TestCase assert_all_attributes(p) end + def test_create_with_bang_with_without_protection_with_attr_accessible_attributes + p = TightPerson.create!(attributes_hash, :without_protection => true) + + assert_all_attributes(p) + end + + def test_create_with_bang_with_without_protection_with_attr_protected_attributes + p = LoosePerson.create!(attributes_hash, :without_protection => true) + + assert_all_attributes(p) + end + def test_protection_against_class_attribute_writers [:logger, :configurations, :primary_key_prefix_type, :table_name_prefix, :table_name_suffix, :pluralize_table_names, :default_timezone, :schema_format, :lock_optimistically, :record_timestamps].each do |method|