Slack integration doesn't work with the new rotating tokens

ricsipontaz commented 2 years ago

Version

21.12.0

Steps to Reproduce

Create a new slack app (with rotating tokens) for Sentry
Setup it on Sentry

Expected Result

It should handle the changed token

Actual Result

It didn't handle it:

08:12:30 [INFO] sentry.integrations.client: integration.http_response (integration='slack' status_string='200') 08:12:30 [INFO] sentry.integrations.slack: rule.slack.conversation_info_failed (error='token_expired')
Traceback (most recent call last): File "/usr/local/lib/python3.8/site-packages/sentry/integrations/slack/utils/channel.py", line 85, in validate_channel_id results = client.get("/conversations.info", headers=headers, params=payload)
File "/usr/local/lib/python3.8/site-packages/sentry/shared_integrations/client.py", line 305, in get return self.request("GET", *args, **kwargs) File "/usr/local/lib/python3.8/site-packages/sentry/integrations/slack/client.py", line 83, in request raise ApiError(response.get("error", "")) sentry.shared_integrations.exceptions.ApiError: token_expired

aminvakil commented 2 years ago

I have not used this feature and don't know if it works or not, but here is the appropriate place to open issue: https://github.com/getsentry/sentry/issues/new?assignees=&labels=Component:%20Integrations&template=bug.yml&title=Slack%20Integration%20Problem

@chadwhitacre If I'm right this can get moved to getsentry/sentry, right?

chadwhitacre commented 2 years ago

Yeah, let's move over there so the Ecosystem team can take a look, if it turns out to be self-hosting specific we can move back here. 👍

getsentry-release commented 2 years ago

Routing to @getsentry/ecosystem for triage. ⏲️

leeandher commented 2 years ago

Thanks for bringing this to us! I've made a note and will be adding it to our backlog.

In the mean time, please avoid "Opting In" to token rotation on the Slack app you wish to integrate with Sentry. If you've already done so, there isn't a way to opt out so you'll have to make a new Slack App and provide your sentry instance with a new Client ID, Client Secret and Signing Secret. More info here.

I'll be keeping this open in case anyone seeing this has similar issues, or wishes us to re-prioritize this feature.

mmospanenko commented 1 year ago

We have the same problem, after one day Slack integration work stops, and I see token_expired in the logs. Any workarounds?

https://api.slack.com/authentication/rotation#turn

if I understood correctly, I should recreate App without token rotation feature

mikejihbe commented 7 months ago

I believe this is related to hybrid cloud. We purposely removed the oauth proxy for Slack to avoid some high-traffic complexity, but it looks like it might be required now that Slack actually rotates tokens. cc @markstory

markstory commented 7 months ago

I believe this is related to hybrid cloud. We purposely removed the oauth proxy for Slack to avoid some high-traffic complexity, but it looks like it might be required now that Slack actually rotates tokens.

I'm not sure it is. Outside of any integration proxy work we did for multi-region, our slack clients don't have any logic to store or handle refresh token, or the refresh flow. That would be net new behavior that would be required.

getsentry / sentry