From 099e87fda8b4b0305aed7af6b8fd856b20785982 Mon Sep 17 00:00:00 2001 From: Eric Davis Date: Tue, 20 Jan 2009 13:58:11 -0800 Subject: [PATCH] Overrode redirect_back_or_default to check the back_url against a whitelist of valid urls to redirect back_to. #1916 --- app/controllers/rates_controller.rb | 20 +++++++++++++++ spec/controllers/rates_controller_spec.rb | 31 ++++++++++++++++++++--- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/app/controllers/rates_controller.rb b/app/controllers/rates_controller.rb index 9e3a16d..fc03b66 100644 --- a/app/controllers/rates_controller.rb +++ b/app/controllers/rates_controller.rb @@ -122,4 +122,24 @@ class RatesController < ApplicationController @back_url = params[:back_url] @back_url end + + # Override defination from ApplicationController to make sure it follows a + # whitelist + def redirect_back_or_default(default) + whitelist = %r{(rates|/users/edit)} + + back_url = CGI.unescape(params[:back_url].to_s) + if !back_url.blank? + begin + uri = URI.parse(back_url) + if uri.path.match(whitelist) + super + return + end + rescue URI::InvalidURIError + # redirect to default + end + end + redirect_to default + end end diff --git a/spec/controllers/rates_controller_spec.rb b/spec/controllers/rates_controller_spec.rb index 094775b..9c900e2 100644 --- a/spec/controllers/rates_controller_spec.rb +++ b/spec/controllers/rates_controller_spec.rb @@ -291,7 +291,7 @@ describe RatesController, "as an administrator" do end it 'should redirect to the back_url if set' do - back_url = '/back_to_this_url' + back_url = '/rates' Rate.stub!(:new).and_return(mock_rate(:save => true)) post :create, :rate => {}, :back_url => back_url response.should redirect_to(back_url) @@ -340,7 +340,7 @@ describe RatesController, "as an administrator" do end it 'should redirect to the back_url if set' do - back_url = '/back_to_this_url' + back_url = '/rates' Rate.stub!(:find).and_return(mock_rate(:update_attributes => true)) put :update, :id => "1", :back_url => back_url response.should redirect_to(back_url) @@ -428,7 +428,7 @@ describe RatesController, "as an administrator" do end it 'should redirect to the back_url if set' do - back_url = '/back_to_this_url' + back_url = '/rates' Rate.stub!(:find).and_return(mock_rate(:destroy => true)) delete :destroy, :id => "1", :back_url => back_url response.should redirect_to(back_url) @@ -450,4 +450,29 @@ describe RatesController, "as an administrator" do controller.send(:set_back_url).should eql('/back') end end + + describe "redirect_back_or_default (private)" do + before(:each) do + @default_url = "/default_response" + end + + it "should allow redirecting back to the user's edit panel" do + allowed = "/users/edit" + controller.should_receive(:redirect_to).with(allowed).and_return(true) + controller.params = { :back_url => allowed } + controller.send(:redirect_back_or_default, @default_url) + end + + it "should allow redirecting back to /rates" do + controller.should_receive(:redirect_to).with("/rates").and_return(true) + controller.params = { :back_url => '/rates' } + controller.send(:redirect_back_or_default, @default_url) + end + + it "should not allow redirecting elsewhere" do + controller.should_receive(:redirect_to).with(@default_url).and_return(true) + controller.params = { :back_url => '/back' } + controller.send(:redirect_back_or_default, @default_url) + end + end end