Skip to content

Commit

Permalink
Move strategy 'identity' method from private to public, as tests expect.
Browse files Browse the repository at this point in the history
Ensure jruby compatibility.
Fix some broken tests.
  • Loading branch information
ginjo committed Aug 25, 2018
1 parent f08c016 commit c37fc90
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 25 deletions.
21 changes: 11 additions & 10 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/omniauth-slack/version.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
module Omniauth
module Slack
VERSION = "2.4.0.pre01"
VERSION = "2.4.0.pre02"
end
end
20 changes: 10 additions & 10 deletions lib/omniauth/strategies/slack.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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|
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion omniauth-slack.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
11 changes: 8 additions & 3 deletions test/test.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
require "helper"
require "omniauth-slack"
require 'helper'
require 'omniauth-slack'

OmniAuth.logger.level = 1

class StrategyTest < StrategyTestCase
include OAuth2StrategyTests
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down

0 comments on commit c37fc90

Please sign in to comment.