From ddfa9037247138e405c2a9364085acec1b82d5ae Mon Sep 17 00:00:00 2001 From: "Michael S. Kazmier" Date: Thu, 14 Dec 2017 09:41:28 -0700 Subject: [PATCH 1/3] forces request query param values to be escaped udpates params parsing function and adds tests Corrects error in merge udpates specs --- api_auth.gemspec | 3 ++- lib/api_auth/headers.rb | 24 ++++++++++++++++++++++-- spec/headers_spec.rb | 16 +++++++++++++++- 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/api_auth.gemspec b/api_auth.gemspec index a7e61591..0e22df8c 100644 --- a/api_auth.gemspec +++ b/api_auth.gemspec @@ -14,9 +14,10 @@ Gem::Specification.new do |s| s.add_development_dependency 'actionpack', '< 6.0', '> 4.0' s.add_development_dependency 'activeresource', '>= 4.0' s.add_development_dependency 'activesupport', '< 6.0', '> 4.0' + s.add_development_dependency 'rails', '>= 4.0' + s.add_development_dependency 'curb', '~> 0.8.1' s.add_development_dependency 'amatch' s.add_development_dependency 'appraisal' - s.add_development_dependency 'curb', '~> 0.8' s.add_development_dependency 'faraday', '>= 0.10' s.add_development_dependency 'httpi' s.add_development_dependency 'multipart-post', '~> 2.0' diff --git a/lib/api_auth/headers.rb b/lib/api_auth/headers.rb index bb0f4c6c..0ebe93b1 100644 --- a/lib/api_auth/headers.rb +++ b/lib/api_auth/headers.rb @@ -94,9 +94,29 @@ def sign_header(header) def parse_uri(uri) parsed_uri = URI.parse(uri) - return parsed_uri.request_uri if parsed_uri.respond_to?(:request_uri) + uri_without_host = parsed_uri.respond_to?(:request_uri) ? parsed_uri.request_uri : uri + return '/' if uri_without_host.empty? + escape_params(uri_without_host) + end - uri.empty? ? '/' : uri + # Different versions of request parsers escape/unescape the param values + # Examples: + # Rails 5.1.3 ApiAuth canonical_string:'GET,application/json,,/api/v1/employees?select=epulse_id%2Cfirst_name%2Clast_name,Thu, 14 Dec 2017 16:19:48 GMT' + # Rails 5.1.4 ApiAuth canonical_string:'GET,application/json,,/api/v1/employees?select=epulse_id,first_name,last_name,Thu, 14 Dec 2017 16:20:57 GMT' + # This will force param values to escaped and fixes issue #123 + def escape_params(uri) + unescaped_uri = CGI.unescape(uri) + uri_array = unescaped_uri.split('?') + return uri unless uri_array.length > 1 + params = uri_array[1].split('&') + encoded_params = "" + params.each do |param| + next unless param.include?('=') + encoded_params += '&' if encoded_params.length.positive? + split_param = param.split('=') + encoded_params += split_param[0] + '=' + CGI.escape(split_param[1]) + end + uri_array[0] + '?' + encoded_params end end end diff --git a/spec/headers_spec.rb b/spec/headers_spec.rb index fe31b794..9dec10a9 100644 --- a/spec/headers_spec.rb +++ b/spec/headers_spec.rb @@ -35,13 +35,27 @@ let(:uri) { 'http://google.com/?redirect_to=https://www.example.com'.freeze } it 'return /?redirect_to=https://www.example.com as canonical string path' do - expect(subject.canonical_string).to eq('GET,,,/?redirect_to=https://www.example.com,') + expect(subject.canonical_string).to eq('GET,,,/?redirect_to=https%3A%2F%2Fwww.example.com,') end it 'does not change request url (by removing host)' do expect(request.url).to eq(uri) end end + + context 'uri param values are not escaped' do + let(:uri) { 'http://google.com/search/advanced?redirect_to=https://www.example.com&account=a12dd334/3444\:23'.freeze } + it 'returns correct anonical string' do + expect(subject.canonical_string).to eq('GET,,,/search/advanced?redirect_to=https%3A%2F%2Fwww.example.com&account=a12dd334%2F3444%5C%3A23,') + end + end + + context 'uri param values are escaped' do + let(:uri) { 'http://google.com/search/advanced?redirect_to=https%3A%2F%2Fwww.example.com&account=a12dd334%2F3444%5C%3A23'.freeze } + it 'returns correct anonical string' do + expect(subject.canonical_string).to eq('GET,,,/search/advanced?redirect_to=https%3A%2F%2Fwww.example.com&account=a12dd334%2F3444%5C%3A23,') + end + end end context 'string construction' do From 1c216bda829658e535b3d2ddb3a87f30fa7220f2 Mon Sep 17 00:00:00 2001 From: "Michael S. Kazmier" Date: Mon, 18 Dec 2017 16:23:31 -0700 Subject: [PATCH 2/3] updates for older versions of ruby --- api_auth.gemspec | 3 +-- lib/api_auth/headers.rb | 10 ++++++---- spec/headers_spec.rb | 18 ++++++++++++++---- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/api_auth.gemspec b/api_auth.gemspec index 0e22df8c..a7e61591 100644 --- a/api_auth.gemspec +++ b/api_auth.gemspec @@ -14,10 +14,9 @@ Gem::Specification.new do |s| s.add_development_dependency 'actionpack', '< 6.0', '> 4.0' s.add_development_dependency 'activeresource', '>= 4.0' s.add_development_dependency 'activesupport', '< 6.0', '> 4.0' - s.add_development_dependency 'rails', '>= 4.0' - s.add_development_dependency 'curb', '~> 0.8.1' s.add_development_dependency 'amatch' s.add_development_dependency 'appraisal' + s.add_development_dependency 'curb', '~> 0.8' s.add_development_dependency 'faraday', '>= 0.10' s.add_development_dependency 'httpi' s.add_development_dependency 'multipart-post', '~> 2.0' diff --git a/lib/api_auth/headers.rb b/lib/api_auth/headers.rb index 0ebe93b1..541aa3e8 100644 --- a/lib/api_auth/headers.rb +++ b/lib/api_auth/headers.rb @@ -101,18 +101,20 @@ def parse_uri(uri) # Different versions of request parsers escape/unescape the param values # Examples: - # Rails 5.1.3 ApiAuth canonical_string:'GET,application/json,,/api/v1/employees?select=epulse_id%2Cfirst_name%2Clast_name,Thu, 14 Dec 2017 16:19:48 GMT' - # Rails 5.1.4 ApiAuth canonical_string:'GET,application/json,,/api/v1/employees?select=epulse_id,first_name,last_name,Thu, 14 Dec 2017 16:20:57 GMT' + # Rails 5.1.3 ApiAuth canonical_string: + # 'GET,application/json,,/api/v1/employees?select=epulse_id%2Cfirst_name%2Clast_name,Thu, 14 Dec 2017 16:19:48 GMT' + # Rails 5.1.4 ApiAuth canonical_string: + # 'GET,application/json,,/api/v1/employees?select=epulse_id,first_name,last_name,Thu, 14 Dec 2017 16:20:57 GMT' # This will force param values to escaped and fixes issue #123 def escape_params(uri) unescaped_uri = CGI.unescape(uri) uri_array = unescaped_uri.split('?') return uri unless uri_array.length > 1 params = uri_array[1].split('&') - encoded_params = "" + encoded_params = '' params.each do |param| next unless param.include?('=') - encoded_params += '&' if encoded_params.length.positive? + encoded_params += '&' if encoded_params.length > 0 split_param = param.split('=') encoded_params += split_param[0] + '=' + CGI.escape(split_param[1]) end diff --git a/spec/headers_spec.rb b/spec/headers_spec.rb index 9dec10a9..a640e1cb 100644 --- a/spec/headers_spec.rb +++ b/spec/headers_spec.rb @@ -44,16 +44,26 @@ end context 'uri param values are not escaped' do - let(:uri) { 'http://google.com/search/advanced?redirect_to=https://www.example.com&account=a12dd334/3444\:23'.freeze } + let(:uri) do + 'http://www.google.com/search/advanced?redirect_to=https://www.example.com&account=a12dd334/3444\:23'.freeze + end + it 'returns correct anonical string' do - expect(subject.canonical_string).to eq('GET,,,/search/advanced?redirect_to=https%3A%2F%2Fwww.example.com&account=a12dd334%2F3444%5C%3A23,') + expect(subject.canonical_string).to( + eq('GET,,,/search/advanced?redirect_to=https%3A%2F%2Fwww.example.com&account=a12dd334%2F3444%5C%3A23,') + ) end end context 'uri param values are escaped' do - let(:uri) { 'http://google.com/search/advanced?redirect_to=https%3A%2F%2Fwww.example.com&account=a12dd334%2F3444%5C%3A23'.freeze } + let(:uri) do + 'http://www.google.com/search/advanced?redirect_to=https%3A%2F%2Fwww.example.com&account=a12dd334%2F3444%5C%3A23'.freeze + end + it 'returns correct anonical string' do - expect(subject.canonical_string).to eq('GET,,,/search/advanced?redirect_to=https%3A%2F%2Fwww.example.com&account=a12dd334%2F3444%5C%3A23,') + expect(subject.canonical_string).to( + eq('GET,,,/search/advanced?redirect_to=https%3A%2F%2Fwww.example.com&account=a12dd334%2F3444%5C%3A23,') + ) end end end From c12a466865e20d397a996a9f8a5fe81b9c860bcf Mon Sep 17 00:00:00 2001 From: "Michael S. Kazmier" Date: Mon, 18 Dec 2017 16:44:23 -0700 Subject: [PATCH 3/3] update specs for rubocop and older ruby --- lib/api_auth/headers.rb | 2 +- spec/headers_spec.rb | 18 ++++++++++-------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/lib/api_auth/headers.rb b/lib/api_auth/headers.rb index 541aa3e8..b8c04a6c 100644 --- a/lib/api_auth/headers.rb +++ b/lib/api_auth/headers.rb @@ -114,7 +114,7 @@ def escape_params(uri) encoded_params = '' params.each do |param| next unless param.include?('=') - encoded_params += '&' if encoded_params.length > 0 + encoded_params += '&' unless encoded_params.empty? split_param = param.split('=') encoded_params += split_param[0] + '=' + CGI.escape(split_param[1]) end diff --git a/spec/headers_spec.rb b/spec/headers_spec.rb index a640e1cb..36dea91d 100644 --- a/spec/headers_spec.rb +++ b/spec/headers_spec.rb @@ -43,15 +43,17 @@ end end - context 'uri param values are not escaped' do - let(:uri) do - 'http://www.google.com/search/advanced?redirect_to=https://www.example.com&account=a12dd334/3444\:23'.freeze - end + if RUBY_VERSION.to_f > 2.1 + context 'uri param values are not escaped' do + let(:uri) do + 'http://www.google.com/search/advanced?redirect_to=https://www.example.com&account=a12dd334/3444\:23'.freeze + end - it 'returns correct anonical string' do - expect(subject.canonical_string).to( - eq('GET,,,/search/advanced?redirect_to=https%3A%2F%2Fwww.example.com&account=a12dd334%2F3444%5C%3A23,') - ) + it 'returns correct anonical string' do + expect(subject.canonical_string).to( + eq('GET,,,/search/advanced?redirect_to=https%3A%2F%2Fwww.example.com&account=a12dd334%2F3444%5C%3A23,') + ) + end end end