From 80d1105c365e6952c72fd86ca61ca49664449b92 Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Thu, 12 Sep 2013 13:36:51 -0700 Subject: [PATCH 01/46] add a license, fixes #36 --- warden-github.gemspec | 1 + 1 file changed, 1 insertion(+) diff --git a/warden-github.gemspec b/warden-github.gemspec index d09671c..216b00d 100644 --- a/warden-github.gemspec +++ b/warden-github.gemspec @@ -9,6 +9,7 @@ Gem::Specification.new do |s| s.email = ["atmos@atmos.org"] s.homepage = "http://github.com/atmos/warden-github" s.summary = "A warden strategy for easy oauth integration with github" + s.license = 'MIT' s.description = s.summary s.rubyforge_project = "warden-github" From 24bd3d49d227c31da3b26c5c263081c7d9aadff8 Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Mon, 24 Mar 2014 14:32:06 -0700 Subject: [PATCH 02/46] grab the first x-forwarded-proto header --- lib/warden/github/config.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/warden/github/config.rb b/lib/warden/github/config.rb index 0248fe9..50aa35a 100644 --- a/lib/warden/github/config.rb +++ b/lib/warden/github/config.rb @@ -131,8 +131,13 @@ def extract_path(uri) end end + def x_forwarded_proto + env['HTTP_X_FORWARDED_PROTO'] && + env['HTTP_X_FORWARDED_PROTO'].split(',')[0] + end + def correct_scheme(uri) - if uri.scheme != 'https' && env['HTTP_X_FORWARDED_PROTO'] == 'https' + if uri.scheme != 'https' && x_forwarded_proto == 'https' uri.port = nil if uri.port == 80 uri.scheme = 'https' # Reparsing will use a different URI subclass, namely URI::HTTPS which From 3dc444929b1ce06da70d2791d903087188e50a5c Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Mon, 24 Mar 2014 14:34:23 -0700 Subject: [PATCH 03/46] version bump --- lib/warden/github/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/warden/github/version.rb b/lib/warden/github/version.rb index 507f87c..e768cc2 100644 --- a/lib/warden/github/version.rb +++ b/lib/warden/github/version.rb @@ -1,5 +1,5 @@ module Warden module GitHub - VERSION = "1.0.0" + VERSION = "1.0.1" end end From d838a8d6fe2c3e178f1f6d78d11023d662aa9c2a Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Mon, 24 Mar 2014 15:17:59 -0700 Subject: [PATCH 04/46] try this now too --- lib/warden/github/config.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/warden/github/config.rb b/lib/warden/github/config.rb index 50aa35a..2930cd1 100644 --- a/lib/warden/github/config.rb +++ b/lib/warden/github/config.rb @@ -131,21 +131,21 @@ def extract_path(uri) end end - def x_forwarded_proto + def https_forwarded_proto? env['HTTP_X_FORWARDED_PROTO'] && - env['HTTP_X_FORWARDED_PROTO'].split(',')[0] + env['HTTP_X_FORWARDED_PROTO'].split(',').include?("https") end def correct_scheme(uri) - if uri.scheme != 'https' && x_forwarded_proto == 'https' - uri.port = nil if uri.port == 80 + if uri.scheme != 'https' && https_forwarded_proto? uri.scheme = 'https' # Reparsing will use a different URI subclass, namely URI::HTTPS which # knows the default port for https and strips it if present. uri = URI(uri.to_s) end + uri.port = nil if uri.port == 80 - uri + URI(uri.to_s) end end end From 2554d4a66899df398999d19a7edf59e32d92f43d Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Mon, 24 Mar 2014 17:05:38 -0700 Subject: [PATCH 05/46] minor fixups --- lib/warden/github/config.rb | 2 +- spec/unit/config_spec.rb | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/warden/github/config.rb b/lib/warden/github/config.rb index 2930cd1..4937ff0 100644 --- a/lib/warden/github/config.rb +++ b/lib/warden/github/config.rb @@ -133,7 +133,7 @@ def extract_path(uri) def https_forwarded_proto? env['HTTP_X_FORWARDED_PROTO'] && - env['HTTP_X_FORWARDED_PROTO'].split(',').include?("https") + env['HTTP_X_FORWARDED_PROTO'].split(',')[0] == "https" end def correct_scheme(uri) diff --git a/spec/unit/config_spec.rb b/spec/unit/config_spec.rb index 9328e97..2d90a7b 100644 --- a/spec/unit/config_spec.rb +++ b/spec/unit/config_spec.rb @@ -141,6 +141,12 @@ def silence_warnings config.redirect_uri.should eq 'https://example.com/the/path' end + it 'returns the expanded redirect uri with adjusted scheme including port 80 with multiple forwarded protocols' do + env['HTTP_X_FORWARDED_PROTO'] = 'https,https' + request.stub(:url => 'https://example.com:80/the/path') + config.redirect_uri.should eq 'https://example.com/the/path' + end + it 'returns the expanded redirect uri(without port) with adjusted scheme' do env['HTTP_X_FORWARDED_PROTO'] = 'https' request.stub(:url => 'http://example.com/the/path') From 14afa811e7ad7d964fe6a398b08ecf740ff89d13 Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Mon, 24 Mar 2014 17:08:07 -0700 Subject: [PATCH 06/46] explain changes in the changelog too --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 112c0e9..b99df6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +v1.0.1 2014/03/24 +----------------- + +* Handle multiple X-Forwarded-Proto headers when comma delimited + v1.0.0 2013/09/03 ----------------- From 94bab24c248faea39cb0580e65e1c6fc5d920dce Mon Sep 17 00:00:00 2001 From: Eugene Kenny Date: Sun, 7 Sep 2014 03:52:26 +0100 Subject: [PATCH 07/46] Make avatar_url available through the user object The gravatar_id attribute has been deprecated in favour of avatar_url: https://developer.github.com/changes/2014-09-05-removing-gravatar-id/ Removing gravatar_id entirely would break clients, but avatar_url should at least be available. --- README.md | 1 + example/simple_app.rb | 2 +- lib/warden/github/user.rb | 2 +- spec/unit/user_spec.rb | 3 ++- 4 files changed, 5 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 4b61475..612c45e 100644 --- a/README.md +++ b/README.md @@ -116,6 +116,7 @@ user.id # => The GitHub user id. user.login # => The GitHub username. user.name user.gravatar_id # => The md5 email hash to construct a gravatar image. +user.avatar_url user.email # => Requires user:email or user scope. user.company diff --git a/example/simple_app.rb b/example/simple_app.rb index 46407ef..628a1b9 100644 --- a/example/simple_app.rb +++ b/example/simple_app.rb @@ -66,7 +66,7 @@ def self.app @@ index <% if env['warden'].authenticated? %>

- .png?r=PG&s=50' /> + ' width='50' height='50' /> Welcome <%= env['warden'].user.name %>

