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

Refresh before request if neccesary and multipart only when resource and media body specified #235

Merged
merged 7 commits into from
Jul 30, 2014

Conversation

ryanseys
Copy link
Contributor

Currently when we make an API request and the request responds with an authentication error, we attempt to refresh the access_token using the refresh_token and then re-attempt the request. If the request has a multipart body, it will not be attached to the re-attempted request due to the constraints of the library. This is exceptionally true for multipart bodies whose data is coming from a readable stream. To mitigate these issues, this PR makes the following changes:

  • Check expiry_date (an added property) of the credentials object to ensure that the token has not expired. If there is no expiry_date value, assume it has not expired. If it has expired, refresh the token before making the request.
  • If there is no access_token in the credentials, and there is a refresh_token, use it to refresh the token before making the request.
  • If there is no refresh_token, never attempt to refresh (because you can't)
  • If you have no access_token nor refresh_token, return an error in the callback.

This will give more responsibility to the developer to ensure that their credentials are kept up to date.

Also added oauth2client.revokeCredentials() which will clear the credentials object and attempt to revoke the access_token.

Bonus: Affects #229 where multipart request was built for all media requests. Now multipart will only be built for requests with resource and media.body. If resource is only specified, it's a simple JSON request, if only media.body is specified, then uploadType=media is used with the body (not a multipart).

@afirstenberg
Copy link

Looks like this should address #98 and #139, correct?

And just to make sure I follow, if for whatever reason (ie - the access_token was revoked) the call fails, there is no attempt to re-auth and re-attempt?

@ryanseys
Copy link
Contributor Author

You're correct, this addresses #98, #139, possibly #81 [due to race condition], and possibly #131 [due to race condition]. No re-attempt to request occurs. Making the request is the last stop before the result is returned to the callback.

@ryanseys
Copy link
Contributor Author

Also #122.

@ryanseys ryanseys changed the title Refresh before request if neccesary Refresh before request if neccesary and multipart only when resource and media body specified Jul 29, 2014
@@ -86,7 +86,8 @@ JWT.prototype.refreshToken_ = function(ignored_, opt_callback) {
opt_callback && opt_callback(err, {
access_token: token,
token_type: 'Bearer',
expires_in: that.gapi.token_expires
expires_in: that.gapi.token_expires,

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@rakyll
Copy link
Contributor

rakyll commented Jul 30, 2014

Media upload changes LGTM, commented on a few issues about auth related changes.

ryanseys added a commit that referenced this pull request Jul 30, 2014
Refresh before request if neccesary and multipart only when resource and media body specified
@ryanseys ryanseys merged commit 2b63ad2 into googleapis:master Jul 30, 2014
@ryanseys ryanseys deleted the refresh-before branch July 30, 2014 23:23
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.

3 participants