diff options
| -rw-r--r-- | actionpack/CHANGELOG | 1 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/mime_type.rb | 19 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/mime_types.rb | 2 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/request_forgery_protection.rb | 2 | ||||
| -rw-r--r-- | actionpack/test/controller/mime_type_test.rb | 25 | ||||
| -rw-r--r-- | actionpack/test/controller/request_forgery_protection_test.rb | 71 | ||||
| -rw-r--r-- | railties/lib/rails/plugin/locator.rb | 5 |
7 files changed, 110 insertions, 15 deletions
diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index baba4ae5ed..852e783f40 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,6 @@ *SVN* +* Change the request forgery protection to go by Content-Type instead of request.format so that you can't bypass it by POSTing to "#{request.uri}.xml" [rick] * InstanceTag#default_time_from_options with hash args uses Time.current as default; respects hash settings when time falls in system local spring DST gap [Geoff Buesing] * select_date defaults to Time.zone.today when config.time_zone is set [Geoff Buesing] diff --git a/actionpack/lib/action_controller/mime_type.rb b/actionpack/lib/action_controller/mime_type.rb index 8c02f20521..f43e2ba06d 100644 --- a/actionpack/lib/action_controller/mime_type.rb +++ b/actionpack/lib/action_controller/mime_type.rb @@ -17,6 +17,10 @@ module Mime # end # end class Type + @@html_types = Set.new [:html, :all] + @@unverifiable_types = Set.new [:text, :json, :csv, :xml, :rss, :atom, :yaml] + cattr_reader :html_types, :unverifiable_types + # A simple helper class used in parsing the accept header class AcceptItem #:nodoc: attr_accessor :order, :name, :q @@ -153,12 +157,21 @@ module Mime synonym.to_s == mime_type.to_s || synonym.to_sym == mime_type.to_sym end end - + + # Returns true if ActionPack should check requests using this Mime Type for possible request forgery. See + # ActionController::RequestForgerProtection. + def verify_request? + !@@unverifiable_types.include?(to_sym) + end + + def html? + @@html_types.include?(to_sym) || @string =~ /html/ + end + private def method_missing(method, *args) if method.to_s =~ /(\w+)\?$/ - mime_type = $1.downcase.to_sym - mime_type == @symbol || (mime_type == :html && @symbol == :all) + $1.downcase.to_sym == to_sym else super end diff --git a/actionpack/lib/action_controller/mime_types.rb b/actionpack/lib/action_controller/mime_types.rb index 71706b4c41..01a266d3fe 100644 --- a/actionpack/lib/action_controller/mime_types.rb +++ b/actionpack/lib/action_controller/mime_types.rb @@ -17,4 +17,4 @@ Mime::Type.register "multipart/form-data", :multipart_form Mime::Type.register "application/x-www-form-urlencoded", :url_encoded_form # http://www.ietf.org/rfc/rfc4627.txt -Mime::Type.register "application/json", :json, %w( text/x-json ) +Mime::Type.register "application/json", :json, %w( text/x-json )
\ No newline at end of file diff --git a/actionpack/lib/action_controller/request_forgery_protection.rb b/actionpack/lib/action_controller/request_forgery_protection.rb index 5daf14eb30..946a0ed152 100644 --- a/actionpack/lib/action_controller/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/request_forgery_protection.rb @@ -99,7 +99,7 @@ module ActionController #:nodoc: end def verifiable_request_format? - request.format.html? || request.format.js? + request.content_type.nil? || request.content_type.verify_request? end # Sets the token value for the current session. Pass a <tt>:secret</tt> option diff --git a/actionpack/test/controller/mime_type_test.rb b/actionpack/test/controller/mime_type_test.rb index 03b0f8bad2..f16a3c68b4 100644 --- a/actionpack/test/controller/mime_type_test.rb +++ b/actionpack/test/controller/mime_type_test.rb @@ -52,16 +52,33 @@ class MimeTypeTest < Test::Unit::TestCase end def test_type_convenience_methods - types = [:html, :xml, :png, :pdf, :yaml, :url_encoded_form] + # Don't test Mime::ALL, since it Mime::ALL#html? == true + types = Mime::SET.to_a.map(&:to_sym).uniq - [:all] + + # Remove custom Mime::Type instances set in other tests, like Mime::GIF and Mime::IPHONE + types.delete_if { |type| !Mime.const_defined?(type.to_s.upcase) } + types.each do |type| mime = Mime.const_get(type.to_s.upcase) - assert mime.send("#{type}?"), "Mime::#{type.to_s.upcase} is not #{type}?" - (types - [type]).each { |t| assert !mime.send("#{t}?"), "Mime::#{t.to_s.upcase} is #{t}?" } + assert mime.send("#{type}?"), "#{mime.inspect} is not #{type}?" + (types - [type]).each { |other_type| assert !mime.send("#{other_type}?"), "#{mime.inspect} is #{other_type}?" } end end - + def test_mime_all_is_html assert Mime::ALL.all?, "Mime::ALL is not all?" assert Mime::ALL.html?, "Mime::ALL is not html?" end + + def test_verifiable_mime_types + unverified_types = Mime::Type.unverifiable_types + all_types = Mime::SET.to_a.map(&:to_sym) + all_types.uniq! + # Remove custom Mime::Type instances set in other tests, like Mime::GIF and Mime::IPHONE + all_types.delete_if { |type| !Mime.const_defined?(type.to_s.upcase) } + + unverified, verified = all_types.partition { |type| Mime::Type.unverifiable_types.include? type } + assert verified.all? { |type| Mime.const_get(type.to_s.upcase).verify_request? }, "Not all Mime Types are verified: #{verified.inspect}" + assert unverified.all? { |type| !Mime.const_get(type.to_s.upcase).verify_request? }, "Some Mime Types are verified: #{unverified.inspect}" + end end diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index d0c3c6e224..833e8d8e00 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -93,19 +93,79 @@ module RequestForgeryProtectionTests post :unsafe assert_response :success end - + def test_should_not_allow_post_without_token assert_raises(ActionController::InvalidAuthenticityToken) { post :index } end - + def test_should_not_allow_put_without_token assert_raises(ActionController::InvalidAuthenticityToken) { put :index } end - + def test_should_not_allow_delete_without_token assert_raises(ActionController::InvalidAuthenticityToken) { delete :index } end - + + def test_should_not_allow_api_formatted_post_without_token + assert_raises(ActionController::InvalidAuthenticityToken) do + post :index, :format => 'xml' + end + end + + def test_should_not_allow_api_formatted_put_without_token + assert_raises(ActionController::InvalidAuthenticityToken) do + put :index, :format => 'xml' + end + end + + def test_should_not_allow_api_formatted_delete_without_token + assert_raises(ActionController::InvalidAuthenticityToken) do + delete :index, :format => 'xml' + end + end + + def test_should_not_allow_api_formatted_post_sent_as_url_encoded_form_without_token + assert_raises(ActionController::InvalidAuthenticityToken) do + @request.env['CONTENT_TYPE'] = Mime::URL_ENCODED_FORM.to_s + post :index, :format => 'xml' + end + end + + def test_should_not_allow_api_formatted_put_sent_as_url_encoded_form_without_token + assert_raises(ActionController::InvalidAuthenticityToken) do + @request.env['CONTENT_TYPE'] = Mime::URL_ENCODED_FORM.to_s + put :index, :format => 'xml' + end + end + + def test_should_not_allow_api_formatted_delete_sent_as_url_encoded_form_without_token + assert_raises(ActionController::InvalidAuthenticityToken) do + @request.env['CONTENT_TYPE'] = Mime::URL_ENCODED_FORM.to_s + delete :index, :format => 'xml' + end + end + + def test_should_not_allow_api_formatted_post_sent_as_multipart_form_without_token + assert_raises(ActionController::InvalidAuthenticityToken) do + @request.env['CONTENT_TYPE'] = Mime::MULTIPART_FORM.to_s + post :index, :format => 'xml' + end + end + + def test_should_not_allow_api_formatted_put_sent_as_multipart_form_without_token + assert_raises(ActionController::InvalidAuthenticityToken) do + @request.env['CONTENT_TYPE'] = Mime::MULTIPART_FORM.to_s + put :index, :format => 'xml' + end + end + + def test_should_not_allow_api_formatted_delete_sent_as_multipart_form_without_token + assert_raises(ActionController::InvalidAuthenticityToken) do + @request.env['CONTENT_TYPE'] = Mime::MULTIPART_FORM.to_s + delete :index, :format => 'xml' + end + end + def test_should_not_allow_xhr_post_without_token assert_raises(ActionController::InvalidAuthenticityToken) { xhr :post, :index } end @@ -134,16 +194,19 @@ module RequestForgeryProtectionTests end def test_should_allow_post_with_xml + @request.env['CONTENT_TYPE'] = Mime::XML.to_s post :index, :format => 'xml' assert_response :success end def test_should_allow_put_with_xml + @request.env['CONTENT_TYPE'] = Mime::XML.to_s put :index, :format => 'xml' assert_response :success end def test_should_allow_delete_with_xml + @request.env['CONTENT_TYPE'] = Mime::XML.to_s delete :index, :format => 'xml' assert_response :success end diff --git a/railties/lib/rails/plugin/locator.rb b/railties/lib/rails/plugin/locator.rb index fd7de4ee28..f06a51a572 100644 --- a/railties/lib/rails/plugin/locator.rb +++ b/railties/lib/rails/plugin/locator.rb @@ -78,11 +78,12 @@ module Rails # a <tt>rails/init.rb</tt> file. class GemLocator < Locator def plugins - specs = initializer.configuration.gems.map(&:specification) - specs + Gem.loaded_specs.values.select do |spec| + specs = initializer.configuration.gems.map(&:specification) + specs += Gem.loaded_specs.values.select do |spec| spec.loaded_from && # prune stubs File.exist?(File.join(spec.full_gem_path, "rails", "init.rb")) end + specs.compact! require "rubygems/dependency_list" |