storage: fix credentials issue.

Previously, if a user wanted to generate a signed URL, there was no guarantee a credentials object would have already been generated. By itself, generating a signed URL doesn't require a network connection, thus the uncertainty that we would have already fetched a token. This change requires a valid credentials object to exist before returning the `credentials` object.
googleapis · Sep 4, 2014 · bb4cd17 · bb4cd17
1 parent 2af7ece
commit bb4cd17
Show file tree

Hide file tree

Showing 6 changed files with 123 additions and 46 deletions.
diff --git a/lib/common/connection.js b/lib/common/connection.js
@@ -95,9 +95,10 @@ module.exports.Token = Token;
 function Connection(opts) {
   events.EventEmitter.call(this);
 
-  this.opts = opts || {};
+  opts = opts || {};
 
   this.credentials = null;
+  this.opts = opts;
   this.scopes = opts.scopes || [];
   this.token = null; // existing access token, if exists
 
@@ -308,4 +309,25 @@ Connection.prototype.authorizeReq = function(requestOptions) {
   return requestOptions;
 };
 
+/**
+ * Get the connection's credentials. If a token hasn't been fetched yet, one
+ * will be triggered now.
+ *
+ * @return {object}
+ */
+Connection.prototype.getCredentials = function(callback) {
+  var that = this;
+  if (this.credentials) {
+    setImmediate(callback, null, that.credentials);
+    return;
+  }
+  this.fetchToken(function(err) {
+    if (err) {
+      callback(err);
+      return;
+    }
+    callback(null, that.credentials);
+  });
+};
+
 module.exports.Connection = Connection;
diff --git a/lib/storage/index.js b/lib/storage/index.js
@@ -263,9 +263,9 @@ Bucket.prototype.remove = function(name, callback) {
  *   action: 'read',
  *   expires: Math.round(Date.now() / 1000) + (60 * 60 * 24 * 14), // 2 weeks.
  *   resource: 'my-dog.jpg'
- * });
+ * }, function(err, url) {});
  */
