Skip to content

Add slack token provider.#31

Open
cadano wants to merge 9 commits into
mainfrom
slack_token_rotation_provider
Open

Add slack token provider.#31
cadano wants to merge 9 commits into
mainfrom
slack_token_rotation_provider

Conversation

@cadano
Copy link
Copy Markdown

@cadano cadano commented Jul 21, 2025

Add Slack token provider.

@IdanAdar IdanAdar requested review from ArikShifer and motyd July 21, 2025 08:29
@motyd
Copy link
Copy Markdown
Contributor

motyd commented Jul 22, 2025

A few questions:

  • Why is there nothing to do for credentials delete? Can't we revoke the token without waiting for it to expire?
  • What is the purpose of exchange_authentication_code.sh file?

Also a few notes:

  • Seems like there are two main.go files.
  • There are some empty functions/structs that are not references or used as far as I could see (SlackRequest and triggerSecretRotation). Do we need them?
  • Folder does not follow the same pattern as other providers. Let's rename to something like slack-token-provider-go.

@cadano
Copy link
Copy Markdown
Author

cadano commented Jul 22, 2025

@motyd

  1. Its possible, but we didn't want to do it, as we have several replicas and we don't want to revoke the previous token right away, as it might take time until the new token is propagated to all replicas.
  2. exchange_authentication_code.sh - It's part of moving to short-term tokens in Slack, basically, you need to provide a redirect URL which, when the user authorizes your app to be installed the redirect URL is invoked with code. Ideally, the redirect URL will be a nice landing page, and in the background, the server will use the code to fetch the refresh token for every app installation. in our case this is not the flow of the installation as it installed only once in the IBM slack workspace (you need to get approval) so we didn't want to invest time and implement a real redirect URL. so I wrote a script which fetch the code and gets the refresh token (this is a one time action). I can show you tomorrow how it's working.
  3. Look like something with the copy paste to your repo I will check that.
  4. I will rename it.

@ArikShifer
Copy link
Copy Markdown
Member

@cadano

  1. Which slack token type does this provider handles? https://api.slack.com/concepts/token-types. It should be clearly described in the readme file.
  2. Current name slack-rotation-provider-go does not align with provider naming convention. Suggested: slack-<token-type>-provider-go
  3. To qualify as a public provider it should:
    • Implement the delete credentials API. SM API contract maintains the newest 2 versions of a secret and during rotation it will delete version n-2.
    • The provider must support explicit revocation of a slack token when the user is using the SM delete secret version data API.
  4. The description for Obtain initial slack refresh token using the exchange_authentication_code.sh script should be improved and provide best practices for production grade usage. It seems to be missing context to slack documentation and/or screen shots.
  5. Can you schedule an internal demo to walkthrough this provider setup and usage?

@cadano
Copy link
Copy Markdown
Author

cadano commented Jul 27, 2025

@ArikShifer

  1. We are using bot tokens, but i think it will work for any token you want, as they always come as pairs (refresh token, and auth token). The Slack provider doesn't care about the token type.
  2. According to 1, let me know what name you want, and I will change it.
  3. When a new token is generated, Slack automatically revokes the n-2 token (you can always hold only 2 tokens at once, even if they are not expired.) - but I can add delete functionality. I will try to find a time to do it.
  4. Basically, it's a workaround for our usage... it's not related directly to the provider. I can remove it...
  5. I can show you. i will try to find a slot for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants