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

App is initialized without WebClient readonly token #250

Closed
4 of 9 tasks
SpencerKaiser opened this issue Sep 12, 2019 · 14 comments
Closed
4 of 9 tasks

App is initialized without WebClient readonly token #250

SpencerKaiser opened this issue Sep 12, 2019 · 14 comments
Labels
discussion M-T: An issue where more input is needed to reach a decision question M-T: User needs support to use the project
Milestone

Comments

@SpencerKaiser
Copy link

Description

After initializing an App, app.client.token is undefined, which forces the user to rely on env variables when using certain WebApi components (such as chat.postMessage).

What type of issue is this? (place an x in one of the [ ])

  • bug
  • enhancement (feature request)
  • question
  • documentation related
  • testing related
  • discussion

Requirements (place an x in each of the [ ])

  • I've read and understood the Contributing guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • I've searched for any related issues and avoided creating a duplicate issue.

Bug Report

Steps to reproduce:

  1. Create an app
  2. Access app.client.token

Expected result:

app.client.token should have the same value as the Slack Bot Token provided to the constructor.

Actual result:

app.client.token is undefined

Workaround

import { WebClient } from '@slack/web-api';

// Initialize your app
const app = new App({
  signingSecret: process.env.SLACK_SIGNING_SECRET,
  token: process.env.SLACK_BOT_TOKEN
});

// Re-initialize the WebClient
app.client = new WebClient(process.env.SLACK_BOT_TOKEN);
@shaydewael
Copy link
Contributor

This is intended behavior because, by default, Bolt will use the SingleTeamAuthorization() authorization function which is run every time that there is an incoming event. This design allows you to swap out the Authorization function and set the token for any API calls on a per-event basis (in case your app is installed on multiple workspaces).

However, the bot token is available in any middleware by accessing context.botToken. Does your app have use cases where you have where you need to access the bot token outside of a middleware function?

@SpencerKaiser
Copy link
Author

I actually don’t need to access the bot token, I need to access the other token... there are a handful of endpoints that require a user-generated token instead of a bot token (for example, channels.rename)

@SpencerKaiser
Copy link
Author

Ah wait, I just conflicted my original description... let me look at where I’m using that and determine why I did that.

@shaydewael
Copy link
Contributor

Oh! For user tokens you can pass that into the constructor with userToken: 'YOUR-TOKEN' and access it with context.userToken

@SpencerKaiser
Copy link
Author

