Merge pull request #1541 from opf/backport/1060_1074

Backport/1060 1074

app/controllers/application_controller.rb

@@ -427,8 +427,15 @@ class ApplicationController < ActionController::Base
     if !back_url.blank?
       begin
         uri = URI.parse(back_url)
-        # do not redirect user to another host or to the login or register page
-        if (uri.relative? || (uri.host == request.host)) && !uri.path.match(%r{/(login|account/register)})
+
+        # do not redirect user to another host (even protocol relative urls have the host set)
+        # whenever a host is set it must match the request's host
+        uri_local_to_host = uri.host.nil? || uri.host == request.host
+
+        # do not redirect user to the login or register page
+        uri_path_allowed  = !uri.path.match(%r{/(login|account/register)})
+
+        if uri_local_to_host && uri_path_allowed
           redirect_to(back_url)
           return
         end

spec/controllers/account_controller_spec.rb

@@ -39,13 +39,24 @@ describe AccountController do
 
     describe "User logging in with back_url" do
 
-      it "should redirect to the same host" do
-        post :login , {:username => admin.login, :password => 'adminADMIN!', :back_url => 'http%3A%2F%2Ftest.host%2Fwork_packages%2Fshow%2F1'}
+      it "should redirect to a relative path" do
+        post :login , {:username => admin.login, :password => 'adminADMIN!', :back_url => '/'}
+        expect(response).to redirect_to '/'
+      end
+
+      it "should redirect to an absolute path given the same host" do
+        # note: test.host is the hostname during tests
+        post :login , {:username => admin.login, :password => 'adminADMIN!', :back_url => 'http://test.host/work_packages/show/1'}
         expect(response).to redirect_to '/work_packages/show/1'
       end
 
       it "should not redirect to another host" do
-        post :login , {:username => admin.login, :password => 'adminADMIN!', :back_url => 'http%3A%2F%2Ftest.foo%2Ffake'}
+        post :login , {:username => admin.login, :password => 'adminADMIN!', :back_url => 'http://test.foo/work_packages/show/1'}
+        expect(response).to redirect_to '/my/page'
+      end
+
+      it "should not redirect to another host with a protocol relative url" do
+        post :login , {:username => admin.login, :password => 'adminADMIN!', :back_url => '//test.foo/fake'}
         expect(response).to redirect_to '/my/page'
       end