From c37fc9071b690a89da42c14a4965cfa89e3d8fbf Mon Sep 17 00:00:00 2001 From: wbr Date: Fri, 24 Aug 2018 22:24:22 -0700 Subject: [PATCH] Move strategy 'identity' method from private to public, as tests expect. Ensure jruby compatibility. Fix some broken tests. --- README.md | 21 +++++++++++---------- lib/omniauth-slack/version.rb | 2 +- lib/omniauth/strategies/slack.rb | 20 ++++++++++---------- omniauth-slack.gemspec | 6 +++++- test/test.rb | 11 ++++++++--- 5 files changed, 35 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index bd25f35..3c96295 100644 --- a/README.md +++ b/README.md @@ -4,12 +4,12 @@ To view the original omniauth-slack from [@kmrshntr](https://github.com/kmrshntr # Omniauth::Slack -This Gem contains the Slack strategy for OmniAuth and supports (almost) all features of -the [Slack OAuth2 authorization API](https://api.slack.com/docs/oauth), including both +This Gem contains the Slack strategy for OmniAuth and supports most features of +the [Slack OAuth2 authorization API](https://api.slack.com/docs/oauth), including both the [Sign in with Slack](https://api.slack.com/docs/sign-in-with-slack) and [Add to Slack](https://api.slack.com/docs/slack-button) approval flows. -This Gem supports Slack "classic" apps and tokens as well as the developerpreview of [Workspace apps and tokens](https://api.slack.com/workspace-apps-preview). +This Gem supports Slack "classic" apps and tokens as well as the developer preview of [Workspace apps and tokens](https://api.slack.com/workspace-apps-preview). ## Before You Begin @@ -68,25 +68,26 @@ end ## Scopes -Slack lets you choose from a [few different scopes](https://api.slack.com/docs/oauth-scopes#scopes). *Here is another [list of Slack scopes](https://api.slack.com/scopes) with classic and new app compatibility information.* +Slack lets you choose from a [few different scopes](https://api.slack.com/docs/oauth-scopes#scopes). +*Here's another [table of Slack scopes](https://api.slack.com/scopes) showing classic and new app compatibility.* -However, you cannot request both `identity` scopes and other scopes at the same time. +**Important:** You cannot request both `identity` scopes and regular scopes in a single authorization request. -If you need to combine regular app scopes with those used for “Sign in with Slack”, you should -configure two providers: +If you need to combine "Add to Slack" scopes with those used for "Sign in with Slack", you should configure two providers: ```ruby provider :slack, 'API_KEY', 'API_SECRET', scope: 'identity.basic', name: :sign_in_with_slack provider :slack, 'API_KEY', 'API_SECRET', scope: 'team:read,users:read,identify,bot' ``` -Use the first provider to sign users in and the second to add the application (and deeper capabilities) to their team. +Use the first provider to sign users in and the second to add the application, and deeper capabilities, to their team. -Slack is designed to allow quick authorization of users with minimally scoped requests. Deeper scope authorizations are intended to be acquired with further passes thru Slack's authorization process, as the needs of the user and the endpoint app require. +Sign-in-with-Slack handles quick authorization of users with minimally scoped requests. +Deeper scope authorizations are acquired with further passes through the Add-to-Slack authorization process. This works because Slack scopes are additive: Once you successfully authorize a scope, the token will possess that scope forever, regardless of what flow or scopes are requested at future authorizations. -Removal of scope(s) requires revocation of the token. +Removal of scope requires revocation of the token. ## Authentication Options diff --git a/lib/omniauth-slack/version.rb b/lib/omniauth-slack/version.rb index 97a4ad6..d951d00 100644 --- a/lib/omniauth-slack/version.rb +++ b/lib/omniauth-slack/version.rb @@ -1,5 +1,5 @@ module Omniauth module Slack - VERSION = "2.4.0.pre01" + VERSION = "2.4.0.pre02" end end diff --git a/lib/omniauth/strategies/slack.rb b/lib/omniauth/strategies/slack.rb index 3c4d9d5..d753e32 100644 --- a/lib/omniauth/strategies/slack.rb +++ b/lib/omniauth/strategies/slack.rb @@ -32,7 +32,7 @@ class Slack < OmniAuth::Strategies::OAuth2 option :preload_data_with_threads, 0 option :additional_data, {} - + # User ID is not guaranteed to be globally unique across all Slack users. # The combination of user ID and team ID, on the other hand, is guaranteed # to be globally unique. @@ -163,7 +163,7 @@ def authorize_params def client st_raw_info = raw_info new_client = super - OmniAuth.logger.send(:debug, "(slack) New client #{new_client}") + log(:debug, "New client #{new_client}") new_client.instance_eval do define_singleton_method(:request) do |*args| @@ -182,6 +182,14 @@ def callback_url full_host + script_name + callback_path end + def identity + return {} unless !skip_info? && has_scope?(identity: ['identity.basic','identity:read:user']) + semaphore.synchronize { + @identity_raw ||= access_token.get('/api/users.identity', headers: {'X-Slack-User' => user_id}) + @identity ||= @identity_raw.parsed + } + end + private @@ -227,14 +235,6 @@ def auth @auth ||= access_token.params.to_h.merge({'token' => access_token.token}) end - def identity - return {} unless !skip_info? && has_scope?(identity: ['identity.basic','identity:read:user']) - semaphore.synchronize { - @identity_raw ||= access_token.get('/api/users.identity', headers: {'X-Slack-User' => user_id}) - @identity ||= @identity_raw.parsed - } - end - def user_identity @user_identity ||= identity['user'].to_h end diff --git a/omniauth-slack.gemspec b/omniauth-slack.gemspec index a45238a..701ba74 100644 --- a/omniauth-slack.gemspec +++ b/omniauth-slack.gemspec @@ -18,7 +18,11 @@ Gem::Specification.new do |spec| spec.add_runtime_dependency 'omniauth-oauth2', '~> 1.4' - spec.add_development_dependency 'bundler', '~> 1.11.2' + if RUBY_PLATFORM =~ /java/ + spec.add_development_dependency 'bundler', '>= 1.11.2' + else + spec.add_development_dependency 'bundler', '>= 1.11.2' + end spec.add_development_dependency 'rake' spec.add_development_dependency 'minitest' spec.add_development_dependency 'mocha' diff --git a/test/test.rb b/test/test.rb index 481f0c0..ca3b56c 100644 --- a/test/test.rb +++ b/test/test.rb @@ -1,5 +1,7 @@ -require "helper" -require "omniauth-slack" +require 'helper' +require 'omniauth-slack' + +OmniAuth.logger.level = 1 class StrategyTest < StrategyTestCase include OAuth2StrategyTests @@ -57,6 +59,7 @@ def setup @access_token.stubs(:expires_at) @access_token.stubs(:refresh_token) @access_token.stubs(:[]) + @access_token.stubs(:params) strategy.stubs(:access_token).returns(@access_token) end @@ -107,12 +110,14 @@ def setup super @access_token = stub("OAuth2::AccessToken") @access_token.stubs(:[]) + @access_token.stubs(:params) + @access_token.stubs(:token) strategy.stubs(:access_token).returns(@access_token) strategy.stubs(:has_scope?).returns true end test "performs a GET to https://slack.com/api/users.identity" do - @access_token.expects(:get).with("/api/users.identity") + @access_token.expects(:get).with("/api/users.identity", {:headers => {"X-Slack-User" => nil}}) .returns(stub_everything("OAuth2::Response")) strategy.identity end