Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

### 2.3.0 (Next)

* [#190](https://github.com/slack-ruby/slack-ruby-bot-server/pull/190): Add opt-in active OAuth v2 reinstallation with credential refresh and a `reinstalled` callback, preserving default rejection and legacy RTM protection; rejected duplicate registrations no longer overwrite OAuth metadata - [@dblock](https://github.com/dblock), [@Copilot](https://github.com/apps/copilot-swe-agent).
* [#188](https://github.com/slack-ruby/slack-ruby-bot-server/pull/188): Reduce the database CI matrices from 22 to 12 jobs while retaining representative legacy ORM majors and modern Ruby/Grape 3/4 coverage - [@dblock](https://github.com/dblock), [@Copilot](https://github.com/apps/copilot-swe-agent).
* [#186](https://github.com/slack-ruby/slack-ruby-bot-server/pull/186): Document persisted custom Team attributes for both adapters and Rails concern loading/reloading - [@dblock](https://github.com/dblock), [@Copilot](https://github.com/apps/copilot-swe-agent).
* [#183](https://github.com/slack-ruby/slack-ruby-bot-server/pull/183): Add a `config.server_pages` block to configure page middleware options centrally without changing the installed library's defaults - [@dblock](https://github.com/dblock), [@Copilot](https://github.com/apps/copilot-swe-agent).
Expand Down
24 changes: 23 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ Build a complete Slack bot service with Slack button integration, in Ruby.
- [Active Record Attributes](#active-record-attributes)
- [Rails Loading and Reloading](#rails-loading-and-reloading)
- [OAuth Version and Scopes](#oauth-version-and-scopes)
- [Active Reinstallation](#active-reinstallation)
- [Slack App](#slack-app)
- [API](#api)
- [Custom Endpoints](#custom-endpoints)
Expand Down Expand Up @@ -197,6 +198,26 @@ end

The "Add to Slack" button uses the standard OAuth code grant flow as described in the [Slack docs](https://api.slack.com/docs/oauth#flow). Once clicked, the user is taken through the authorization process at Slack's site. Upon successful completion, a callback containing a temporary code is sent to the redirect URL you specified. The endpoint at that URL contains code that persists the bot token each time a Slack client is instantiated for the specific team.

#### Active Reinstallation

By default, installing an already active bot returns an "already registered" error. Stateless OAuth v2 applications, such as Events API bots, can opt into refreshing an existing installation:

```ruby
SlackRubyBotServer.configure do |config|
config.allow_reinstall = true
end
```

Both the existing installation and the new authorization must use OAuth v2. Successful active reinstalls update the stored token, scopes, and authorizing-user metadata, then call `Service#reinstall!` and emit `:reinstalled` with the optional OAuth `state`. They do not create another team or invoke `:creating`, `:created`, `:starting`, or `:started`, so no second worker is started:

```ruby
SlackRubyBotServer::Service.instance.on :reinstalled do |team, error, options|
# Apply application-specific installation settings using options[:state].
end
```

Only enable this for applications that do not need an existing connection or worker restarted when credentials change. Active OAuth v1 installations and active OAuth-version migrations still return an error, even with this option enabled, preserving legacy RTM protection. New installations and previously inactive installations retain the normal creation/start lifecycle. `Config.reset!` restores the default `false`.

### Slack App

Create a new Slack App [here](https://api.slack.com/applications/new).
Expand Down Expand Up @@ -339,6 +360,7 @@ The following callbacks are supported. All callbacks receive a `team`, except `e
| error | an error has occurred |
| creating | a new team is being registered |
| created | a new team has been registered |
| reinstalled | an active OAuth v2 installation has been refreshed without starting a worker |
| booting | the service is starting and is connecting a team to Slack |
| booted | the service is starting and has connected a team to Slack |
| stopping | the service is about to disconnect a team from Slack |
Expand All @@ -348,7 +370,7 @@ The following callbacks are supported. All callbacks receive a `team`, except `e
| deactivating | a team is being deactivated |
| deactivated | a team has been deactivated |

The [Add to Slack button](https://api.slack.com/docs/slack-button) also allows for an optional `state` parameter that will be returned on completion of the request. The `creating` and `created` callbacks include an options hash where this value can be accessed (to check for forgery attacks for instance).
The [Add to Slack button](https://api.slack.com/docs/slack-button) also allows for an optional `state` parameter that will be returned on completion of the request. The `creating`, `created`, and `reinstalled` callbacks include an options hash where this value can be accessed (to check for forgery attacks for instance).
```ruby
auth = OpenSSL::HMAC.hexdigest("SHA256", "key", "data")
```
Expand Down
46 changes: 26 additions & 20 deletions lib/slack-ruby-bot-server/api/endpoints/teams_endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,36 +88,42 @@ class Create < Grape::API
team ||= Team.where(team_id: team_id, oauth_version: oauth_version).first
team ||= Team.where(team_id: team_id).first

attributes = {
oauth_version: oauth_version,
oauth_scope: oauth_scope,
activated_user_id: user_id,
activated_user_access_token: access_token,
bot_user_id: bot_user_id
}
reinstall = false

if team
team.ping_if_active!

team.update!(
oauth_version: oauth_version,
oauth_scope: oauth_scope,
activated_user_id: user_id,
activated_user_access_token: access_token,
bot_user_id: bot_user_id
)
if team.active?
raise "Team #{team.name} is already registered." unless SlackRubyBotServer::Config.allow_reinstall && team.oauth_version == 'v2' && oauth_version == :v2

raise "Team #{team.name} is already registered." if team.active?
reinstall = true
attributes[:token] = token
end

team.activate!(token)
team.update!(attributes)
team.activate!(token) unless reinstall
else
team = Team.create!(
token: token,
oauth_version: oauth_version,
oauth_scope: oauth_scope,
team_id: team_id,
name: team_name,
activated_user_id: user_id,
activated_user_access_token: access_token,
bot_user_id: bot_user_id
)
team = Team.create!(attributes.merge(
token: token,
team_id: team_id,
name: team_name
))
end

options = params.slice(:state)

Service.instance.create!(team, options)
if reinstall
Service.instance.reinstall!(team, options)
else
Service.instance.create!(team, options)
end
present team, with: Presenters::TeamPresenter
end
end
Expand Down
2 changes: 2 additions & 0 deletions lib/slack-ruby-bot-server/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@ module Config
attr_accessor :view_paths
attr_accessor :oauth_scope
attr_accessor :oauth_version
attr_accessor :allow_reinstall
attr_accessor :api

def reset!
self.logger = nil
self.service_class = SlackRubyBotServer::Service
self.oauth_scope = nil
self.oauth_version = :v2
self.allow_reinstall = false
@server_pages = nil
self.api = nil

Expand Down
5 changes: 5 additions & 0 deletions lib/slack-ruby-bot-server/service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ def start!(team)
logger.error e
end

def reinstall!(team, options = {})
logger.info "Reinstalled team #{team}."
run_callbacks :reinstalled, team, nil, options
end

def restart!(team)
logger.info "Restarting team #{team}."
run_callbacks :restarting, team
Expand Down
59 changes: 57 additions & 2 deletions spec/api/endpoints/teams_endpoint_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,13 @@
end
it 'returns a useful error when team already exists' do
expect_any_instance_of(Team).to receive(:ping_if_active!)
existing_team = Fabricate(:team, token: 'access_token')
existing_team = Fabricate(:team, token: 'access_token', oauth_scope: 'original-scope', activated_user_id: 'original-user')
expect { client.teams._post(code: 'code') }.to raise_error Faraday::ClientError do |e|
json = JSON.parse(e.response[:body])
expect(json['message']).to eq "Team #{existing_team.name} is already registered."
end
expect(existing_team.reload.oauth_scope).to eq 'original-scope'
expect(existing_team.activated_user_id).to eq 'original-user'
end
it 'reactivates a deactivated team with a different code' do
expect(SlackRubyBotServer::Service.instance).to receive(:start!)
Expand All @@ -151,6 +153,58 @@
expect(team.bot_user_id).to be nil
end.to_not change(Team, :count)
end

context 'with active reinstallation enabled' do
before do
SlackRubyBotServer.config.allow_reinstall = true
allow_any_instance_of(Team).to receive(:ping_if_active!)
end

it 'refreshes credentials without creating a record or starting another worker' do
existing_team = Fabricate(:team, token: 'old-token', team_id: 'team_id', oauth_version: 'v2')
expect(SlackRubyBotServer::Service.instance).not_to receive(:start!)
expect(SlackRubyBotServer::Service.instance).not_to receive(:create!)
expect(SlackRubyBotServer::Service.instance).to receive(:reinstall!).with(
instance_of(Team), { state: 'application-state' }
).and_call_original

expect do
response = client.teams._post(code: 'code', state: 'application-state')
expect(response.id.to_s).to eq existing_team.id.to_s
end.not_to change(Team, :count)
existing_team.reload
expect(existing_team.token).to eq 'access_token'
expect(existing_team.active?).to be true
expect(existing_team.oauth_scope).to eq 'commands,incoming-webhook'
expect(existing_team.activated_user_access_token).to eq 'access_token'
expect(existing_team.activated_user_id).to eq 'user_id'
end

it 'does not bypass the active legacy installation guard' do
existing_team = Fabricate(:team, token: 'old-token', team_id: 'team_id', oauth_version: 'v1', oauth_scope: 'original-scope')
expect(SlackRubyBotServer::Service.instance).not_to receive(:reinstall!)
expect { client.teams._post(code: 'code') }.to raise_error(Faraday::ClientError)
existing_team.reload
expect(existing_team.token).to eq 'old-token'
expect(existing_team.oauth_version).to eq 'v1'
expect(existing_team.oauth_scope).to eq 'original-scope'
end

it 'still starts newly created installations normally' do
expect(SlackRubyBotServer::Service.instance).to receive(:start!).and_call_original
expect(SlackRubyBotServer::Service.instance).not_to receive(:reinstall!)
expect { client.teams._post(code: 'code') }.to change(Team, :count).by(1)
end

it 'still starts previously deactivated installations normally' do
existing_team = Fabricate(:team, token: 'old-token', team_id: 'team_id', oauth_version: 'v2', active: false)
expect(SlackRubyBotServer::Service.instance).to receive(:start!).and_call_original
expect(SlackRubyBotServer::Service.instance).not_to receive(:reinstall!)
client.teams._post(code: 'code')
expect(existing_team.reload.token).to eq 'access_token'
expect(existing_team.active?).to be true
end
end
end

context 'register a bot via oauth v1' do
Expand Down Expand Up @@ -235,8 +289,9 @@
end.to_not change(Team, :count)
end
it 'returns a useful error when team already exists' do
SlackRubyBotServer.config.allow_reinstall = true
expect_any_instance_of(Team).to receive(:ping_if_active!)
existing_team = Fabricate(:team, token: 'access_token')
existing_team = Fabricate(:team, token: 'access_token', oauth_version: 'v2')
expect { client.teams._post(code: 'code') }.to raise_error Faraday::ClientError do |e|
json = JSON.parse(e.response[:body])
expect(json['message']).to eq "Team #{existing_team.name} is already registered."
Expand Down
8 changes: 8 additions & 0 deletions spec/slack-ruby-bot-server/service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,14 @@
end
context 'callbacks' do
let(:instance) { SlackRubyBotServer::Service.instance }
it 'notifies applications of reinstallation without starting a worker' do
expect(instance).not_to receive(:start!)
expect do |callback|
instance.on(:reinstalled, &callback)
instance.reinstall!(team, state: 'application-state')
end.to yield_with_args(team, nil, { state: 'application-state' })
end

context 'single' do
before do
@events = []
Expand Down
Loading