Skip to content
This repository has been archived by the owner on Sep 20, 2024. It is now read-only.

Add flexibility to Slack auth flow via custom redirect params #343

Merged
merged 1 commit into from
Nov 7, 2016

Conversation

sundeepgupta
Copy link
Contributor

@sundeepgupta sundeepgupta commented Jul 25, 2016

#329

This will enable handling auth flows for various use cases by way of redirect_uri query params.

If we stay with the existing design, I see two approaches:
a) Pass params around and only use them if the Slack App controller is configured with the redirectUri value.
b) Pass the entire redirect URI around. This options gives more flexibility in case apps want to have multiple redirect URI endpoints. This might be useful if they want greater control over their auth flows than what Botkit provides.

For the app we're building, we don't have a use case for b), so I thought to try out a) and get some feedback.

Question (if the approach is ok):
Should the redirect_params be passed into the create/update_team and create_bot events also?

@sundeepgupta sundeepgupta changed the title Proposal on how we could pass redirect params through the auth flow. Add flexibility Slack auth flow by passing custom redirect params Jul 27, 2016
@sundeepgupta
Copy link
Contributor Author

bump, I welcome any comments/questions. Or better yet, a merge if this is good as is :)

@sundeepgupta sundeepgupta changed the title Add flexibility Slack auth flow by passing custom redirect params Add flexibility to Slack auth flow via custom redirect params Jul 27, 2016
@sundeepgupta sundeepgupta force-pushed the redirect_params branch 2 times, most recently from c9764a8 to 1338377 Compare August 14, 2016 16:01
@sundeepgupta
Copy link
Contributor Author

Hey Botkit folks, I welcome any comments/feedback on this. We're using this (via our fork) quite successfully so far.

var redirect_params = {};
if (slack_botkit.config.redirectUri) {
Object.assign(redirect_params, req.query);
delete redirect_params.code
Copy link
Contributor

Choose a reason for hiding this comment

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

missing ;

@sundeepgupta
Copy link
Contributor Author

@anonrig Great thanks for explaining. I've updated the branch to address the feedback.

@sundeepgupta
Copy link
Contributor Author

@anonrig quick bump to see if anything else was needed. I think I've addressed everything, but otherwise, please let me know... thanks.

@peterswimm
Copy link
Contributor

@sundeepgupta sorry for the delay, we'll get on evaluating this and other pending PRs very shortly.

@sundeepgupta
Copy link
Contributor Author

Awesome thanks!

Cheers,
Sundeep

On Aug 29, 2016, at 11:19 AM, Peter Swimm notifications@github.com wrote:

@sundeepgupta https://github.com/sundeepgupta sorry for the delay, we'll get on evaluating this and other pending PRs very shortly.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub #343 (comment), or mute the thread https://github.com/notifications/unsubscribe-auth/ACQWbZqS6gzP1Lq4zhQghDb3vMOZDJtKks5qkvhrgaJpZM4JUPXY.

@sundeepgupta
Copy link
Contributor Author

Thought I'd give this another bump...

Allow passing of custom params into the Slack auth flow. The custom params are passed through the flow and are passed back into the `create_user` and `update_user` events.

howdyai#329
@sundeepgupta
Copy link
Contributor Author

:nudge:

@josephfinlayson
Copy link

Please merge!

@benbrown benbrown merged commit 908171f into howdyai:master Nov 7, 2016
@benbrown
Copy link
Contributor

benbrown commented Nov 7, 2016

AT LONG LAST, merged!

Thanks for your patience, @sundeepgupta

@sundeepgupta
Copy link
Contributor Author

yay!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants