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

Conversation

evanpurkhiser
Copy link
Member

@evanpurkhiser evanpurkhiser commented May 13, 2019

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 appropriate scopes depending on if the workspace token app setting is enabled. Otherwise we use bot app scopes. 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, as unfurling cannot be done with just a bot token.

  • We can still pre-link the authorizing user, but we have to make a API request first to receive the users ID.

/cc @Roach @hwz

Fixes #9004

NOTE TO SELF: Do not merge until sentry.io has the associated WST configuration enabled: https://github.com/getsentry/getsentry/pull/2917

@evanpurkhiser evanpurkhiser requested a review from wedamija May 13, 2019 20:27
@evanpurkhiser evanpurkhiser force-pushed the refslack-implement-bot-app-switch branch 2 times, most recently from bc8cc13 to ddb26f8 Compare May 13, 2019 20:34
Copy link
Member

@markstory markstory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we'd encourage users/ourselves to migrate back to a bot account now that workspace apps have been deprecated?

@evanpurkhiser
Copy link
Member Author

evanpurkhiser commented May 13, 2019

Do you think we'd encourage users/ourselves to migrate back to a bot account now that workspace apps have been deprecated?

Ourselves, No:

This is something we've been discussing with the folks over at Slack. Unfortunately because the workspace tokens and bot tokens are different, we would have to ask all of our users to re-authorize their application.

The team over at Slack is working on a solution in the mid-term (I believe) to provide a migration path for workspace token apps back to bot apps.

Users (onprem), Yes:

We should document this change in the on-premise forum post for the slack integration. /cc @jessla

@@ -141,3 +143,109 @@ def test_reassign_user(self):
self.assert_setup_flow(authorizing_user_id='UXXXXXXX2')
identity = Identity.objects.get()
assert identity.external_id == 'UXXXXXXX2'


class SlackIntegrationBotAppTest(IntegrationTestCase):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is almost identical to SlackIntegrationTest, can this be abstracted?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I'll pull out a base class

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.
@evanpurkhiser evanpurkhiser force-pushed the refslack-implement-bot-app-switch branch from ddb26f8 to 1e2a63b Compare May 15, 2019 21:50
@evanpurkhiser evanpurkhiser merged commit f6937d4 into master May 20, 2019
@evanpurkhiser evanpurkhiser deleted the refslack-implement-bot-app-switch branch May 20, 2019 22:13
@ArcticSnowman
Copy link

When will this fix make a release?

@BYK BYK mentioned this pull request Jul 5, 2019
BYK pushed a commit that referenced this pull request Jul 5, 2019
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.
BYK pushed a commit that referenced this pull request Jul 22, 2019
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.
@github-actions github-actions bot locked and limited conversation to collaborators Dec 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Slack Global Integration: oauth error (WST apps deprecated)
4 participants