<% else %> diff --git a/lib/warden/github/user.rb b/lib/warden/github/user.rb index 4a6b222..27447af 100644 --- a/lib/warden/github/user.rb +++ b/lib/warden/github/user.rb @@ -3,7 +3,7 @@ module Warden module GitHub class User < Struct.new(:attribs, :token) - ATTRIBUTES = %w[id login name gravatar_id email company site_admin].freeze + ATTRIBUTES = %w[id login name gravatar_id avatar_url email company site_admin].freeze def self.load(access_token) api = Octokit::Client.new(:access_token => access_token) diff --git a/spec/unit/user_spec.rb b/spec/unit/user_spec.rb index 594041c..14c9feb 100644 --- a/spec/unit/user_spec.rb +++ b/spec/unit/user_spec.rb @@ -5,6 +5,7 @@ { 'login' => 'john', 'name' => 'John Doe', 'gravatar_id' => '38581cb351a52002548f40f8066cfecg', + 'avatar_url' => 'http://example.com/avatar.jpg', 'email' => 'john@doe.com', 'company' => 'Doe, Inc.' } end @@ -20,7 +21,7 @@ end end - %w[login name gravatar_id email company].each do |name| + %w[login name gravatar_id avatar_url email company].each do |name| describe "##{name}" do it "returns the #{name}" do user.send(name).should eq default_attrs[name] From fc62d8863c4a5fb7b5a7593164d5774c02abd5d6 Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Wed, 1 Oct 2014 11:24:02 -0700 Subject: [PATCH 08/46] cut a version 1.0.2 gem --- lib/warden/github/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/warden/github/version.rb b/lib/warden/github/version.rb index e768cc2..a3b50ce 100644 --- a/lib/warden/github/version.rb +++ b/lib/warden/github/version.rb @@ -1,5 +1,5 @@ module Warden module GitHub - VERSION = "1.0.1" + VERSION = "1.0.2" end end From 0f7a4d29649e7e4d116f9f37edfe0c09a1b978f3 Mon Sep 17 00:00:00 2001 From: Philipe Fatio Date: Sat, 13 Dec 2014 11:32:34 +0100 Subject: [PATCH 09/46] Cache team memberships This was accidentally removed in 8023a53. --- lib/warden/github/user.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/warden/github/user.rb b/lib/warden/github/user.rb index 27447af..57d967e 100644 --- a/lib/warden/github/user.rb +++ b/lib/warden/github/user.rb @@ -59,7 +59,9 @@ def organization_member?(org_name) # # Returns: true if the user has access, false otherwise def team_member?(team_id) - api.team_member?(team_id, login) + memberships.fetch_membership(:team, team_id) do + api.team_member?(team_id, login) + end end # Identify GitHub employees/staff members. From 44afe6c05530a14fedd6284b713418f0a9da48bb Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Sat, 13 Dec 2014 08:30:18 -0800 Subject: [PATCH 10/46] cut version 1.0.3 --- CHANGELOG.md | 4 ++++ lib/warden/github/version.rb | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b99df6f..2d9ccdf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +v1.0.3 2014/12/13 + +* Reintroduce membership caching to reduce API hits for validating team membership. + v1.0.1 2014/03/24 ----------------- diff --git a/lib/warden/github/version.rb b/lib/warden/github/version.rb index a3b50ce..2b15ca0 100644 --- a/lib/warden/github/version.rb +++ b/lib/warden/github/version.rb @@ -1,5 +1,5 @@ module Warden module GitHub - VERSION = "1.0.2" + VERSION = "1.0.3" end end From 6e3d4d840ef131e5598f6aa3ba5c2a5486886923 Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Wed, 7 Jan 2015 13:59:45 -0800 Subject: [PATCH 11/46] appease the rspec gods --- spec/unit/membership_cache_spec.rb | 10 +++++----- spec/unit/user_spec.rb | 4 ++-- warden-github.gemspec | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/unit/membership_cache_spec.rb b/spec/unit/membership_cache_spec.rb index d080c50..97120d3 100644 --- a/spec/unit/membership_cache_spec.rb +++ b/spec/unit/membership_cache_spec.rb @@ -7,7 +7,7 @@ describe '#fetch_membership' do it 'returns false by default' do - cache.fetch_membership('foo', 'bar').should be_false + cache.fetch_membership('foo', 'bar').should be_falsey end context 'when cache valid' do @@ -17,7 +17,7 @@ end it 'returns true' do - cache.fetch_membership('foo', 'bar').should be_true + cache.fetch_membership('foo', 'bar').should be_truthy end it 'does not invoke the block' do @@ -34,7 +34,7 @@ context 'when no block given' do it 'returns false' do - cache.fetch_membership('foo', 'bar').should be_false + cache.fetch_membership('foo', 'bar').should be_falsey end end @@ -51,12 +51,12 @@ it 'caches the value when block returns true' do cache.fetch_membership('foo', 'bar') { true } - cache.fetch_membership('foo', 'bar').should be_true + cache.fetch_membership('foo', 'bar').should be_truthy end it 'does not cache the value when block returns false' do cache.fetch_membership('foo', 'bar') { false } - cache.fetch_membership('foo', 'bar').should be_false + cache.fetch_membership('foo', 'bar').should be_falsey end end end diff --git a/spec/unit/user_spec.rb b/spec/unit/user_spec.rb index 14c9feb..71afd07 100644 --- a/spec/unit/user_spec.rb +++ b/spec/unit/user_spec.rb @@ -50,14 +50,14 @@ def stub_api(user, method, args, ret) context 'when user is not member' do it 'returns false' do stub_api(user, method, ['rails', user.login], false) - user.send(method, 'rails').should be_false + user.send(method, 'rails').should be_falsey end end context 'when user is member' do it 'returns true' do stub_api(user, method, ['rails', user.login], true) - user.send(method, 'rails').should be_true + user.send(method, 'rails').should be_truthy end end end diff --git a/warden-github.gemspec b/warden-github.gemspec index 216b00d..bec3c50 100644 --- a/warden-github.gemspec +++ b/warden-github.gemspec @@ -24,7 +24,7 @@ Gem::Specification.new do |s| s.add_development_dependency "webmock", "~>1.9" s.add_development_dependency "sinatra" s.add_development_dependency "shotgun" - s.add_development_dependency "addressable", "~>2.2.0" + s.add_development_dependency "addressable", ">2.2.0" s.add_development_dependency "rack-test", "~>0.5.3" s.add_development_dependency "yajl-ruby" From 2d9cccc292feb2cf9d5a1f4af1b0bf767067d72e Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Wed, 7 Jan 2015 19:34:28 -0800 Subject: [PATCH 12/46] encrypt user objects in the session --- config.ru | 15 +----------- lib/warden/github.rb | 2 ++ lib/warden/github/session_serializer.rb | 32 +++++++++++++++++++++++++ warden-github.gemspec | 5 ++-- 4 files changed, 38 insertions(+), 16 deletions(-) create mode 100644 lib/warden/github/session_serializer.rb diff --git a/config.ru b/config.ru index 10e1170..2415247 100644 --- a/config.ru +++ b/config.ru @@ -1,19 +1,6 @@ ENV['RACK_ENV'] ||= 'development' -begin - require File.expand_path('../.bundle/environment', __FILE__) -rescue LoadError - require "rubygems" - require "bundler" - Bundler.setup -end - -begin - require 'debugger' -rescue LoadError - require 'ruby-debug' -end - +require "bundler/setup" require 'warden/github' if ENV['MULTI_SCOPE_APP'] diff --git a/lib/warden/github.rb b/lib/warden/github.rb index e4c8012..0284f52 100644 --- a/lib/warden/github.rb +++ b/lib/warden/github.rb @@ -7,5 +7,7 @@ require 'warden/github/hook' require 'warden/github/config' require 'warden/github/membership_cache' +require 'warden/github/session_serializer' +require 'active_support/message_verifier' require 'securerandom' diff --git a/lib/warden/github/session_serializer.rb b/lib/warden/github/session_serializer.rb new file mode 100644 index 0000000..42db4ba --- /dev/null +++ b/lib/warden/github/session_serializer.rb @@ -0,0 +1,32 @@ +module Warden + class SessionSerializer + attr_reader :env + + def initialize(env) + @env = env + end + + def serialize(user) + cookie_verifier.generate(user) + end + + def deserialize(key) + cookie_verifier.verify(key) + rescue ::ActiveSupport::MessageVerifier::InvalidSignature + nil + end + + private + def verifier_key + self.class.verifier_key + end + + def self.verifier_key + @verifier_key ||= ENV['WARDEN_GITHUB_VERIFIER_SECRET'] || SecureRandom.hex + end + + def cookie_verifier + @cookie_verifier ||= ::ActiveSupport::MessageVerifier.new(verifier_key, serializer: Marshal) + end + end +end diff --git a/warden-github.gemspec b/warden-github.gemspec index bec3c50..655c17c 100644 --- a/warden-github.gemspec +++ b/warden-github.gemspec @@ -14,8 +14,9 @@ Gem::Specification.new do |s| s.rubyforge_project = "warden-github" - s.add_dependency "warden", ">1.0" - s.add_dependency "octokit", ">2.1.0" + s.add_dependency "warden", ">1.0" + s.add_dependency "octokit", ">2.1.0" + s.add_dependency "activesupport", ">3.0" s.add_development_dependency "rack", "~>1.4.1" s.add_development_dependency "rake" From 31db5f60952778df3d2254dfb393f70b397f5e5c Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Wed, 7 Jan 2015 20:42:07 -0800 Subject: [PATCH 13/46] dumb down the verifier stuff --- example/simple_app.rb | 2 ++ lib/warden/github.rb | 1 + lib/warden/github/verifier.rb | 28 ++++++++++++++++++++++++++++ 3 files changed, 31 insertions(+) create mode 100644 lib/warden/github/verifier.rb diff --git a/example/simple_app.rb b/example/simple_app.rb index 628a1b9..4cd9128 100644 --- a/example/simple_app.rb +++ b/example/simple_app.rb @@ -15,6 +15,8 @@ class SimpleApp < BaseApp config.default_strategies :github config.scope_defaults :default, :config => GITHUB_CONFIG end + Warden::Manager.serialize_from_session { |key| puts 'lol'; Warden::GitHub::Verifier.new.deserialize(key) } + Warden::Manager.serialize_into_session { |user| puts 'wut'; Warden::GitHub::Verifier.new.serialize(user) } get '/' do erb :index diff --git a/lib/warden/github.rb b/lib/warden/github.rb index 0284f52..774363e 100644 --- a/lib/warden/github.rb +++ b/lib/warden/github.rb @@ -8,6 +8,7 @@ require 'warden/github/config' require 'warden/github/membership_cache' require 'warden/github/session_serializer' +require 'warden/github/verifier' require 'active_support/message_verifier' require 'securerandom' diff --git a/lib/warden/github/verifier.rb b/lib/warden/github/verifier.rb new file mode 100644 index 0000000..3e90916 --- /dev/null +++ b/lib/warden/github/verifier.rb @@ -0,0 +1,28 @@ +module Warden + module GitHub + class Verifier + def serialize(user) + cookie_verifier.generate(user) + end + + def deserialize(key) + cookie_verifier.verify(key) + rescue ::ActiveSupport::MessageVerifier::InvalidSignature + nil + end + + private + def verifier_key + self.class.verifier_key + end + + def self.verifier_key + @verifier_key ||= ENV['WARDEN_GITHUB_VERIFIER_SECRET'] || SecureRandom.hex + end + + def cookie_verifier + @cookie_verifier ||= ::ActiveSupport::MessageVerifier.new(verifier_key, serializer: Marshal) + end + end + end +end From 84291a31ec9d97df3d005c5ba942b77850233176 Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Wed, 7 Jan 2015 21:21:48 -0800 Subject: [PATCH 14/46] use a custom serializer for sensitive info --- example/simple_app.rb | 4 ++-- lib/warden/github.rb | 1 - lib/warden/github/session_serializer.rb | 32 ------------------------- lib/warden/github/verifier.rb | 10 +++++++- 4 files changed, 11 insertions(+), 36 deletions(-) delete mode 100644 lib/warden/github/session_serializer.rb diff --git a/example/simple_app.rb b/example/simple_app.rb index 4cd9128..1a3ee4d 100644 --- a/example/simple_app.rb +++ b/example/simple_app.rb @@ -14,9 +14,9 @@ class SimpleApp < BaseApp config.failure_app = BadAuthentication config.default_strategies :github config.scope_defaults :default, :config => GITHUB_CONFIG + config.serialize_from_session { |key| Warden::GitHub::Verifier.load(key) } + config.serialize_into_session { |user| Warden::GitHub::Verifier.dump(user) } end - Warden::Manager.serialize_from_session { |key| puts 'lol'; Warden::GitHub::Verifier.new.deserialize(key) } - Warden::Manager.serialize_into_session { |user| puts 'wut'; Warden::GitHub::Verifier.new.serialize(user) } get '/' do erb :index diff --git a/lib/warden/github.rb b/lib/warden/github.rb index 774363e..3ac8c3d 100644 --- a/lib/warden/github.rb +++ b/lib/warden/github.rb @@ -7,7 +7,6 @@ require 'warden/github/hook' require 'warden/github/config' require 'warden/github/membership_cache' -require 'warden/github/session_serializer' require 'warden/github/verifier' require 'active_support/message_verifier' diff --git a/lib/warden/github/session_serializer.rb b/lib/warden/github/session_serializer.rb deleted file mode 100644 index 42db4ba..0000000 --- a/lib/warden/github/session_serializer.rb +++ /dev/null @@ -1,32 +0,0 @@ -module Warden - class SessionSerializer - attr_reader :env - - def initialize(env) - @env = env - end - - def serialize(user) - cookie_verifier.generate(user) - end - - def deserialize(key) - cookie_verifier.verify(key) - rescue ::ActiveSupport::MessageVerifier::InvalidSignature - nil - end - - private - def verifier_key - self.class.verifier_key - end - - def self.verifier_key - @verifier_key ||= ENV['WARDEN_GITHUB_VERIFIER_SECRET'] || SecureRandom.hex - end - - def cookie_verifier - @cookie_verifier ||= ::ActiveSupport::MessageVerifier.new(verifier_key, serializer: Marshal) - end - end -end diff --git a/lib/warden/github/verifier.rb b/lib/warden/github/verifier.rb index 3e90916..330a951 100644 --- a/lib/warden/github/verifier.rb +++ b/lib/warden/github/verifier.rb @@ -1,6 +1,14 @@ module Warden module GitHub class Verifier + def self.dump(user) + new.serialize(user) + end + + def self.load(key) + new.deserialize(key) + end + def serialize(user) cookie_verifier.generate(user) end @@ -11,11 +19,11 @@ def deserialize(key) nil end - private def verifier_key self.class.verifier_key end + private def self.verifier_key @verifier_key ||= ENV['WARDEN_GITHUB_VERIFIER_SECRET'] || SecureRandom.hex end From ff7941c04d0982c7a4987d2436d11a2196305c15 Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Mon, 12 Jan 2015 13:29:23 -0800 Subject: [PATCH 15/46] use json marshaling instead of marshaling objects --- lib/warden/github/verifier.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/warden/github/verifier.rb b/lib/warden/github/verifier.rb index 330a951..d0ec95a 100644 --- a/lib/warden/github/verifier.rb +++ b/lib/warden/github/verifier.rb @@ -10,11 +10,13 @@ def self.load(key) end def serialize(user) - cookie_verifier.generate(user) + cookie_verifier.generate(user.marshal_dump) end def deserialize(key) - cookie_verifier.verify(key) + User.new.tap do |u| + u.marshal_load(cookie_verifier.verify(key)) + end rescue ::ActiveSupport::MessageVerifier::InvalidSignature nil end @@ -29,7 +31,7 @@ def self.verifier_key end def cookie_verifier - @cookie_verifier ||= ::ActiveSupport::MessageVerifier.new(verifier_key, serializer: Marshal) + @cookie_verifier ||= ::ActiveSupport::MessageVerifier.new(verifier_key, serializer: JSON) end end end From bd1596a63f228429e07f3124a4718d0bc2b1ace2 Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Mon, 12 Jan 2015 20:55:15 -0800 Subject: [PATCH 16/46] document defaults for securely storing api tokens --- README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/README.md b/README.md index 612c45e..d6b5e74 100644 --- a/README.md +++ b/README.md @@ -53,9 +53,14 @@ use Warden::Manager do |config| :client_secret => 'barfoo', :scope => 'user,repo', :redirect_uri => '/admin/oauth/callback' } + + config.serialize_from_session { |key| Warden::GitHub::Verifier.load(key) } + config.serialize_into_session { |user| Warden::GitHub::Verifier.dump(user) } end ``` +The two serialization methods store the API token in the session securely via the `WARDEN_GITHUB_VERIFIER_SECRET` environmental variable. + ### Parameters The config parameters and their defaults are listed below. From 601e171042c9aa8474a7fded32fd4ca04d80d50c Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Mon, 12 Jan 2015 20:56:34 -0800 Subject: [PATCH 17/46] version bump --- lib/warden/github/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/warden/github/version.rb b/lib/warden/github/version.rb index 2b15ca0..16d34f3 100644 --- a/lib/warden/github/version.rb +++ b/lib/warden/github/version.rb @@ -1,5 +1,5 @@ module Warden module GitHub - VERSION = "1.0.3" + VERSION = "1.1.0" end end From 70876d1a442e096531fea154be12973da9476480 Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Wed, 14 Jan 2015 16:01:28 -0800 Subject: [PATCH 18/46] explicitly require json for verifier marshaling --- lib/warden/github.rb | 1 + lib/warden/github/version.rb | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/warden/github.rb b/lib/warden/github.rb index 3ac8c3d..e2cc00b 100644 --- a/lib/warden/github.rb +++ b/lib/warden/github.rb @@ -1,3 +1,4 @@ +require 'json' require 'warden' require 'warden/github/user' diff --git a/lib/warden/github/version.rb b/lib/warden/github/version.rb index 16d34f3..4ef54c9 100644 --- a/lib/warden/github/version.rb +++ b/lib/warden/github/version.rb @@ -1,5 +1,5 @@ module Warden module GitHub - VERSION = "1.1.0" + VERSION = "1.1.1" end end From b617bec960a5a9a5d6fd5fda586c5834adbd0cab Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Fri, 13 Feb 2015 12:53:01 -0800 Subject: [PATCH 19/46] add basic single sign out support for github properties --- lib/warden/github.rb | 1 + lib/warden/github/strategy.rb | 6 ++++- lib/warden/github/user.rb | 43 +++++++++++++++++++++++++++---- spec/integration/oauth_spec.rb | 17 ++++++++++++ spec/spec_helper.rb | 6 +++++ spec/unit/user_spec.rb | 47 ++++++++++++++++++++++++++++++++++ 6 files changed, 114 insertions(+), 6 deletions(-) diff --git a/lib/warden/github.rb b/lib/warden/github.rb index e2cc00b..df309a8 100644 --- a/lib/warden/github.rb +++ b/lib/warden/github.rb @@ -1,5 +1,6 @@ require 'json' require 'warden' +require 'octokit' require 'warden/github/user' require 'warden/github/oauth' diff --git a/lib/warden/github/strategy.rb b/lib/warden/github/strategy.rb index 4b457fb..af5e1fb 100644 --- a/lib/warden/github/strategy.rb +++ b/lib/warden/github/strategy.rb @@ -70,6 +70,10 @@ def validate_flow! elsif (error = params['error']) && !error.empty? abort_flow!(error.gsub(/_/, ' ')) end + + if params['browser_session_id'] + custom_session['browser_session_id'] = params['browser_session_id'] + end end def custom_session @@ -77,7 +81,7 @@ def custom_session end def load_user - User.load(oauth.access_token) + User.load(oauth.access_token, custom_session['browser_session_id']) rescue OAuth::BadVerificationCode => e abort_flow!(e.message) end diff --git a/lib/warden/github/user.rb b/lib/warden/github/user.rb index 57d967e..438ea8e 100644 --- a/lib/warden/github/user.rb +++ b/lib/warden/github/user.rb @@ -1,11 +1,9 @@ -require 'octokit' - module Warden module GitHub - class User < Struct.new(:attribs, :token) + class User < Struct.new(:attribs, :token, :browser_session_id, :browser_session_verified_at) ATTRIBUTES = %w[id login name gravatar_id avatar_url email company site_admin].freeze - def self.load(access_token) + def self.load(access_token, browser_session_id = nil) api = Octokit::Client.new(:access_token => access_token) data = { } @@ -13,7 +11,7 @@ def self.load(access_token) data[k.to_s] = v if ATTRIBUTES.include?(k.to_s) end - new(data, access_token) + new(data, access_token, browser_session_id, Time.now.utc.to_i) end def marshal_dump @@ -71,6 +69,41 @@ def site_admin? !!site_admin end + # Identify if the browser session is still valid + # + # Returns: true if the browser session is still active or the GitHub API is unavailable + def browser_session_valid?(since = 120) + return true unless needs_browser_reverification?(since) + client = api + client.get("/user/sessions/active", :browser_session_id => browser_session_id) + self.browser_session_verified_at = Time.now.utc.to_i + client.last_response.status == 204 + rescue Octokit::ServerError # GitHub API unavailable + true + rescue Octokit::ClientError => e # GitHub API failed + false + end + + # Identify whether or not the browser has been reverified since a time + # + # since - The number of seconds since the last browser session verification + # + # Returns: true if the user is using single signout and needs to be + # reverified. false if it has been verified in a recent enough amount of + # time. + def needs_browser_reverification?(since = 120) + return false unless using_single_sign_out? + browser_session_verified_at && + (browser_session_verified_at <= (Time.now.utc.to_i - since)) + end + + # Identify if the user is on a GitHub SSO property + # + # Returns: true if a browser_session_id is present, false otherwise. + def using_single_sign_out? + !(browser_session_id.nil? || browser_session_id == "") + end + # Access the GitHub API from Octokit # # Octokit is a robust client library for the GitHub API diff --git a/spec/integration/oauth_spec.rb b/spec/integration/oauth_spec.rb index 8a38d9a..2ebc4bc 100644 --- a/spec/integration/oauth_spec.rb +++ b/spec/integration/oauth_spec.rb @@ -100,6 +100,23 @@ def redirect_uri(response) authenticated_uri.query.should eq 'foo=bar' end end + + context 'with GitHub SSO and code was exchanged for an access token' do + it 'redirects back to the original path' do + stub_code_for_token_exchange + stub_user_retrieval + + unauthenticated_response = get '/profile?foo=bar' + github_uri = redirect_uri(unauthenticated_response) + state = github_uri.query_values['state'] + + callback_response = get "/profile?code=#{code}&state=#{state}&browser_session_id=abcdefghijklmnop" + authenticated_uri = redirect_uri(callback_response) + + authenticated_uri.path.should eq '/profile' + authenticated_uri.query.should eq 'foo=bar' + end + end end context 'when not inside OAuth flow' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 2cbdf21..295a1b6 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -4,6 +4,7 @@ add_filter '/example' end +require 'pry' require 'warden/github' require File.expand_path('../../example/simple_app', __FILE__) require 'rack/test' @@ -16,4 +17,9 @@ def app Example.app end + + def stub_user_session_request + stub_request(:get, "https://api.github.com/user/sessions/active?browser_session_id=abcdefghijklmnop"). + with(:headers => {'Accept'=>'application/vnd.github.v3+json', 'Accept-Encoding'=>'gzip;q=1.0,deflate;q=0.6,identity;q=0.3', 'Authorization'=>'token the_token', 'Content-Type'=>'application/json', 'User-Agent'=>'Octokit Ruby Gem 3.7.0'}) + end end diff --git a/spec/unit/user_spec.rb b/spec/unit/user_spec.rb index 71afd07..c141468 100644 --- a/spec/unit/user_spec.rb +++ b/spec/unit/user_spec.rb @@ -15,6 +15,10 @@ described_class.new(default_attrs, token) end + let(:sso_user) do + described_class.new(default_attrs, token, "abcdefghijklmnop", Time.now.utc.to_i) + end + describe '#token' do it 'returns the token' do user.token.should eq token @@ -108,4 +112,47 @@ def stub_api(user, method, args, ret) it 'marshals correctly' do Marshal.load(Marshal.dump(user)).should eq user end + + describe 'single sign out' do + it "knows if the user is using single sign out" do + user.should_not be_using_single_sign_out + sso_user.should be_using_single_sign_out + end + + it "identifies when browsers need to be reverified" do + sso_user.should_not be_needs_browser_reverification + sso_user.browser_session_verified_at = Time.now.utc.to_i - 300 + sso_user.should be_needs_browser_reverification + end + + context "browser reverification" do + before do + sso_user.browser_session_verified_at = Time.now.utc.to_i - 300 + end + + it "handles success" do + sso_user.should be_needs_browser_reverification + stub_user_session_request.to_return(:status => 204, :body => "", :headers => {}) + sso_user.should be_browser_session_valid + end + + it "handles failure" do + sso_user.should be_needs_browser_reverification + stub_user_session_request.to_return(:status => 403, :body => "", :headers => {}) + sso_user.should_not be_browser_session_valid + end + + it "handles GitHub being unavailable" do + sso_user.should be_needs_browser_reverification + stub_user_session_request.to_raise(Octokit::ServerError.new) + sso_user.should be_browser_session_valid + end + + it "handles authentication failures" do + sso_user.should be_needs_browser_reverification + stub_user_session_request.to_return(:status => 403, :body => "", :headers => {}) + sso_user.should_not be_browser_session_valid + end + end + end end From 9c8dc8bb69645e0caf93d4e287445068c930f184 Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Fri, 13 Feb 2015 13:41:12 -0800 Subject: [PATCH 20/46] fixup sso examples in the simple app --- example/simple_app.rb | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/example/simple_app.rb b/example/simple_app.rb index 1a3ee4d..6de5108 100644 --- a/example/simple_app.rb +++ b/example/simple_app.rb @@ -18,16 +18,24 @@ class SimpleApp < BaseApp config.serialize_into_session { |user| Warden::GitHub::Verifier.dump(user) } end + def verify_browser_session + if env['warden'].user && !env['warden'].user.browser_session_valid?(10) + env['warden'].logout + end + end + get '/' do erb :index end get '/profile' do + verify_browser_session env['warden'].authenticate! erb :profile end get '/login' do + verify_browser_session env['warden'].authenticate! redirect '/' end @@ -86,4 +94,14 @@ def self.app
<%= env['warden'].user.team_member?(632) %>
GitHub Site Admin:
<%= env['warden'].user.site_admin? %>
+ <% if env['warden'].user.using_single_sign_out? %> +
GitHub Browser Session ID
+
<%= env['warden'].user.browser_session_id %>
+
GitHub Browser Session Valid
+
<%= env['warden'].user.browser_session_valid?(10) %>
+
GitHub Browser Session Verified At
+
<%= Time.at(env['warden'].user.browser_session_verified_at) %>
+
GitHub Browser Session Needs Reverification
+
<%= env['warden'].user.needs_browser_reverification?(10) %>
+ <% end %> From 398fec0759c7dcec345c49cc32f7a7d7630621be Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Fri, 13 Feb 2015 13:41:39 -0800 Subject: [PATCH 21/46] do a version bump for sso support --- lib/warden/github/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/warden/github/version.rb b/lib/warden/github/version.rb index 4ef54c9..f20c5c5 100644 --- a/lib/warden/github/version.rb +++ b/lib/warden/github/version.rb @@ -1,5 +1,5 @@ module Warden module GitHub - VERSION = "1.1.1" + VERSION = "1.2.0.pre1" end end From 3bba8e5c83e925765e32d94deffb8e84aaf385b4 Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Wed, 18 Feb 2015 10:54:17 -0800 Subject: [PATCH 22/46] fixup tests --- lib/warden/github.rb | 1 + spec/spec_helper.rb | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/warden/github.rb b/lib/warden/github.rb index df309a8..7a5b87f 100644 --- a/lib/warden/github.rb +++ b/lib/warden/github.rb @@ -2,6 +2,7 @@ require 'warden' require 'octokit' +require 'warden/github/sso' require 'warden/github/user' require 'warden/github/oauth' require 'warden/github/version' diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 295a1b6..e812ad9 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -4,7 +4,6 @@ add_filter '/example' end -require 'pry' require 'warden/github' require File.expand_path('../../example/simple_app', __FILE__) require 'rack/test' @@ -20,6 +19,6 @@ def app def stub_user_session_request stub_request(:get, "https://api.github.com/user/sessions/active?browser_session_id=abcdefghijklmnop"). - with(:headers => {'Accept'=>'application/vnd.github.v3+json', 'Accept-Encoding'=>'gzip;q=1.0,deflate;q=0.6,identity;q=0.3', 'Authorization'=>'token the_token', 'Content-Type'=>'application/json', 'User-Agent'=>'Octokit Ruby Gem 3.7.0'}) + with(:headers => {'Accept'=>'application/vnd.github.v3+json', 'Accept-Encoding'=>'gzip;q=1.0,deflate;q=0.6,identity;q=0.3', 'Authorization'=>'token the_token', 'Content-Type'=>'application/json', 'User-Agent'=>"Octokit Ruby Gem #{Octokit::VERSION}"}) end end From 69cd3bfda976ec27d08702ee01f47c6f8157eb3f Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Wed, 18 Feb 2015 12:51:00 -0800 Subject: [PATCH 23/46] move browser verified at out of the user model --- lib/warden/github/user.rb | 20 +++----------------- spec/unit/user_spec.rb | 18 ++---------------- 2 files changed, 5 insertions(+), 33 deletions(-) diff --git a/lib/warden/github/user.rb b/lib/warden/github/user.rb index 438ea8e..b5e517c 100644 --- a/lib/warden/github/user.rb +++ b/lib/warden/github/user.rb @@ -1,6 +1,6 @@ module Warden module GitHub - class User < Struct.new(:attribs, :token, :browser_session_id, :browser_session_verified_at) + class User < Struct.new(:attribs, :token, :browser_session_id) ATTRIBUTES = %w[id login name gravatar_id avatar_url email company site_admin].freeze def self.load(access_token, browser_session_id = nil) @@ -11,7 +11,7 @@ def self.load(access_token, browser_session_id = nil) data[k.to_s] = v if ATTRIBUTES.include?(k.to_s) end - new(data, access_token, browser_session_id, Time.now.utc.to_i) + new(data, access_token, browser_session_id) end def marshal_dump @@ -73,10 +73,9 @@ def site_admin? # # Returns: true if the browser session is still active or the GitHub API is unavailable def browser_session_valid?(since = 120) - return true unless needs_browser_reverification?(since) + return true unless using_single_sign_out? client = api client.get("/user/sessions/active", :browser_session_id => browser_session_id) - self.browser_session_verified_at = Time.now.utc.to_i client.last_response.status == 204 rescue Octokit::ServerError # GitHub API unavailable true @@ -84,19 +83,6 @@ def browser_session_valid?(since = 120) false end - # Identify whether or not the browser has been reverified since a time - # - # since - The number of seconds since the last browser session verification - # - # Returns: true if the user is using single signout and needs to be - # reverified. false if it has been verified in a recent enough amount of - # time. - def needs_browser_reverification?(since = 120) - return false unless using_single_sign_out? - browser_session_verified_at && - (browser_session_verified_at <= (Time.now.utc.to_i - since)) - end - # Identify if the user is on a GitHub SSO property # # Returns: true if a browser_session_id is present, false otherwise. diff --git a/spec/unit/user_spec.rb b/spec/unit/user_spec.rb index c141468..8a3b371 100644 --- a/spec/unit/user_spec.rb +++ b/spec/unit/user_spec.rb @@ -16,7 +16,7 @@ end let(:sso_user) do - described_class.new(default_attrs, token, "abcdefghijklmnop", Time.now.utc.to_i) + described_class.new(default_attrs, token, "abcdefghijklmnop") end describe '#token' do @@ -119,37 +119,23 @@ def stub_api(user, method, args, ret) sso_user.should be_using_single_sign_out end - it "identifies when browsers need to be reverified" do - sso_user.should_not be_needs_browser_reverification - sso_user.browser_session_verified_at = Time.now.utc.to_i - 300 - sso_user.should be_needs_browser_reverification - end - context "browser reverification" do - before do - sso_user.browser_session_verified_at = Time.now.utc.to_i - 300 - end - it "handles success" do - sso_user.should be_needs_browser_reverification stub_user_session_request.to_return(:status => 204, :body => "", :headers => {}) sso_user.should be_browser_session_valid end it "handles failure" do - sso_user.should be_needs_browser_reverification - stub_user_session_request.to_return(:status => 403, :body => "", :headers => {}) + stub_user_session_request.to_return(:status => 404, :body => "", :headers => {}) sso_user.should_not be_browser_session_valid end it "handles GitHub being unavailable" do - sso_user.should be_needs_browser_reverification stub_user_session_request.to_raise(Octokit::ServerError.new) sso_user.should be_browser_session_valid end it "handles authentication failures" do - sso_user.should be_needs_browser_reverification stub_user_session_request.to_return(:status => 403, :body => "", :headers => {}) sso_user.should_not be_browser_session_valid end From 96d598d4ff64e84aa1b6a833eae592329b7327d5 Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Wed, 18 Feb 2015 13:12:35 -0800 Subject: [PATCH 24/46] make an easily included module for sso support --- lib/warden/github/sso.rb | 30 ++++++++++++++++++++++++++++ spec/unit/sso_spec.rb | 42 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 lib/warden/github/sso.rb create mode 100644 spec/unit/sso_spec.rb diff --git a/lib/warden/github/sso.rb b/lib/warden/github/sso.rb new file mode 100644 index 0000000..23d0635 --- /dev/null +++ b/lib/warden/github/sso.rb @@ -0,0 +1,30 @@ +module Warden + module GitHub + module SSO + def warden_github_sso_session_valid?(user, expiry_in_seconds = 30) + return true if defined?(::Rails) && ::Rails.env.test? + if warden_github_sso_session_needs_reverification?(user, expiry_in_seconds) + if user.browser_session_valid?(expiry_in_seconds) + warden_github_sso_session_reverify! + return true + end + return false + end + true + end + + def warden_github_sso_session_verified_at + session[:warden_github_sso_session_verified_at] || Time.now.utc.to_i - 86400 + end + + def warden_github_sso_session_reverify! + session[:warden_github_sso_session_verified_at] = Time.now.utc.to_i + end + + def warden_github_sso_session_needs_reverification?(user, expiry_in_seconds) + user.using_single_sign_out? && + (warden_github_sso_session_verified_at <= (Time.now.utc.to_i - expiry_in_seconds)) + end + end + end +end diff --git a/spec/unit/sso_spec.rb b/spec/unit/sso_spec.rb new file mode 100644 index 0000000..157ff08 --- /dev/null +++ b/spec/unit/sso_spec.rb @@ -0,0 +1,42 @@ +require 'spec_helper' + +class FakeController + def session + @session ||= {} + end + include Warden::GitHub::SSO +end + +describe Warden::GitHub::SSO do + let(:default_attrs) do + { 'login' => 'john', + 'name' => 'John Doe', + 'gravatar_id' => '38581cb351a52002548f40f8066cfecg', + 'avatar_url' => 'http://example.com/avatar.jpg', + 'email' => 'john@doe.com', + 'company' => 'Doe, Inc.' } + end + + let(:user) do + Warden::GitHub::User.new(default_attrs, "the_token", "abcdefghijklmnop") + end + + subject do + FakeController.new + end + + describe "whatever" do + it "identifies when browsers need to be reverified" do + subject.session[:warden_github_sso_session_verified_at] = Time.now.utc.to_i - 10 + subject.should be_warden_github_sso_session_valid(user) + + subject.session[:warden_github_sso_session_verified_at] = Time.now.utc.to_i - 300 + stub_user_session_request.to_return(:status => 204, :body => "", :headers => {}) + subject.should be_warden_github_sso_session_valid(user) + + subject.session[:warden_github_sso_session_verified_at] = Time.now.utc.to_i - 300 + stub_user_session_request.to_return(:status => 404, :body => "", :headers => {}) + subject.should_not be_warden_github_sso_session_valid(user) + end + end +end From 0fb6eb6957a7e800bb4686c809d43310bc8a9b6e Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Wed, 18 Feb 2015 13:12:53 -0800 Subject: [PATCH 25/46] another pre-release --- lib/warden/github/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/warden/github/version.rb b/lib/warden/github/version.rb index f20c5c5..898e78e 100644 --- a/lib/warden/github/version.rb +++ b/lib/warden/github/version.rb @@ -1,5 +1,5 @@ module Warden module GitHub - VERSION = "1.2.0.pre1" + VERSION = "1.2.0.pre2" end end From c48ffdfe1e005317fa4782d5ba3c9eb660323ab5 Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Wed, 18 Feb 2015 13:22:30 -0800 Subject: [PATCH 26/46] use the helper methods --- example/simple_app.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/example/simple_app.rb b/example/simple_app.rb index 6de5108..2efda13 100644 --- a/example/simple_app.rb +++ b/example/simple_app.rb @@ -2,6 +2,8 @@ module Example class SimpleApp < BaseApp + include Warden::GitHub::SSO + enable :inline_templates GITHUB_CONFIG = { @@ -98,10 +100,8 @@ def self.app
GitHub Browser Session ID
<%= env['warden'].user.browser_session_id %>
GitHub Browser Session Valid
-
<%= env['warden'].user.browser_session_valid?(10) %>
+
<%= warden_github_sso_session_valid?(env['warden'].user, 10) %>
GitHub Browser Session Verified At
-
<%= Time.at(env['warden'].user.browser_session_verified_at) %>
-
GitHub Browser Session Needs Reverification
-
<%= env['warden'].user.needs_browser_reverification?(10) %>
+
<%= Time.at(session[:warden_github_sso_session_verified_at]) %>
<% end %> From 6ca97cda259f13949da6daac6b38397d84724f88 Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Wed, 18 Feb 2015 13:35:09 -0800 Subject: [PATCH 27/46] use the helper methods --- example/simple_app.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/example/simple_app.rb b/example/simple_app.rb index 2efda13..f46414e 100644 --- a/example/simple_app.rb +++ b/example/simple_app.rb @@ -21,7 +21,7 @@ class SimpleApp < BaseApp end def verify_browser_session - if env['warden'].user && !env['warden'].user.browser_session_valid?(10) + if env['warden'].user && !warden_github_sso_session_valid?(env['warden'].user, 10) env['warden'].logout end end From 77f74ffbfc08e39e5a840832ab05a23231acbbcd Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Wed, 18 Feb 2015 13:59:52 -0800 Subject: [PATCH 28/46] cut a 1.2.0 release --- CHANGELOG.md | 4 ++++ lib/warden/github/version.rb | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d9ccdf..6979168 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +v1.2.0 2015/02/18 + +* Implement single sign out for cookie sessions of GitHub properties + v1.0.3 2014/12/13 * Reintroduce membership caching to reduce API hits for validating team membership. diff --git a/lib/warden/github/version.rb b/lib/warden/github/version.rb index 898e78e..b49a6d9 100644 --- a/lib/warden/github/version.rb +++ b/lib/warden/github/version.rb @@ -1,5 +1,5 @@ module Warden module GitHub - VERSION = "1.2.0.pre2" + VERSION = "1.2.0" end end From c786e48569e507763235785b48ca2ee879a375a2 Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Wed, 18 Feb 2015 14:05:24 -0800 Subject: [PATCH 29/46] provide docs for the sso session valid stuff --- README.md | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/README.md b/README.md index d6b5e74..c700812 100644 --- a/README.md +++ b/README.md @@ -143,6 +143,38 @@ If you're looking for an easy way to integrate this into a Sinatra or Rails appl - [sinatra_auth_github](https://github.com/atmos/sinatra_auth_github) - [warden-github-rails](https://github.com/fphilipe/warden-github-rails) +## Single Sign Out + +OAuth applications owned by the GitHub organization are sent an extra browser parameter to ensure that the user remains logged in to github.com. Taking advantage of this is provided by a small module you include into your controller and a before filter. Your `ApplicationController` should resemble something like this. + + +```ruby +class ApplicationController < ActionController::Base + include Warden::GitHub::SSO + + protect_from_forgery with: :exception + + before_filter :verify_logged_in_user + + helper_method :current_user + + private + + def verify_logged_in_user + unless current_user && warden_github_sso_session_valid?(current_user, 30) + request.env['warden'].logout + request.env['warden'].authenticate! + end + end + + def current_user + github_user + end +end +``` + +You can also see single sign out in action in the example app. + ## Additional Information - [warden](https://github.com/hassox/warden) From f53ab667522bcd5bde4fbdf9871e45cd43a75d35 Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Wed, 18 Feb 2015 14:08:04 -0800 Subject: [PATCH 30/46] use the helper method instead of accessing the session directly --- example/simple_app.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/example/simple_app.rb b/example/simple_app.rb index f46414e..4dae3f5 100644 --- a/example/simple_app.rb +++ b/example/simple_app.rb @@ -102,6 +102,6 @@ def self.app
GitHub Browser Session Valid
<%= warden_github_sso_session_valid?(env['warden'].user, 10) %>
GitHub Browser Session Verified At
-
<%= Time.at(session[:warden_github_sso_session_verified_at]) %>
+
<%= Time.at(warden_github_sso_session_verified_at) %>
<% end %> From e7a9fa2f6ea6991765f6c847fb5bef4b56a8a080 Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Wed, 18 Feb 2015 14:09:33 -0800 Subject: [PATCH 31/46] simplify the readme --- README.md | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/README.md b/README.md index c700812..0c83c1d 100644 --- a/README.md +++ b/README.md @@ -156,20 +156,14 @@ class ApplicationController < ActionController::Base before_filter :verify_logged_in_user - helper_method :current_user - private def verify_logged_in_user - unless current_user && warden_github_sso_session_valid?(current_user, 30) + unless github_user && warden_github_sso_session_valid?(github_user, 120) request.env['warden'].logout request.env['warden'].authenticate! end end - - def current_user - github_user - end end ``` From a97811e3d71f562dea24e282293854ec2f472d8c Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Wed, 18 Feb 2015 14:11:28 -0800 Subject: [PATCH 32/46] use a meaningful name for the test description --- spec/unit/sso_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/unit/sso_spec.rb b/spec/unit/sso_spec.rb index 157ff08..3ad506b 100644 --- a/spec/unit/sso_spec.rb +++ b/spec/unit/sso_spec.rb @@ -25,7 +25,7 @@ def session FakeController.new end - describe "whatever" do + describe "warden_github_sso_session_valid?" do it "identifies when browsers need to be reverified" do subject.session[:warden_github_sso_session_verified_at] = Time.now.utc.to_i - 10 subject.should be_warden_github_sso_session_valid(user) From c6177776ace8898f098b2ced032090110c8864d8 Mon Sep 17 00:00:00 2001 From: Philipe Fatio Date: Thu, 14 Apr 2016 13:37:51 +0200 Subject: [PATCH 33/46] Improve JSON serialization compatibility of User Previously, the MembershipCache was a subclass of Hash. When serializing to and deserializing from JSON, the MembershipCache would become a Hash and blow up. This is a problem in warden-github-rails where I switched to a custom warden (de)serialization in order to properly support JSON serialization and also to support mocked users in test environments between requests[1]. [1]: https://github.com/fphilipe/warden-github-rails/commit/db82470e6fa66b0b546cfef501abcd04f91529ca --- lib/warden/github/membership_cache.rb | 12 ++++--- lib/warden/github/user.rb | 13 +++---- spec/unit/membership_cache_spec.rb | 51 +++++++++++++++++++-------- 3 files changed, 51 insertions(+), 25 deletions(-) diff --git a/lib/warden/github/membership_cache.rb b/lib/warden/github/membership_cache.rb index 91add5f..89bc3e9 100644 --- a/lib/warden/github/membership_cache.rb +++ b/lib/warden/github/membership_cache.rb @@ -3,9 +3,13 @@ module GitHub # A hash subclass that acts as a cache for organization and team # membership states. Only membership states that are true are cached. These # are invalidated after a certain time. - class MembershipCache < ::Hash + class MembershipCache CACHE_TIMEOUT = 60 * 5 + def initialize(data) + @data = data + end + # Fetches a membership status by type and id (e.g. 'org', 'my_company') # from cache. If no cached value is present or if the cached value # expired, the block will be invoked and the return value, if true, @@ -27,10 +31,10 @@ def fetch_membership(type, id) private def cached_membership_valid?(type, id) - timestamp = fetch(type).fetch(id) + timestamp = @data.fetch(type).fetch(id) if Time.now.to_i > timestamp + CACHE_TIMEOUT - fetch(type).delete(id) + @data.fetch(type).delete(id) false else true @@ -40,7 +44,7 @@ def cached_membership_valid?(type, id) end def cache_membership(type, id) - hash = self[type] ||= {} + hash = @data[type] ||= {} hash[id] = Time.now.to_i end end diff --git a/lib/warden/github/user.rb b/lib/warden/github/user.rb index b5e517c..5844cc0 100644 --- a/lib/warden/github/user.rb +++ b/lib/warden/github/user.rb @@ -1,6 +1,6 @@ module Warden module GitHub - class User < Struct.new(:attribs, :token, :browser_session_id) + class User < Struct.new(:attribs, :token, :browser_session_id, :memberships) ATTRIBUTES = %w[id login name gravatar_id avatar_url email company site_admin].freeze def self.load(access_token, browser_session_id = nil) @@ -32,7 +32,7 @@ def marshal_load(hash) # # Returns: true if the user is publicized as an org member def organization_public_member?(org_name) - memberships.fetch_membership(:org_pub, org_name) do + membership_cache.fetch_membership(:org_pub, org_name) do api.organization_public_member?(org_name, login) end end @@ -46,7 +46,7 @@ def organization_public_member?(org_name) # # Returns: true if the user has access, false otherwise def organization_member?(org_name) - memberships.fetch_membership(:org, org_name) do + membership_cache.fetch_membership(:org, org_name) do api.organization_member?(org_name, login) end end @@ -57,7 +57,7 @@ def organization_member?(org_name) # # Returns: true if the user has access, false otherwise def team_member?(team_id) - memberships.fetch_membership(:team, team_id) do + membership_cache.fetch_membership(:team, team_id) do api.team_member?(team_id, login) end end @@ -105,8 +105,9 @@ def api private - def memberships - attribs['member'] ||= MembershipCache.new + def membership_cache + self.memberships ||= {} + @membership_cache ||= MembershipCache.new(memberships) end end end diff --git a/spec/unit/membership_cache_spec.rb b/spec/unit/membership_cache_spec.rb index 97120d3..80a695b 100644 --- a/spec/unit/membership_cache_spec.rb +++ b/spec/unit/membership_cache_spec.rb @@ -1,19 +1,17 @@ require 'spec_helper' describe Warden::GitHub::MembershipCache do - subject(:cache) do - described_class.new - end - describe '#fetch_membership' do it 'returns false by default' do + cache = described_class.new({}) cache.fetch_membership('foo', 'bar').should be_falsey end context 'when cache valid' do - before do - cache['foo'] = {} - cache['foo']['bar'] = Time.now.to_i - described_class::CACHE_TIMEOUT + 5 + let(:cache) do + described_class.new('foo' => { + 'bar' => Time.now.to_i - described_class::CACHE_TIMEOUT + 5 + }) end it 'returns true' do @@ -27,9 +25,10 @@ end context 'when cache expired' do - before do - cache['foo'] = {} - cache['foo']['bar'] = Time.now.to_i - described_class::CACHE_TIMEOUT - 5 + let(:cache) do + described_class.new('foo' => { + 'bar' => Time.now.to_i - described_class::CACHE_TIMEOUT - 5 + }) end context 'when no block given' do @@ -38,11 +37,6 @@ end end - it 'deletes the cached value' do - cache.fetch_membership('foo', 'bar') - cache['foo'].should_not have_key('bar') - end - it 'invokes the block' do expect { |b| cache.fetch_membership('foo', 'bar', &b) }. to yield_control @@ -50,13 +44,40 @@ end it 'caches the value when block returns true' do + cache = described_class.new({}) cache.fetch_membership('foo', 'bar') { true } cache.fetch_membership('foo', 'bar').should be_truthy end it 'does not cache the value when block returns false' do + cache = described_class.new({}) cache.fetch_membership('foo', 'bar') { false } cache.fetch_membership('foo', 'bar').should be_falsey end end + + it 'uses the hash that is passed to the initializer as storage' do + time = Time.now.to_i + hash = { + 'foo' => { + 'valid' => time, + 'timedout' => time - described_class::CACHE_TIMEOUT - 5 + } + } + cache = described_class.new(hash) + + # Verify that existing data in the hash is used: + expect(cache.fetch_membership('foo', 'valid')).to be_true + expect(cache.fetch_membership('foo', 'timedout')).to be_false + + # Add new data to the hash: + cache.fetch_membership('foo', 'new') { true } + cache.fetch_membership('foo', 'false') { false } + + # Verify the final hash state: + expect(hash).to eq('foo' => { + 'valid' => time, + 'new' => time + }) + end end From bee9e2a36971fc92494e188e4b8f96dcc4e4f92e Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Tue, 19 Apr 2016 23:51:18 -0700 Subject: [PATCH 34/46] update ruby versions we care about --- .travis.yml | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/.travis.yml b/.travis.yml index 28a0ca8..05fd001 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,10 +1,7 @@ language: ruby rvm: - 1.8.7 - - 1.9.2 - 1.9.3 - - jruby-18mode # JRuby in 1.8 mode - - jruby-19mode # JRuby in 1.9 mode - - rbx-18mode - - ree + - 2.2.0 + - 2.3.0 - ruby-head From a2e4b6a0726d3eb941aab9b5e0e68126ab6b5669 Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Tue, 19 Apr 2016 23:52:46 -0700 Subject: [PATCH 35/46] can't install on 1.8.7, don't care --- .travis.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 05fd001..9197bac 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,5 @@ language: ruby rvm: - - 1.8.7 - 1.9.3 - 2.2.0 - 2.3.0 From 8082f751bf8e3559947484c0c9b845e15a659bb5 Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Wed, 20 Apr 2016 00:01:27 -0700 Subject: [PATCH 36/46] remove ruby versions that no longer work on travis --- .travis.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 9197bac..c350511 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,4 @@ language: ruby rvm: - - 1.9.3 - - 2.2.0 - 2.3.0 - ruby-head From c45060654b47455daee6c422f167c925a71a0e22 Mon Sep 17 00:00:00 2001 From: Corey Donohoe Date: Wed, 20 Apr 2016 00:04:06 -0700 Subject: [PATCH 37/46] cut 1.3.0 --- lib/warden/github/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/warden/github/version.rb b/lib/warden/github/version.rb index b49a6d9..75d9c59 100644 --- a/lib/warden/github/version.rb +++ b/lib/warden/github/version.rb @@ -1,5 +1,5 @@ module Warden module GitHub - VERSION = "1.2.0" + VERSION = "1.3.0" end end From b2ebac2578dd380fb9ecdadc930b9d528bcb11cc Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Fri, 15 Jul 2016 20:45:49 -0600 Subject: [PATCH 38/46] Always use strings This fixes a really difficult issue to track down, particularly when the user specifies a GitHub team as an integer instead of a string. The resulting API call includes a string, but the cache will be an integer. As such, it will always force a cache lookup. --- lib/warden/github/membership_cache.rb | 2 +- spec/unit/membership_cache_spec.rb | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/warden/github/membership_cache.rb b/lib/warden/github/membership_cache.rb index 89bc3e9..b8f5310 100644 --- a/lib/warden/github/membership_cache.rb +++ b/lib/warden/github/membership_cache.rb @@ -16,7 +16,7 @@ def initialize(data) # cached for e certain time. def fetch_membership(type, id) type = type.to_s - id = id.to_s if id.is_a?(Symbol) + id = id.to_s if cached_membership_valid?(type, id) true diff --git a/spec/unit/membership_cache_spec.rb b/spec/unit/membership_cache_spec.rb index 80a695b..e14a065 100644 --- a/spec/unit/membership_cache_spec.rb +++ b/spec/unit/membership_cache_spec.rb @@ -22,6 +22,10 @@ expect { |b| cache.fetch_membership('foo', 'bar', &b) }. to_not yield_control end + + it 'converts type and id to strings' do + cache.fetch_membership(:foo, :bar).should be_truthy + end end context 'when cache expired' do From d1cd1273330e104e4ec28e28bf40c2cd2311b687 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Fri, 15 Jul 2016 20:49:53 -0600 Subject: [PATCH 39/46] Bump version --- lib/warden/github/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/warden/github/version.rb b/lib/warden/github/version.rb index 75d9c59..9543c8e 100644 --- a/lib/warden/github/version.rb +++ b/lib/warden/github/version.rb @@ -1,5 +1,5 @@ module Warden module GitHub - VERSION = "1.3.0" + VERSION = "1.3.1" end end From 65aae456677fc7b3eb336278cc8e251b433c0cd0 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Fri, 15 Jul 2016 20:54:49 -0600 Subject: [PATCH 40/46] Yank ruby-head --- .travis.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index c350511..cc13928 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,4 +1,3 @@ language: ruby rvm: - 2.3.0 - - ruby-head From 63d3549d8bc173d036050152b9354ce4910a6a27 Mon Sep 17 00:00:00 2001 From: Philipe Fatio Date: Sat, 13 May 2017 20:12:23 +0200 Subject: [PATCH 41/46] Persist membership cache between requests The membership cache was being serialized into the user object. It was assumed that the user object was serialized at the end of each request. It turns out that this is not the case. Instead, the user object only gets serialized once, namely after setting the user. This means that the membership cache wasn't working at all between requests. This commit fixes it by not including the memberships in the serialized user as there's no point in doing so. Instead, after the user object is set up, the memberships hash get retrieved from the user session (warden provides a session object for each scope; if there is none, a new hash is stored in the session) and assigned to the user object. The hash is then used as before and we get the session serialization from rack for free. Fixes #52. --- lib/warden/github/hook.rb | 7 +++++++ lib/warden/github/user.rb | 4 +++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/warden/github/hook.rb b/lib/warden/github/hook.rb index f72bba7..c006c36 100644 --- a/lib/warden/github/hook.rb +++ b/lib/warden/github/hook.rb @@ -4,3 +4,10 @@ strategy.finalize_flow! if strategy.class == Warden::GitHub::Strategy end + +Warden::Manager.after_set_user do |user, auth, opts| + if user.is_a?(Warden::GitHub::User) + session = auth.session(opts.fetch(:scope)) + user.memberships = session[:_memberships] ||= {} + end +end diff --git a/lib/warden/github/user.rb b/lib/warden/github/user.rb index 5844cc0..cd699a9 100644 --- a/lib/warden/github/user.rb +++ b/lib/warden/github/user.rb @@ -1,8 +1,10 @@ module Warden module GitHub - class User < Struct.new(:attribs, :token, :browser_session_id, :memberships) + class User < Struct.new(:attribs, :token, :browser_session_id) ATTRIBUTES = %w[id login name gravatar_id avatar_url email company site_admin].freeze + attr_accessor :memberships + def self.load(access_token, browser_session_id = nil) api = Octokit::Client.new(:access_token => access_token) data = { } From 242b75bcbcc52b46e508783cecaa475f04a9dbd9 Mon Sep 17 00:00:00 2001 From: Philipe Fatio Date: Sat, 13 May 2017 21:47:10 +0200 Subject: [PATCH 42/46] Upgrade to rspec 3.6 This conversion is done by Transpec 3.3.0 with the following command: transpec * 58 conversions from: obj.should to: expect(obj).to * 9 conversions from: obj.should_not to: expect(obj).not_to * 8 conversions from: obj.stub(:message => value) to: allow(obj).to receive_messages(:message => value) * 4 conversions from: obj.stub(:message) to: allow(obj).to receive(:message) * 3 conversions from: =~ /pattern/ to: match(/pattern/) * 3 conversions from: obj.should_receive(:message) to: expect(obj).to receive(:message) * 1 conversion from: =~ [1, 2] to: match_array([1, 2]) For more details: https://github.com/yujinakayama/transpec#supported-conversions --- spec/integration/oauth_spec.rb | 38 ++++++++++----------- spec/unit/config_spec.rb | 52 ++++++++++++++-------------- spec/unit/membership_cache_spec.rb | 16 ++++----- spec/unit/oauth_spec.rb | 8 ++--- spec/unit/sso_spec.rb | 6 ++-- spec/unit/user_spec.rb | 54 +++++++++++++++--------------- warden-github.gemspec | 2 +- 7 files changed, 88 insertions(+), 88 deletions(-) diff --git a/spec/integration/oauth_spec.rb b/spec/integration/oauth_spec.rb index 2ebc4bc..f897dc0 100644 --- a/spec/integration/oauth_spec.rb +++ b/spec/integration/oauth_spec.rb @@ -27,12 +27,12 @@ def redirect_uri(response) unauthenticated_response = get '/profile' github_uri = redirect_uri(unauthenticated_response) - github_uri.scheme.should eq 'https' - github_uri.host.should eq 'github.com' - github_uri.path.should eq '/login/oauth/authorize' - github_uri.query_values['client_id'].should =~ /\w+/ - github_uri.query_values['state'].should =~ /\w+/ - github_uri.query_values['redirect_uri'].should =~ /^http.*\/profile$/ + expect(github_uri.scheme).to eq 'https' + expect(github_uri.host).to eq 'github.com' + expect(github_uri.path).to eq '/login/oauth/authorize' + expect(github_uri.query_values['client_id']).to match(/\w+/) + expect(github_uri.query_values['state']).to match(/\w+/) + expect(github_uri.query_values['redirect_uri']).to match(/^http.*\/profile$/) end end @@ -53,8 +53,8 @@ def redirect_uri(response) get '/login' response = get "/login?code=#{code}&state=foobar" - response.should_not be_successful - response.body.should include 'State mismatch' + expect(response).not_to be_successful + expect(response.body).to include 'State mismatch' end end @@ -67,8 +67,8 @@ def redirect_uri(response) state = github_uri.query_values['state'] response = get "/login?code=#{code}&state=#{state}" - response.should_not be_successful - response.body.should include 'Bad verification code' + expect(response).not_to be_successful + expect(response.body).to include 'Bad verification code' end end @@ -79,8 +79,8 @@ def redirect_uri(response) state = github_uri.query_values['state'] response = get "/login?error=access_denied&state=#{state}" - response.should_not be_successful - response.body.should include 'access denied' + expect(response).not_to be_successful + expect(response.body).to include 'access denied' end end @@ -96,8 +96,8 @@ def redirect_uri(response) callback_response = get "/profile?code=#{code}&state=#{state}" authenticated_uri = redirect_uri(callback_response) - authenticated_uri.path.should eq '/profile' - authenticated_uri.query.should eq 'foo=bar' + expect(authenticated_uri.path).to eq '/profile' + expect(authenticated_uri.query).to eq 'foo=bar' end end @@ -113,8 +113,8 @@ def redirect_uri(response) callback_response = get "/profile?code=#{code}&state=#{state}&browser_session_id=abcdefghijklmnop" authenticated_uri = redirect_uri(callback_response) - authenticated_uri.path.should eq '/profile' - authenticated_uri.query.should eq 'foo=bar' + expect(authenticated_uri.path).to eq '/profile' + expect(authenticated_uri.query).to eq 'foo=bar' end end end @@ -123,8 +123,8 @@ def redirect_uri(response) it 'does not recognize a seeming callback url as an actual callback' do response = get '/profile?state=foo&code=bar' - a_request(:post, 'https://github.com/login/oauth/access_token'). - should have_not_been_made + expect(a_request(:post, 'https://github.com/login/oauth/access_token')). + to have_not_been_made end end @@ -142,7 +142,7 @@ def redirect_uri(response) get authenticated_uri.path logged_in_response = get '/login' - redirect_uri(logged_in_response).path.should eq '/' + expect(redirect_uri(logged_in_response).path).to eq '/' end end end diff --git a/spec/unit/config_spec.rb b/spec/unit/config_spec.rb index 2d90a7b..8f17dd1 100644 --- a/spec/unit/config_spec.rb +++ b/spec/unit/config_spec.rb @@ -24,7 +24,7 @@ end before do - config.stub(:request => request) + allow(config).to receive_messages(:request => request) end def silence_warnings @@ -38,7 +38,7 @@ def silence_warnings context 'when specified in scope config' do it 'returns the client id' do scope_config[:client_id] = 'foobar' - config.client_id.should eq 'foobar' + expect(config.client_id).to eq 'foobar' end end @@ -46,15 +46,15 @@ def silence_warnings it 'returns the client id' do warden_config[:github_client_id] = 'foobar' silence_warnings do - config.client_id.should eq 'foobar' + expect(config.client_id).to eq 'foobar' end end end context 'when specified in ENV' do it 'returns the client id' do - ENV.stub(:[]).with('GITHUB_CLIENT_ID').and_return('foobar') - config.client_id.should eq 'foobar' + allow(ENV).to receive(:[]).with('GITHUB_CLIENT_ID').and_return('foobar') + expect(config.client_id).to eq 'foobar' end end @@ -69,7 +69,7 @@ def silence_warnings context 'when specified in scope config' do it 'returns the client secret' do scope_config[:client_secret] = 'foobar' - config.client_secret.should eq 'foobar' + expect(config.client_secret).to eq 'foobar' end end @@ -77,16 +77,16 @@ def silence_warnings it 'returns the client secret' do warden_config[:github_secret] = 'foobar' silence_warnings do - config.client_secret.should eq 'foobar' + expect(config.client_secret).to eq 'foobar' end end end context 'when specified in ENV' do it 'returns the client secret' do - ENV.stub(:[]).with('GITHUB_CLIENT_SECRET').and_return('foobar') + allow(ENV).to receive(:[]).with('GITHUB_CLIENT_SECRET').and_return('foobar') silence_warnings do - config.client_secret.should eq 'foobar' + expect(config.client_secret).to eq 'foobar' end end end @@ -102,14 +102,14 @@ def silence_warnings context 'when specified in scope config' do it 'returns the expanded redirect uri' do scope_config[:redirect_uri] = '/callback' - config.redirect_uri.should eq 'http://example.com/callback' + expect(config.redirect_uri).to eq 'http://example.com/callback' end end context 'when specified path lacks leading slash' do it 'corrects the path and returns the expanded uri' do scope_config[:redirect_uri] = 'callback' - config.redirect_uri.should eq 'http://example.com/callback' + expect(config.redirect_uri).to eq 'http://example.com/callback' end end @@ -117,40 +117,40 @@ def silence_warnings it 'returns the expanded redirect uri' do warden_config[:github_callback_url] = '/callback' silence_warnings do - config.redirect_uri.should eq 'http://example.com/callback' + expect(config.redirect_uri).to eq 'http://example.com/callback' end end end context 'when not specified' do it 'returns the expanded redirect uri with the current path' do - config.redirect_uri.should eq 'http://example.com/the/path' + expect(config.redirect_uri).to eq 'http://example.com/the/path' end end context 'when HTTP_X_FORWARDED_PROTO is set to https' do it 'returns the expanded redirect uri(with port) with adjusted scheme' do env['HTTP_X_FORWARDED_PROTO'] = 'https' - request.stub(:url => 'http://example.com:443/the/path') - config.redirect_uri.should eq 'https://example.com/the/path' + allow(request).to receive_messages(:url => 'http://example.com:443/the/path') + expect(config.redirect_uri).to eq 'https://example.com/the/path' end it 'returns the expanded redirect uri with adjusted scheme including port 80' do env['HTTP_X_FORWARDED_PROTO'] = 'https' - request.stub(:url => 'http://example.com:80/the/path') - config.redirect_uri.should eq 'https://example.com/the/path' + allow(request).to receive_messages(:url => 'http://example.com:80/the/path') + expect(config.redirect_uri).to eq 'https://example.com/the/path' end it 'returns the expanded redirect uri with adjusted scheme including port 80 with multiple forwarded protocols' do env['HTTP_X_FORWARDED_PROTO'] = 'https,https' - request.stub(:url => 'https://example.com:80/the/path') - config.redirect_uri.should eq 'https://example.com/the/path' + allow(request).to receive_messages(:url => 'https://example.com:80/the/path') + expect(config.redirect_uri).to eq 'https://example.com/the/path' end it 'returns the expanded redirect uri(without port) with adjusted scheme' do env['HTTP_X_FORWARDED_PROTO'] = 'https' - request.stub(:url => 'http://example.com/the/path') - config.redirect_uri.should eq 'https://example.com/the/path' + allow(request).to receive_messages(:url => 'http://example.com/the/path') + expect(config.redirect_uri).to eq 'https://example.com/the/path' end end end @@ -159,7 +159,7 @@ def silence_warnings context 'when specified in scope config' do it 'returns the client secret' do scope_config[:scope] = 'user' - config.scope.should eq 'user' + expect(config.scope).to eq 'user' end end @@ -167,14 +167,14 @@ def silence_warnings it 'returns the client secret' do warden_config[:github_scopes] = 'user' silence_warnings do - config.scope.should eq 'user' + expect(config.scope).to eq 'user' end end end context 'when not specified' do it 'returns nil' do - config.scope.should be_nil + expect(config.scope).to be_nil end end end @@ -187,8 +187,8 @@ def silence_warnings :client_secret => '123', :redirect_uri => '/foo') - config.to_hash.keys. - should =~ [:scope, :client_id, :client_secret, :redirect_uri] + expect(config.to_hash.keys). + to match_array([:scope, :client_id, :client_secret, :redirect_uri]) end end end diff --git a/spec/unit/membership_cache_spec.rb b/spec/unit/membership_cache_spec.rb index e14a065..3a37954 100644 --- a/spec/unit/membership_cache_spec.rb +++ b/spec/unit/membership_cache_spec.rb @@ -4,7 +4,7 @@ describe '#fetch_membership' do it 'returns false by default' do cache = described_class.new({}) - cache.fetch_membership('foo', 'bar').should be_falsey + expect(cache.fetch_membership('foo', 'bar')).to be_falsey end context 'when cache valid' do @@ -15,7 +15,7 @@ end it 'returns true' do - cache.fetch_membership('foo', 'bar').should be_truthy + expect(cache.fetch_membership('foo', 'bar')).to be_truthy end it 'does not invoke the block' do @@ -24,7 +24,7 @@ end it 'converts type and id to strings' do - cache.fetch_membership(:foo, :bar).should be_truthy + expect(cache.fetch_membership(:foo, :bar)).to be_truthy end end @@ -37,7 +37,7 @@ context 'when no block given' do it 'returns false' do - cache.fetch_membership('foo', 'bar').should be_falsey + expect(cache.fetch_membership('foo', 'bar')).to be_falsey end end @@ -50,13 +50,13 @@ it 'caches the value when block returns true' do cache = described_class.new({}) cache.fetch_membership('foo', 'bar') { true } - cache.fetch_membership('foo', 'bar').should be_truthy + expect(cache.fetch_membership('foo', 'bar')).to be_truthy end it 'does not cache the value when block returns false' do cache = described_class.new({}) cache.fetch_membership('foo', 'bar') { false } - cache.fetch_membership('foo', 'bar').should be_falsey + expect(cache.fetch_membership('foo', 'bar')).to be_falsey end end @@ -71,8 +71,8 @@ cache = described_class.new(hash) # Verify that existing data in the hash is used: - expect(cache.fetch_membership('foo', 'valid')).to be_true - expect(cache.fetch_membership('foo', 'timedout')).to be_false + expect(cache.fetch_membership('foo', 'valid')).to be(true) + expect(cache.fetch_membership('foo', 'timedout')).to be(false) # Add new data to the hash: cache.fetch_membership('foo', 'new') { true } diff --git a/spec/unit/oauth_spec.rb b/spec/unit/oauth_spec.rb index bbf60eb..6fbc330 100644 --- a/spec/unit/oauth_spec.rb +++ b/spec/unit/oauth_spec.rb @@ -14,7 +14,7 @@ def oauth(attrs=default_attrs) describe '#authorize_uri' do it 'contains the base uri' do - oauth.authorize_uri.to_s.should \ + expect(oauth.authorize_uri.to_s).to \ include Octokit.web_endpoint end @@ -22,7 +22,7 @@ def oauth(attrs=default_attrs) it "contains the correct #{name} param" do uri = Addressable::URI.parse(oauth.authorize_uri) - uri.query_values[name].should eq default_attrs[name.to_sym] + expect(uri.query_values[name]).to eq default_attrs[name.to_sym] end end @@ -30,7 +30,7 @@ def oauth(attrs=default_attrs) it "does not contain the scope param if #{desc}" do uri = oauth(default_attrs.merge(:scope => value)).authorize_uri - uri.to_s.should_not include 'scope' + expect(uri.to_s).not_to include 'scope' end end end @@ -46,7 +46,7 @@ def expect_request(attrs={}) it 'exchanges the code for an access token' do expect_request(:answer => 'access_token=the_token&token_type=bearer') - oauth.access_token.should eq 'the_token' + expect(oauth.access_token).to eq 'the_token' end it 'raises BadVerificationCode if no access token is returned' do diff --git a/spec/unit/sso_spec.rb b/spec/unit/sso_spec.rb index 3ad506b..758d332 100644 --- a/spec/unit/sso_spec.rb +++ b/spec/unit/sso_spec.rb @@ -28,15 +28,15 @@ def session describe "warden_github_sso_session_valid?" do it "identifies when browsers need to be reverified" do subject.session[:warden_github_sso_session_verified_at] = Time.now.utc.to_i - 10 - subject.should be_warden_github_sso_session_valid(user) + expect(subject).to be_warden_github_sso_session_valid(user) subject.session[:warden_github_sso_session_verified_at] = Time.now.utc.to_i - 300 stub_user_session_request.to_return(:status => 204, :body => "", :headers => {}) - subject.should be_warden_github_sso_session_valid(user) + expect(subject).to be_warden_github_sso_session_valid(user) subject.session[:warden_github_sso_session_verified_at] = Time.now.utc.to_i - 300 stub_user_session_request.to_return(:status => 404, :body => "", :headers => {}) - subject.should_not be_warden_github_sso_session_valid(user) + expect(subject).not_to be_warden_github_sso_session_valid(user) end end end diff --git a/spec/unit/user_spec.rb b/spec/unit/user_spec.rb index 8a3b371..914d297 100644 --- a/spec/unit/user_spec.rb +++ b/spec/unit/user_spec.rb @@ -21,14 +21,14 @@ describe '#token' do it 'returns the token' do - user.token.should eq token + expect(user.token).to eq token end end %w[login name gravatar_id avatar_url email company].each do |name| describe "##{name}" do it "returns the #{name}" do - user.send(name).should eq default_attrs[name] + expect(user.send(name)).to eq default_attrs[name] end end end @@ -37,16 +37,16 @@ it 'returns a preconfigured Octokit client for the user' do api = user.api - api.should be_an Octokit::Client - api.login.should eq user.login - api.access_token.should eq user.token + expect(api).to be_an Octokit::Client + expect(api.login).to eq user.login + expect(api.access_token).to eq user.token end end def stub_api(user, method, args, ret) api = double - user.stub(:api => api) - api.should_receive(method).with(*args).and_return(ret) + allow(user).to receive_messages(:api => api) + expect(api).to receive(method).with(*args).and_return(ret) end [:organization_public_member?, :organization_member?].each do |method| @@ -54,14 +54,14 @@ def stub_api(user, method, args, ret) context 'when user is not member' do it 'returns false' do stub_api(user, method, ['rails', user.login], false) - user.send(method, 'rails').should be_falsey + expect(user.send(method, 'rails')).to be_falsey end end context 'when user is member' do it 'returns true' do stub_api(user, method, ['rails', user.login], true) - user.send(method, 'rails').should be_truthy + expect(user.send(method, 'rails')).to be_truthy end end end @@ -71,21 +71,21 @@ def stub_api(user, method, args, ret) context 'when user is not member' do it 'returns false' do api = double() - user.stub(:api => api) + allow(user).to receive_messages(:api => api) - api.stub(:team_member?).with(123, user.login).and_return(false) + allow(api).to receive(:team_member?).with(123, user.login).and_return(false) - user.should_not be_team_member(123) + expect(user).not_to be_team_member(123) end end context 'when user is member' do it 'returns true' do api = double() - user.stub(:api => api) - api.stub(:team_member?).with(123, user.login).and_return(true) + allow(user).to receive_messages(:api => api) + allow(api).to receive(:team_member?).with(123, user.login).and_return(true) - user.should be_team_member(123) + expect(user).to be_team_member(123) end end end @@ -95,49 +95,49 @@ def stub_api(user, method, args, ret) client = double attrs = {} - Octokit::Client. - should_receive(:new). + expect(Octokit::Client). + to receive(:new). with(:access_token => token). and_return(client) - client.should_receive(:user).and_return(attrs) + expect(client).to receive(:user).and_return(attrs) user = described_class.load(token) - user.attribs.should eq attrs - user.token.should eq token + expect(user.attribs).to eq attrs + expect(user.token).to eq token end end # NOTE: This always passes on MRI 1.9.3 because of ruby bug #7627. it 'marshals correctly' do - Marshal.load(Marshal.dump(user)).should eq user + expect(Marshal.load(Marshal.dump(user))).to eq user end describe 'single sign out' do it "knows if the user is using single sign out" do - user.should_not be_using_single_sign_out - sso_user.should be_using_single_sign_out + expect(user).not_to be_using_single_sign_out + expect(sso_user).to be_using_single_sign_out end context "browser reverification" do it "handles success" do stub_user_session_request.to_return(:status => 204, :body => "", :headers => {}) - sso_user.should be_browser_session_valid + expect(sso_user).to be_browser_session_valid end it "handles failure" do stub_user_session_request.to_return(:status => 404, :body => "", :headers => {}) - sso_user.should_not be_browser_session_valid + expect(sso_user).not_to be_browser_session_valid end it "handles GitHub being unavailable" do stub_user_session_request.to_raise(Octokit::ServerError.new) - sso_user.should be_browser_session_valid + expect(sso_user).to be_browser_session_valid end it "handles authentication failures" do stub_user_session_request.to_return(:status => 403, :body => "", :headers => {}) - sso_user.should_not be_browser_session_valid + expect(sso_user).not_to be_browser_session_valid end end end diff --git a/warden-github.gemspec b/warden-github.gemspec index 655c17c..245e23c 100644 --- a/warden-github.gemspec +++ b/warden-github.gemspec @@ -20,7 +20,7 @@ Gem::Specification.new do |s| s.add_development_dependency "rack", "~>1.4.1" s.add_development_dependency "rake" - s.add_development_dependency "rspec", "~>2.8" + s.add_development_dependency "rspec", "~>3.6" s.add_development_dependency "simplecov" s.add_development_dependency "webmock", "~>1.9" s.add_development_dependency "sinatra" From 0a2d8fabc30819c8742e4f12f8546d6bed962e75 Mon Sep 17 00:00:00 2001 From: Philipe Fatio Date: Sat, 13 May 2017 21:23:42 +0200 Subject: [PATCH 43/46] Use new symbol syntax --- Gemfile | 4 ++-- README.md | 10 +++++----- Rakefile | 2 +- example/multi_scope_app.rb | 10 +++++----- example/simple_app.rb | 8 ++++---- lib/warden/github/config.rb | 16 ++++++++-------- lib/warden/github/oauth.rb | 14 +++++++------- lib/warden/github/strategy.rb | 2 +- lib/warden/github/user.rb | 6 +++--- spec/integration/oauth_spec.rb | 12 ++++++------ spec/spec_helper.rb | 2 +- spec/unit/config_spec.rb | 24 ++++++++++++------------ spec/unit/oauth_spec.rb | 26 +++++++++++++------------- spec/unit/sso_spec.rb | 4 ++-- spec/unit/user_spec.rb | 14 +++++++------- 15 files changed, 77 insertions(+), 77 deletions(-) diff --git a/Gemfile b/Gemfile index b4d4a51..6aa0bb1 100644 --- a/Gemfile +++ b/Gemfile @@ -1,7 +1,7 @@ source "https://rubygems.org" -gem 'debugger', :platforms => :ruby_19, :require => false -gem 'ruby-debug', :platforms => :ruby_18, :require => false +gem 'debugger', platforms: :ruby_19, require: false +gem 'ruby-debug', platforms: :ruby_18, require: false # Specify your gem's dependencies in warden-github.gemspec gemspec diff --git a/README.md b/README.md index 0c83c1d..4f26c34 100644 --- a/README.md +++ b/README.md @@ -48,11 +48,11 @@ use Warden::Manager do |config| config.failure_app = BadAuthentication config.default_strategies :github - config.scope_defaults :default, :config => { :scope => 'user:email' } - config.scope_defaults :admin, :config => { :client_id => 'foobar', - :client_secret => 'barfoo', - :scope => 'user,repo', - :redirect_uri => '/admin/oauth/callback' } + config.scope_defaults :default, config: { scope: 'user:email' } + config.scope_defaults :admin, config: { client_id: 'foobar', + client_secret: 'barfoo', + scope: 'user,repo', + redirect_uri: '/admin/oauth/callback' } config.serialize_from_session { |key| Warden::GitHub::Verifier.load(key) } config.serialize_into_session { |user| Warden::GitHub::Verifier.dump(user) } diff --git a/Rakefile b/Rakefile index 7422ba1..3b9f150 100644 --- a/Rakefile +++ b/Rakefile @@ -3,7 +3,7 @@ require 'rubygems/specification' require 'date' require 'bundler' -task :default => [:spec] +task default: [:spec] require 'rspec/core/rake_task' desc "Run specs" diff --git a/example/multi_scope_app.rb b/example/multi_scope_app.rb index 82454ac..edbcccc 100644 --- a/example/multi_scope_app.rb +++ b/example/multi_scope_app.rb @@ -5,15 +5,15 @@ class MultiScopeApp < BaseApp enable :inline_templates GITHUB_CONFIG = { - :client_id => ENV['GITHUB_CLIENT_ID'] || 'test_client_id', - :client_secret => ENV['GITHUB_CLIENT_SECRET'] || 'test_client_secret' + client_id: ENV['GITHUB_CLIENT_ID'] || 'test_client_id', + client_secret: ENV['GITHUB_CLIENT_SECRET'] || 'test_client_secret' } use Warden::Manager do |config| config.failure_app = BadAuthentication config.default_strategies :github - config.scope_defaults :default, :config => GITHUB_CONFIG - config.scope_defaults :admin, :config => GITHUB_CONFIG.merge(:scope => 'user,notifications') + config.scope_defaults :default, config: GITHUB_CONFIG + config.scope_defaults :admin, config: GITHUB_CONFIG.merge(scope: 'user,notifications') end get '/' do @@ -26,7 +26,7 @@ class MultiScopeApp < BaseApp end get '/admin/login' do - env['warden'].authenticate!(:scope => :admin) + env['warden'].authenticate!(scope: :admin) redirect '/' end diff --git a/example/simple_app.rb b/example/simple_app.rb index 4dae3f5..9a083f3 100644 --- a/example/simple_app.rb +++ b/example/simple_app.rb @@ -7,15 +7,15 @@ class SimpleApp < BaseApp enable :inline_templates GITHUB_CONFIG = { - :client_id => ENV['GITHUB_CLIENT_ID'] || 'test_client_id', - :client_secret => ENV['GITHUB_CLIENT_SECRET'] || 'test_client_secret', - :scope => 'user' + client_id: ENV['GITHUB_CLIENT_ID'] || 'test_client_id', + client_secret: ENV['GITHUB_CLIENT_SECRET'] || 'test_client_secret', + scope: 'user' } use Warden::Manager do |config| config.failure_app = BadAuthentication config.default_strategies :github - config.scope_defaults :default, :config => GITHUB_CONFIG + config.scope_defaults :default, config: GITHUB_CONFIG config.serialize_from_session { |key| Warden::GitHub::Verifier.load(key) } config.serialize_into_session { |user| Warden::GitHub::Verifier.dump(user) } end diff --git a/lib/warden/github/config.rb b/lib/warden/github/config.rb index 4937ff0..3ab4b4e 100644 --- a/lib/warden/github/config.rb +++ b/lib/warden/github/config.rb @@ -43,10 +43,10 @@ module GitHub # # # This configures an additional scope that uses the github strategy # # with custom configuration. - # config.scope_defaults :admin, :config => { :client_id => 'foobar', - # :client_secret => 'barfoo', - # :scope => 'user,repo', - # :redirect_uri => '/admin/oauth/callback' } + # config.scope_defaults :admin, config: { client_id: 'foobar', + # client_secret: 'barfoo', + # scope: 'user,repo', + # redirect_uri: '/admin/oauth/callback' } # end class Config BadConfig = Class.new(StandardError) @@ -88,10 +88,10 @@ def scope end def to_hash - { :client_id => client_id, - :client_secret => client_secret, - :redirect_uri => redirect_uri, - :scope => scope } + { client_id: client_id, + client_secret: client_secret, + redirect_uri: redirect_uri, + scope: scope } end private diff --git a/lib/warden/github/oauth.rb b/lib/warden/github/oauth.rb index c7e3afa..3a0dfdc 100644 --- a/lib/warden/github/oauth.rb +++ b/lib/warden/github/oauth.rb @@ -25,10 +25,10 @@ def initialize(attrs={}) def authorize_uri @authorize_uri ||= build_uri( '/login/oauth/authorize', - :client_id => client_id, - :redirect_uri => redirect_uri, - :scope => scope, - :state => state) + client_id: client_id, + redirect_uri: redirect_uri, + scope: scope, + state: state) end def access_token @@ -53,9 +53,9 @@ def load_access_token def access_token_uri @access_token_uri ||= build_uri( '/login/oauth/access_token', - :client_id => client_id, - :client_secret => client_secret, - :code => code) + client_id: client_id, + client_secret: client_secret, + code: code) end def build_uri(path, params) diff --git a/lib/warden/github/strategy.rb b/lib/warden/github/strategy.rb index af5e1fb..6ddd6fd 100644 --- a/lib/warden/github/strategy.rb +++ b/lib/warden/github/strategy.rb @@ -92,7 +92,7 @@ def state def oauth @oauth ||= OAuth.new( - config.to_hash.merge(:code => params['code'], :state => state)) + config.to_hash.merge(code: params['code'], state: state)) end def config diff --git a/lib/warden/github/user.rb b/lib/warden/github/user.rb index 5844cc0..ec52848 100644 --- a/lib/warden/github/user.rb +++ b/lib/warden/github/user.rb @@ -4,7 +4,7 @@ class User < Struct.new(:attribs, :token, :browser_session_id, :memberships) ATTRIBUTES = %w[id login name gravatar_id avatar_url email company site_admin].freeze def self.load(access_token, browser_session_id = nil) - api = Octokit::Client.new(:access_token => access_token) + api = Octokit::Client.new(access_token: access_token) data = { } api.user.to_hash.each do |k,v| @@ -75,7 +75,7 @@ def site_admin? def browser_session_valid?(since = 120) return true unless using_single_sign_out? client = api - client.get("/user/sessions/active", :browser_session_id => browser_session_id) + client.get("/user/sessions/active", browser_session_id: browser_session_id) client.last_response.status == 204 rescue Octokit::ServerError # GitHub API unavailable true @@ -100,7 +100,7 @@ def api # Don't cache instance for now because of a ruby marshaling bug present # in MRI 1.9.3 (Bug #7627) that causes instance variables to be # marshaled even when explicitly specifying #marshal_dump. - Octokit::Client.new(:login => login, :access_token => token) + Octokit::Client.new(login: login, access_token: token) end private diff --git a/spec/integration/oauth_spec.rb b/spec/integration/oauth_spec.rb index f897dc0..c687d50 100644 --- a/spec/integration/oauth_spec.rb +++ b/spec/integration/oauth_spec.rb @@ -5,17 +5,17 @@ def stub_code_for_token_exchange(answer='access_token=the_token') stub_request(:post, 'https://github.com/login/oauth/access_token'). - with(:body => hash_including(:code => code)). - to_return(:status => 200, :body => answer) + with(body: hash_including(code: code)). + to_return(status: 200, body: answer) end def stub_user_retrieval stub_request(:get, 'https://api.github.com/user'). - with(:headers => { 'Authorization' => 'token the_token' }). + with(headers: { 'Authorization' => 'token the_token' }). to_return( - :status => 200, - :body => File.read('spec/fixtures/user.json'), - :headers => { 'Content-Type' => 'application/json; charset=utf-8' }) + status: 200, + body: File.read('spec/fixtures/user.json'), + headers: { 'Content-Type' => 'application/json; charset=utf-8' }) end def redirect_uri(response) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index e812ad9..6379fcd 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -19,6 +19,6 @@ def app def stub_user_session_request stub_request(:get, "https://api.github.com/user/sessions/active?browser_session_id=abcdefghijklmnop"). - with(:headers => {'Accept'=>'application/vnd.github.v3+json', 'Accept-Encoding'=>'gzip;q=1.0,deflate;q=0.6,identity;q=0.3', 'Authorization'=>'token the_token', 'Content-Type'=>'application/json', 'User-Agent'=>"Octokit Ruby Gem #{Octokit::VERSION}"}) + with(headers: {'Accept'=>'application/vnd.github.v3+json', 'Accept-Encoding'=>'gzip;q=1.0,deflate;q=0.6,identity;q=0.3', 'Authorization'=>'token the_token', 'Content-Type'=>'application/json', 'User-Agent'=>"Octokit Ruby Gem #{Octokit::VERSION}"}) end end diff --git a/spec/unit/config_spec.rb b/spec/unit/config_spec.rb index 8f17dd1..88215aa 100644 --- a/spec/unit/config_spec.rb +++ b/spec/unit/config_spec.rb @@ -4,11 +4,11 @@ let(:warden_scope) { :test_scope } let(:env) do - { 'warden' => double(:config => warden_config) } + { 'warden' => double(config: warden_config) } end let(:warden_config) do - { :scope_defaults => { warden_scope => { :config => scope_config } } } + { scope_defaults: { warden_scope => { config: scope_config } } } end let(:scope_config) do @@ -16,7 +16,7 @@ end let(:request) do - double(:url => 'http://example.com/the/path', :path => '/the/path') + double(url: 'http://example.com/the/path', path: '/the/path') end subject(:config) do @@ -24,7 +24,7 @@ end before do - allow(config).to receive_messages(:request => request) + allow(config).to receive_messages(request: request) end def silence_warnings @@ -131,25 +131,25 @@ def silence_warnings context 'when HTTP_X_FORWARDED_PROTO is set to https' do it 'returns the expanded redirect uri(with port) with adjusted scheme' do env['HTTP_X_FORWARDED_PROTO'] = 'https' - allow(request).to receive_messages(:url => 'http://example.com:443/the/path') + allow(request).to receive_messages(url: 'http://example.com:443/the/path') expect(config.redirect_uri).to eq 'https://example.com/the/path' end it 'returns the expanded redirect uri with adjusted scheme including port 80' do env['HTTP_X_FORWARDED_PROTO'] = 'https' - allow(request).to receive_messages(:url => 'http://example.com:80/the/path') + allow(request).to receive_messages(url: 'http://example.com:80/the/path') expect(config.redirect_uri).to eq 'https://example.com/the/path' end it 'returns the expanded redirect uri with adjusted scheme including port 80 with multiple forwarded protocols' do env['HTTP_X_FORWARDED_PROTO'] = 'https,https' - allow(request).to receive_messages(:url => 'https://example.com:80/the/path') + allow(request).to receive_messages(url: 'https://example.com:80/the/path') expect(config.redirect_uri).to eq 'https://example.com/the/path' end it 'returns the expanded redirect uri(without port) with adjusted scheme' do env['HTTP_X_FORWARDED_PROTO'] = 'https' - allow(request).to receive_messages(:url => 'http://example.com/the/path') + allow(request).to receive_messages(url: 'http://example.com/the/path') expect(config.redirect_uri).to eq 'https://example.com/the/path' end end @@ -182,10 +182,10 @@ def silence_warnings describe '#to_hash' do it 'includes all configs' do scope_config.merge!( - :scope => 'user', - :client_id => 'abc', - :client_secret => '123', - :redirect_uri => '/foo') + scope: 'user', + client_id: 'abc', + client_secret: '123', + redirect_uri: '/foo') expect(config.to_hash.keys). to match_array([:scope, :client_id, :client_secret, :redirect_uri]) diff --git a/spec/unit/oauth_spec.rb b/spec/unit/oauth_spec.rb index 6fbc330..ab901f2 100644 --- a/spec/unit/oauth_spec.rb +++ b/spec/unit/oauth_spec.rb @@ -2,10 +2,10 @@ describe Warden::GitHub::OAuth do let(:default_attrs) do - { :state => 'abc', - :client_id => 'foo', - :client_secret => 'bar', - :redirect_uri => 'http://example.com/callback' } + { state: 'abc', + client_id: 'foo', + client_secret: 'bar', + redirect_uri: 'http://example.com/callback' } end def oauth(attrs=default_attrs) @@ -26,9 +26,9 @@ def oauth(attrs=default_attrs) end end - { :nil => nil, :empty => '' }.each do |desc, value| + { nil: nil, empty: '' }.each do |desc, value| it "does not contain the scope param if #{desc}" do - uri = oauth(default_attrs.merge(:scope => value)).authorize_uri + uri = oauth(default_attrs.merge(scope: value)).authorize_uri expect(uri.to_s).not_to include 'scope' end @@ -38,19 +38,19 @@ def oauth(attrs=default_attrs) describe '#access_token' do def expect_request(attrs={}) stub_request(:post, %r{\/login\/oauth\/access_token$}). - with(:body => hash_including(attrs.fetch(:params, {}))). - to_return(:status => 200, - :body => attrs.fetch(:answer, 'access_token=foobar')) + with(body: hash_including(attrs.fetch(:params, {}))). + to_return(status: 200, + body: attrs.fetch(:answer, 'access_token=foobar')) end it 'exchanges the code for an access token' do - expect_request(:answer => 'access_token=the_token&token_type=bearer') + expect_request(answer: 'access_token=the_token&token_type=bearer') expect(oauth.access_token).to eq 'the_token' end it 'raises BadVerificationCode if no access token is returned' do - expect_request(:answer => 'error=bad_verification_code') + expect_request(answer: 'error=bad_verification_code') expect { oauth.access_token }. to raise_error(described_class::BadVerificationCode) @@ -58,8 +58,8 @@ def expect_request(attrs={}) %w[ client_id client_secret code ].each do |name| it "performs a request containing the correct #{name} param" do - oauth(default_attrs.merge(:code => 'the_code')).tap do |o| - expect_request(:params => { name => o.send(name) }) + oauth(default_attrs.merge(code: 'the_code')).tap do |o| + expect_request(params: { name => o.send(name) }) o.access_token end end diff --git a/spec/unit/sso_spec.rb b/spec/unit/sso_spec.rb index 758d332..0894937 100644 --- a/spec/unit/sso_spec.rb +++ b/spec/unit/sso_spec.rb @@ -31,11 +31,11 @@ def session expect(subject).to be_warden_github_sso_session_valid(user) subject.session[:warden_github_sso_session_verified_at] = Time.now.utc.to_i - 300 - stub_user_session_request.to_return(:status => 204, :body => "", :headers => {}) + stub_user_session_request.to_return(status: 204, body: "", headers: {}) expect(subject).to be_warden_github_sso_session_valid(user) subject.session[:warden_github_sso_session_verified_at] = Time.now.utc.to_i - 300 - stub_user_session_request.to_return(:status => 404, :body => "", :headers => {}) + stub_user_session_request.to_return(status: 404, body: "", headers: {}) expect(subject).not_to be_warden_github_sso_session_valid(user) end end diff --git a/spec/unit/user_spec.rb b/spec/unit/user_spec.rb index 914d297..026a712 100644 --- a/spec/unit/user_spec.rb +++ b/spec/unit/user_spec.rb @@ -45,7 +45,7 @@ def stub_api(user, method, args, ret) api = double - allow(user).to receive_messages(:api => api) + allow(user).to receive_messages(api: api) expect(api).to receive(method).with(*args).and_return(ret) end @@ -71,7 +71,7 @@ def stub_api(user, method, args, ret) context 'when user is not member' do it 'returns false' do api = double() - allow(user).to receive_messages(:api => api) + allow(user).to receive_messages(api: api) allow(api).to receive(:team_member?).with(123, user.login).and_return(false) @@ -82,7 +82,7 @@ def stub_api(user, method, args, ret) context 'when user is member' do it 'returns true' do api = double() - allow(user).to receive_messages(:api => api) + allow(user).to receive_messages(api: api) allow(api).to receive(:team_member?).with(123, user.login).and_return(true) expect(user).to be_team_member(123) @@ -97,7 +97,7 @@ def stub_api(user, method, args, ret) expect(Octokit::Client). to receive(:new). - with(:access_token => token). + with(access_token: token). and_return(client) expect(client).to receive(:user).and_return(attrs) @@ -121,12 +121,12 @@ def stub_api(user, method, args, ret) context "browser reverification" do it "handles success" do - stub_user_session_request.to_return(:status => 204, :body => "", :headers => {}) + stub_user_session_request.to_return(status: 204, body: "", headers: {}) expect(sso_user).to be_browser_session_valid end it "handles failure" do - stub_user_session_request.to_return(:status => 404, :body => "", :headers => {}) + stub_user_session_request.to_return(status: 404, body: "", headers: {}) expect(sso_user).not_to be_browser_session_valid end @@ -136,7 +136,7 @@ def stub_api(user, method, args, ret) end it "handles authentication failures" do - stub_user_session_request.to_return(:status => 403, :body => "", :headers => {}) + stub_user_session_request.to_return(status: 403, body: "", headers: {}) expect(sso_user).not_to be_browser_session_valid end end From 4a5af45e0e6a59bfe1772823fd0ad55d65a6dab0 Mon Sep 17 00:00:00 2001 From: Philipe Fatio Date: Sat, 13 May 2017 20:39:45 +0200 Subject: [PATCH 44/46] Replace old debugger gems by byebug --- Gemfile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Gemfile b/Gemfile index 6aa0bb1..caa0989 100644 --- a/Gemfile +++ b/Gemfile @@ -1,7 +1,6 @@ source "https://rubygems.org" -gem 'debugger', platforms: :ruby_19, require: false -gem 'ruby-debug', platforms: :ruby_18, require: false +gem 'byebug', require: false # Specify your gem's dependencies in warden-github.gemspec gemspec From d716c62649c8f00386fbb2337b32c8453f3b2dbe Mon Sep 17 00:00:00 2001 From: Philipe Fatio Date: Sat, 13 May 2017 21:29:25 +0200 Subject: [PATCH 45/46] Update ruby version on Travis --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index cc13928..a01d453 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,3 +1,3 @@ language: ruby rvm: - - 2.3.0 + - 2.4.1 From 8f121d40908dce3388a5c4034f44a8393a29b617 Mon Sep 17 00:00:00 2001 From: Philipe Fatio Date: Sat, 13 May 2017 20:45:28 +0200 Subject: [PATCH 46/46] Bump version --- lib/warden/github/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/warden/github/version.rb b/lib/warden/github/version.rb index 9543c8e..2126db8 100644 --- a/lib/warden/github/version.rb +++ b/lib/warden/github/version.rb @@ -1,5 +1,5 @@ module Warden module GitHub - VERSION = "1.3.1" + VERSION = "1.3.2" end end