From 822435f616dac23a84b5b5d7a3258f5ac8bf7d2d Mon Sep 17 00:00:00 2001 From: Heng Xiong Date: Wed, 12 Jul 2017 15:04:03 -0700 Subject: [PATCH 1/3] Fix rubocop violations. Fix Travis build. --- .rubocop.yml | 19 ++++++++-- .rubocop_todo.yml | 41 --------------------- .travis.yml | 3 +- Gemfile | 12 +++--- Rakefile | 2 +- googleauth.gemspec | 1 + lib/googleauth/user_authorizer.rb | 26 +++++++------ lib/googleauth/user_refresh.rb | 2 +- lib/googleauth/web_user_authorizer.rb | 2 +- spec/googleauth/scope_util_spec.rb | 2 +- spec/googleauth/user_authorizer_spec.rb | 4 +- spec/googleauth/user_refresh_spec.rb | 6 +-- spec/googleauth/web_user_authorizer_spec.rb | 2 +- 13 files changed, 50 insertions(+), 72 deletions(-) delete mode 100644 .rubocop_todo.yml diff --git a/.rubocop.yml b/.rubocop.yml index 0ebe5d0..8fede7b 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,6 +1,19 @@ -inherit_from: .rubocop_todo.yml +AllCops: + Exclude: + - "spec/**/*" +Metrics/AbcSize: + Max: 25 Metrics/BlockLength: Exclude: - - 'spec/**/*.rb' - - 'googleauth.gemspec' + - "googleauth.gemspec" +Metrics/CyclomaticComplexity: + Max: 8 +Metrics/MethodLength: + Max: 20 +Metrics/ClassLength: + Enabled: false +Style/IndentHeredoc: + Enabled: false +Style/FormatString: + Enabled: false diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml deleted file mode 100644 index 5440c1f..0000000 --- a/.rubocop_todo.yml +++ /dev/null @@ -1,41 +0,0 @@ -# This configuration was generated by -# `rubocop --auto-gen-config` -# on 2017-02-25 23:23:21 +0900 using RuboCop version 0.46.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: 5 -# Configuration parameters: Include. -# Include: **/Gemfile, **/gems.rb -Bundler/OrderedGems: - Exclude: - - 'Gemfile' - -# Offense count: 3 -Metrics/AbcSize: - Max: 27 - -# Offense count: 1 -Metrics/CyclomaticComplexity: - Max: 7 - -# Offense count: 18 -# Configuration parameters: CountComments. -Metrics/MethodLength: - Max: 22 - -# Offense count: 3 -# Configuration parameters: EnforcedStyle, SupportedStyles. -# SupportedStyles: format, sprintf, percent -Style/FormatString: - Exclude: - - 'lib/googleauth/user_authorizer.rb' - - 'lib/googleauth/web_user_authorizer.rb' - -# Offense count: 1 -# Configuration parameters: MinBodyLength. -Style/GuardClause: - Exclude: - - 'lib/googleauth/web_user_authorizer.rb' diff --git a/.travis.yml b/.travis.yml index 7ae7a34..9e3b43b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,7 @@ sudo: false language: ruby rvm: + - 2.4 - 2.3.3 - 2.2 - 2.0.0 @@ -33,6 +34,6 @@ before_install: notifications: email: recipients: - - temiola@google.com + - ruby-cloud-eng@google.com on_success: change on_failure: change diff --git a/Gemfile b/Gemfile index ebeec76..af430a6 100755 --- a/Gemfile +++ b/Gemfile @@ -5,17 +5,17 @@ gemspec group :development do gem 'bundler', '~> 1.9' - gem 'simplecov', '~> 0.9' gem 'coveralls', '~> 0.7' gem 'fakefs', '~> 0.6' - gem 'rake', '~> 10.0' - gem 'rubocop', '~> 0.30' - gem 'rspec', '~> 3.0' - gem 'redis', '~> 3.2' gem 'fakeredis', '~> 0.5' - gem 'webmock', '~> 1.21' gem 'rack-test', '~> 0.6' + gem 'rake', '~> 10.0' + gem 'redis', '~> 3.2' + gem 'rspec', '~> 3.0' + gem 'rubocop', '~> 0.30' + gem 'simplecov', '~> 0.9' gem 'sinatra' + gem 'webmock', '~> 1.21' end platforms :jruby do diff --git a/Rakefile b/Rakefile index 2c95e80..c4d4ffc 100755 --- a/Rakefile +++ b/Rakefile @@ -10,6 +10,6 @@ desc 'Run rake task' RSpec::Core::RakeTask.new(:spec) desc 'Does rubocop lint and runs the specs' -task all: [:rubocop, :spec] +task all: %i[rubocop spec] task default: :all diff --git a/googleauth.gemspec b/googleauth.gemspec index 9a3a0fc..ffba4e9 100755 --- a/googleauth.gemspec +++ b/googleauth.gemspec @@ -1,5 +1,6 @@ # -*- ruby -*- # encoding: utf-8 + $LOAD_PATH.push File.expand_path('../lib', __FILE__) require 'googleauth/version' diff --git a/lib/googleauth/user_authorizer.rb b/lib/googleauth/user_authorizer.rb index 68fea74..2adf4c1 100644 --- a/lib/googleauth/user_authorizer.rb +++ b/lib/googleauth/user_authorizer.rb @@ -32,7 +32,6 @@ require 'multi_json' require 'googleauth/signet' require 'googleauth/user_refresh' -# rubocop:disable ClassLength module Google module Auth # Handles an interactive 3-Legged-OAuth2 (3LO) user consent authorization. @@ -125,11 +124,7 @@ module Google # @return [Google::Auth::UserRefreshCredentials] # Stored credentials, nil if none present def get_credentials(user_id, scope = nil) - raise NIL_USER_ID_ERROR if user_id.nil? - raise NIL_TOKEN_STORE_ERROR if @token_store.nil? - - scope ||= @scope - saved_token = @token_store.load(user_id) + saved_token = stored_token(user_id) return nil if saved_token.nil? data = MultiJson.load(saved_token) @@ -146,6 +141,7 @@ module Google refresh_token: data['refresh_token'], expires_at: data.fetch('expiration_time_millis', 0) / 1000 ) + scope ||= @scope if credentials.includes_scope?(scope) return monitor_credentials(user_id, credentials) end @@ -245,6 +241,18 @@ module Google private + # @private Fetch stored token with given user_id + # + # @param [String] user_id + # Unique ID of the user for loading/storing credentials. + # @return [String] The saved token from @token_store + def stored_token(user_id) + raise NIL_USER_ID_ERROR if user_id.nil? + raise NIL_TOKEN_STORE_ERROR if @token_store.nil? + + @token_store.load(user_id) + end + # Begin watching a credential for refreshes so the access token can be # saved. # @@ -268,14 +276,10 @@ module Google def redirect_uri_for(base_url) return @callback_uri unless URI(@callback_uri).scheme.nil? if base_url.nil? || URI(base_url).scheme.nil? - raise sprintf( - MISSING_ABSOLUTE_URL_ERROR, - @callback_uri - ) + raise sprintf(ISSING_ABSOLUTE_URL_ERROR, @callback_uri) end URI.join(base_url, @callback_uri).to_s end end end end -# rubocop:enable ClassLength diff --git a/lib/googleauth/user_refresh.rb b/lib/googleauth/user_refresh.rb index 14c1990..4abe7a3 100644 --- a/lib/googleauth/user_refresh.rb +++ b/lib/googleauth/user_refresh.rb @@ -75,7 +75,7 @@ module Google # JSON key. def self.read_json_key(json_key_io) json_key = MultiJson.load(json_key_io.read) - wanted = %w(client_id client_secret refresh_token) + wanted = %w[client_id client_secret refresh_token] wanted.each do |key| raise "the json is missing the #{key} field" unless json_key.key?(key) end diff --git a/lib/googleauth/web_user_authorizer.rb b/lib/googleauth/web_user_authorizer.rb index cfe8eac..9b09d06 100644 --- a/lib/googleauth/web_user_authorizer.rb +++ b/lib/googleauth/web_user_authorizer.rb @@ -227,7 +227,7 @@ module Google # @param [Rack::Request] request # Current request def self.validate_callback_state(state, request) - if state[AUTH_CODE_KEY].nil? + if state[AUTH_CODE_KEY].nil? # rubocop:disable Style/GuardClause raise Signet::AuthorizationError, MISSING_AUTH_CODE_ERROR elsif state[ERROR_CODE_KEY] raise Signet::AuthorizationError, diff --git a/spec/googleauth/scope_util_spec.rb b/spec/googleauth/scope_util_spec.rb index d69b820..f401b9e 100644 --- a/spec/googleauth/scope_util_spec.rb +++ b/spec/googleauth/scope_util_spec.rb @@ -70,7 +70,7 @@ describe Google::Auth::ScopeUtil do context 'with scope as Array' do let(:source) do - %w(email profile openid https://www.googleapis.com/auth/drive) + %w[email profile openid https://www.googleapis.com/auth/drive] end it_behaves_like 'normalizes scopes' end diff --git a/spec/googleauth/user_authorizer_spec.rb b/spec/googleauth/user_authorizer_spec.rb index 5e8d62e..d0bcfe2 100644 --- a/spec/googleauth/user_authorizer_spec.rb +++ b/spec/googleauth/user_authorizer_spec.rb @@ -41,7 +41,7 @@ describe Google::Auth::UserAuthorizer do include TestHelpers let(:client_id) { Google::Auth::ClientId.new('testclient', 'notasecret') } - let(:scope) { %w(email profile) } + let(:scope) { %w[email profile] } let(:token_store) { DummyTokenStore.new } let(:callback_uri) { 'https://www.example.com/oauth/callback' } let(:authorizer) do @@ -173,7 +173,7 @@ describe Google::Auth::UserAuthorizer do end it 'should return credentials with a valid scope' do - expect(credentials.scope).to eq %w(email profile) + expect(credentials.scope).to eq %w[email profile] end it 'should return credentials with a valid expiration time' do diff --git a/spec/googleauth/user_refresh_spec.rb b/spec/googleauth/user_refresh_spec.rb index d80f6ba..a2bb059 100644 --- a/spec/googleauth/user_refresh_spec.rb +++ b/spec/googleauth/user_refresh_spec.rb @@ -115,7 +115,7 @@ describe Google::Auth::UserRefreshCredentials do end it 'fails if the GOOGLE_APPLICATION_CREDENTIALS path file is invalid' do - needed = %w(client_id client_secret refresh_token) + needed = %w[client_id client_secret refresh_token] needed.each do |missing| Dir.mktmpdir do |dir| key_path = File.join(dir, 'my_cert_file') @@ -169,7 +169,7 @@ describe Google::Auth::UserRefreshCredentials do end it 'fails if the file is invalid' do - needed = %w(client_id client_secret refresh_token) + needed = %w[client_id client_secret refresh_token] needed.each do |missing| Dir.mktmpdir do |dir| key_path = File.join(dir, '.config', @known_path) @@ -208,7 +208,7 @@ describe Google::Auth::UserRefreshCredentials do end it 'fails if the file is invalid' do - needed = %w(client_id client_secret refresh_token) + needed = %w[client_id client_secret refresh_token] needed.each do |missing| FakeFS do FileUtils.mkdir_p(File.dirname(@path)) diff --git a/spec/googleauth/web_user_authorizer_spec.rb b/spec/googleauth/web_user_authorizer_spec.rb index 04e30dd..939bb3f 100644 --- a/spec/googleauth/web_user_authorizer_spec.rb +++ b/spec/googleauth/web_user_authorizer_spec.rb @@ -42,7 +42,7 @@ describe Google::Auth::WebUserAuthorizer do include TestHelpers let(:client_id) { Google::Auth::ClientId.new('testclient', 'notasecret') } - let(:scope) { %w(email profile) } + let(:scope) { %w[email profile] } let(:token_store) { DummyTokenStore.new } let(:authorizer) do Google::Auth::WebUserAuthorizer.new(client_id, scope, token_store) From 374b342ea9efe6ca7089165b49cc5856ec89c739 Mon Sep 17 00:00:00 2001 From: Heng Xiong Date: Wed, 12 Jul 2017 16:13:10 -0700 Subject: [PATCH 2/3] restore travis config back to ruby 2.3, which aliases to 2.3.4 --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 9e3b43b..94cccbd 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,7 +2,7 @@ sudo: false language: ruby rvm: - 2.4 - - 2.3.3 + - 2.3 - 2.2 - 2.0.0 - 2.1 From 1db30f6db126c2e644fea802e316af1ec73b4b49 Mon Sep 17 00:00:00 2001 From: Heng Xiong Date: Wed, 12 Jul 2017 17:11:49 -0700 Subject: [PATCH 3/3] Fix rubocop Ruby 1.9.3 compatibility issues --- .rubocop.yml | 7 +++++++ Rakefile | 2 +- lib/googleauth/user_refresh.rb | 2 +- lib/googleauth/web_user_authorizer.rb | 2 +- spec/googleauth/scope_util_spec.rb | 2 +- spec/googleauth/user_authorizer_spec.rb | 4 ++-- spec/googleauth/user_refresh_spec.rb | 6 +++--- spec/googleauth/web_user_authorizer_spec.rb | 2 +- 8 files changed, 17 insertions(+), 10 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 8fede7b..019c78d 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -17,3 +17,10 @@ Style/IndentHeredoc: Enabled: false Style/FormatString: Enabled: false +Style/GuardClause: + Enabled: false +Style/PercentLiteralDelimiters: # Contradicting rule + Enabled: false +Style/SymbolArray: # Undefined syntax in Ruby 1.9.3 + Enabled: false + diff --git a/Rakefile b/Rakefile index c4d4ffc..2c95e80 100755 --- a/Rakefile +++ b/Rakefile @@ -10,6 +10,6 @@ desc 'Run rake task' RSpec::Core::RakeTask.new(:spec) desc 'Does rubocop lint and runs the specs' -task all: %i[rubocop spec] +task all: [:rubocop, :spec] task default: :all diff --git a/lib/googleauth/user_refresh.rb b/lib/googleauth/user_refresh.rb index 4abe7a3..14c1990 100644 --- a/lib/googleauth/user_refresh.rb +++ b/lib/googleauth/user_refresh.rb @@ -75,7 +75,7 @@ module Google # JSON key. def self.read_json_key(json_key_io) json_key = MultiJson.load(json_key_io.read) - wanted = %w[client_id client_secret refresh_token] + wanted = %w(client_id client_secret refresh_token) wanted.each do |key| raise "the json is missing the #{key} field" unless json_key.key?(key) end diff --git a/lib/googleauth/web_user_authorizer.rb b/lib/googleauth/web_user_authorizer.rb index 9b09d06..cfe8eac 100644 --- a/lib/googleauth/web_user_authorizer.rb +++ b/lib/googleauth/web_user_authorizer.rb @@ -227,7 +227,7 @@ module Google # @param [Rack::Request] request # Current request def self.validate_callback_state(state, request) - if state[AUTH_CODE_KEY].nil? # rubocop:disable Style/GuardClause + if state[AUTH_CODE_KEY].nil? raise Signet::AuthorizationError, MISSING_AUTH_CODE_ERROR elsif state[ERROR_CODE_KEY] raise Signet::AuthorizationError, diff --git a/spec/googleauth/scope_util_spec.rb b/spec/googleauth/scope_util_spec.rb index f401b9e..d69b820 100644 --- a/spec/googleauth/scope_util_spec.rb +++ b/spec/googleauth/scope_util_spec.rb @@ -70,7 +70,7 @@ describe Google::Auth::ScopeUtil do context 'with scope as Array' do let(:source) do - %w[email profile openid https://www.googleapis.com/auth/drive] + %w(email profile openid https://www.googleapis.com/auth/drive) end it_behaves_like 'normalizes scopes' end diff --git a/spec/googleauth/user_authorizer_spec.rb b/spec/googleauth/user_authorizer_spec.rb index d0bcfe2..5e8d62e 100644 --- a/spec/googleauth/user_authorizer_spec.rb +++ b/spec/googleauth/user_authorizer_spec.rb @@ -41,7 +41,7 @@ describe Google::Auth::UserAuthorizer do include TestHelpers let(:client_id) { Google::Auth::ClientId.new('testclient', 'notasecret') } - let(:scope) { %w[email profile] } + let(:scope) { %w(email profile) } let(:token_store) { DummyTokenStore.new } let(:callback_uri) { 'https://www.example.com/oauth/callback' } let(:authorizer) do @@ -173,7 +173,7 @@ describe Google::Auth::UserAuthorizer do end it 'should return credentials with a valid scope' do - expect(credentials.scope).to eq %w[email profile] + expect(credentials.scope).to eq %w(email profile) end it 'should return credentials with a valid expiration time' do diff --git a/spec/googleauth/user_refresh_spec.rb b/spec/googleauth/user_refresh_spec.rb index a2bb059..d80f6ba 100644 --- a/spec/googleauth/user_refresh_spec.rb +++ b/spec/googleauth/user_refresh_spec.rb @@ -115,7 +115,7 @@ describe Google::Auth::UserRefreshCredentials do end it 'fails if the GOOGLE_APPLICATION_CREDENTIALS path file is invalid' do - needed = %w[client_id client_secret refresh_token] + needed = %w(client_id client_secret refresh_token) needed.each do |missing| Dir.mktmpdir do |dir| key_path = File.join(dir, 'my_cert_file') @@ -169,7 +169,7 @@ describe Google::Auth::UserRefreshCredentials do end it 'fails if the file is invalid' do - needed = %w[client_id client_secret refresh_token] + needed = %w(client_id client_secret refresh_token) needed.each do |missing| Dir.mktmpdir do |dir| key_path = File.join(dir, '.config', @known_path) @@ -208,7 +208,7 @@ describe Google::Auth::UserRefreshCredentials do end it 'fails if the file is invalid' do - needed = %w[client_id client_secret refresh_token] + needed = %w(client_id client_secret refresh_token) needed.each do |missing| FakeFS do FileUtils.mkdir_p(File.dirname(@path)) diff --git a/spec/googleauth/web_user_authorizer_spec.rb b/spec/googleauth/web_user_authorizer_spec.rb index 939bb3f..04e30dd 100644 --- a/spec/googleauth/web_user_authorizer_spec.rb +++ b/spec/googleauth/web_user_authorizer_spec.rb @@ -42,7 +42,7 @@ describe Google::Auth::WebUserAuthorizer do include TestHelpers let(:client_id) { Google::Auth::ClientId.new('testclient', 'notasecret') } - let(:scope) { %w[email profile] } + let(:scope) { %w(email profile) } let(:token_store) { DummyTokenStore.new } let(:authorizer) do Google::Auth::WebUserAuthorizer.new(client_id, scope, token_store)