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

Using the built-in OAuth, how can I access the rest of the fetched Installation from a listener? #685

Open
3 tasks done
aoberoi opened this issue Nov 21, 2020 · 14 comments
Open
3 tasks done
Labels
enhancement M-T: A feature request for new functionality semver:major
Milestone

Comments

@aoberoi
Copy link
Contributor

aoberoi commented Nov 21, 2020

Description

The built in OAuth is super convenient and great! It fetches the appropriate Installation from my InstallationStore and uses the tokens in that installation to authorize the incoming events. It also puts the relevant User ID and Bot User ID in context so I can access it from middleware and listeners.

But the Installation has so much more in it than just those properties. For example, the installed scopes are stored, an incoming webhook may be stored, etc. If I wanted to access these, I'd currently have to perform installationStore.fetchInstallation() again inside my listener. The framework did that already though, so that's a waste.

This could be solved in probably a few different ways, but I think one of the simplest would just be to add a new (optional) property called installation on AuthorizeResult. The implementation of authorize() that the built-in OAuth library uses would set that property to the whole Installation it got from fetchInstallation(). Any custom authorize() implementations could also set this value. That installation property would then be added to the context, just like botToken, userToken, etc. Then listeners and middleware could use any installation data.

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.
@stevengill
Copy link
Member

I like this idea! So developers could access the entire installation object via context.installation..

@seratch seratch removed the untriaged label Nov 30, 2020
@seratch seratch added this to the 4.0.0 milestone Mar 23, 2021
@willyxiao
Copy link

willyxiao commented May 9, 2021

Ah, just posted about something similar here: https://stackoverflow.com/questions/67453098/slackbolt-app-how-can-i-get-scopes-associated-with-the-current-request.

Is this the same issue / will this resolve it?

