Skip to content

Commit

Permalink
ref(slack): Implement bot app switch (#13184)
Browse files Browse the repository at this point in the history
Introduces as SLACK_INTEGRATION_USER_WST setting that can be enabled to
use the legacy workspace token apps (which is what sentry.io uses).

 - When installing the integration we now use scopes depending on if
   the workspace token app setting is enabled. Otherwise we use bot
   tokens. NOTE that the WST app scopes have not changed.

 - When using bot tokens we must store the bot token _and_ user token to
   handle unfurling.

 - We can still pre-link the authorizing user, but we have to make a API
   request first to receive the users ID.
  • Loading branch information
evanpurkhiser authored and BYK committed Jul 22, 2019
1 parent 763ab01 commit b11debc
Show file tree
Hide file tree
Showing 7 changed files with 136 additions and 27 deletions.
5 changes: 5 additions & 0 deletions src/sentry/conf/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -1606,3 +1606,8 @@ def get_sentry_sdk_config():
'topic': KAFKA_EVENTS,
},
}

# Enable this to use the legacy Slack Workspace Token apps. You will likely
# never need to switch this unless you created a workspace app before slack
# disabled them.
SLACK_INTEGRATION_USE_WST = False
14 changes: 10 additions & 4 deletions src/sentry/identity/slack/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,9 @@ class SlackIdentityProvider(OAuth2Provider):
key = 'slack'
name = 'Slack'

# TODO(epurkhiser): This identity provider is actually used for authorizing
# the Slack application through their Workspace Token OAuth flow, not a
# standard user access token flow.
oauth_access_token_url = 'https://slack.com/api/oauth.token'
# This identity provider is used for authorizing the Slack application
# through their Bot token (or legacy Workspace Token if enabled) flow.
oauth_access_token_url = 'https://slack.com/api/oauth.access'
oauth_authorize_url = 'https://slack.com/oauth/authorize'