@shanedewael alright I just had a chance to test... I have a group member cache that I update every 15 minutes for a specific private channel (it's basically my auth for whether someone should be able to use the Slash command we have). Here's my code for making the conversations.members request.

const channelInfo = await this.app.client.conversations.members({
  channel: announcementChannel,
  token: this.app.client.token,
  cursor: nextPage,
});

So definitely ignore my stuff about the bot/user token, this was just an issue of me needing to do something and wanting to use my app singleton to do it (and because I didn't want to include @slack/web-api as a separate dependency). Is there a better way to do this? I love having a singleton app I can use throughout my code (and the implementation is really readable) and I want to be able to leverage the bolt functionality for that component of the Web API.

@shaydewael
Copy link
Contributor

This brings up an interesting point. I'm assuming you're running this outside of any of the built-in methods and in some kind of timer loop?

The design decision that poses the problem is implied a little bit in my previous comment. To support apps that are installed on multiple workspaces, we run the authorization function and tie the token to the individual HTTP request, rather than tying tokens to the actual WebClient (this works because token is optional, so we just leave it out).

Is your app meant to installed on multiple workspaces? If it is installed on multiple workspaces, what value would you expect this token to be? And if it's only installed on one workspace, is it unexpected to just pass the token you used to initialize Bolt?

I'm happy to help brainstorm solutions on the Bolt side, but I'm not sure I fully understand the problem.

@seratch seratch added discussion M-T: An issue where more input is needed to reach a decision question M-T: User needs support to use the project labels Jan 4, 2020
@billyvg
Copy link

billyvg commented Feb 11, 2021

@shaydewael Sorry to dig this issue up, but I'm running into this where my app is only installed on a single workspace. I just swapped out web-api in favor of bolt and didn't expect that I needed to pass token when I used client outside of an event. Will I be breaking things if I use the workaround presented in the original post?

@shaydewael
Copy link
Contributor

@billyvg No worries bringing this up - that really shouldn't break anything if it's only installed on one workspace. Though I just tested this and it works fine and is a bit cleaner:

const app = new App({
  token: process.env.SLACK_BOT_TOKEN,
  signingSecret: process.env.SLACK_SIGNING_SECRET,
  ignoreSelf: false
});

app.client.token = process.env.SLACK_BOT_TOKEN;

You still have to instantiate it with the same token because it verifies it exists within the constructor, but I was able to use it outside of event listeners. Although I'll tack on a warning that I haven't thoroughly looked through the code to see if there could be unexpected behavior like the token getting overridden at some point.

Part of me wonders if we should offer a configuration flag that tells Bolt to instantiate the internal client with a token rather than only associating the token when an event comes in. Or just allowing folks to pass in a token to the WebClientOptions constructor object which would overwrite the default behavior.

@billyvg
Copy link

billyvg commented Feb 11, 2021

@shaydewael Thanks, that is cleaner! It would be nice to have an api for this, but judging from this issue activity, doesn't seem like it affects that many others?

It was a bit frustrating trying to figure out why my calls were un-authed. Is there a case where people use the un-authed client?

@shaydewael
Copy link
Contributor

@billyvg No, there are very few methods you can call without authentication, but folks will often pass in the token to the individual API calls (which seems fine if you're calling the API a few times outside of listeners). I can't speak for the whole team, but I think the basis of the decision is something like:

  1. Developers may be passing user or bot tokens interchangeably. We should leave explicit flexibility for devs using this (and other) use cases.
  2. It would go against the token model within the listeners, which could cause unexpected/unpredictable behavior when devs try to use it outside of a listener. And again, there may be repercussions to changing the token at the top level.

That said, we don't see a large sect of devs using a client outside of listeners while using Bolt, or if they do it's in a separate context so they'll just instantiate a different client using WebClient. I'm not sure how thoroughly this kind of pattern has been thought through—there is a real possibility that we just don't have enough visibility into those use cases. If those surfaces, it may be worthwhile to think through this a bit more.

@SpencerKaiser
Copy link
Author

@shaydewael I'm so sorry I didn't respond to your question way back! No it wasn't in some sort of a timer loop, the need to do this on my end was really just a side effect of the response time requirements... with several of the apps I've written, I'm so dependent on 3rd party services/connections that I can't be 100% certain I'll be able to respond in time, so I take the approach above ☹️

Definitely not ideal and I've had a few discussions in the Slack Community workspace and with the developer advocacy team about it. This is a whole separate can of worms, but I'd love to have extra flexibility on that side of things to continue processing but give the user a heads up that it's taking longer than normal.

Back to the discussion above; isn't adding that ignoreSelf flag going to allow the user to override other readonly values? And to your point, I would love to see a singleWorkspace flag in the app constructor!!!

For more context, 99% of the apps I write are to help folks at my company, which means they're always single workspace apps and they're always apps that I setup directly without submitting through the formal Slack app submission processes. I'm sure a lot of your users are in that bucket (although many may be in grid and that changes things I guess), so I'd love to see y'all research that flow a bit more 🙂

@seratch
Copy link
Member

seratch commented Mar 19, 2021

This is not a direct answer to this issue but, with the latest version of Bolt, theclient argument in middleware/listeners holds a valid token unlike app.client.

const token = process.env.SLACK_BOT_TOKEN;
const app = new App({token, ...});
app.action("action-id", async ({ client, ack }) => {
  await ack();

  await client.auth.test();
  // the above code is equivalent to the following
  // await app.client.auth.test({token});
});

I personally do understand the lack of token in app.client in the case where you pass token in App constructor could be confusing but current recommendation is to always use client listener/middleware argument instead.

@SpencerKaiser
Copy link
Author

Thanks for the extra info! I'll see if we can use that to simplify some of our logic.

That still doesn't address the use case for an app that will only ever be in a single workspace, it would be super convenient to just use app within a file that isn't a Slack handler of any sort (like a listener for Webhook events from another service). Not the end of the world, just unclear why that value is readable if it isn't set.

@seratch
Copy link
Member

seratch commented Jun 2, 2021

Hi @SpencerKaiser, thanks for waiting for a long time! This issue will be resolved in the next minor version (v3.4.0), which will be out within a few days. Refer to #947 #948 for details and the context behind the change. Let us know at #947 if you have comments / feedback on it 👋

For this reason, let me add this to v3.4 milestone and close it now.

@seratch seratch closed this as completed Jun 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion M-T: An issue where more input is needed to reach a decision question M-T: User needs support to use the project
Projects
None yet
Development

No branches or pull requests

5 participants