diff --git a/CHANGELOG.md b/CHANGELOG.md index fb5c208..4d01957 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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). diff --git a/README.md b/README.md index a4d915e..cf3dfeb 100644 --- a/README.md +++ b/README.md @@ -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) @@ -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). @@ -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 | @@ -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") ``` diff --git a/lib/slack-ruby-bot-server/api/endpoints/teams_endpoint.rb b/lib/slack-ruby-bot-server/api/endpoints/teams_endpoint.rb index 972737c..25a771c 100644 --- a/lib/slack-ruby-bot-server/api/endpoints/teams_endpoint.rb +++ b/lib/slack-ruby-bot-server/api/endpoints/teams_endpoint.rb @@ -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 diff --git a/lib/slack-ruby-bot-server/config.rb b/lib/slack-ruby-bot-server/config.rb index 672e592..be3c782 100644 --- a/lib/slack-ruby-bot-server/config.rb +++ b/lib/slack-ruby-bot-server/config.rb @@ -8,6 +8,7 @@ module Config attr_accessor :view_paths attr_accessor :oauth_scope attr_accessor :oauth_version + attr_accessor :allow_reinstall attr_accessor :api def reset! @@ -15,6 +16,7 @@ def reset! self.service_class = SlackRubyBotServer::Service self.oauth_scope = nil self.oauth_version = :v2 + self.allow_reinstall = false @server_pages = nil self.api = nil diff --git a/lib/slack-ruby-bot-server/service.rb b/lib/slack-ruby-bot-server/service.rb index 8aac577..4a87cbd 100644 --- a/lib/slack-ruby-bot-server/service.rb +++ b/lib/slack-ruby-bot-server/service.rb @@ -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 diff --git a/spec/api/endpoints/teams_endpoint_spec.rb b/spec/api/endpoints/teams_endpoint_spec.rb index 0b7a687..1956269 100644 --- a/spec/api/endpoints/teams_endpoint_spec.rb +++ b/spec/api/endpoints/teams_endpoint_spec.rb @@ -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!) @@ -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 @@ -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." diff --git a/spec/slack-ruby-bot-server/service_spec.rb b/spec/slack-ruby-bot-server/service_spec.rb index 956fc09..3dbbc67 100644 --- a/spec/slack-ruby-bot-server/service_spec.rb +++ b/spec/slack-ruby-bot-server/service_spec.rb @@ -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 = []