The main use case I care about now is if I want to add permissions over the course of many months to my app (but I don't want to force companies/users to install when I do the update, I can manage that separately), I want a way to basically know which teamIds have which scopes.

Also if there's a workaround in the meantime or a suggested way of doing this, I'd love to hear it! I haven't found anything yet on the public internets :).

maybe.... cc: @seratch will know?

@seratch
Copy link
Member

seratch commented May 9, 2021

Hi @willyxiao, thanks for sharing your use case. Yes, if we resolve this issue, the update may be a solution for you. However, we haven't decided how/when we enhance the installation data access yet.

A workaround I can suggest as of today would be defining our own authorize function that utilizes installation store. With this way, you can add any additional fields to the context object. Refer to https://slack.dev/bolt-js/concepts#authorization for learning how to implement it.

For the authorize with custom fields, I need to mention a limitation with TypeScript. If you write your app in TS, the AuthorizeResult interface is not yet customizable as a type.

@willyxiao
Copy link

willyxiao commented May 10, 2021

Thank you SO much! Super helpful. @seratch one question - the current default authorize function looks like it's making a call to slack (via client.auth.test on line 890 in App.ts).

  1. Is that what's ultimately calling fetchInstallation as I see in my logs, or am I misunderstanding? If I am, where is fetchInstallation called now? I just want to understand what default behavior, so that I can emulate the parts that I need when I override it with a new authorize function.

  2. Kind of on a separate note, it kind of seems like I can pass tokenVerificationEnabled = false and that should actually improve my response times / decrease latency if I keep the default authorize function. Especially if ack() is called after 3 seconds every once in a while in our case, passing that option in tokenVerificationEnabled should actually have an effect of making ack succeed more often, right?

@seratch
Copy link
Member

seratch commented May 10, 2021

@willyxiao

the current default authorize function looks like it's making a call to slack (via client.auth.test on line 890 in App.ts).

This is only for the case where you pass a token in App constructor. In this case, your app works only for a single workspace , so that it uses singleAuthorization for its authorize. This is not the case you mentioned here.

bolt-js/src/App.ts

Lines 359 to 367 in 9867e38

this.authorize = singleAuthorization(
this.client,
{
botId,
botUserId,
botToken: token,
},
tokenVerificationEnabled,
);

Is that what's ultimately calling fetchInstallation as I see in my logs, or am I misunderstanding? If I am, where is fetchInstallation called now?

If your app enables OAuth and its installation store functions, Bolt uses the HTTPReceiver's default authorize function for OAuth:

this.authorize = (this.receiver as HTTPReceiver).installer!.authorize;
This authorize function calls fetchInstallation internally.

tokenVerificationEnabled

tokenVerificationEnabled is a flag option for turning the eager verification of the given token value in App constructor on/off. Therefore, this is not an option for you. This option works only with the bult-in singleAuthorization.

Although it's not active recently, this is an issue for bolt-js's enhancement discussion. If you have followup questions or related ones, would you mind creating a new issue for your question or asking them in the Slack Platform Community workspace? In the community workspace, #lang-javascript #tools-bolt would be good places to have this type of Q&A. I would appreciate it if you could understand this!

@anthonygualandri
Copy link

What is supposed to be returned exactly from the fetchInstallation call if using a publically installed app with Oauth? I am returning the authorized bot user token stored in my database as a string for the user invoking the app, but I get the error below:

[WARN] bolt-app Authorization of incoming event did not succeed. No listeners will be called.
[ERROR] bolt-app Error: Cannot read property 'token' of undefined

@seratch
Copy link
Member

seratch commented Jan 11, 2022

@anthonygualandri
Please refer to the document: https://slack.dev/bolt-js/concepts#authorization

To know the full list of available properties, you can check the AuthroizeResult interface here: https://github.com/slackapi/bolt-js/blob/%40slack/bolt%403.8.1/src/App.ts#L113-L139

@anthonygualandri
Copy link

Thanks @seratch and I'm so sorry, but I'm confused. The spot in the docs you sent me to says "If you’re using the built-in OAuth support authorization is handled by default, so you do not need to pass in an authorize option." Can it be something else here instead that you can point me to?

@seratch
Copy link
Member

seratch commented Jan 12, 2022

@anthonygualandri Oh, I'm sorry. Somehow, I assumed that you've asked about authorize function. Please check this interface to learn the expected data structure as a returned value form fetchInstallation method: https://github.com/slackapi/node-slack-sdk/blob/%40slack/oauth%402.3.0/packages/oauth/src/index.ts#L636-L700

@anthonygualandri
Copy link

Thanks @seratch, this is exactly what I needed and helped me get the right return values into the right data structure to get the app installing / loading properly. Will look to the code in the package in the future for anything that isn't detailed out in the Bolt concepts guide.

@LeoDOD
Copy link

LeoDOD commented May 24, 2022

is there an estimated release date for 4.x?

@acerbisgianluca
Copy link

Hey there, I got here because I'd like to access the team name from the installation object without getting it again from the DB.

The "problem" as you said is that the authorize function get the whole installation, but it only returns the fields needed by the AuthroizeResult interface, so I basically need to override that method.
The fact is that all the properties and utility methods are private, so I ended up basically rewriting the whole InstallProvider class because I can't access those methods from the subclass I created to override the authorize method.

Have you came up with a better solution that doesn't involve an useless database query?

@filmaj
Copy link
Contributor

filmaj commented Sep 12, 2024

Wondering if this one needs to be set to the next major version milestone - @seratch thoughts? Could this be a minor update? Could we simply expose an additional parameter with the entire object, as discussed, to the relevant functions? E.g. have the @slack/oauth library expose the entire installation object, then ensure bolt exposes that too?

@seratch
Copy link
Member

seratch commented Sep 13, 2024

@filmaj Simlpy allowing installation?: Installation in AuthorizeResult is a qiuck solution here, but I am not sure if it's a great design yet. If we do it, it requires @slack/oauth's change as well. Say, oauth 3.1 + bolt 3.22, 4.1 or whatever. The reason why I set 4.0 milestone was that simply we were not planning to make immediate changes for this. If you think 4.0 can come within a few weeks, you can set 4.1 milestone to this issue for now. Including this enhancement in 4.0 as well is an option too, tho.

@filmaj filmaj modified the milestones: 4.0.0, 4.x Sep 27, 2024
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:major
Projects
None yet
Development

No branches or pull requests

8 participants