diff --git a/src/sentry/conf/server.py b/src/sentry/conf/server.py index cb6aa38699cde2..340bd98a59f3ca 100644 --- a/src/sentry/conf/server.py +++ b/src/sentry/conf/server.py @@ -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 diff --git a/src/sentry/identity/slack/provider.py b/src/sentry/identity/slack/provider.py index c989ae973a2d55..2d08299bf7931f 100644 --- a/src/sentry/identity/slack/provider.py +++ b/src/sentry/identity/slack/provider.py @@ -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 = ( @@ -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(',')), diff --git a/src/sentry/integrations/slack/event_endpoint.py b/src/sentry/integrations/slack/event_endpoint.py index 6d57f7234440bf..8c20b7464a58de 100644 --- a/src/sentry/integrations/slack/event_endpoint.py +++ b/src/sentry/integrations/slack/event_endpoint.py @@ -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 @@ -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({ diff --git a/src/sentry/integrations/slack/integration.py b/src/sentry/integrations/slack/integration.py index 7646c81656ee4e..0926a1132dd7fe 100644 --- a/src/sentry/integrations/slack/integration.py +++ b/src/sentry/integrations/slack/integration.py @@ -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 @@ -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', @@ -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': {}, }, diff --git a/tests/sentry/integrations/slack/test_event_endpoint.py b/tests/sentry/integrations/slack/test_event_endpoint.py index ac40aab2dcc188..92c2ed0cc3ee2e 100644 --- a/tests/sentry/integrations/slack/test_event_endpoint.py +++ b/tests/sentry/integrations/slack/test_event_endpoint.py @@ -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 @@ -40,6 +41,7 @@ }""" +@override_settings(SLACK_INTEGRATION_USE_WST=True) class BaseEventTest(APITestCase): def setUp(self): super(BaseEventTest, self).setUp() @@ -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( diff --git a/tests/sentry/integrations/slack/test_integration.py b/tests/sentry/integrations/slack/test_integration.py index 3a4b52b4f32c2c..cc085d8c403885 100644 --- a/tests/sentry/integrations/slack/test_integration.py +++ b/tests/sentry/integrations/slack/test_integration.py @@ -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 @@ -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) @@ -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, } ) @@ -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' @@ -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, @@ -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' diff --git a/tests/sentry/integrations/slack/test_notify_action.py b/tests/sentry/integrations/slack/test_notify_action.py index a879cb6ed04f46..127ffa5861239b 100644 --- a/tests/sentry/integrations/slack/test_notify_action.py +++ b/tests/sentry/integrations/slack/test_notify_action.py @@ -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)