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

Enable to use app.client with passed token for single workspace apps #947

Closed
4 of 10 tasks
seratch opened this issue May 31, 2021 · 4 comments
Closed
4 of 10 tasks
Assignees
Labels
enhancement M-T: A feature request for new functionality semver:minor
Milestone

Comments

@seratch
Copy link
Member

seratch commented May 31, 2021

Description

tl;dr

tl;dr - I propose that app.client should hold a token value when a fixed single workspace token is passed to App constructor. Refer to #250 to learn the context. If you have comments / feedback on this, please let us know within a few days.

How it currently works

As mentioned in #250, the following does not work as you may expect.

const app = new App({
  token: process.env.SLACK_BOT_TOKEN,
  signingSecret: process.env.SLACK_SIGNING_SECRET,
});
(async () => {
  const authTestResponse = await app.client.auth.test();
  console.log(authTestResponse);
})();

You will see the following error even when you pass a valid token as SLACK_BOT_TOKEN.

(node:95474) UnhandledPromiseRejectionWarning: Error: An API error occurred: not_authed
    at Object.platformErrorFromResult (/Users/ksera/github/bolt-starter/node_modules/@slack/web-api/dist/errors.js:51:33)
    at WebClient.apiCall (/Users/ksera/github/bolt-starter/node_modules/@slack/web-api/dist/WebClient.js:157:28)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)

The reason is that the underlying token in app.client is undefined, while the client in listener arguments holds the passed value. As described here, this has been intentionally done in this line.

Workaround as of v3.3 (not available for TypeScript)

As mentioned in this comment, you can modify the internal token value in JavaScript. However, this approach does not work in TypeScript as the property is read-only.

(async () => {
  app.client.token = process.env.SLACK_BOT_TOKEN;
  const authTestResponse = await app.client.auth.test();
  console.log(authTestResponse);
})();

Why we are going to change

For a cleaner framework design, the team decided to intentionally set undefined token for the app.client as the instance is not yet initialized by an authorize function in Bolt app.

However, we have been receiving questions / requests like #250 (not only in GitHub issues but also in the community workspace) several times. @seratch proposed this internally, and the Bolt maintainer team discussed this topic again. Then, we reached a consensus that, regarding this specific use case (a single workspace app with a fixed access token), we can prioritize the convenience for developers over the framework design policies we had in the early days.

Pros/Cons

These are pros/cons of the change I propose here.

Pros:

  • Obviously, the behavior is more intuitive for developers
  • The Bolt dependency is usable not only for event handling but also for simple Web API calls
  • Bolt can provide a way to use app.client with a given token in TypeScript too

Cons:

  • A bit inconsistency with the cases where App constructor does not have token but has authorize or OAuth settings (the app.client.token exists only when the constructor takes token argument).

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

  • bug
  • enhancement (feature request)
  • question
  • documentation related
  • example code 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.
@seratch seratch added enhancement M-T: A feature request for new functionality semver:minor labels May 31, 2021
@seratch seratch added this to the 3.4.0 milestone May 31, 2021
@seratch seratch self-assigned this May 31, 2021
seratch added a commit to seratch/bolt-js that referenced this issue May 31, 2021
@stevengill
Copy link
Member

This looks great! I wonder if in a token rotation world, we can assign a new token to app.client.token

@stevengill
Copy link
Member

Well I guess this solution is only for single workspace install. Should we consider assigning the token when users are using OAuth too?

@seratch
Copy link
Member Author

seratch commented May 31, 2021

@stevengill

Well I guess this solution is only for single workspace install. Should we consider assigning the token when users are using OAuth too?

In the case of OAuth supported apps, multiple bot tokens / user tokens can be associated with client. Thus, it's not possible to assign a fixed token in the case.

If an app uses OAuth only for bot token rotation along with a single workspace installation, it may be possible to enable app.client to hold refresh token in it. To make this happen, WebClient must have a feature to automatically refresh rotated tokens:

const app = new App({
    refreshToken: '...',
    clientId: '...',
    clientSecret: '...',
});

(async () => {
  // before calling auth.test API, a new access token will be retrieved by the refresh token
  await app.client.auth.test();
})();

@seratch
Copy link
Member Author

seratch commented Jun 1, 2021

I'm thinking to merge #948 by the end of this week. Write in if you have any comments on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement M-T: A feature request for new functionality semver:minor
Projects
None yet
Development

No branches or pull requests

2 participants