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

Remove the authentication process for OAuth 2.0 #5

Closed
wants to merge 1 commit into from
Closed

Remove the authentication process for OAuth 2.0 #5

wants to merge 1 commit into from

Conversation

withsmilo
Copy link
Contributor

  • Change all http:// urls to https:// urls.
  • Replace https module of request module for https request.
  • Remove OAuth 1.0 module.
  • Remove consumer-key, comsumer-secret, yuser-secret and yuser-sessionHandle.
    • We do not need these variables anymore.
    • External OAuth 2.0 application must handle them.
  • Remove the codes to refresh userToken.
    • External app using OAuth 2.0 must handle it.
    • Set set new token using setUserToken API.

* Change all `http://` urls to `https://` urls.
* Replace `https` module of `request` module for https request.
* Remove OAuth 1.0 module.
* Remove consumer-key, comsumer-secret, yuser-secret and yuser-sessionHandle.
  * We do not need these variables anymore.
  * External OAuth 2.0 application must handle them.
* Remove the codes to refresh userToken.
  * External app using OAuth 2.0 must handle it.
  * Set set new token using setUserToken API.
oauth_session_handle: self.yuser.sessionHandle
});

https.get('https://api.login.yahoo.com/oauth/v2/get_token?' + refresh_data, function(res) {
Copy link
Owner

Choose a reason for hiding this comment

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

So we no longer need the refresh token when we move to OAuth2?

Copy link
Contributor Author

@withsmilo withsmilo Apr 21, 2016

Choose a reason for hiding this comment

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

To exchange the refresh token in OAuth 2.0, we need a redirectionUri and must handle Yahoo's response in it. So external app must handle the refresh token like below.

  1. [externalApp] Call some yfsapi API.
  2. [yfsapi] If session is expired, we return an error.
  3. [externalApp] Exchange the refresh token for new access token.
  4. [externalApp] Call yf.setUserToken(NEW_USER_TOKEN) API.
  5. [externalApp] Call some yfsapi API.

@whatadewitt
Copy link
Owner

So I've been thinking a lot about this, here's my plan:

I'm going to hold off on implementing OAuth2, simply because we cannot use client credentials to make public queries. This is a big issue for me as it will limit the ability to easily use the API.

That said, I am going to take all of the OAuth specific stuff out of my API and that will be on the user to set up going forward. When I require the refresh token I am going to trigger an event that the user will have to catch in order to refresh the token (if the user is signed in). I plan to also keep a branch up to date next to "master" which I will call "oauth2" which will, when OAuth2 supports client credentials for the Fantasy Sports API, I will make the "master" branch of the project.

Does that make sense? I hate not merging in your hard work, but for the API I just want to make sure it's easiest for the user.

@withsmilo
Copy link
Contributor Author

@whatadewitt
I understood your opinion. 😃
I think that removing OAuth specific stuff from yfsapi's API is not clear because OAuth1.0 needs 'http://' and OAuth2.0 needs 'https://'. Anyway, I close this pull request now and eagerly look forward to your next work. But I would like to maintain my yfsapi-without-auth module seperately to develop my yahoo-fantasy app.

Thank you a lot for your reply!

@withsmilo withsmilo closed this Apr 24, 2016
@withsmilo withsmilo deleted the remove_auth_process_for_oauth2.0 branch April 24, 2016 13:44
@whatadewitt
Copy link
Owner

Thank YOU for taking the time with the code!!!

@whatadewitt
Copy link
Owner

Thinking about it more... it's a bit of a catch-22...

Best case for a user is for them to just have a client ID and secret and use that to query the API. If I remove all of the OAuth stuff, then I end up having the user needing to do some sort of OAuth for the client in their app, which doesn't make things as easy as possible for them... This is frustrating...

@whatadewitt
Copy link
Owner

... should be like Twitter where they just let you create these tokens :)

@withsmilo
Copy link
Contributor Author

@whatadewitt
I agree with you. So complicated problem.

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

Successfully merging this pull request may close these issues.

2 participants