GitHub PullRequestReviewCommentEvent at 2014-07-30 22:41:14
Repository: google-api-nodejs-client
Actor: ryanseys
Payload: {"action":"created","comment":{"url":"https://api.github.com/repos/google/google-api-nodejs-client/pulls/comments/15617432","id":15617432,"diff_hunk":"@@ -200,51 +210,78 @@ OAuth2Client.prototype.revokeToken = function(token, opt_callback) {\n };\n \n /**\n+ * Revokes access token and clears the credentials object\n+ * @param  {Function=} callback callback\n+ */\n+OAuth2Client.prototype.revokeCredentials = function(callback) {\n+  var token = this.credentials.access_token;\n+  this.credentials = {};\n+  if (token) {\n+    this.revokeToken(token, callback);\n+  } else {\n+    callback(new Error('No access token to revoke.'), null);\n+  }\n+};\n+\n+/**\n  * Provides a request implementation with OAuth 2.0 flow.\n  * If credentials have a refresh_token, in cases of HTTP\n  * 401 and 403 responses, it automatically asks for a new\n  * access token and replays the unsuccessful request.\n  * @param {object} opts Request options.\n  * @param {function} callback callback.\n- * @param {boolean=} opt_dontForceRefresh If set, don't ask for a new token\n+ * @param {boolean=} opt_forceDontRefresh If set, don't ask for a new token\n  *   with refresh_token.\n  * @return {Request} Request object\n  */\n-OAuth2Client.prototype.request = function(opts, callback, opt_dontForceRefresh) {\n+OAuth2Client.prototype.request = function(opts, callback, opt_forceDontRefresh) {\n \n   var that = this;\n   var credentials = this.credentials;\n+  var shouldRefresh = !opt_forceDontRefresh;\n+  var expiryDate = credentials.expiry_date;\n+  // if no expiry time, assume it's not expired\n+  var isTokenExpired = expiryDate ? expiryDate <= (new Date()).getTime() : false;\n \n   if (!credentials.access_token && !credentials.refresh_token) {\n     callback(new Error('No access or refresh token is set.'), null);\n     return;\n+  } else if (!credentials.refresh_token) {\n+    shouldRefresh = false;\n+  } else if (credentials.access_token && !isTokenExpired) {\n+    shouldRefresh = false;\n   }\n \n+  if (shouldRefresh) {","path":"lib/auth/oauth2client.js","position":76,"original_position":76,"commit_id":"10303fdc9fdbdbae0f4ee35a1c5deab22b107f0d","original_commit_id":"10303fdc9fdbdbae0f4ee35a1c5deab22b107f0d","user":{"login":"ryanseys","id":163873,"avatar_url":"https://avatars.githubusercontent.com/u/163873?v=1","gravatar_id":"a0d3ecbe29f500fc9664ca6f7a73c735","url":"https://api.github.com/users/ryanseys","html_url":"https://github.com/ryanseys","followers_url":"https://api.github.com/users/ryanseys/followers","following_url":"https://api.github.com/users/ryanseys/following{/other_user}","gists_url":"https://api.github.com/users/ryanseys/gists{/gist_id}","starred_url":"https://api.github.com/users/ryanseys/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ryanseys/subscriptions","organizations_url":"https://api.github.com/users/ryanseys/orgs","repos_url":"https://api.github.com/users/ryanseys/repos","events_url":"https://api.github.com/users/ryanseys/events{/privacy}","received_events_url":"https://api.github.com/users/ryanseys/received_events","type":"User","site_admin":false},"body":"Removing it.","created_at":"2014-07-30T22:41:14Z","updated_at":"2014-07-30T22:41:14Z","html_url":"https://github.com/google/google-api-nodejs-client/pull/235#discussion_r15617432","pull_request_url":"https://api.github.com/repos/google/google-api-nodejs-client/pulls/235","_links":{"self":{"href":"https://api.github.com/repos/google/google-api-nodejs-client/pulls/comments/15617432"},"html":{"href":"https://github.com/google/google-api-nodejs-client/pull/235#discussion_r15617432"},"pull_request":{"href":"https://api.github.com/repos/google/google-api-nodejs-client/pulls/235"}}},"pull_request":{"url":"https://api.github.com/repos/google/google-api-nodejs-client/pulls/235","id":19036734,"html_url":"https://github.com/google/google-api-nodejs-client/pull/235","diff_url":"https://github.com/google/google-api-nodejs-client/pull/235.diff","patch_url":"https://github.com/google/google-api-nodejs-client/pull/235.patch","issue_url":"https://api.github.com/repos/google/google-api-nodejs-client/issues/235","number":235,"state":"open","title":"Refresh before request if neccesary and multipart only when resource and media body specified","user":{"login":"ryanseys","id":163873,"avatar_url":"https://avatars.githubusercontent.com/u/163873?v=1","gravatar_id":"a0d3ecbe29f500fc9664ca6f7a73c735","url":"https://api.github.com/users/ryanseys","html_url":"https://github.com/ryanseys","followers_url":"https://api.github.com/users/ryanseys/followers","following_url":"https://api.github.com/users/ryanseys/following{/other_user}","gists_url":"https://api.github.com/users/ryanseys/gists{/gist_id}","starred_url":"https://api.github.com/users/ryanseys/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ryanseys/subscriptions","organizations_url":"https://api.github.com/users/ryanseys/orgs","repos_url":"https://api.github.com/users/ryanseys/repos","events_url":"https://api.github.com/users/ryanseys/events{/privacy}","received_events_url":"https://api.github.com/users/ryanseys/received_events","type":"User","site_admin":false},"body":"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:\r\n- 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.\r\n- 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.\r\n- If there is no `refresh_token`, never attempt to refresh (because you can't)\r\n- If you have no `access_token` nor `refresh_token`, return an error in the callback.\r\n\r\nThis will give more responsibility to the developer to ensure that their credentials are kept up to date.\r\n\r\nAlso added `oauth2client.revokeCredentials()` which will clear the credentials object and attempt to revoke the `access_token`.\r\n\r\nBonus: 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).","created_at":"2014-07-29T16:09:05Z","updated_at":"2014-07-30T22:41:14Z","closed_at":null,"merged_at":null,"merge_commit_sha":"aa5a91185baa582020807f6fb6b8cc051ff5c2a4","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/google/google-api-nodejs-client/pulls/235/commits","review_comments_url":"https://api.github.com/repos/google/google-api-nodejs-client/pulls/235/comments","review_comment_url":"https://api.github.com/repos/google/google-api-nodejs-client/pulls/comments/{number}","comments_url":"https://api.github.com/repos/google/google-api-nodejs-client/issues/235/comments","statuses_url":"https://api.github.com/repos/google/google-api-nodejs-client/statuses/10303fdc9fdbdbae0f4ee35a1c5deab22b107f0d","head":{"label":"ryanseys:refresh-before","ref":"refresh-before","sha":"10303fdc9fdbdbae0f4ee35a1c5deab22b107f0d","user":{"login":"ryanseys","id":163873,"avatar_url":"https://avatars.githubusercontent.com/u/163873?v=1","gravatar_id":"a0d3ecbe29f500fc9664ca6f7a73c735","url":"https://api.github.com/users/ryanseys","html_url":"https://github.com/ryanseys","followers_url":"https://api.github.com/users/ryanseys/followers","following_url":"https://api.github.com/users/ryanseys/following{/other_user}","gists_url":"https://api.github.com/users/ryanseys/gists{/gist_id}","starred_url":"https://api.github.com/users/ryanseys/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ryanseys/subscriptions","organizations_url":"https://api.github.com/users/ryanseys/orgs","repos_url":"https://api.github.com/users/ryanseys/repos","events_url":"https://api.github.com/users/ryanseys/events{/privacy}","received_events_url":"https://api.github.com/users/ryanseys/received_events","type":"User","site_admin":false},"repo":{"id":19864273,"name":"google-api-nodejs-client","full_name":"ryanseys/google-api-nodejs-client","owner":{"login":"ryanseys","id":163873,"avatar_url":"https://avatars.githubusercontent.com/u/163873?v=1","gravatar_id":"a0d3ecbe29f500fc9664ca6f7a73c735","url":"https://api.github.com/users/ryanseys","html_url":"https://github.com/ryanseys","followers_url":"https://api.github.com/users/ryanseys/followers","following_url":"https://api.github.com/users/ryanseys/following{/other_user}","gists_url":"https://api.github.com/users/ryanseys/gists{/gist_id}","starred_url":"https://api.github.com/users/ryanseys/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ryanseys/subscriptions","organizations_url":"https://api.github.com/users/ryanseys/orgs","repos_url":"https://api.github.com/users/ryanseys/repos","events_url":"https://api.github.com/users/ryanseys/events{/privacy}","received_events_url":"https://api.github.com/users/ryanseys/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/ryanseys/google-api-nodejs-client","description":"Google's officially supported Node.js client library for accessing Google APIs, it comes with OAuth 2.0 support.","fork":true,"url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client","forks_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/forks","keys_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/keys{/key_id}","collaborators_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/teams","hooks_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/hooks","issue_events_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/issues/events{/number}","events_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/events","assignees_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/assignees{/user}","branches_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/branches{/branch}","tags_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/tags","blobs_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/git/refs{/sha}","trees_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/git/trees{/sha}","statuses_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/statuses/{sha}","languages_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/languages","stargazers_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/stargazers","contributors_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/contributors","subscribers_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/subscribers","subscription_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/subscription","commits_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/commits{/sha}","git_commits_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/git/commits{/sha}","comments_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/comments{/number}","issue_comment_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/issues/comments/{number}","contents_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/contents/{+path}","compare_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/compare/{base}...{head}","merges_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/merges","archive_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/downloads","issues_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/issues{/number}","pulls_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/pulls{/number}","milestones_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/milestones{/number}","notifications_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/labels{/name}","releases_url":"https://api.github.com/repos/ryanseys/google-api-nodejs-client/releases{/id}","created_at":"2014-05-16T17:07:32Z","updated_at":"2014-06-30T21:32:20Z","pushed_at":"2014-07-30T22:33:01Z","git_url":"git://github.com/ryanseys/google-api-nodejs-client.git","ssh_url":"git@github.com:ryanseys/google-api-nodejs-client.git","clone_url":"https://github.com/ryanseys/google-api-nodejs-client.git","svn_url":"https://github.com/ryanseys/google-api-nodejs-client","homepage":"","size":2086,"stargazers_count":0,"watchers_count":0,"language":"JavaScript","has_issues":false,"has_downloads":true,"has_wiki":false,"forks_count":0,"mirror_url":null,"open_issues_count":1,"forks":0,"open_issues":1,"watchers":0,"default_branch":"master"}},"base":{"label":"google:master","ref":"master","sha":"9b30fa8dc632bb7d9c41aabfcb8c0e6f1ef7fd61","user":{"login":"google","id":1342004,"avatar_url":"https://avatars.githubusercontent.com/u/1342004?v=1","gravatar_id":"24ba30616d2a20673f54c2aee36d159e","url":"https://api.github.com/users/google","html_url":"https://github.com/google","followers_url":"https://api.github.com/users/google/followers","following_url":"https://api.github.com/users/google/following{/other_user}","gists_url":"https://api.github.com/users/google/gists{/gist_id}","starred_url":"https://api.github.com/users/google/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/google/subscriptions","organizations_url":"https://api.github.com/users/google/orgs","repos_url":"https://api.github.com/users/google/repos","events_url":"https://api.github.com/users/google/events{/privacy}","received_events_url":"https://api.github.com/users/google/received_events","type":"Organization","site_admin":false},"repo":{"id":8608480,"name":"google-api-nodejs-client","full_name":"google/google-api-nodejs-client","owner":{"login":"google","id":1342004,"avatar_url":"https://avatars.githubusercontent.com/u/1342004?v=1","gravatar_id":"24ba30616d2a20673f54c2aee36d159e","url":"https://api.github.com/users/google","html_url":"https://github.com/google","followers_url":"https://api.github.com/users/google/followers","following_url":"https://api.github.com/users/google/following{/other_user}","gists_url":"https://api.github.com/users/google/gists{/gist_id}","starred_url":"https://api.github.com/users/google/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/google/subscriptions","organizations_url":"https://api.github.com/users/google/orgs","repos_url":"https://api.github.com/users/google/repos","events_url":"https://api.github.com/users/google/events{/privacy}","received_events_url":"https://api.github.com/users/google/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/google/google-api-nodejs-client","description":"Google's officially supported Node.js client library for accessing Google APIs, it comes with OAuth 2.0 support.","fork":false,"url":"https://api.github.com/repos/google/google-api-nodejs-client","forks_url":"https://api.github.com/repos/google/google-api-nodejs-client/forks","keys_url":"https://api.github.com/repos/google/google-api-nodejs-client/keys{/key_id}","collaborators_url":"https://api.github.com/repos/google/google-api-nodejs-client/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/google/google-api-nodejs-client/teams","hooks_url":"https://api.github.com/repos/google/google-api-nodejs-client/hooks","issue_events_url":"https://api.github.com/repos/google/google-api-nodejs-client/issues/events{/number}","events_url":"https://api.github.com/repos/google/google-api-nodejs-client/events","assignees_url":"https://api.github.com/repos/google/google-api-nodejs-client/assignees{/user}","branches_url":"https://api.github.com/repos/google/google-api-nodejs-client/branches{/branch}","tags_url":"https://api.github.com/repos/google/google-api-nodejs-client/tags","blobs_url":"https://api.github.com/repos/google/google-api-nodejs-client/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/google/google-api-nodejs-client/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/google/google-api-nodejs-client/git/refs{/sha}","trees_url":"https://api.github.com/repos/google/google-api-nodejs-client/git/trees{/sha}","statuses_url":"https://api.github.com/repos/google/google-api-nodejs-client/statuses/{sha}","languages_url":"https://api.github.com/repos/google/google-api-nodejs-client/languages","stargazers_url":"https://api.github.com/repos/google/google-api-nodejs-client/stargazers","contributors_url":"https://api.github.com/repos/google/google-api-nodejs-client/contributors","subscribers_url":"https://api.github.com/repos/google/google-api-nodejs-client/subscribers","subscription_url":"https://api.github.com/repos/google/google-api-nodejs-client/subscription","commits_url":"https://api.github.com/repos/google/google-api-nodejs-client/commits{/sha}","git_commits_url":"https://api.github.com/repos/google/google-api-nodejs-client/git/commits{/sha}","comments_url":"https://api.github.com/repos/google/google-api-nodejs-client/comments{/number}","issue_comment_url":"https://api.github.com/repos/google/google-api-nodejs-client/issues/comments/{number}","contents_url":"https://api.github.com/repos/google/google-api-nodejs-client/contents/{+path}","compare_url":"https://api.github.com/repos/google/google-api-nodejs-client/compare/{base}...{head}","merges_url":"https://api.github.com/repos/google/google-api-nodejs-client/merges","archive_url":"https://api.github.com/repos/google/google-api-nodejs-client/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/google/google-api-nodejs-client/downloads","issues_url":"https://api.github.com/repos/google/google-api-nodejs-client/issues{/number}","pulls_url":"https://api.github.com/repos/google/google-api-nodejs-client/pulls{/number}","milestones_url":"https://api.github.com/repos/google/google-api-nodejs-client/milestones{/number}","notifications_url":"https://api.github.com/repos/google/google-api-nodejs-client/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/google/google-api-nodejs-client/labels{/name}","releases_url":"https://api.github.com/repos/google/google-api-nodejs-client/releases{/id}","created_at":"2013-03-06T17:02:23Z","updated_at":"2014-07-30T20:14:50Z","pushed_at":"2014-07-30T22:33:00Z","git_url":"git://github.com/google/google-api-nodejs-client.git","ssh_url":"git@github.com:google/google-api-nodejs-client.git","clone_url":"https://github.com/google/google-api-nodejs-client.git","svn_url":"https://github.com/google/google-api-nodejs-client","homepage":"","size":2445,"stargazers_count":2167,"watchers_count":2167,"language":"JavaScript","has_issues":true,"has_downloads":true,"has_wiki":false,"forks_count":237,"mirror_url":null,"open_issues_count":18,"forks":237,"open_issues":18,"watchers":2167,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/google/google-api-nodejs-client/pulls/235"},"html":{"href":"https://github.com/google/google-api-nodejs-client/pull/235"},"issue":{"href":"https://api.github.com/repos/google/google-api-nodejs-client/issues/235"},"comments":{"href":"https://api.github.com/repos/google/google-api-nodejs-client/issues/235/comments"},"review_comments":{"href":"https://api.github.com/repos/google/google-api-nodejs-client/pulls/235/comments"},"review_comment":{"href":"https://api.github.com/repos/google/google-api-nodejs-client/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/google/google-api-nodejs-client/pulls/235/commits"},"statuses":{"href":"https://api.github.com/repos/google/google-api-nodejs-client/statuses/10303fdc9fdbdbae0f4ee35a1c5deab22b107f0d"}}}}