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

Support OAuth flow for Connect accounts #555

Merged
merged 10 commits into from
Jan 25, 2019
Merged

Conversation

robz-stripe
Copy link
Contributor

r? @remi-stripe
cc @stripe/api-libraries

Fixes #422

@robz-stripe
Copy link
Contributor Author

Tested this via the steps at https://stripe.com/docs/connect/standard-accounts and the access code redeemed correctly for a token.

@ob-stripe
Copy link
Contributor

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:

  • a utility authorizeURL method for creating the https://connect.stripe.com/oauth/authorize?... redirection URL (doc)
    • this method should have an option for Express accounts, to change the resulting URL to https://connect.stripe.com/express/oauth/authorize?...
  • support for deauthorizing accounts via the /oauth/deauthorize endpoint (doc)

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 Oauth to OAuth?

lib/stripe.js Outdated Show resolved Hide resolved
lib/stripe.js Outdated Show resolved Hide resolved
@robz-stripe
Copy link
Contributor Author

r? @ob-stripe

This implements authorizeUrl and deauthorize as well, now.

@robz-stripe robz-stripe assigned ob-stripe and unassigned robz-stripe Jan 23, 2019
@robz-stripe robz-stripe changed the title Allow creation of OAuth tokens for Connect accounts Support OAuth flow for Connect accounts Jan 23, 2019
@robz-stripe
Copy link
Contributor Author

Making a note here to update https://github.com/stripe/stripe-node/wiki/Using-Stripe-Connect-with-node.js once the interface is finalized.

Copy link
Contributor

@ob-stripe ob-stripe left a 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?

lib/stripe.js Outdated Show resolved Hide resolved
lib/stripe.js Outdated Show resolved Hide resolved
@robz-stripe robz-stripe assigned remi-stripe and unassigned ob-stripe Jan 25, 2019
@remi-stripe remi-stripe removed their assignment Jan 25, 2019
@remi-stripe
Copy link
Contributor

r? @rattrayalex-stripe as our node expert

Copy link
Contributor

@rattrayalex-stripe rattrayalex-stripe left a 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.

lib/resources/OAuth.js Outdated Show resolved Hide resolved
lib/resources/OAuth.js Show resolved Hide resolved
test/resources/OAuth.spec.js Outdated Show resolved Hide resolved
test/resources/OAuth.spec.js Outdated Show resolved Hide resolved
test/resources/OAuth.spec.js Outdated Show resolved Hide resolved
@robz-stripe
Copy link
Contributor Author

Cleaned up the tests to be more granular and simpler to grok. Let me know what you think!

Copy link
Contributor

@rattrayalex-stripe rattrayalex-stripe left a 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 !

@robz-stripe robz-stripe merged commit b5fb564 into master Jan 25, 2019
@robz-stripe robz-stripe deleted the robz/oauth-tokens branch January 25, 2019 21:49
@samheutmaker
Copy link

Thanks @robz-stripe!

@Nisthar
Copy link

Nisthar commented Aug 3, 2019

@robz-stripe is there an example to do oauth with this lib?

@noah-witt
Copy link

Any way to get this documented in the API?

@remi-stripe
Copy link
Contributor

@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!

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

Successfully merging this pull request may close these issues.

Authenticate express account tokens documentation?
8 participants