55

Raise error on unpermitted open redirects. · rails/rails@5e93cff · GitHub

 3 years ago
source link: https://github.com/rails/rails/commit/5e93cff83599833380b4cb3d99c020b5efc7dd96
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
neoserver,ios ssh client

Raise error on unpermitted open redirects. · rails/rails@5e93cff · GitHubPermalink

Browse files

Raise error on unpermitted open redirects. 
Add `allow_other_host` options to `redirect_to`.
Opt in to this behaviour with `ActionController::Base.raise_on_open_redirects = true`.

gmcgibbon

authored and eileencodes

committed 2 days ago

Verified

1 parent 25d8c1c commit 5e93cff83599833380b4cb3d99c020b5efc7dd96
Showing with 119 additions and 6 deletions.

@@ -1,3 +1,10 @@

* Raise error on unpermitted open redirects.

Add `allow_other_host` options to `redirect_to`.

Opt in to this behaviour with `ActionController::Base.raise_on_open_redirects = true`.

*Gannon McGibbon*

* Deprecate `poltergeist` and `webkit` (capybara-webkit) driver registration for system testing (they will be removed in Rails 7.1). Add `cuprite` instead.

[Poltergeist](https://github.com/teampoltergeist/poltergeist) and [capybara-webkit](https://github.com/thoughtbot/capybara-webkit) are already not maintained. These usage in Rails are removed for avoiding confusing users.

@@ -7,6 +7,10 @@ module Redirecting

include AbstractController::Logger

include ActionController::UrlFor

included do

mattr_accessor :raise_on_open_redirects, default: false

end

# Redirects the browser to the target specified in +options+. This parameter can be any one of:

#

# * <tt>Hash</tt> - The URL will be generated by calling url_for with the +options+.

@@ -60,17 +64,19 @@ module Redirecting

# Passing user input directly into +redirect_to+ is considered dangerous (e.g. `redirect_to(params[:location])`).

# Always use regular expressions or a permitted list when redirecting to a user specified location.

def redirect_to(options = {}, response_options = {})

response_options[:allow_other_host] ||= _allow_other_host unless response_options.key?(:allow_other_host)

raise ActionControllerError.new("Cannot redirect to nil!") unless options

raise AbstractController::DoubleRenderError if response_body

self.status = _extract_redirect_to_status(options, response_options)

self.location = _compute_redirect_to_location(request, options)

self.location = _compute_safe_redirect_to_location(request, options, response_options)

self.response_body = "<html><body>You are being <a href=\"#{ERB::Util.unwrapped_html_escape(response.location)}\">redirected</a>.</body></html>"

end

# Soft deprecated alias for <tt>redirect_back_or_to</tt> where the fallback_location location is supplied as a keyword argument instead

# of the first positional argument.

def redirect_back(fallback_location:, allow_other_host: true, **args)

def redirect_back(fallback_location:, allow_other_host: _allow_other_host, **args)

redirect_back_or_to fallback_location, allow_other_host: allow_other_host, **args

end

@@ -96,10 +102,25 @@ def redirect_back(fallback_location:, allow_other_host: true, **args)

#

# All other options that can be passed to #redirect_to are accepted as

# options and the behavior is identical.

def redirect_back_or_to(fallback_location, allow_other_host: true, **args)

referer = request.headers["Referer"]

redirect_to_referer = referer && (allow_other_host || _url_host_allowed?(referer))

redirect_to redirect_to_referer ? referer : fallback_location, **args

def redirect_back_or_to(fallback_location, allow_other_host: _allow_other_host, **options)

location = request.referer || fallback_location

location = fallback_location unless allow_other_host || _url_host_allowed?(request.referer)

allow_other_host = true if _allow_other_host && !allow_other_host # if the fallback is an open redirect

redirect_to location, allow_other_host: allow_other_host, **options

end

def _compute_safe_redirect_to_location(request, options, response_options)

location = _compute_redirect_to_location(request, options)

if response_options[:allow_other_host] || _url_host_allowed?(location)

location

else

raise(ArgumentError, <<~MSG.squish)

Unsafe redirect #{location.truncate(100).inspect},

use :allow_other_host to redirect anyway.

MSG

end

end

def _compute_redirect_to_location(request, options) #:nodoc:

@@ -123,6 +144,10 @@ def _compute_redirect_to_location(request, options) #:nodoc:

public :_compute_redirect_to_location

private

def _allow_other_host

!raise_on_open_redirects

end

def _extract_redirect_to_status(options, response_options)

if options.is_a?(Hash) && options.key?(:status)

Rack::Utils.status_code(options.delete(:status))

@@ -10,6 +10,7 @@

module ActionController

class Railtie < Rails::Railtie #:nodoc:

config.action_controller = ActiveSupport::OrderedOptions.new

config.action_controller.raise_on_open_redirects = false

config.eager_load_namespaces << ActionController

@@ -80,6 +80,14 @@ def safe_redirect_back_with_status_and_fallback_location_to_another_host

redirect_back_or_to "http://www.rubyonrails.org/", status: 307, allow_other_host: false

end

def unsafe_redirect

redirect_to "http://www.rubyonrails.org/"

end

def unsafe_redirect_back

redirect_back_or_to "http://www.rubyonrails.org/"

end

def redirect_back_with_explicit_fallback_kwarg

redirect_back(fallback_location: "/things/stuff", status: 307)

end

@@ -467,6 +475,41 @@ def test_redirect_to_with_block_and_accepted_options

assert_redirected_to "http://test.host/redirect/hello_world"

end

end

def test_unsafe_redirect

with_raise_on_open_redirects do

error = assert_raise(ArgumentError) do

get :unsafe_redirect

end

assert_equal(<<~MSG.squish, error.message)

Unsafe redirect \"http://www.rubyonrails.org/\",

use :allow_other_host to redirect anyway.

MSG

end

end

def test_unsafe_redirect_back

with_raise_on_open_redirects do

error = assert_raise(ArgumentError) do

get :unsafe_redirect_back

end

assert_equal(<<~MSG.squish, error.message)

Unsafe redirect \"http://www.rubyonrails.org/\",

use :allow_other_host to redirect anyway.

MSG

end

end

private

def with_raise_on_open_redirects

old_raise_on_open_redirects = ActionController::Base.raise_on_open_redirects

ActionController::Base.raise_on_open_redirects = true

yield

ensure

ActionController::Base.raise_on_open_redirects = old_raise_on_open_redirects

end

end

module ModuleTest

@@ -571,6 +571,8 @@ The schema dumper adds two additional configuration options:

Rendered recordings/threads/_thread.html.erb in 1.5 ms [cache miss]

```

* `config.action_controller.raise_on_open_redirects` raises an `ArgumentError` when an unpermitted open redirect occurs. The default value is `false`.

### Configuring Action Dispatch

* `config.action_dispatch.session_store` sets the name of the store for session data. The default is `:cookie_store`; other valid options include `:active_record_store`, `:mem_cache_store` or the name of your own custom class.

@@ -1095,6 +1097,7 @@ text/javascript image/svg+xml application/postscript application/x-shockwave-fla

#### For '7.0', defaults from previous versions below and:

- `config.action_controller.raise_on_open_redirects`: `true`

- `config.action_view.button_to_generates_button_tag`: `true`

- `config.action_view.apply_stylesheet_media_default`: `false`

- `config.active_support.key_generator_hash_digest_class`: `OpenSSL::Digest::SHA256`

@@ -1162,6 +1165,7 @@ text/javascript image/svg+xml application/postscript application/x-shockwave-fla

#### Baseline defaults:

- `config.action_controller.default_protect_from_forgery`: `false`

- `config.action_controller.raise_on_open_redirects`: `false`

- `config.action_controller.urlsafe_csrf_tokens`: `false`

- `config.action_dispatch.cookies_same_site_protection`: `nil`

- `config.action_mailer.delivery_job`: `ActionMailer::DeliveryJob`

@@ -231,6 +231,10 @@ def load_defaults(target_version)

active_record.verify_foreign_keys_for_fixtures = true

active_record.partial_inserts = false

end

if respond_to?(:action_controller)

action_controller.raise_on_open_redirects = true

end

else

raise "Unknown version #{target_version.to_s.inspect}"

end

@@ -60,3 +60,6 @@

# This default means that all columns will be referenced in INSERT queries

# regardless of whether they have a default or not.

# Rails.application.config.active_record.partial_inserts = false

#

# Protect from open redirect attacks in `redirect_back_or_to` and `redirect_to`.

# Rails.application.config.action_controller.raise_on_open_redirects = true

@@ -1330,6 +1330,32 @@ def index

assert_equal false, ActionView::Resolver.caching?

end

test "ActionController::Base.raise_on_open_redirects is true by default for new apps" do

app "development"

assert_equal true, ActionController::Base.raise_on_open_redirects

end

test "ActionController::Base.raise_on_open_redirects is false by default for upgraded apps" do

remove_from_config '.*config\.load_defaults.*\n'

add_to_config 'config.load_defaults "6.1"'

app "development"

assert_equal false, ActionController::Base.raise_on_open_redirects

end

test "ActionController::Base.raise_on_open_redirects can be configured in the new framework defaults" do

remove_from_config '.*config\.load_defaults.*\n'

app_file "config/initializers/new_framework_defaults_6_2.rb", <<-RUBY

Rails.application.config.action_controller.raise_on_open_redirects = true

RUBY

app "development"

assert_equal true, ActionController::Base.raise_on_open_redirects

end

test "config.action_dispatch.show_exceptions is sent in env" do

make_basic_app do |application|

application.config.action_dispatch.show_exceptions = true

0 comments on commit 5e93cff

Please sign in to comment.


report

Recommend

About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK