From 98816f7b6728a6f68cba7c9ae40723e40639a1dd Mon Sep 17 00:00:00 2001 From: Tim Emiola Date: Thu, 23 Apr 2015 09:50:11 -0700 Subject: [PATCH 1/3] Adds more robust checking of the status code during compute engine auth. Currently the implementation does not verify that a 200 response is received before attempting to parse the JSON response. This PR updates the behaviour to fail with an authorization error, similar to how other auth library implementations. --- .rubocop_todo.yml | 8 +++--- lib/googleauth/compute_engine.rb | 20 +++++++++++++- spec/googleauth/compute_engine_spec.rb | 36 ++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 5 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 8c99036..919ad7d 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,15 +1,15 @@ # This configuration was generated by `rubocop --auto-gen-config` -# on 2015-03-06 19:51:00 -0800 using RuboCop version 0.28.0. +# on 2015-04-23 09:39:10 -0700 using RuboCop version 0.30.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. -# Offense count: 3 +# Offense count: 4 Metrics/AbcSize: Max: 24 -# Offense count: 3 +# Offense count: 6 # Configuration parameters: CountComments. Metrics/MethodLength: - Max: 11 + Max: 12 diff --git a/lib/googleauth/compute_engine.rb b/lib/googleauth/compute_engine.rb index b820af1..358130a 100644 --- a/lib/googleauth/compute_engine.rb +++ b/lib/googleauth/compute_engine.rb @@ -35,13 +35,24 @@ module Google # Module Auth provides classes that provide Google-specific authorization # used to access Google APIs. module Auth + NO_METADATA_SERVER_ERROR = < 'Google' } resp = c.get(COMPUTE_AUTH_TOKEN_URI) + if resp.status == 404 + fail(Signet::AuthorizationError, NO_METADATA_SERVER_ERROR) + end + if resp.status != 200 + msg = "Unexpected error code #{resp.status}" + UNEXPECTED_ERROR_SUFFIX + fail(Signet::AuthorizationError, msg) + end Signet::OAuth2.parse_credentials(resp.body, resp.headers['content-type']) end diff --git a/spec/googleauth/compute_engine_spec.rb b/spec/googleauth/compute_engine_spec.rb index 95785cd..4666792 100644 --- a/spec/googleauth/compute_engine_spec.rb +++ b/spec/googleauth/compute_engine_spec.rb @@ -60,6 +60,42 @@ describe Google::Auth::GCECredentials do it_behaves_like 'apply/apply! are OK' + context 'metadata is unavailable' do + describe '#fetch_access_token' do + it 'should fail if the metadata request returns a 404' do + stubs = Faraday::Adapter::Test::Stubs.new do |stub| + stub.get(MD_URI) do |_env| + [404, + { 'Metadata-Flavor' => 'Google' }, + ''] + end + end + c = Faraday.new do |b| + b.adapter(:test, stubs) + end + blk = proc { @client.fetch_access_token!(connection: c) } + expect(&blk).to raise_error Signet::AuthorizationError + stubs.verify_stubbed_calls + end + + it 'should fail if metadata the request returns an unexpected codes' do + stubs = Faraday::Adapter::Test::Stubs.new do |stub| + stub.get(MD_URI) do |_env| + [503, + { 'Metadata-Flavor' => 'Google' }, + ''] + end + end + c = Faraday.new do |b| + b.adapter(:test, stubs) + end + blk = proc { @client.fetch_access_token!(connection: c) } + expect(&blk).to raise_error Signet::AuthorizationError + stubs.verify_stubbed_calls + end + end + end + describe '#on_gce?' do it 'should be true when Metadata-Flavor is Google' do stubs = Faraday::Adapter::Test::Stubs.new do |stub| From 194128d22094bdf4ade121fb002492d72534566c Mon Sep 17 00:00:00 2001 From: Tim Emiola Date: Thu, 23 Apr 2015 11:19:16 -0700 Subject: [PATCH 2/3] Updates conditional logic to use a case clause --- .rubocop_todo.yml | 6 +++--- lib/googleauth/compute_engine.rb | 11 ++++++----- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 919ad7d..b909cd2 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,15 +1,15 @@ # This configuration was generated by `rubocop --auto-gen-config` -# on 2015-04-23 09:39:10 -0700 using RuboCop version 0.30.0. +# on 2015-04-23 11:18:24 -0700 using RuboCop version 0.30.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. -# Offense count: 4 +# Offense count: 3 Metrics/AbcSize: Max: 24 # Offense count: 6 # Configuration parameters: CountComments. Metrics/MethodLength: - Max: 12 + Max: 13 diff --git a/lib/googleauth/compute_engine.rb b/lib/googleauth/compute_engine.rb index 358130a..76fc92f 100644 --- a/lib/googleauth/compute_engine.rb +++ b/lib/googleauth/compute_engine.rb @@ -89,15 +89,16 @@ END c = options[:connection] || Faraday.default_connection c.headers = { 'Metadata-Flavor' => 'Google' } resp = c.get(COMPUTE_AUTH_TOKEN_URI) - if resp.status == 404 + case resp.status + when 200 + Signet::OAuth2.parse_credentials(resp.body, + resp.headers['content-type']) + when 404 fail(Signet::AuthorizationError, NO_METADATA_SERVER_ERROR) - end - if resp.status != 200 + else msg = "Unexpected error code #{resp.status}" + UNEXPECTED_ERROR_SUFFIX fail(Signet::AuthorizationError, msg) end - Signet::OAuth2.parse_credentials(resp.body, - resp.headers['content-type']) end end end From fe1c46cc1408997a72859561f3ee1596a491a201 Mon Sep 17 00:00:00 2001 From: Tim Emiola Date: Thu, 23 Apr 2015 11:21:19 -0700 Subject: [PATCH 3/3] Corrects grammar --- spec/googleauth/compute_engine_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/googleauth/compute_engine_spec.rb b/spec/googleauth/compute_engine_spec.rb index 4666792..c14ddca 100644 --- a/spec/googleauth/compute_engine_spec.rb +++ b/spec/googleauth/compute_engine_spec.rb @@ -78,7 +78,7 @@ describe Google::Auth::GCECredentials do stubs.verify_stubbed_calls end - it 'should fail if metadata the request returns an unexpected codes' do + it 'should fail if the metadata request returns an unexpected code' do stubs = Faraday::Adapter::Test::Stubs.new do |stub| stub.get(MD_URI) do |_env| [503,