Credentials Environment Variable Refactor (#211)

* Refactor env var methods
  Have the iteration return for the first match, instead of comparing
  all elements and choosing the first one that matches.
* Combine file and json credentials detection
  Allow an environment variable to contain either a file path
  or JSON to describe the credentials.
  This change matches how these variables are used in Google Cloud.

lib/googleauth/credentials.rb

@@ -85,13 +85,10 @@ module Google
       # previously stated locations do not contain keyfile information,
       # this method defaults to use the application default.
       def self.default options = {}
-        # First try to find keyfile file from environment variables.
+        # First try to find keyfile file or json from environment variables.
-        client = from_path_vars options
+        client = from_env_vars options
 
-        # Second try to find keyfile json from environment variables.
+        # Second try to find keyfile file from known file paths.
-        client ||= from_json_vars options
-
-        # Third try to find keyfile file from known file paths.
         client ||= from_default_paths options
 
         # Finally get instantiated client from Google::Auth
@@ -99,31 +96,16 @@ module Google
         client
       end
 
-      def self.from_path_vars options
+      def self.from_env_vars options
-        self::PATH_ENV_VARS
+        (self::PATH_ENV_VARS + self::JSON_ENV_VARS).each do |env_var|
-          .map { |v| ENV[v] }
+          str = ENV[env_var]
-          .compact
+          next if str.nil?
-          .select { |p| ::File.file? p }
+          return new str, options if ::File.file? str
-          .each do |file|
+          return new ::JSON.parse(str), options rescue nil
-            return new file, options
         end
         nil
       end
 
-      def self.from_json_vars options
-        json = lambda do |v|
-          unless ENV[v].nil?
-            begin
-              JSON.parse ENV[v]
-            rescue StandardError
-              nil
-            end
-          end
-        end
-        self::JSON_ENV_VARS.map(&json).compact.each { |hash| return new hash, options }
-        nil
-      end
-
       def self.from_default_paths options
         self::DEFAULT_PATHS
           .select { |p| ::File.file? p }
@@ -138,8 +120,7 @@ module Google
         client = Google::Auth.get_application_default scope
         new client, options
       end
-      private_class_method :from_path_vars,
+      private_class_method :from_env_vars,
-                           :from_json_vars,
                            :from_default_paths,
                            :from_application_default
 

spec/googleauth/credentials_spec.rb

@@ -81,13 +81,14 @@ describe Google::Auth::Credentials, :private do
     Google::Auth::Credentials.new default_keyfile_hash, scope: "http://example.com/scope"
   end
 
-  it "can be subclassed to pass in other env paths" do
+  it 'can be subclassed to pass in other env paths' do
-    TEST_PATH_ENV_VAR = "TEST_PATH".freeze
+    TEST_PATH_ENV_VAR = 'TEST_PATH'.freeze
-    TEST_PATH_ENV_VAL = "/unknown/path/to/file.txt".freeze
+    TEST_PATH_ENV_VAL = '/unknown/path/to/file.txt'.freeze
-    TEST_JSON_ENV_VAR = "TEST_JSON_VARS".freeze
+    TEST_JSON_ENV_VAR = 'TEST_JSON_VARS'.freeze
+    TEST_JSON_ENV_VAL = JSON.generate(default_keyfile_hash)
 
     ENV[TEST_PATH_ENV_VAR] = TEST_PATH_ENV_VAL
-    ENV[TEST_JSON_ENV_VAR] = JSON.generate default_keyfile_hash
+    ENV[TEST_JSON_ENV_VAR] = TEST_JSON_ENV_VAL
 
     class TestCredentials < Google::Auth::Credentials
       SCOPE = "http://example.com/scope".freeze
@@ -96,6 +97,7 @@ describe Google::Auth::Credentials, :private do
     end
 
     allow(::File).to receive(:file?).with(TEST_PATH_ENV_VAL) { false }
+    allow(::File).to receive(:file?).with(TEST_JSON_ENV_VAL) { false }
 
     mocked_signet = double "Signet::OAuth2::Client"
     allow(mocked_signet).to receive(:configure_connection).and_return(mocked_signet)
@@ -151,7 +153,9 @@ describe Google::Auth::Credentials, :private do
     expect(creds.project_id).to eq(default_keyfile_hash["project_id"])
   end
 
-  it "subclasses can use JSON_ENV_VARS to get keyfile contents" do
+  it 'subclasses can use JSON_ENV_VARS to get keyfile contents' do
+    TEST_JSON_ENV_VAL = JSON.generate(default_keyfile_hash)
+
     class TestCredentials < Google::Auth::Credentials
       SCOPE = "http://example.com/scope".freeze
       PATH_ENV_VARS = ["PATH_ENV_DUMMY"].freeze
@@ -159,10 +163,11 @@ describe Google::Auth::Credentials, :private do
       DEFAULT_PATHS = ["~/default/path/to/file.txt"].freeze
     end
 
-    allow(::ENV).to receive(:[]).with("PATH_ENV_DUMMY") { "/fake/path/to/file.txt" }
+    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(::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(TEST_JSON_ENV_VAL) { false }
-    allow(::ENV).to receive(:[]).with("JSON_ENV_TEST") { JSON.generate default_keyfile_hash }
+    allow(::ENV).to receive(:[]).with('JSON_ENV_DUMMY') { nil }
+    allow(::ENV).to receive(:[]).with('JSON_ENV_TEST') { TEST_JSON_ENV_VAL }
 
     mocked_signet = double "Signet::OAuth2::Client"
     allow(mocked_signet).to receive(:configure_connection).and_return(mocked_signet)