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

botId required for authorization, but users.info not immediately consistent after oauth.access returns #315

Closed
4 of 9 tasks
ianmacartney opened this issue Nov 21, 2019 · 9 comments
Labels
discussion M-T: An issue where more input is needed to reach a decision

Comments

@ianmacartney
Copy link

ianmacartney commented Nov 21, 2019

Description

Describe your issue here.

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

Filling out the following details about bugs will help us solve your issue sooner.

Reproducible in:

package version: 1.4.1

node version: 13.1.0

OS version(s): 10.15.1

Steps to reproduce:

  1. implement a custom oauth handler
  2. on oauth success, immediately try to fetch the users.info using the newly acquired bot.bot_access_token and bot.bot_user_id.

Expected result:

I expect to get back the bot user's info to get the bot_id from user.profile.bot_id.

Actual result:

I get a user_not_found response. Trying again a second later, it works.
Not knowing how it works under the hood, my suspicion is that this is an issue of eventual consistency. I may be hitting one server that is doing reads on an out of date database after the oauth server did the write.

My workaround is to lazily fetch the bot_id in the authorize function if it's missing, and then update the database. This could lead to database contention and is non-ideal.

Other thoughts:

I would prefer to get the bot_id in the oauth success handler alongside bot_user_id, and not need to do another call.

I would also like it made clear that botId doesn't need to be returned from authorize if you pass ignoreSelf: false in initialization (I think this is true but haven't tested it). I don't need ignoreSelf for my use case, so I could have saved myself a lot of trouble if I could have skipped this debugging session.

#196 is related to this, but they didn't mention the problem of not being able to immediately access this info.

@ianmacartney ianmacartney changed the title botId required for authorization, but users.info not immediately consistent from oauth.access botId required for authorization, but users.info not immediately consistent after oauth.access returns Nov 21, 2019
@seratch
Copy link
Member

seratch commented Nov 28, 2019

I get a user_not_found response. Trying again a second later, it works. Not knowing how it works under the hood, my suspicion is that this is an issue of eventual consistency.

I've never experienced this but, as you guessed, it looks like a consistency issue on the server-side. I hate to say this, but Slack apps have to have a workaround like the one you did to deal with the problem.

I would prefer to get the bot_id in the oauth success handler alongside bot_user_id, and not need to do another call.

Indeed, it's better to have bot_id in the response. I don't have anything to share about this at the moment but will share this feedback internally.

I would also like it made clear that botId doesn't need to be returned from authorize if you pass ignoreSelf: false in initialization (I think this is true but haven't tested it). I don't need ignoreSelf for my use case, so I could have saved myself a lot of trouble if I could have skipped this debugging session.

I believe ignoreSelf: false should work as you expect.

@seratch seratch added the discussion M-T: An issue where more input is needed to reach a decision label Nov 28, 2019
@seratch
Copy link
Member

seratch commented Jan 30, 2020

Update:
bot_id is still not available in the response of oauth.access and oauth.v2.access. But auth.test recently started returning bot_id in the case a given token is a bot token.

Slack apps need to run an additional API call (= auth.test instead of users.info) after oauth.v2.access call but the scope users:read is no longer required.

@pertschuk
Copy link

@seratch thanks for the update -

so to clarify does replacing users.info call with auth.test solve the consistency issue?

@seratch
Copy link
Member

seratch commented Apr 1, 2020

@pertschuk I believe so. As I haven't reproduced your situation yet, I'm not 100% sure though. auth.test should behave more firmly.

@Oghenebrume50
Copy link

@seratch so I had this issue some days back and the comment here helped me, auth.test would actually send the bot_id as part of the object returned, is it possible to update the documentation to reflect this, I have seen more than 4 issues around authorization, this , this , this and some others, I feel the details on here is not clear enough, after struggling for days with help from the issues above, I would like to make this contribution if I can be pointed to the direct direction, I can open a pull request to make the authorization flow clearer.
Also, this is my first time here so if it is inappropriate to point this out here, I can open a separate issue for it.

@seratch
Copy link
Member

seratch commented Jun 9, 2020

Bolt v2.1 came out with its built-in OAuth support almost three weeks ago.

The module provides a much smoother way to implement OAuth flows so that developers no longer deal with the details mentioned in this issue.

If you have feedback on the OAuth module, let us know in a new GitHub issue! Allow me to close this issue now.

@seratch seratch closed this as completed Jun 9, 2020
@abdel-ships-it
Copy link

@seratch Can you please expose Installation interface which is required by fetchInstallation ? There are a lot of interfaces that some methods require which are not exposed by the bolt sdk :/ This forces typescript users to cast to any. The View interface is a good example of this.

@stevengill
Copy link
Member

@realappie that is a good suggestion. Can you create a new issue for exporting Installation? For now, you could import the Installation interface from the @slack/oauth package directly.

@stevengill
Copy link
Member

@realappie I have a PR open where I am exporting Installation in bolt-js. #586

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
Projects
None yet
Development

No branches or pull requests

6 participants