Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ref(slack): Implement bot app switch #13184

Merged
merged 1 commit into from
May 20, 2019
Merged
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
5 changes: 5 additions & 0 deletions src/sentry/conf/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -1608,3 +1608,8 @@ def get_sentry_sdk_config():
'topic': KAFKA_OUTCOMES,
},
}

# 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 @@ -133,11 +191,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