-Bucket.prototype.getSignedUrl = function(options) {
+Bucket.prototype.getSignedUrl = function(options, callback) {
   options.action = {
       read: 'GET',
       write: 'PUT',
@@ -277,22 +277,29 @@ Bucket.prototype.getSignedUrl = function(options) {
       resource: options.resource
     });
 
-  var sign = crypto.createSign('RSA-SHA256');
-  sign.update([
-      options.action,
-      (options.contentMd5 || ''),
-      (options.contentType || ''),
-      options.expires,
-      (options.extensionHeaders || '') + options.resource
-    ].join('\n'));
-  var signature = sign.sign(this.conn.credentials.private_key, 'base64');
+  this.conn.getCredentials(function(err, credentials) {
+    if (err) {
+      callback(err);
+      return;
+    }
+
+    var sign = crypto.createSign('RSA-SHA256');
+    sign.update([
+        options.action,
+        (options.contentMd5 || ''),
+        (options.contentType || ''),
+        options.expires,
+        (options.extensionHeaders || '') + options.resource
+      ].join('\n'));
+    var signature = sign.sign(credentials.private_key, 'base64');
 
-  return [
-      'http://storage.googleapis.com' + options.resource,
-      '?GoogleAccessId=' + this.conn.credentials.client_email,
-      '&Expires=' + options.expires,
-      '&Signature=' + encodeURIComponent(signature)
-    ].join('');
+    callback(null, [
+        'http://storage.googleapis.com' + options.resource,
+        '?GoogleAccessId=' + credentials.client_email,
+        '&Expires=' + options.expires,
+        '&Signature=' + encodeURIComponent(signature)
+      ].join(''));
+  });
 };
 
 /**

diff --git a/regression/storage.js b/regression/storage.js
@@ -185,6 +185,7 @@ describe('storage', function() {
 
   describe('sign urls', function() {
     var filename = 'LogoToSign.jpg';
+    var localFile = fs.readFileSync(files.logo.path);
 
     beforeEach(function(done) {
       fs.createReadStream(files.logo.path)
@@ -194,49 +195,54 @@ describe('storage', function() {
     });
 
     it('should create a signed read url', function(done) {
-      var signedReadUrl = bucket.getSignedUrl({
+      bucket.getSignedUrl({
           action: 'read',
           expires: Math.round(Date.now() / 1000) + 5,
           resource: filename
+        }, function(err, signedReadUrl) {
+          assert.ifError(err);
+          request.get(signedReadUrl, function(err, resp, body) {
+            assert.equal(body, localFile);
+            bucket.remove(filename, done);
+          });
         });
-      var localFile = fs.readFileSync(files.logo.path);
-      request.get(signedReadUrl, function(err, resp, body) {
-        assert.equal(body, localFile);
-        bucket.remove(filename, done);
-      });
     });
 
     it('should create a signed delete url', function(done) {
-      var signedDeleteUrl = bucket.getSignedUrl({
+      bucket.getSignedUrl({
           action: 'delete',
           expires: Math.round(Date.now() / 1000) + 5,
           resource: filename
+        }, function(err, signedDeleteUrl) {
+          assert.ifError(err);
+          request.del(signedDeleteUrl, function(err, resp) {
+            assert.equal(resp.statusCode, 204);
+            bucket.stat(filename, function(err) {
+              assert.equal(err.code, 404);
+              done();
+            });
+          });
         });
-      request.del(signedDeleteUrl, function(err, resp) {
-        assert.equal(resp.statusCode, 204);
-        bucket.stat(filename, function(err) {
-          assert.equal(err.code, 404);
-          done();
-        });
-      });
     });
 
     it('should allow control of expiration', function(done) {
-      var OFFSET_SECONDS = 5;
-      var signedReadUrl = bucket.getSignedUrl({
+      var offsetSeconds = 5;
+      bucket.getSignedUrl({
           action: 'read',
-          expires: Math.round(Date.now() / 1000) + OFFSET_SECONDS,
+          expires: Math.round(Date.now() / 1000) + offsetSeconds,
           resource: filename
-        });
-      request.get(signedReadUrl, function(err, resp, body) {
-        assert.equal(body, fs.readFileSync(files.logo.path));
-        setTimeout(function() {
-          request.get(signedReadUrl, function(err, resp) {
-            assert.equal(resp.statusCode, 400);
-            bucket.remove(filename, done);
+        }, function(err, signedReadUrl) {
+          assert.ifError(err);
+          request.get(signedReadUrl, function(err, resp, body) {
+            assert.equal(body, localFile);
           });
-        }, (OFFSET_SECONDS + 5) * 1000);
-      });
+          setTimeout(function() {
+            request.get(signedReadUrl, function(err, resp) {
+              assert.equal(resp.statusCode, 400);
+              bucket.remove(filename, done);
+            });
+          }, (offsetSeconds + 1) * 1000);
+        });
     });
   });
 });
diff --git a/test/common/connection.js b/test/common/connection.js
@@ -49,6 +49,35 @@ describe('Connection', function() {
   });
 
   describe('credentials object', function() {
+    describe('getCredentials', function() {
+      it('should expose a method to retrieve credentials', function() {
+        var conn = new connection.Connection();
+        assert.equal(typeof conn.getCredentials, 'function');
+      });
+
+      it('should return credentials object if provided', function(done) {
+        var conn = new connection.Connection({
+          credentials: privateKeyFileJson
+        });
+        conn.getCredentials(function(err, credentials) {
+          assert.deepEqual(credentials, privateKeyFileJson);
+          done();
+        });
+      });
+
+      it('should fetch token if missing credentials object', function(done) {
+        var conn = new connection.Connection();
+        conn.fetchToken = function(cb) {
+          conn.credentials = privateKeyFileJson;
+          cb();
+        };
+        conn.getCredentials(function(err, credentials) {
+          assert.deepEqual(credentials, privateKeyFileJson);
+          done();
+        });
+      });
+    });
+
     it('should accept and assign a complete credentials object', function() {
       var credConnection = new connection.Connection({
         credentials: privateKeyFileJson

diff --git a/test/storage/index.js b/test/storage/index.js
@@ -27,7 +27,7 @@ function createBucket() {
   return new storage.Bucket({
     bucketName: 'bucket-name',
     email: 'xxx@email.com',
-    pemFilePath: '/some/path'
+    credentials: require('../testdata/privateKeyFile.json')
   });
 }
 
@@ -130,4 +130,17 @@ describe('Bucket', function() {
     };
     bucket.remove('file-name');
   });
+
+  it('should create a signed url', function(done) {
+    var bucket = createBucket();
+    bucket.getSignedUrl({
+        action: 'read',
+        resource: 'filename',
+        expires: Date.now() / 1000
+      }, function(err, url) {
+        assert.ifError(err);
+        assert.equal(typeof url, 'string');
+        done();
+      });
+  });
 });
diff --git a/test/testdata/privateKeyFile.json b/test/testdata/privateKeyFile.json
@@ -1,6 +1,6 @@
 {
   "private_key_id": "7",
-  "private_key": "-----BEGIN PRIVATE KEY-----\n\n-----END PRIVATE KEY-----\n",
+  "private_key": "-----BEGIN RSA PRIVATE KEY-----\nMIIBOgIBAAJBAK8Q+ToR4tWGshaKYRHKJ3ZmMUF6jjwCS/u1A8v1tFbQiVpBlxYB\npaNcT2ENEXBGdmWqr8VwSl0NBIKyq4p0rhsCAQMCQHS1+3wL7I5ZzA8G62Exb6RE\nINZRtCgBh/0jV91OeDnfQUc07SE6vs31J8m7qw/rxeB3E9h6oGi9IVRebVO+9zsC\nIQDWb//KAzrSOo0P0yktnY57UF9Q3Y26rulWI6LqpsxZDwIhAND/cmlg7rUz34Pf\nSmM61lJEmMEjKp8RB/xgghzmCeI1AiEAjvVVMVd8jCcItTdwyRO0UjWU4JOz0cnw\n5BfB8cSIO18CIQCLVPbw60nOIpUClNxCJzmMLbsrbMcUtgVS6wFomVvsIwIhAK+A\nYqT6WwsMW2On5l9di+RPzhDT1QdGyTI5eFNS+GxY\n-----END RSA PRIVATE KEY-----",
   "client_email": "firstpart@secondpart.com",
   "client_id": "8",
   "type": "service_account"