Record Slack app permission scopes to allow feature checking logic
Problem
Some new Slack app features require new granular bot permission scopes to be added to our Slack app. We do not currently know what permission scopes a workspace has authorized.
This has UX and backend implications, as we currently cannot:
- UX Use conditional logic in the UI to display upgrade notices, or toggle the display of sections of the integration form that require certain scopes to first have been authorized before they will work.
- backend Make checks on the backend for certain permissions before we make an API call which is destined to fail.
Solution
When we release features that require a new granular bot permission scope to our Slack app, we need to track what permission scopes the workspace has authorized. This information can then be used by the backend and passed to the frontend.
Technical Proposal
Track the list of authorized scopes
We would save the scope list against the bot token in the SlackIntegration
record within SlackApplicationInstallService#execute
.
In conversation with Adam Marinelli (Slack), they pointed out that Slack API requests return a list of authorized scopes for the bot token used in the request in the response headers.
All API methods return this information but we could use a call to auth.test
as it has a generous rate limit and requires no permission scopes itself.
Example:
BOT_ACCESS_TOKEN = '...' # We can test using the token from https://api.slack.com/apps/A039N0NBYFL/oauth
slack_installation = SlackInstallation.new(bot_access_token: BOT_ACCESS_TOKEN)
response = ::Slack::API.new(slack_installation).post('auth.test', {})
response.headers['x-oauth-scopes'] # => "commands,incoming-webhook,users:read,chat:write,channels:manage"
To save space in the database we could store numbers corresponding to the list's scopes (enum-style), rather than strings.
Making it user-friendly
We would make a map of feature => required scopes
in order to abstract the checks:
SlackIntegration.first.feature_available?(:notifications)
Feature to scopes mapping
Feature | scopes required |
---|---|
Slash commands | commands |
Notifications &8670 (closed) |
chat:write and chat:write.public
|
Note, we only care about the above scopes. When you conduct the test above, response.headers['x-oauth-scopes']
returns a lot more, because it includes all scopes that our development app has ever been configured with, and due to testing this includes many that we don't actually use.
SlackIntegration
records
Mass updating When we update a SlackIntegration
record we would save the refreshed scope list.
Due to unusual modelling, where we can have multiple SlackIntegration
records for a single workspace, all of which have the same bot token, we would also need to update all other SlackIntegration
records for the workspace (team_id
) with the same updated scope list. We currently do something similar when we update legacy records (ones within bot tokens) for the team, however, in this case, we would also want to update non-legacy records (ones with bot tokens) too. It would be safe to change this line (and this comment) and instead update all SlackIntegration
records for the team (regardless of legacy or non-legacy), however, I think this will require a new index to be added to the table.
SlackIntegration
records
Backfilling old We should backfill this information into SlackIntegration
records that have #bot_access_token
s but no authorized scope information in a background data migration. The migration would similarly call auth.test
to fetch the authorized scopes for each record and save it. The background migration should be written to make small measures of calls to the Slack API at a time, to avoid flooding the Slack API while it runs even though auth.test
has a generous rate limit. We should update records in batches of around 100 at a time, with a pause of around 10 seconds between each batch.
Availability & Testing
Suggestions:
- Write unit tests around the proposed method
Slack::Integration#feature_available?
- Write unit tests to persist the scopes based on a mocked response from Slack
- Alternatively, this would be great behavior to test on the feature level when the backend/frontend work is done. More info here: #376240 (closed)