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

Update oauth_v2_access to include redirect_uri #652

Merged
merged 3 commits into from May 17, 2020

Conversation

tomasreimers
Copy link
Contributor

Summary

Following the docs here, we want to add the redirect URI as a named parameter: https://api.slack.com/legacy/oauth

Requirements (place an x in each [ ])

Following the docs here, we want to add the redirect URI as a named parameter: https://api.slack.com/legacy/oauth
"""
kwargs.update({"code": code})
kwargs.update({"code": code, "redirect_uri": redirect_uri})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A potential gotcha: while redirect_uri is optional, it is required if your app passed it as a parameter to oauth/authorization in the first step of the OAuth flow.

https://api.slack.com/methods/oauth.v2.access#arg_redirect_uri

As described in the above page, it's also possible to omit the redirect_uri parameter when starting OAuth flow. If this named arg is optional, I'm fine with this change for more developer-friendliness.

@seratch
Copy link
Member

seratch commented Apr 17, 2020

We have two endpoints as below. Applying the same to oauth.access is preferred for consistency.

@seratch seratch added enhancement M-T: A feature request for new functionality Version: 2x web-client labels Apr 17, 2020
@tomasreimers
Copy link
Contributor Author

Thank you @seratch , and apologies about being slow to update, work was a lot this month.

Went ahead and added it to both methods as well as making the param optional, lmk if that's what you were thinking :)

@seratch seratch merged commit 6f85a9d into slackapi:master May 17, 2020
@seratch
Copy link
Member

seratch commented May 17, 2020

@tomasreimers Thanks - I've fixed minor warnings by flake8 and merged your changes!

@seratch seratch added this to the 2.6.0 milestone May 17, 2020
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 Version: 2x web-client
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants