From 9f395a2338d873bf6263c4dfe2cc791e972802a1 Mon Sep 17 00:00:00 2001
From: Mike Moore <mike@blowmage.com>
Date: Fri, 10 Nov 2017 15:47:17 -0700
Subject: [PATCH] Fix Credentials.default bug

If no PATH_ENV_VARS were found Credentials.default would return an empty array.
Update each default lookup method to return nil if no match was found.
Fix call to from_default_paths method, was calling from_default_vars instead.
Add spec coverage for path, json, and default cases.
Add spec coverage for application_default case.
Fix spec coverage to check the return type from calling Credentials.default.
---
 lib/googleauth/credentials.rb       |   5 +-
 spec/googleauth/credentials_spec.rb | 133 +++++++++++++++++++++++++++-
 2 files changed, 136 insertions(+), 2 deletions(-)

diff --git a/lib/googleauth/credentials.rb b/lib/googleauth/credentials.rb
index c5a6d06..30f9502 100644
--- a/lib/googleauth/credentials.rb
+++ b/lib/googleauth/credentials.rb
@@ -84,7 +84,7 @@ module Google
         client ||= from_json_vars(scope)
 
         # Third try to find keyfile file from known file paths.
-        client ||= from_default_vars(scope)
+        client ||= from_default_paths(scope)
 
         # Finally get instantiated client from Google::Auth
         client ||= from_application_default(scope)
@@ -99,6 +99,7 @@ module Google
           .each do |file|
             return new file, scope: scope
           end
+        nil
       end
 
       def self.from_json_vars(scope)
@@ -114,6 +115,7 @@ module Google
         self::JSON_ENV_VARS.map(&json).compact.each do |hash|
           return new hash, scope: scope
         end
+        nil
       end
 
       def self.from_default_paths(scope)
@@ -122,6 +124,7 @@ module Google
           .each do |file|
             return new file, scope: scope
           end
+        nil
       end
 
       def self.from_application_default(scope)
diff --git a/spec/googleauth/credentials_spec.rb b/spec/googleauth/credentials_spec.rb
index aa10a51..3f653fb 100644
--- a/spec/googleauth/credentials_spec.rb
+++ b/spec/googleauth/credentials_spec.rb
@@ -103,6 +103,137 @@ describe Google::Auth::Credentials, :private do
       mocked_signet
     end
 
-    TestCredentials.default
+    creds = TestCredentials.default
+    expect(creds).to be_a_kind_of(TestCredentials)
+    expect(creds.client).to eq(mocked_signet)
+  end
+
+  it 'subclasses can use PATH_ENV_VARS to get keyfile path' do
+    class TestCredentials < Google::Auth::Credentials
+      SCOPE = 'http://example.com/scope'.freeze
+      PATH_ENV_VARS = ['PATH_ENV_DUMMY', 'PATH_ENV_TEST'].freeze
+      JSON_ENV_VARS = ['JSON_ENV_DUMMY'].freeze
+      DEFAULT_PATHS = ['~/default/path/to/file.txt'].freeze
+    end
+
+    allow(::ENV).to receive(:[]).with('PATH_ENV_DUMMY') { '/fake/path/to/file.txt' }
+    allow(::File).to receive(:file?).with('/fake/path/to/file.txt') { false }
+    allow(::ENV).to receive(:[]).with('PATH_ENV_TEST') { '/unknown/path/to/file.txt' }
+    allow(::File).to receive(:file?).with('/unknown/path/to/file.txt') { true }
+    allow(::File).to receive(:read).with('/unknown/path/to/file.txt') { JSON.generate(default_keyfile_hash) }
+
+    mocked_signet = double('Signet::OAuth2::Client')
+    allow(mocked_signet).to receive(:fetch_access_token!).and_return(true)
+    allow(Signet::OAuth2::Client).to receive(:new) do |options|
+      expect(options[:token_credential_uri]).to eq('https://accounts.google.com/o/oauth2/token')
+      expect(options[:audience]).to eq('https://accounts.google.com/o/oauth2/token')
+      expect(options[:scope]).to eq(['http://example.com/scope'])
+      expect(options[:issuer]).to eq(default_keyfile_hash['client_email'])
+      expect(options[:signing_key]).to be_a_kind_of(OpenSSL::PKey::RSA)
+
+      mocked_signet
+    end
+
+    creds = TestCredentials.default
+    expect(creds).to be_a_kind_of(TestCredentials)
+    expect(creds.client).to eq(mocked_signet)
+  end
+
+  it 'subclasses can use JSON_ENV_VARS to get keyfile contents' do
+    class TestCredentials < Google::Auth::Credentials
+      SCOPE = 'http://example.com/scope'.freeze
+      PATH_ENV_VARS = ['PATH_ENV_DUMMY'].freeze
+      JSON_ENV_VARS = ['JSON_ENV_DUMMY', 'JSON_ENV_TEST'].freeze
+      DEFAULT_PATHS = ['~/default/path/to/file.txt'].freeze
+    end
+
+    allow(::ENV).to receive(:[]).with('PATH_ENV_DUMMY') { '/fake/path/to/file.txt' }
+    allow(::File).to receive(:file?).with('/fake/path/to/file.txt') { false }
+    allow(::ENV).to receive(:[]).with('JSON_ENV_DUMMY') { nil }
+    allow(::ENV).to receive(:[]).with('JSON_ENV_TEST') { JSON.generate(default_keyfile_hash) }
+
+    mocked_signet = double('Signet::OAuth2::Client')
+    allow(mocked_signet).to receive(:fetch_access_token!).and_return(true)
+    allow(Signet::OAuth2::Client).to receive(:new) do |options|
+      expect(options[:token_credential_uri]).to eq('https://accounts.google.com/o/oauth2/token')
+      expect(options[:audience]).to eq('https://accounts.google.com/o/oauth2/token')
+      expect(options[:scope]).to eq(['http://example.com/scope'])
+      expect(options[:issuer]).to eq(default_keyfile_hash['client_email'])
+      expect(options[:signing_key]).to be_a_kind_of(OpenSSL::PKey::RSA)
+
+      mocked_signet
+    end
+
+    creds = TestCredentials.default
+    expect(creds).to be_a_kind_of(TestCredentials)
+    expect(creds.client).to eq(mocked_signet)
+  end
+
+  it 'subclasses can use DEFAULT_PATHS to get keyfile path' do
+    class TestCredentials < Google::Auth::Credentials
+      SCOPE = 'http://example.com/scope'.freeze
+      PATH_ENV_VARS = ['PATH_ENV_DUMMY'].freeze
+      JSON_ENV_VARS = ['JSON_ENV_DUMMY'].freeze
+      DEFAULT_PATHS = ['~/default/path/to/file.txt'].freeze
+    end
+
+    allow(::ENV).to receive(:[]).with('PATH_ENV_DUMMY') { '/fake/path/to/file.txt' }
+    allow(::File).to receive(:file?).with('/fake/path/to/file.txt') { false }
+    allow(::ENV).to receive(:[]).with('JSON_ENV_DUMMY') { nil }
+    allow(::File).to receive(:file?).with('~/default/path/to/file.txt') { true }
+    allow(::File).to receive(:read).with('~/default/path/to/file.txt') { JSON.generate(default_keyfile_hash) }
+
+    mocked_signet = double('Signet::OAuth2::Client')
+    allow(mocked_signet).to receive(:fetch_access_token!).and_return(true)
+    allow(Signet::OAuth2::Client).to receive(:new) do |options|
+      expect(options[:token_credential_uri]).to eq('https://accounts.google.com/o/oauth2/token')
+      expect(options[:audience]).to eq('https://accounts.google.com/o/oauth2/token')
+      expect(options[:scope]).to eq(['http://example.com/scope'])
+      expect(options[:issuer]).to eq(default_keyfile_hash['client_email'])
+      expect(options[:signing_key]).to be_a_kind_of(OpenSSL::PKey::RSA)
+
+      mocked_signet
+    end
+
+    creds = TestCredentials.default
+    expect(creds).to be_a_kind_of(TestCredentials)
+    expect(creds.client).to eq(mocked_signet)
+  end
+
+  it 'subclasses that find no matches default to Google::Auth.get_application_default' do
+    class TestCredentials < Google::Auth::Credentials
+      SCOPE = 'http://example.com/scope'.freeze
+      PATH_ENV_VARS = ['PATH_ENV_DUMMY'].freeze
+      JSON_ENV_VARS = ['JSON_ENV_DUMMY'].freeze
+      DEFAULT_PATHS = ['~/default/path/to/file.txt'].freeze
+    end
+
+    allow(::ENV).to receive(:[]).with('PATH_ENV_DUMMY') { '/fake/path/to/file.txt' }
+    allow(::File).to receive(:file?).with('/fake/path/to/file.txt') { false }
+    allow(::ENV).to receive(:[]).with('JSON_ENV_DUMMY') { nil }
+    allow(::File).to receive(:file?).with('~/default/path/to/file.txt') { false }
+
+    mocked_signet = double('Signet::OAuth2::Client')
+    allow(mocked_signet).to receive(:fetch_access_token!).and_return(true)
+    allow(Google::Auth).to receive(:get_application_default) do |scope|
+      expect(scope).to eq(TestCredentials::SCOPE)
+
+      # This should really be a Signet::OAuth2::Client object,
+      # but mocking is making that difficult, so return a valid hash instead.
+      default_keyfile_hash
+    end
+    allow(Signet::OAuth2::Client).to receive(:new) do |options|
+      expect(options[:token_credential_uri]).to eq('https://accounts.google.com/o/oauth2/token')
+      expect(options[:audience]).to eq('https://accounts.google.com/o/oauth2/token')
+      expect(options[:scope]).to eq(['http://example.com/scope'])
+      expect(options[:issuer]).to eq(default_keyfile_hash['client_email'])
+      expect(options[:signing_key]).to be_a_kind_of(OpenSSL::PKey::RSA)
+
+      mocked_signet
+    end
+
+    creds = TestCredentials.default
+    expect(creds).to be_a_kind_of(TestCredentials)
+    expect(creds.client).to eq(mocked_signet)
   end
 end