From 0db6a14ae16b143e078375ff7f3c940cf707290b Mon Sep 17 00:00:00 2001 From: Tim Masliuchenko Date: Tue, 10 Oct 2017 14:15:56 +0300 Subject: [PATCH] Add allow_other_host option to redirect_back method --- .../action_controller/metal/redirecting.rb | 23 +++++++++++++------ actionpack/test/controller/redirect_test.rb | 21 +++++++++++++++++ 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb index 5cd8568d8d..b8a80eef31 100644 --- a/actionpack/lib/action_controller/metal/redirecting.rb +++ b/actionpack/lib/action_controller/metal/redirecting.rb @@ -79,15 +79,18 @@ module ActionController # redirect_back fallback_location: "/images/screenshot.jpg" # redirect_back fallback_location: posts_url # redirect_back fallback_location: proc { edit_post_url(@post) } + # redirect_back fallback_location: '/', allow_other_host: false # - # All options that can be passed to redirect_to are accepted as + # ==== Options + # * :fallback_location - The default fallback location that will be used on missing `Referer` header. + # * :allow_other_host - Allows or dissallow redirection to the host that is different to the current host + # + # All other options that can be passed to redirect_to are accepted as # options and the behavior is identical. - def redirect_back(fallback_location:, **args) - if referer = request.headers["Referer"] - redirect_to referer, **args - else - redirect_to fallback_location, **args - end + def redirect_back(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 end def _compute_redirect_to_location(request, options) #:nodoc: @@ -120,5 +123,11 @@ module ActionController 302 end end + + def _url_host_allowed?(url) + URI(url.to_s).host == request.host + rescue ArgumentError, URI::Error + false + end end end diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb index e447b66486..2959dc3e4d 100644 --- a/actionpack/test/controller/redirect_test.rb +++ b/actionpack/test/controller/redirect_test.rb @@ -62,6 +62,10 @@ class RedirectController < ActionController::Base redirect_back(fallback_location: "/things/stuff", status: 307) end + def safe_redirect_back_with_status + redirect_back(fallback_location: "/things/stuff", status: 307, allow_other_host: false) + end + def host_redirect redirect_to action: "other_host", only_path: false, host: "other.test.host" end @@ -259,6 +263,23 @@ class RedirectTest < ActionController::TestCase assert_equal "http://test.host/things/stuff", redirect_to_url end + def test_safe_redirect_back_from_other_host + @request.env["HTTP_REFERER"] = "http://another.host/coming/from" + get :safe_redirect_back_with_status + + assert_response 307 + assert_equal "http://test.host/things/stuff", redirect_to_url + end + + def test_safe_redirect_back_from_the_same_host + referer = "http://test.host/coming/from" + @request.env["HTTP_REFERER"] = referer + get :safe_redirect_back_with_status + + assert_response 307 + assert_equal referer, redirect_to_url + end + def test_redirect_to_record with_routing do |set| set.draw do