-
Notifications
You must be signed in to change notification settings - Fork 757
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
Support OAuth flow for Connect accounts #555
Conversation
Tested this via the steps at https://stripe.com/docs/connect/standard-accounts and the access code redeemed correctly for a token. |
Hey @robz-stripe, thanks for taking a stab at this! This looks like a great start. Before we merge this however, I think we need some additional features for bring this to parity with our other libraries:
If you haven't already, I recommend you take a look at the implementations in other languages, particularly stripe-ruby's: https://github.com/stripe/stripe-ruby/blob/master/lib/stripe/oauth.rb. (If I'm not mistaken, stripe-ruby is the only one that supports Express accounts right now.) Also, very minor nit, but would you mind changing the capitalization from |
r? @ob-stripe This implements |
Making a note here to update https://github.com/stripe/stripe-node/wiki/Using-Stripe-Connect-with-node.js once the interface is finalized. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job @robz-stripe, this looks great. I've left a couple of comments, but nothing I feel too strongly about.
@rattrayalex-stripe @jlomas-stripe Would one of you Node-fu masters mind taking a look at this?
r? @rattrayalex-stripe as our node expert |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Few nits, would be nice to see the tests get a bit more granular but not bad as they are; fine to leave if you prefer as-is.
Cleaned up the tests to be more granular and simpler to grok. Let me know what you think! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✨
Awesome! Thanks @robz-stripe !
Thanks @robz-stripe! |
@robz-stripe is there an example to do oauth with this lib? |
Any way to get this documented in the API? |
@Nisthar You can find examples in the repo here: https://github.com/stripe/stripe-node/blob/master/test/resources/OAuth.spec.js @noah-witt Yes we're working on adding examples to our docs! |
r? @remi-stripe
cc @stripe/api-libraries
Fixes #422