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

Add test to confirm clientOptions get passed to web-api #1083

Closed
4 tasks done
stevengill opened this issue Aug 20, 2020 · 4 comments · Fixed by #1359
Closed
4 tasks done

Add test to confirm clientOptions get passed to web-api #1083

stevengill opened this issue Aug 20, 2020 · 4 comments · Fixed by #1359
Assignees
Labels
good first issue pkg:oauth applies to `@slack/oauth-helper` tests M-T: Testing work only
Milestone

Comments

@stevengill
Copy link
Member

Description

For @slack/oauth, clientOptions should be getting passed to @slack/web-api. Lets add a test for this.

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

  • testing related

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 stevengill added tests M-T: Testing work only pkg:oauth applies to `@slack/oauth-helper` labels Aug 20, 2020
@seratch seratch added this to the oauth@2.1 milestone Mar 24, 2021
@seratch seratch modified the milestones: oauth@2.1, oauth@2.2 Jun 3, 2021
@seratch seratch modified the milestones: oauth@2.2, oauth@2.3 Jul 13, 2021
@filmaj filmaj self-assigned this Aug 10, 2021
@filmaj
Copy link
Contributor

filmaj commented Aug 10, 2021

I will give this issue a shot 🤠

A question: is it expected that the handleCallback method could potentially run an "auth test" using both a bot and a user token in one invocation of handleCallback? I ask this because the way the tests run through the v2 success callback test scenario currently, I think due to the way the response is mocked out, both a access_token for a bot user as well as an authed_user access token are provided. Is this expected?

If it is, then I can expand these tests to verify that the WebClientOptions as well as the the correct access token type(s) are forwarded along during API calls.

Thanks for any info!

@stevengill
Copy link
Member Author

hmm. It seems like this code could use a bit of improvement. I think the second authResult in the section i highlighted should be moved into the if statement.

          if (v2Resp.is_enterprise_install && v2Installation.enterpriseUrl === undefined) {
            const authResult = await runAuthTest(v2Resp.authed_user.access_token, this.clientOptions);
            v2Installation.enterpriseUrl = authResult.url;
          }

I don't think authResult should ever have to run more than once.

@seratch seratch modified the milestones: oauth@2.3, oauth@2.4 Sep 12, 2021
@filmaj
Copy link
Contributor

filmaj commented Sep 13, 2021

@stevengill when you say "the second authResult in the section I highlighted", which one do you mean? I see three authResult constructions:

@stevengill
Copy link
Member Author

Move 472 to 475.

The v2 user install flow. Being in that if statement would mean it would only run in the circumstance that it is a v2 app and no bot token (only user token).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue pkg:oauth applies to `@slack/oauth-helper` tests M-T: Testing work only
Projects
None yet
3 participants