protect_from_forgery is not quite class-wise
Reported by Robin Lu | July 6th, 2008 @ 06:03 PM | in 2.x
protect_from_forgery is not quite class-wise currently. When one controller declares protect_from_forgery, protect_against_forgery? of every controller returns true even for controllers without protect_from_forgery. This causes the inconsistency of forgery verification and code generation of helpers. The controller without verify_authenticity_token filter still needs a :secret to generate the token for helpers like link_to_remote.
If I want controller without protect_from_forgery not being affected, I have to explicitly set allow_forgery_protection to false foe each of them or set allow_forgery_protection to false in ApplicationController but still explicitly set allow_forgery_protection to true whenerver protect_from_forgery is called.
This patch exchange the usage of allow_forgery_protection and request_forgery_protection_token which seems a bit misplaced for me. It means to make protect_from_forgery more class-wise without breaking default behavior of the protection. You can still declare protect_from_forgery in ApplicationController to enable all or you can make it controller by controller without affecting other controllers.
Comments and changes to this ticket
-
-
Robin Lu July 7th, 2008 @ 05:59 PM
Maybe I should give an example here.
For two controllers, one has protect_from_forgery and not.
Class A < ApplicationController
protect_from_forgery
...
end
Class B < ApplicationController
session :off
...
end
If I did not use cookie session or declare controller B as session off, when I use link_to_remote in the views for B, I get a crash for no :secret is given in B.
By default allow_forgery_protection is true and request_forgery_protection_token is a cattr_accessor. So no matter where protect_from_forgery is called once, protect_against_forgery? will return true everywhere.
-
Michael Koziarski July 8th, 2008 @ 07:35 PM
- → Assigned user changed from “” to “Michael Koziarski”
This looks ok but needs tests.
-
Florian Aßmann July 26th, 2008 @ 09:35 AM
I think the controller should modify the router tree and mark paths as protected.
When the path for the xhr is marked as forgery protected it should request a token.
Just an idea though...
Since I'm not using action specific FP I only:
-- a/actionpack/lib/action_controller/base.rb
+++ b/actionpack/lib/action_controller/base.rb
@@ -340,7 +340,7 @@ module ActionController #:nodoc:
- Sets the token parameter name for RequestForgery. Calling +protect_from_forgery+
- sets it to :authenticity_token by default.
- cattr_accessor :request_forgery_protection_token
- class_inheritable_accessor :request_forgery_protection_token
- Indicates whether or not optimise the generated named
- route helper methods
Regards
Florian Aßmann
-
Joshua Peek October 28th, 2008 @ 04:33 PM
- → State changed from “new” to “open”
Please Login or create a free account to add a new comment.
You can update this ticket by sending an email to from your email client. (help)
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
Source available from github
The Git repository resides at http://github.com/rails
Check out the current development trunk (Edge Rails) with:
git clone git://github.com/rails/rails.git
Creating or reviewing a patch
See the contributor guide.
Creating a feature request
Please don't. If you want a new feature in Rails, you'll have to pull up your sleeves and get busy yourself. Or convince someone else to do it. See the contributor guide on how to get going. But posting them here is just going to lead to ticket root.
Creating a bug report
When creating a bug report, be sure to include as much relevant information as possible. Post the code sample that causes the problem. Preferably, alter the unit tests and show through either changed or added tests how the expected behavior is not occuring.
Security vulnerabilities should be reported via an email to security@rubyonrails.org, do not use trac for reporting security vulnerabilities. All content in trac is publicly available as soon as it is posted.
Then don't get your hopes up. Unless you have a "Code Red, Mission Critical, The World is Coming to an End" kinda bug, you're creating this ticket in the hope that others with the same problem will be able to collaborate with you on solving it. Do not expect that the ticket automatically will see any activity or that others will jump to fix it. Creating a ticket like this is mostly to help yourself start on the path of fixing the problem and for others to sign on to with a "I'm having this problem too".
Tarmo Tänav