Merge pull request #235 from ryanseys/refresh-before

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

lib/apirequest.js

@@ -20,7 +20,12 @@ var Multipart = require('multipart-stream');
 var utils = require('./utils.js');
 var DefaultTransporter = require('./transporters.js');
 var transporter = new DefaultTransporter();
+var stream = require('stream');
 
+function isReadableStream(obj) {
+  return obj instanceof stream.Stream && typeof obj._read == 'function' &&
+      typeof obj._readableState == 'object';
+}
 
 function logErrorOnly(err) {
   if (err) {
@@ -49,7 +54,7 @@ function isValidParams(params, keys, callback) {
  * @return {Request}             Returns Request object or null
  */
 function createAPIRequest(parameters, callback) {
-  var req;
+  var req, body;
   var mediaUrl = parameters.mediaUrl;
   var context = parameters.context;
   var params = parameters.params;
@@ -78,6 +83,7 @@ function createAPIRequest(parameters, callback) {
   var media = params.media || {};
   var resource = params.resource;
   var authClient = params.auth || context._options.auth || context.google._options.auth;
+  var defaultMime = typeof media.body === 'string' ? 'text/plain' : 'application/octet-stream';
   delete params.media;
   delete params.resource;
   delete params.auth;
@@ -95,37 +101,45 @@ function createAPIRequest(parameters, callback) {
 
   if (mediaUrl && media && media.body) {
     options.url = mediaUrl;
-    // Create a boundary identifier and multipart read stream
-    var boundary = Math.random().toString(36).slice(2);
-    var mp = new Multipart(boundary);
+    if (resource) {
+      // Create a boundary identifier and multipart read stream
+      var boundary = Math.random().toString(36).slice(2);
+      body = new Multipart(boundary);
 
-    // Always a multipart upload
-    params.uploadType = 'multipart';
+      // Use multipart upload
+      params.uploadType = 'multipart';
 
-    options.headers = {
-      'Content-Type': 'multipart/related; boundary="' + boundary + '"'
-    };
+      options.headers = {
+        'Content-Type': 'multipart/related; boundary="' + boundary + '"'
+      };
 
-    // Add parts to multipart request
-    if (resource) {
-      mp.addPart({
+      // Add parts to multipart request
+      body.addPart({
         headers: {
           'Content-Type': 'application/json'
         },
         body: JSON.stringify(resource)
       });
-    }
-
-    var defaultMime = typeof media.body === 'string' ? 'text/plain' : 'application/octet-stream';
 
-    mp.addPart({
-      headers: {
-        'Content-Type': media.mimeType || resource && resource.mimeType || defaultMime
-      },
-      body: media.body // can be a readable stream or raw string!
-    });
-  }
-  else {
+      body.addPart({
+        headers: {
+          'Content-Type': media.mimeType || resource && resource.mimeType || defaultMime
+        },
+        body: media.body // can be a readable stream or raw string!
+      });
+    } else {
+      params.uploadType = 'media';
+      options.headers = {
+        'Content-Type': media.mimeType || defaultMime
+      };
+
+      if (isReadableStream(media.body)) {
+        body = media.body;
+      } else {
+        options.body = media.body;
+      }
+    }
+  } else {
     options.json = resource || ((method === 'GET' || method === 'DELETE') ? true : {});
   }
 
@@ -140,7 +154,7 @@ function createAPIRequest(parameters, callback) {
     req = transporter.request(options, callback);
   }
 
-  if (mp) mp.pipe(req);
+  if (body) body.pipe(req);
   return req;
 }
 

lib/auth/computeclient.js

@@ -70,7 +70,13 @@ Compute.prototype.refreshToken_ = function(ignored_, opt_callback) {
     method: 'GET',
     uri: uri,
     json: true
-  }, opt_callback);
+  }, function(err, tokens) {
+    if (!err && tokens && tokens.expires_in) {
+      tokens.expiry_date = ((new Date()).getTime() + (tokens.expires_in * 1000));
+      delete tokens.expires_in;
+    }
+    opt_callback && opt_callback(err, tokens);
+  });
 };
 
 /**

lib/auth/jwtclient.js

@@ -86,7 +86,7 @@ JWT.prototype.refreshToken_ = function(ignored_, opt_callback) {
     opt_callback && opt_callback(err, {
       access_token: token,
       token_type: 'Bearer',
-      expires_in: that.gapi.token_expires
+      expiry_date: ((new Date()).getTime() + (that.gapi.token_expires * 1000))
     });
   });
 };

lib/auth/oauth2client.js

@@ -135,7 +135,13 @@ OAuth2Client.prototype.getToken = function(code, opt_callback) {
     uri: uri,
     form: values,
     json: true
-  }, opt_callback);
+  }, function(err, tokens) {
+    if (!err && tokens && tokens.expires_in) {
+      tokens.expiry_date = ((new Date()).getTime() + (tokens.expires_in * 1000));
+      delete tokens.expires_in;
+    }
+    opt_callback && opt_callback(err, tokens);
+  });
 };
 
 /**
@@ -159,7 +165,13 @@ OAuth2Client.prototype.refreshToken_ = function(refresh_token, opt_callback) {
     uri: uri,
     form: values,
     json: true
-  }, opt_callback);
+  }, function(err, tokens) {
+    if (!err && tokens && tokens.expires_in) {
+      tokens.expiry_date = ((new Date()).getTime() + (tokens.expires_in * 1000));
+      delete tokens.expires_in;
+    }
+    opt_callback && opt_callback(err, tokens);
+  });
 };
 
 /**
@@ -199,52 +211,74 @@ OAuth2Client.prototype.revokeToken = function(token, opt_callback) {
   }, opt_callback);
 };
 
+/**
+ * Revokes access token and clears the credentials object
+ * @param  {Function=} callback callback
+ */
+OAuth2Client.prototype.revokeCredentials = function(callback) {
+  var token = this.credentials.access_token;
+  this.credentials = {};
+  if (token) {
+    this.revokeToken(token, callback);
+  } else {
+    callback(new Error('No access token to revoke.'), null);
+  }
+};
+
 /**
  * Provides a request implementation with OAuth 2.0 flow.
  * If credentials have a refresh_token, in cases of HTTP
  * 401 and 403 responses, it automatically asks for a new
  * access token and replays the unsuccessful request.
  * @param {object} opts Request options.
  * @param {function} callback callback.
- * @param {boolean=} opt_dontForceRefresh If set, don't ask for a new token
- *   with refresh_token.
  * @return {Request} Request object
  */
-OAuth2Client.prototype.request = function(opts, callback, opt_dontForceRefresh) {
+OAuth2Client.prototype.request = function(opts, callback) {
 
   var that = this;
   var credentials = this.credentials;
+  var expiryDate = credentials.expiry_date;
+  // if no expiry time, assume it's not expired
+  var isTokenExpired = expiryDate ? expiryDate <= (new Date()).getTime() : false;
 
   if (!credentials.access_token && !credentials.refresh_token) {
     callback(new Error('No access or refresh token is set.'), null);
     return;
   }
 
+  var shouldRefresh = !credentials.access_token || isTokenExpired;
+
+  if (shouldRefresh && credentials.refresh_token) {
+    this.refreshAccessToken(function(err, tokens) {
+      if (err) {
+        callback(err, null);
+      } else if (!tokens || (tokens && !tokens.access_token)) {
+        callback(new Error('Could not refresh access token.'), null);
+      } else {
+        return that._makeRequest(opts, callback);
+      }
+    });
+  } else {
+    return this._makeRequest(opts, callback);
+  }
+};
+
+/**
+ * Makes a request without paying attention to refreshing or anything
+ * Assumes that all credentials are set correctly.
+ * @param  {object}   opts     Options for request
+ * @param  {Function} callback callback function
+ * @return {Request}           The request object created
+ */
+OAuth2Client.prototype._makeRequest = function(opts, callback) {
+  var that = this;
+  var credentials = this.credentials;
   credentials.token_type = credentials.token_type || 'Bearer';
   opts.headers = opts.headers || {};
   opts.headers['Authorization'] = credentials.token_type + ' ' + credentials.access_token;
 
-  return this.transporter.request(opts, function(err, body, res) {
-    // TODO: Check if it's not userRateLimitExceeded
-    var hasAuthError = res && (res.statusCode === 401 || res.statusCode === 403);
-    // if there is an auth error, refresh the token
-    // and make the request again
-    if (!opt_dontForceRefresh && hasAuthError && credentials.refresh_token) {
-      // refresh access token and re-request
-      that.refreshAccessToken(function(err, result) {
-        if (err || (result && !result.access_token)) {
-          callback(err, result, res);
-        } else {
-          var tokens = result;
-          tokens.refresh_token = credentials.refresh_token;
-          that.credentials = tokens;
-          that.request(opts, callback, true);
-        }
-      });
-    } else {
-      callback(err, body, res);
-    }
-  });
+  return this.transporter.request(opts, callback);
 };
 
 /**

test/fixtures/media-response.txt

@@ -3,7 +3,7 @@ Content-Type: application/json
 
 $resource
 --$boundary
-Content-Type: text/plain
+Content-Type: $mimeType
 
 $media
 --$boundary--

test/fixtures/mediabody.txt

@@ -0,0 +1 @@
+hello world abc123

test/test.compute.js

@@ -18,6 +18,9 @@
 
 var assert = require('assert');
 var googleapis = require('../lib/googleapis.js');
+var nock = require('nock');
+
+nock.disableNetConnect();
 
 describe('Compute auth client', function() {
 
@@ -27,7 +30,8 @@ describe('Compute auth client', function() {
       request: function(opts, opt_callback) {
         opt_callback(null, {
           'access_token': 'initial-access-token',
-          'token_type': 'Bearer'
+          'token_type': 'Bearer',
+          'expires_in': 3600
         }, {});
       }
     };
@@ -38,25 +42,36 @@ describe('Compute auth client', function() {
     });
   });
 
-  it('should refresh token when request fails', function(done) {
+  it('should refresh if access token has expired', function(done) {
+    var scope = nock('http://metadata')
+        .get('/computeMetadata/v1beta1/instance/service-accounts/default/token')
+        .reply(200, { access_token: 'abc123', expires_in: 10000 });
     var compute = new googleapis.auth.Compute();
     compute.credentials = {
       access_token: 'initial-access-token',
-      refresh_token: 'compute-placeholder'
-    };
-    compute.transporter = {
-      request: function(opts, opt_callback) {
-        opt_callback({}, {}, { statusCode: 401 });
-      }
+      refresh_token: 'compute-placeholder',
+      expiry_date: (new Date()).getTime() - 2000
     };
-    compute.refreshToken_ = function(token, callback) {
-      callback(null, {
-        'access_token': 'another-access-token',
-        'token_type': 'Bearer'
-      });
+    compute.request({}, function() {
+      assert.equal(compute.credentials.access_token, 'abc123');
+      scope.done();
+      done();
+    });
+  });
+
+  it('should not refresh if access token has expired', function(done) {
+    var scope = nock('http://metadata')
+        .get('/computeMetadata/v1beta1/instance/service-accounts/default/token')
+        .reply(200, { access_token: 'abc123', expires_in: 10000 });
+    var compute = new googleapis.auth.Compute();
+    compute.credentials = {
+      access_token: 'initial-access-token',
+      refresh_token: 'compute-placeholder'
     };
     compute.request({}, function() {
-      assert.equal('another-access-token', compute.credentials.access_token);
+      assert.equal(compute.credentials.access_token, 'initial-access-token');
+      assert.equal(false, scope.isDone());
+      nock.cleanAll();
       done();
     });
   });