oauth_scopes = (
Expand All @@ -25,11 +24,18 @@ def get_oauth_client_id(self):
def get_oauth_client_secret(self):
return options.get('slack.client-secret')

def get_oauth_data(self, payload):
# TODO(epurkhiser): This flow isn't actually used right now in sentry.
# In slack-bot world we would need to make an API call to the 'me'
# endpoint to get their user ID here.
return super(SlackIdentityProvider, self).get_oauth_data(self, payload)

def build_identity(self, data):
data = data['data']

return {
'type': 'slack',
# TODO(epurkhiser): See note above
'id': data['user']['id'],
'email': data['user']['email'],
'scopes': sorted(data['scope'].split(',')),
Expand Down
9 changes: 8 additions & 1 deletion src/sentry/integrations/slack/event_endpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import re
import six

from django.conf import settings

from sentry import http
from sentry.api.base import Endpoint
from sentry.models import Group, Project
Expand Down Expand Up @@ -58,8 +60,13 @@ def on_link_shared(self, request, integration, token, data):
if not results:
return

if settings.SLACK_INTEGRATION_USE_WST:
access_token = integration.metadata['access_token'],
else:
access_token = integration.metadata['user_access_token'],

payload = {
'token': integration.metadata['access_token'],
'token': access_token,
'channel': data['channel'],
'ts': data['message_ts'],
'unfurls': json.dumps({
Expand Down
48 changes: 40 additions & 8 deletions src/sentry/integrations/slack/integration.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import absolute_import

from django.utils.translation import ugettext_lazy as _
from django.conf import settings

from sentry import http
from sentry.identity.pipeline import IdentityProviderPipeline
Expand Down Expand Up @@ -68,7 +69,12 @@ class SlackIntegrationProvider(IntegrationProvider):
IntegrationFeatures.ALERT_RULE,
])

# Scopes differ depending on if it's a workspace app
identity_oauth_scopes = frozenset([
'bot',
'links:read',
'links:write',
]) if not settings.SLACK_INTEGRATION_USE_WST else frozenset([
'channels:read',
'groups:read',
'users:read',
Expand Down Expand Up @@ -110,25 +116,51 @@ def get_team_info(self, access_token):

return resp['team']

def get_identity(self, user_token):
payload = {
'token': user_token,
}

session = http.build_session()
resp = session.get('https://slack.com/api/auth.test', params=payload)
resp.raise_for_status()
resp = resp.json()

return resp['user_id']

def build_integration(self, state):
data = state['identity']['data']
assert data['ok']

if settings.SLACK_INTEGRATION_USE_WST:
access_token = data['access_token']
user_id_slack = data['authorizing_user_id']
else:
access_token = data['bot']['bot_access_token']
user_id_slack = self.get_identity(data['access_token'])

scopes = sorted(self.identity_oauth_scopes)
team_data = self.get_team_info(data['access_token'])
team_data = self.get_team_info(access_token)

metadata = {
'access_token': access_token,
'scopes': scopes,
'icon': team_data['icon']['image_132'],
'domain_name': team_data['domain'] + '.slack.com',
}

# When using bot tokens, we must use the user auth token for URL
# unfurling
if not settings.SLACK_INTEGRATION_USE_WST:
metadata['user_access_token'] = data['access_token']

return {
'name': data['team_name'],
'external_id': data['team_id'],
'metadata': {
'access_token': data['access_token'],
'scopes': scopes,
'icon': team_data['icon']['image_132'],
'domain_name': team_data['domain'] + '.slack.com',
},
'metadata': metadata,
'user_identity': {
'type': 'slack',
'external_id': data['authorizing_user_id'],
'external_id': user_id_slack,
'scopes': [],
'data': {},
},
Expand Down
3 changes: 2 additions & 1 deletion tests/sentry/integrations/slack/test_event_endpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import json
import responses
from django.test.utils import override_settings

from sentry import options
from sentry.models import Integration, OrganizationIntegration
Expand Down Expand Up @@ -40,6 +41,7 @@
}"""


@override_settings(SLACK_INTEGRATION_USE_WST=True)
class BaseEventTest(APITestCase):
def setUp(self):
super(BaseEventTest, self).setUp()
Expand All @@ -50,7 +52,6 @@ def setUp(self):
external_id='TXXXXXXX1',
metadata={
'access_token': 'xoxp-xxxxxxxxx-xxxxxxxxxx-xxxxxxxxxxxx',
'bot_access_token': 'xoxb-xxxxxxxxx-xxxxxxxxxx-xxxxxxxxxxxx',
}
)
OrganizationIntegration.objects.create(
Expand Down
83 changes: 71 additions & 12 deletions tests/sentry/integrations/slack/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import responses
import six

from django.test.utils import override_settings
from six.moves.urllib.parse import parse_qs, urlencode, urlparse

from sentry.integrations.slack import SlackIntegrationProvider
Expand All @@ -13,7 +14,8 @@
class SlackIntegrationTest(IntegrationTestCase):
provider = SlackIntegrationProvider

def assert_setup_flow(self, team_id='TXXXXXXX1', authorizing_user_id='UXXXXXXX1'):
def assert_setup_flow(self, team_id='TXXXXXXX1', authorizing_user_id='UXXXXXXX1',
access_token='xoxp-xxxxxxxxx-xxxxxxxxxx-xxxxxxxxxxxx', access_extras=None):
responses.reset()

resp = self.client.get(self.init_path)
Expand All @@ -32,14 +34,27 @@ def assert_setup_flow(self, team_id='TXXXXXXX1', authorizing_user_id='UXXXXXXX1'
# easier
authorize_params = {k: v[0] for k, v in six.iteritems(params)}

access_json = {
'ok': True,
'access_token': access_token,
'team_id': team_id,
'team_name': 'Example',
'authorizing_user_id': authorizing_user_id,
}

if access_extras is not None:
access_json.update(access_extras)

responses.add(
responses.POST, 'https://slack.com/api/oauth.token',
responses.POST, 'https://slack.com/api/oauth.access',
json=access_json,
)

responses.add(
responses.GET, 'https://slack.com/api/auth.test',
json={
'ok': True,
'access_token': 'xoxp-xxxxxxxxx-xxxxxxxxxx-xxxxxxxxxxxx',
'team_id': team_id,
'team_name': 'Example',
'authorizing_user_id': authorizing_user_id,
'user_id': authorizing_user_id,
}
)

Expand Down Expand Up @@ -74,8 +89,50 @@ def assert_setup_flow(self, team_id='TXXXXXXX1', authorizing_user_id='UXXXXXXX1'
self.assertDialogSuccess(resp)

@responses.activate
def test_basic_flow(self):
self.assert_setup_flow()
def test_bot_flow(self):
self.assert_setup_flow(
access_token='xoxa-xxxxxxxxx-xxxxxxxxxx-xxxxxxxxxxxx',
access_extras={
'bot': {
'bot_access_token': 'xoxb-xxxxxxxxx-xxxxxxxxxx-xxxxxxxxxxxx',
}
})

integration = Integration.objects.get(provider=self.provider.key)
assert integration.external_id == 'TXXXXXXX1'
assert integration.name == 'Example'
assert integration.metadata == {
'access_token': 'xoxb-xxxxxxxxx-xxxxxxxxxx-xxxxxxxxxxxx',
'user_access_token': 'xoxa-xxxxxxxxx-xxxxxxxxxx-xxxxxxxxxxxx',
'scopes': sorted(self.provider.identity_oauth_scopes),
'icon': 'http://example.com/ws_icon.jpg',
'domain_name': 'test-slack-workspace.slack.com',
}
oi = OrganizationIntegration.objects.get(
integration=integration,
organization=self.organization,
)
assert oi.config == {}

idp = IdentityProvider.objects.get(
type='slack',
external_id='TXXXXXXX1',
)
identity = Identity.objects.get(
idp=idp,
user=self.user,
external_id='UXXXXXXX1',
)
assert identity.status == IdentityStatus.VALID

@override_settings(SLACK_INTEGRATION_USE_WST=True)
def assert_wst_setup_flow(self, *args, **kwargs):
self.assert_setup_flow(*args, **kwargs)

@responses.activate
@override_settings(SLACK_INTEGRATION_USE_WST=True)
def test_wst_flow(self):
self.assert_wst_setup_flow()

integration = Integration.objects.get(provider=self.provider.key)
assert integration.external_id == 'TXXXXXXX1'
Expand Down Expand Up @@ -104,9 +161,10 @@ def test_basic_flow(self):
assert identity.status == IdentityStatus.VALID

@responses.activate
@override_settings(SLACK_INTEGRATION_USE_WST=True)
def test_multiple_integrations(self):
self.assert_setup_flow()
self.assert_setup_flow(team_id='TXXXXXXX2', authorizing_user_id='UXXXXXXX2')
self.assert_wst_setup_flow()
self.assert_wst_setup_flow(team_id='TXXXXXXX2', authorizing_user_id='UXXXXXXX2')

integrations = Integration.objects.filter(provider=self.provider.key)

Expand All @@ -131,11 +189,12 @@ def test_multiple_integrations(self):
assert identities[0].idp != identities[1].idp

@responses.activate
@override_settings(SLACK_INTEGRATION_USE_WST=True)
def test_reassign_user(self):
self.assert_setup_flow()
self.assert_wst_setup_flow()
identity = Identity.objects.get()
assert identity.external_id == 'UXXXXXXX1'

self.assert_setup_flow(authorizing_user_id='UXXXXXXX2')
self.assert_wst_setup_flow(authorizing_user_id='UXXXXXXX2')
identity = Identity.objects.get()
assert identity.external_id == 'UXXXXXXX2'
1 change: 0 additions & 1 deletion tests/sentry/integrations/slack/test_notify_action.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ def setUp(self):
external_id='TXXXXXXX1',
metadata={
'access_token': 'xoxp-xxxxxxxxx-xxxxxxxxxx-xxxxxxxxxxxx',
'bot_access_token': 'xoxb-xxxxxxxxx-xxxxxxxxxx-xxxxxxxxxxxx',
}
)
self.integration.add_organization(event.project.organization, self.user)
Expand Down

0 comments on commit b11debc

Please sign in to comment.