Save OAuth tokens #2810
Save OAuth tokens #2810
Conversation
|
Why no tests? This isn't fixing an urgent production issue, for example. |
| BEGIN; | ||
| ALTER TABLE elsewhere DROP COLUMN access_token, | ||
| DROP COLUMN refresh_token, | ||
| DROP COLUMN expires; |
There was a problem hiding this comment.
What about the data that's already in these columns? It looks like we're throwing it away, yes? Why?
There was a problem hiding this comment.
Because keeping it is not worth the effort, it only exists in 59 rows (Venmo accounts), and I think it might be obsolete anyway.
There was a problem hiding this comment.
Okay. We've been using an OAuth app for Venmo that is attached to my personal account. Due to bugs on their end we were unable to set up an app under our Gratipay account, but I believe that's resolved now. What are the implications of throwing away these access/refresh tokens? Does it make sense to also switch to a different Venmo OAuth app along with this PR?
There was a problem hiding this comment.
Yes, if we want to change some OAuth keys now is the time.
There was a problem hiding this comment.
Okay, I'll look into it.
|
Venmo looks good. We end up with the entire user object in the |
We can't really test interactions with the elsewhere platforms. There may be other things we could cover in the tests though, I haven't really looked into it, I wanted to get this PR opened. |
|
I have the new Venmo app ready to go. Can we think this through? So we're tossing away these access and refresh tokens. That doesn't matter, because we aren't actually using them for anything yet. In fact, once a Venmo connection is made, that's all we care about: that a connection is made, that we have a Venmo user id to include in |
|
Specifically: we're not using Venmo as a sign-in method. |
|
I've switched over to the new Venmo app, and I've deleted the old one. |
|
I'm closing #1995, leaving this comment here for reference. |
|
@whit537's commits LGTM. I guess what's left to do here is to test the remaining platforms: Bitbucket, Facebook, Google, OpenStreetMap. |
|
I just created oauthlib/oauthlib#283 so we can be notified when oauthlib 0.6.4 is released. We're currently using a non-released version. |
|
I guess that's an FYI. Not directly related to this PR but I just happened to be reminded since this is oauth code. |
|
Bitbucket and OpenStreetMap both populate |
|
Venmo gives us both |
We need that for #2809 if we're going to show suggestions about who to tip based on elsewhere data.
I've included the simplate I used to check that it works for Twitter and GitHub. Still need to confirm that it works with the other OAuth platforms (Bitbucket, Facebook, Google, OpenStreetMap and Venmo), feel free to help (especially with Venmo since I can't test that).