From cfc7bd7f7cb87011c751f9b5a655973c30048a40 Mon Sep 17 00:00:00 2001 From: Aidan Coyle Date: Wed, 27 Jul 2016 14:19:00 -0500 Subject: [PATCH 1/2] Fix Rails path mutation ActionDispatch will mutate the uri of a request before the application actually gets a chance to see it. This can lead to auth failures where the client signed with the original version of the path, and the server is comparing against a mutated version. Instead for ActionDispatch::Request we should use #original_fullpath, which is passed to Rails from Rack. --- .../request_drivers/action_dispatch.rb | 2 +- spec/request_drivers/action_dispatch_spec.rb | 25 +++++++++++++++++-- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/lib/api_auth/request_drivers/action_dispatch.rb b/lib/api_auth/request_drivers/action_dispatch.rb index fe09cebb..6c5c3212 100644 --- a/lib/api_auth/request_drivers/action_dispatch.rb +++ b/lib/api_auth/request_drivers/action_dispatch.rb @@ -2,7 +2,7 @@ module ApiAuth module RequestDrivers # :nodoc: class ActionDispatchRequest < ActionControllerRequest # :nodoc: def request_uri - @request.fullpath + @request.original_fullpath end end end diff --git a/spec/request_drivers/action_dispatch_spec.rb b/spec/request_drivers/action_dispatch_spec.rb index 44439533..ff2b2dd5 100644 --- a/spec/request_drivers/action_dispatch_spec.rb +++ b/spec/request_drivers/action_dispatch_spec.rb @@ -30,8 +30,29 @@ expect(driven_request.content_md5).to eq('1B2M2Y8AsgTpgAmY7PhCfg==') end - it 'gets the request_uri' do - expect(driven_request.request_uri).to eq('/resource.xml?foo=bar&bar=foo') + describe 'request_uri' do + context 'with url parameters' do + it 'gets the request_uri' do + expect(driven_request.request_uri).to eq('/resource.xml?foo=bar&bar=foo') + end + end + + context 'with mutated path' do + let(:request) do + ActionDispatch::Request.new( + 'PATH_INFO' => '/resource/', + 'ORIGINAL_FULLPATH' => '/resource/' + ) + end + + before do + request.path_info = 'overwritten_in_action_dispatch' + end + + it 'gets the original path' do + expect(driven_request.request_uri).to eq('/resource/') + end + end end it 'gets the timestamp' do From 79176e34e1e6b957609dd59c37d5bce780314a01 Mon Sep 17 00:00:00 2001 From: Aidan Coyle Date: Wed, 27 Jul 2016 14:28:54 -0500 Subject: [PATCH 2/2] Fall back to #fullpath Some versions of Rails (notably 3.0) don't have #originl_fullpath, so we have to fall back to #fullpath. --- lib/api_auth/request_drivers/action_dispatch.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/api_auth/request_drivers/action_dispatch.rb b/lib/api_auth/request_drivers/action_dispatch.rb index 6c5c3212..0ec36c63 100644 --- a/lib/api_auth/request_drivers/action_dispatch.rb +++ b/lib/api_auth/request_drivers/action_dispatch.rb @@ -2,7 +2,11 @@ module ApiAuth module RequestDrivers # :nodoc: class ActionDispatchRequest < ActionControllerRequest # :nodoc: def request_uri - @request.original_fullpath + if @request.respond_to?(:original_fullpath) + @request.original_fullpath + else + @request.fullpath + end end end end