Skip to content

Commit

Permalink
Merge pull request #1573 from padrino/clarify-csrf
Browse files Browse the repository at this point in the history
add report_csrf_failure, enable custom reports
  • Loading branch information
ujifgc committed Feb 13, 2014
2 parents dffac89 + 70e7fe3 commit aa9d108
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 19 deletions.
13 changes: 6 additions & 7 deletions padrino-core/lib/padrino-core/application.rb
Expand Up @@ -249,9 +249,9 @@ def default_paths!

def default_security!
set :protection, :except => :path_traversal
set :authentication, false
set :sessions, false
set :protect_from_csrf, false
set :report_csrf_failure, false
set :allow_disabled_csrf, false
end

Expand Down Expand Up @@ -353,12 +353,11 @@ def setup_csrf_protection(builder)
# returns the options used in the builder for csrf protection setup
def options_for_csrf_protection_setup
options = { :logger => logger }

if allow_disabled_csrf?
options.merge!({
:reaction => :report,
:report_key => 'protection.csrf.failed'
})
if report_csrf_failure? || allow_disabled_csrf?
options.merge!(
:reaction => :report,
:report_key => 'protection.csrf.failed'
)
end
options
end
Expand Down
22 changes: 12 additions & 10 deletions padrino-core/lib/padrino-core/application/routing.rb
Expand Up @@ -678,9 +678,9 @@ def route(verb, path, *args, &block)
route_options = options.dup
route_options[:provides] = @_provides if @_provides

# CSRF protection is always active except when explicitly switched off.
if allow_disabled_csrf
unless route_options[:csrf_protection] == false
# Add Sinatra condition to check rack-protection failure.
if protect_from_csrf && (report_csrf_failure || allow_disabled_csrf)
unless route_options.has_key?(:csrf_protection)
route_options[:csrf_protection] = true
end
end
Expand Down Expand Up @@ -932,17 +932,19 @@ def provides(*types)
end

##
# Implements CSRF checking when `allow_disabled_csrf` is set to true.
#
# This condition is always on, except when it is explicitly switched
# off.
# Implements checking for rack-protection failure flag when
# `report_csrf_failure` is enabled.
#
# @example
# post("/", :csrf_protection => false)
#
def csrf_protection(on = true)
if on
condition { halt 403 if request.env['protection.csrf.failed'] }
def csrf_protection(enabled)
return unless enabled
condition do
if request.env['protection.csrf.failed']
message = settings.protect_from_csrf.kind_of?(Hash) && settings.protect_from_csrf[:message] || 'Forbidden'
halt(403, message)
end
end
end
end
Expand Down
31 changes: 29 additions & 2 deletions padrino-core/test/test_csrf_protection.rb
Expand Up @@ -60,7 +60,7 @@
mock_app do
enable :sessions
enable :protect_from_csrf
set :allow_disabled_csrf, true
enable :allow_disabled_csrf
post('/on') { 'HI' }
post('/off', :csrf_protection => false) { 'HI' }
end
Expand All @@ -74,6 +74,7 @@
should "not allow access to routes with csrf_protection on" do
post "/on"
assert_equal 403, status
assert_equal 'Forbidden', body
end
end

Expand Down Expand Up @@ -123,7 +124,7 @@
before do
mock_app do
enable :sessions
set :protect_from_csrf, :authenticity_param => 'foobar'
set :protect_from_csrf, :authenticity_param => 'foobar', :message => 'sucker!'
post("/a") { "a" }
end
end
Expand All @@ -132,6 +133,12 @@
post "/a", {"foobar" => "a"}, 'rack.session' => {:csrf => "a"}
assert_equal 200, status
end

should "allow configuring message" do
post "/a"
assert_equal 403, status
assert_equal 'sucker!', body
end
end

context "with middleware" do
Expand All @@ -157,5 +164,25 @@ class Middleware < Sinatra::Base
assert_equal 403, status
end
end

context "with standard report layout" do
before do
mock_app do
enable :sessions
set :protect_from_csrf, :message => 'sucker!'
enable :report_csrf_failure
post("/a") { "a" }
error 403 do
halt 406, 'please, do not hack'
end
end
end

should "allow configuring protection options" do
post "/a"
assert_equal 406, status
assert_equal 'please, do not hack', body
end
end
end
end

0 comments on commit aa9d108

Please sign in to comment.