Skip to content

Commit

Permalink
Make session_hijacking an optional protection (#1984)
Browse files Browse the repository at this point in the history
Also remove the very old `does not include ...` comment from 0985552

Close #1930
  • Loading branch information
dentarg committed Jan 5, 2024
1 parent ceb14ed commit 157e307
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 6 deletions.
2 changes: 1 addition & 1 deletion README.md
Expand Up @@ -1922,7 +1922,7 @@ set :protection, :except => :path_traversal
You can also hand in an array in order to disable a list of protections:
```ruby
set :protection, :except => [:path_traversal, :session_hijacking]
set :protection, :except => [:path_traversal, :remote_token]
```
By default, Sinatra will only set up session based protection if `:sessions`
Expand Down
2 changes: 1 addition & 1 deletion rack-protection/README.md
Expand Up @@ -69,7 +69,7 @@ Prevented by:

Prevented by:

* [`Rack::Protection::SessionHijacking`][session-hijacking]
* [`Rack::Protection::SessionHijacking`][session-hijacking] (not included by `use Rack::Protection`)

## Cookie Tossing

Expand Down
5 changes: 2 additions & 3 deletions rack-protection/lib/rack/protection.rb
Expand Up @@ -27,12 +27,11 @@ module Protection
autoload :XSSHeader, 'rack/protection/xss_header'

def self.new(app, options = {})
# does not include: RemoteReferrer, AuthenticityToken and FormToken
except = Array options[:except]
use_these = Array options[:use]

if options.fetch(:without_session, false)
except += %i[session_hijacking remote_token]
except += %i[remote_token]
end

Rack::Builder.new do
Expand All @@ -44,6 +43,7 @@ def self.new(app, options = {})
use ::Rack::Protection::FormToken, options if use_these.include? :form_token
use ::Rack::Protection::ReferrerPolicy, options if use_these.include? :referrer_policy
use ::Rack::Protection::RemoteReferrer, options if use_these.include? :remote_referrer
use ::Rack::Protection::SessionHijacking, options if use_these.include? :session_hijacking
use ::Rack::Protection::StrictTransport, options if use_these.include? :strict_transport

# On by default, unless skipped
Expand All @@ -53,7 +53,6 @@ def self.new(app, options = {})
use ::Rack::Protection::JsonCsrf, options unless except.include? :json_csrf
use ::Rack::Protection::PathTraversal, options unless except.include? :path_traversal
use ::Rack::Protection::RemoteToken, options unless except.include? :remote_token
use ::Rack::Protection::SessionHijacking, options unless except.include? :session_hijacking
use ::Rack::Protection::XSSHeader, options unless except.include? :xss_header
run app
end.to_app
Expand Down
5 changes: 4 additions & 1 deletion rack-protection/spec/lib/rack/protection/protection_spec.rb
Expand Up @@ -5,7 +5,8 @@

it 'passes on options' do
mock_app do
use Rack::Protection, track: ['HTTP_FOO']
# the :track option is used by session_hijacking
use Rack::Protection, track: ['HTTP_FOO'], use: [:session_hijacking], except: [:remote_token]
run proc { |_e| [200, { 'content-type' => 'text/plain' }, ['hi']] }
end

Expand All @@ -15,6 +16,8 @@
expect(session[:foo]).to eq(:bar)

get '/', {}, 'rack.session' => session, 'HTTP_FOO' => 'BAR'
# wont be empty if the remote_token middleware runs after session_hijacking
# why we run the mock app without remote_token
expect(session).to be_empty
end

Expand Down

0 comments on commit 157e307

Please sign in to comment.