Better error messages if you leave out the :secret option for request forgery...

Better error messages if you leave out the :secret option for request forgery protection.  Closes #9670 [rick]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@7671 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

actionpack/CHANGELOG

 *SVN*
 
+* Better error messages if you leave out the :secret option for request forgery protection.  Closes #9670 [rick]
+
 * Allow ability to disable request forgery protection, disable it in test mode by default.  Closes #9693 [lifofifo]
 
 * Avoid calling is_missing on LoadErrors. Closes #7460. [ntalbott]

actionpack/lib/action_controller/request_forgery_protection.rb

@@ -93,8 +93,12 @@ def verifiable_request_format?
       def form_authenticity_token
         @form_authenticity_token ||= if request_forgery_protection_options[:secret]
           authenticity_token_from_session_id
-        else
+        elsif session.respond_to?(:dbman) && session.dbman.respond_to?(:generate_digest)
           authenticity_token_from_cookie_session
+        elsif session.nil?
+          raise InvalidAuthenticityToken, "Request Forgery Protection requires a valid session.  Use #allow_forgery_protection to disable it, or use a valid session."
+        else
+          raise InvalidAuthenticityToken, "No :secret given to the #protect_from_forgery call.  Set that or use a session store capable of generating its own keys (Cookie Session Store)."
         end
       end
       

actionpack/test/controller/request_forgery_protection_test.rb

@@ -5,6 +5,61 @@
   map.connect ':controller/:action/:id'
 end
 
+# simulates cookie session store
+class FakeSessionDbMan
+  def self.generate_digest(data)
+    Digest::SHA1.hexdigest("secure")
+  end
+end
+
+# common controller actions
+module RequestForgeryProtectionActions
+  def index
+    render :inline => "<%= form_tag('/') {} %>"
+  end
+  
+  def show_button
+    render :inline => "<%= button_to('New', '/') {} %>"
+  end
+  
+  def unsafe
+    render :text => 'pwn'
+  end
+  
+  def rescue_action(e) raise e end
+end
+
+# sample controllers
+class RequestForgeryProtectionController < ActionController::Base
+  include RequestForgeryProtectionActions
+  protect_from_forgery :only => :index, :secret => 'abc'
+end
+
+class RequestForgeryProtectionWithoutSecretController < ActionController::Base
+  include RequestForgeryProtectionActions
+  protect_from_forgery
+end
+
+# no token is given, assume the cookie store is used
+class CsrfCookieMonsterController < ActionController::Base
+  include RequestForgeryProtectionActions
+  protect_from_forgery :only => :index
+end
+
+class FreeCookieController < CsrfCookieMonsterController
+  self.allow_forgery_protection = false
+  
+  def index
+    render :inline => "<%= form_tag('/') {} %>"
+  end
+  
+  def show_button
+    render :inline => "<%= button_to('New', '/') {} %>"
+  end
+end
+
+# common test methods
+
 module RequestForgeryProtectionTests
   def teardown
     ActionController::Base.request_forgery_protection_token = nil
@@ -85,26 +140,7 @@ def test_should_allow_delete_with_xml
   end
 end
 
-module RequestForgeryProtectionActions
-  def index
-    render :inline => "<%= form_tag('/') {} %>"
-  end
-  
-  def show_button
-    render :inline => "<%= button_to('New', '/') {} %>"
-  end
-  
-  def unsafe
-    render :text => 'pwn'
-  end
-  
-  def rescue_action(e) raise e end
-end
-
-class RequestForgeryProtectionController < ActionController::Base
-  include RequestForgeryProtectionActions
-  protect_from_forgery :only => :index, :secret => 'abc'
-end
+# OK let's get our test on
 
 class RequestForgeryProtectionControllerTest < Test::Unit::TestCase
   include RequestForgeryProtectionTests
@@ -120,27 +156,22 @@ def session_id() '123' end
   end
 end
 
-# no token is given, assume the cookie store is used
-class CsrfCookieMonsterController < ActionController::Base
-  include RequestForgeryProtectionActions
-  protect_from_forgery :only => :index
-end
-
-class FreeCookieController < CsrfCookieMonsterController
-  self.allow_forgery_protection = false
-  
-  def index
-    render :inline => "<%= form_tag('/') {} %>"
+class RequestForgeryProtectionWithoutSecretControllerTest < Test::Unit::TestCase
+  def setup
+    @controller = RequestForgeryProtectionWithoutSecretController.new
+    @request    = ActionController::TestRequest.new
+    @response   = ActionController::TestResponse.new
+    class << @request.session
+      def session_id() '123' end
+    end
+    @token = OpenSSL::HMAC.hexdigest(OpenSSL::Digest::Digest.new('SHA1'), 'abc', '123')
+    ActionController::Base.request_forgery_protection_token = :authenticity_token
   end
   
-  def show_button
-    render :inline => "<%= button_to('New', '/') {} %>"
-  end  
-end
-
-class FakeSessionDbMan
-  def self.generate_digest(data)
-    Digest::SHA1.hexdigest("secure")
+  def test_should_raise_error_without_secret
+    assert_raises ActionController::InvalidAuthenticityToken do
+      get :index
+    end
   end
 end
 
@@ -150,18 +181,17 @@ def setup
     @controller = CsrfCookieMonsterController.new
     @request    = ActionController::TestRequest.new
     @response   = ActionController::TestResponse.new
-    # simulate a cookie session store
-    @request.session.instance_variable_set(:@dbman, FakeSessionDbMan)
     class << @request.session
-      attr_reader :dbman
+      attr_accessor :dbman
     end
+    # simulate a cookie session store
+    @request.session.dbman = FakeSessionDbMan
     @token = Digest::SHA1.hexdigest("secure")
     ActionController::Base.request_forgery_protection_token = :authenticity_token
   end
 end
 
 class FreeCookieControllerTest < Test::Unit::TestCase
-  
   def setup
     @controller = FreeCookieController.new
     @request    = ActionController::TestRequest.new
@@ -184,5 +214,4 @@ def test_should_allow_all_methods_without_token
       assert_nothing_raised { send(method, :index)}
     end
   end
-  
-end
+end
\ No newline at end of file

railties/helpers/application.rb

@@ -3,5 +3,8 @@
 
 class ApplicationController < ActionController::Base
   helper :all # include all helpers, all the time
-  protect_from_forgery # See ActionController::RequestForgeryProtection for details
+
+  # See ActionController::RequestForgeryProtection for details
+  # If you're using the Cookie Session Store you can leave out the :secret
+  protect_from_forgery :secret => '<%= app_secret %>'
 end
\ No newline at end of file

railties/lib/rails_generator/generators/applications/app/app_generator.rb

@@ -43,7 +43,7 @@ def manifest
       m.file "README",         "README"
 
       # Application
-      m.template "helpers/application.rb",        "app/controllers/application.rb", :assigns => { :app_name => @app_name }
+      m.template "helpers/application.rb",        "app/controllers/application.rb", :assigns => { :app_name => @app_name, :app_secret => md5.hexdigest }
       m.template "helpers/application_helper.rb", "app/helpers/application_helper.rb"
       m.template "helpers/test_helper.rb",        "test/test_helper.rb"