From 5d42d3b4be2f22c60f355443271e6a327f980c07 Mon Sep 17 00:00:00 2001 From: Graham Paye Date: Wed, 18 Jul 2018 13:54:03 -0700 Subject: [PATCH] Warn when using cloud sdk credentials (#145) * Issue warning when cloud sdk credentials are used. --- .rubocop.yml | 2 +- lib/googleauth/client_id.rb | 6 ++-- lib/googleauth/credentials.rb | 11 +++--- lib/googleauth/credentials_loader.rb | 16 +++++++++ lib/googleauth/default_credentials.rb | 4 ++- spec/googleauth/client_id_spec.rb | 25 ++++++++++--- spec/googleauth/credentials_spec.rb | 20 +++++++++++ .../get_application_default_spec.rb | 35 +++++++++++++------ spec/googleauth/user_refresh_spec.rb | 2 +- 9 files changed, 96 insertions(+), 25 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 019c78d..0e9126a 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -13,7 +13,7 @@ Metrics/MethodLength: Max: 20 Metrics/ClassLength: Enabled: false -Style/IndentHeredoc: +Layout/IndentHeredoc: Enabled: false Style/FormatString: Enabled: false diff --git a/lib/googleauth/client_id.rb b/lib/googleauth/client_id.rb index ffecde9..b702e9d 100644 --- a/lib/googleauth/client_id.rb +++ b/lib/googleauth/client_id.rb @@ -28,6 +28,7 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. require 'multi_json' +require 'googleauth/credentials_loader' module Google module Auth @@ -63,13 +64,14 @@ module Google # & secrets in source. See {#from_file} to load from # `client_secrets.json` files. def initialize(id, secret) + CredentialsLoader.warn_if_cloud_sdk_credentials id raise 'Client id can not be nil' if id.nil? raise 'Client secret can not be nil' if secret.nil? @id = id @secret = secret end - # Constructs a Client ID from a JSON file downloaed from the + # Constructs a Client ID from a JSON file downloaded from the # Google Developers Console. # # @param [String, File] file @@ -79,7 +81,7 @@ module Google raise 'File can not be nil.' if file.nil? File.open(file.to_s) do |f| json = f.read - config = MultiJson.load(json) + config = MultiJson.load json from_hash(config) end end diff --git a/lib/googleauth/credentials.rb b/lib/googleauth/credentials.rb index 30f9502..6ebe8ff 100644 --- a/lib/googleauth/credentials.rb +++ b/lib/googleauth/credentials.rb @@ -31,7 +31,7 @@ require 'forwardable' require 'json' require 'signet/oauth_2/client' -require 'googleauth/default_credentials' +require 'googleauth/credentials_loader' module Google module Auth @@ -68,6 +68,7 @@ module Google json['scope'] ||= scope @client = init_client json end + CredentialsLoader.warn_if_cloud_sdk_credentials @client.client_id @client.fetch_access_token! end @@ -78,16 +79,16 @@ module Google def self.default(options = {}) scope = options[:scope] # First try to find keyfile file from environment variables. - client = from_path_vars(scope) + client = from_path_vars scope # Second try to find keyfile json from environment variables. - client ||= from_json_vars(scope) + client ||= from_json_vars scope # Third try to find keyfile file from known file paths. - client ||= from_default_paths(scope) + client ||= from_default_paths scope # Finally get instantiated client from Google::Auth - client ||= from_application_default(scope) + client ||= from_application_default scope client end diff --git a/lib/googleauth/credentials_loader.rb b/lib/googleauth/credentials_loader.rb index cdcd43f..260a3d9 100644 --- a/lib/googleauth/credentials_loader.rb +++ b/lib/googleauth/credentials_loader.rb @@ -57,6 +57,17 @@ module Google SYSTEM_DEFAULT_ERROR = 'Unable to read the system default credential file'.freeze + CLOUD_SDK_CLIENT_ID = '764086051850-6qr4p6gpi6hn506pt8ejuq83di341hur.app'\ + 's.googleusercontent.com'.freeze + + CLOUD_SDK_CREDENTIALS_WARNING = 'Your application has authenticated '\ + 'using end user credentials from Google Cloud SDK. We recommend that '\ + 'most server applications use service accounts instead. If your '\ + 'application continues to use end user credentials from Cloud SDK, '\ + 'you might receive a "quota exceeded" or "API not enabled" error. For'\ + ' more information about service accounts, see '\ + 'https://cloud.google.com/docs/authentication/.'.freeze + # make_creds proxies the construction of a credentials instance # # By default, it calls #new on the current class, but this behaviour can @@ -119,6 +130,11 @@ module Google raise "#{SYSTEM_DEFAULT_ERROR}: #{e}" end + # Issues warning if cloud sdk client id is used + def warn_if_cloud_sdk_credentials(client_id) + warn CLOUD_SDK_CREDENTIALS_WARNING if client_id == CLOUD_SDK_CLIENT_ID + end + private def service_account_env_vars? diff --git a/lib/googleauth/default_credentials.rb b/lib/googleauth/default_credentials.rb index cebb015..0926f8a 100644 --- a/lib/googleauth/default_credentials.rb +++ b/lib/googleauth/default_credentials.rb @@ -49,9 +49,11 @@ module Google json_key_io, scope = options.values_at(:json_key_io, :scope) if json_key_io json_key, clz = determine_creds_class(json_key_io) + warn_if_cloud_sdk_credentials json_key['client_id'] clz.make_creds(json_key_io: StringIO.new(MultiJson.dump(json_key)), scope: scope) else + warn_if_cloud_sdk_credentials ENV[CLIENT_ID_VAR] clz = read_creds clz.make_creds(scope: scope) end @@ -73,7 +75,7 @@ module Google # Reads the input json and determines which creds class to use. def self.determine_creds_class(json_key_io) - json_key = MultiJson.load(json_key_io.read) + json_key = MultiJson.load json_key_io.read key = 'type' raise "the json is missing the '#{key}' field" unless json_key.key?(key) type = json_key[key] diff --git a/spec/googleauth/client_id_spec.rb b/spec/googleauth/client_id_spec.rb index 857df0d..693d4b4 100644 --- a/spec/googleauth/client_id_spec.rb +++ b/spec/googleauth/client_id_spec.rb @@ -48,7 +48,7 @@ describe Google::Auth::ClientId do shared_examples 'it can successfully load client_id' do context 'loaded from hash' do - let(:client_id) { Google::Auth::ClientId.from_hash(config) } + let(:client_id) { Google::Auth::ClientId.from_hash config } it_behaves_like 'it has a valid config' end @@ -103,7 +103,7 @@ describe Google::Auth::ClientId do end it 'should raise error' do - expect { Google::Auth::ClientId.from_hash(config) }.to raise_error( + expect { Google::Auth::ClientId.from_hash config }.to raise_error( /Expected top level property/ ) end @@ -119,7 +119,7 @@ describe Google::Auth::ClientId do end it 'should raise error' do - expect { Google::Auth::ClientId.from_hash(config) }.to raise_error( + expect { Google::Auth::ClientId.from_hash config }.to raise_error( /Client id can not be nil/ ) end @@ -135,9 +135,26 @@ describe Google::Auth::ClientId do end it 'should raise error' do - expect { Google::Auth::ClientId.from_hash(config) }.to raise_error( + expect { Google::Auth::ClientId.from_hash config }.to raise_error( /Client secret can not be nil/ ) end end + + context 'with cloud sdk credentials' do + let(:config) do + { + 'web' => { + 'client_id' => Google::Auth::CredentialsLoader::CLOUD_SDK_CLIENT_ID, + 'client_secret' => 'notasecret' + } + } + end + + it 'should raise warning' do + expect { Google::Auth::ClientId.from_hash config }.to output( + Google::Auth::CredentialsLoader::CLOUD_SDK_CREDENTIALS_WARNING + "\n" + ).to_stderr + end + end end diff --git a/spec/googleauth/credentials_spec.rb b/spec/googleauth/credentials_spec.rb index 3f653fb..119d850 100644 --- a/spec/googleauth/credentials_spec.rb +++ b/spec/googleauth/credentials_spec.rb @@ -29,6 +29,7 @@ require 'googleauth' + # This test is testing the private class Google::Auth::Credentials. We want to # make sure that the passed in scope propogates to the Signet object. This means # testing the private API, which is generally frowned on. @@ -46,6 +47,7 @@ describe Google::Auth::Credentials, :private do it 'uses a default scope' do mocked_signet = double('Signet::OAuth2::Client') allow(mocked_signet).to receive(:fetch_access_token!).and_return(true) + allow(mocked_signet).to receive(:client_id) 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') @@ -62,6 +64,7 @@ describe Google::Auth::Credentials, :private do it 'uses a custom scope' do mocked_signet = double('Signet::OAuth2::Client') allow(mocked_signet).to receive(:fetch_access_token!).and_return(true) + allow(mocked_signet).to receive(:client_id) 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') @@ -93,6 +96,7 @@ describe Google::Auth::Credentials, :private do mocked_signet = double('Signet::OAuth2::Client') allow(mocked_signet).to receive(:fetch_access_token!).and_return(true) + allow(mocked_signet).to receive(:client_id) 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') @@ -124,6 +128,7 @@ describe Google::Auth::Credentials, :private do mocked_signet = double('Signet::OAuth2::Client') allow(mocked_signet).to receive(:fetch_access_token!).and_return(true) + allow(mocked_signet).to receive(:client_id) 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') @@ -154,6 +159,7 @@ describe Google::Auth::Credentials, :private do mocked_signet = double('Signet::OAuth2::Client') allow(mocked_signet).to receive(:fetch_access_token!).and_return(true) + allow(mocked_signet).to receive(:client_id) 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') @@ -185,6 +191,7 @@ describe Google::Auth::Credentials, :private do mocked_signet = double('Signet::OAuth2::Client') allow(mocked_signet).to receive(:fetch_access_token!).and_return(true) + allow(mocked_signet).to receive(:client_id) 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') @@ -215,6 +222,7 @@ describe Google::Auth::Credentials, :private do mocked_signet = double('Signet::OAuth2::Client') allow(mocked_signet).to receive(:fetch_access_token!).and_return(true) + allow(mocked_signet).to receive(:client_id) allow(Google::Auth).to receive(:get_application_default) do |scope| expect(scope).to eq(TestCredentials::SCOPE) @@ -236,4 +244,16 @@ describe Google::Auth::Credentials, :private do expect(creds).to be_a_kind_of(TestCredentials) expect(creds.client).to eq(mocked_signet) end + + it 'warns when cloud sdk credentials are used' do + 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| + mocked_signet + end + allow(mocked_signet).to receive(:client_id).and_return(Google::Auth::CredentialsLoader::CLOUD_SDK_CLIENT_ID) + expect { Google::Auth::Credentials.new default_keyfile_hash }.to output( + Google::Auth::CredentialsLoader::CLOUD_SDK_CREDENTIALS_WARNING + "\n" + ).to_stderr + end end diff --git a/spec/googleauth/get_application_default_spec.rb b/spec/googleauth/get_application_default_spec.rb index ffb0a69..2794bc7 100644 --- a/spec/googleauth/get_application_default_spec.rb +++ b/spec/googleauth/get_application_default_spec.rb @@ -63,7 +63,7 @@ describe '#get_application_default' do Dir.mktmpdir do |dir| key_path = File.join(dir, 'does-not-exist') ENV[@var_name] = key_path - expect { Google::Auth.get_application_default(@scope, options) } + expect { Google::Auth.get_application_default @scope, options } .to raise_error RuntimeError end end @@ -76,7 +76,7 @@ describe '#get_application_default' do ENV.delete(@var_name) unless ENV[@var_name].nil? # no env var ENV['HOME'] = dir # no config present in this tmp dir expect do - Google::Auth.get_application_default(@scope, options) + Google::Auth.get_application_default @scope, options end.to raise_error RuntimeError end expect(stub).to have_been_requested @@ -90,7 +90,7 @@ describe '#get_application_default' do FileUtils.mkdir_p(File.dirname(key_path)) File.write(key_path, cred_json_text) ENV[@var_name] = key_path - expect(Google::Auth.get_application_default(@scope, options)) + expect(Google::Auth.get_application_default @scope, options) .to_not be_nil end end @@ -102,7 +102,7 @@ describe '#get_application_default' do FileUtils.mkdir_p(File.dirname(key_path)) File.write(key_path, cred_json_text) ENV['HOME'] = dir - expect(Google::Auth.get_application_default(@scope, options)) + expect(Google::Auth.get_application_default @scope, options) .to_not be_nil end end @@ -114,7 +114,7 @@ describe '#get_application_default' do FileUtils.mkdir_p(File.dirname(key_path)) File.write(key_path, cred_json_text) ENV['HOME'] = dir - expect(Google::Auth.get_application_default(nil, options)).to_not be_nil + expect(Google::Auth.get_application_default nil, options).to_not be_nil end end @@ -125,7 +125,7 @@ describe '#get_application_default' do Dir.mktmpdir do |dir| ENV.delete(@var_name) unless ENV[@var_name].nil? # no env var ENV['HOME'] = dir # no config present in this tmp dir - creds = Google::Auth.get_application_default(@scope, options) + creds = Google::Auth.get_application_default @scope, options expect(creds).to_not be_nil end expect(stub).to have_been_requested @@ -137,7 +137,7 @@ describe '#get_application_default' do key_path = File.join('/etc/google/auth/', CREDENTIALS_FILE_NAME) FileUtils.mkdir_p(File.dirname(key_path)) File.write(key_path, cred_json_text) - expect(Google::Auth.get_application_default(@scope, options)) + expect(Google::Auth.get_application_default @scope, options) .to_not be_nil File.delete(key_path) end @@ -151,9 +151,22 @@ describe '#get_application_default' do ENV[CLIENT_SECRET_VAR] = cred_json[:client_secret] ENV[REFRESH_TOKEN_VAR] = cred_json[:refresh_token] ENV[ACCOUNT_TYPE_VAR] = cred_json[:type] - expect(Google::Auth.get_application_default(@scope, options)) + expect(Google::Auth.get_application_default @scope, options) .to_not be_nil end + + it 'warns when using cloud sdk credentials' do + ENV.delete(@var_name) unless ENV[@var_name].nil? # no env var + ENV[PRIVATE_KEY_VAR] = cred_json[:private_key] + ENV[CLIENT_EMAIL_VAR] = cred_json[:client_email] + ENV[CLIENT_ID_VAR] = Google::Auth::CredentialsLoader::CLOUD_SDK_CLIENT_ID + ENV[CLIENT_SECRET_VAR] = cred_json[:client_secret] + ENV[REFRESH_TOKEN_VAR] = cred_json[:refresh_token] + ENV[ACCOUNT_TYPE_VAR] = cred_json[:type] + expect { Google::Auth.get_application_default @scope, options }.to output( + Google::Auth::CredentialsLoader::CLOUD_SDK_CREDENTIALS_WARNING + "\n" + ).to_stderr + end end describe 'when credential type is service account' do @@ -216,7 +229,7 @@ describe '#get_application_default' do File.write(key_path, cred_json_text) ENV[@var_name] = key_path expect do - Google::Auth.get_application_default(@scope, options) + Google::Auth.get_application_default @scope, options end.to raise_error RuntimeError end end @@ -229,7 +242,7 @@ describe '#get_application_default' do File.write(key_path, cred_json_text) ENV['HOME'] = dir expect do - Google::Auth.get_application_default(@scope, options) + Google::Auth.get_application_default @scope, options end.to raise_error RuntimeError end end @@ -238,7 +251,7 @@ describe '#get_application_default' do ENV[PRIVATE_KEY_VAR] = cred_json[:private_key] ENV[CLIENT_EMAIL_VAR] = cred_json[:client_email] expect do - Google::Auth.get_application_default(@scope, options) + Google::Auth.get_application_default @scope, options end.to raise_error RuntimeError end end diff --git a/spec/googleauth/user_refresh_spec.rb b/spec/googleauth/user_refresh_spec.rb index 3162b18..e2316b1 100644 --- a/spec/googleauth/user_refresh_spec.rb +++ b/spec/googleauth/user_refresh_spec.rb @@ -294,7 +294,7 @@ describe Google::Auth::UserRefreshCredentials do end end - describe 'when erros occurred with request' do + describe 'when errors occurred with request' do it 'should fail with Signet::AuthorizationError if request times out' do allow_any_instance_of(Faraday::Connection).to receive(:get) .and_raise(Faraday::TimeoutError)