From 84249014354610cfa386c57b6755a94b766557af Mon Sep 17 00:00:00 2001 From: Stephen Sawchuk Date: Wed, 1 Nov 2017 16:06:28 -0400 Subject: [PATCH 1/4] Support un-authenticated requests. --- packages/common/src/service.js | 17 +++-- packages/common/src/util.js | 39 +++++++---- packages/common/test/service.js | 4 +- packages/common/test/util.js | 112 ++++++++++++++++++++++++++------ 4 files changed, 132 insertions(+), 40 deletions(-) diff --git a/packages/common/src/service.js b/packages/common/src/service.js index 12a7d335072..2cbfeb118ba 100644 --- a/packages/common/src/service.js +++ b/packages/common/src/service.js @@ -48,7 +48,17 @@ var PROJECT_ID_TOKEN = '{{projectId}}'; function Service(config, options) { options = options || {}; + this.baseUrl = config.baseUrl; + this.globalInterceptors = arrify(options.interceptors_); + this.interceptors = []; + this.packageJson = config.packageJson; + this.projectId = options.projectId || PROJECT_ID_TOKEN; + this.projectIdRequired = config.projectIdRequired !== false; + this.Promise = options.promise || Promise; + var reqCfg = extend({}, config, { + projectIdRequired: this.projectIdRequired, + projectId: this.projectId, credentials: options.credentials, keyFile: options.keyFilename, email: options.email @@ -56,14 +66,7 @@ function Service(config, options) { this.makeAuthenticatedRequest = util.makeAuthenticatedRequestFactory(reqCfg); this.authClient = this.makeAuthenticatedRequest.authClient; - this.baseUrl = config.baseUrl; this.getCredentials = this.makeAuthenticatedRequest.getCredentials; - this.globalInterceptors = arrify(options.interceptors_); - this.interceptors = []; - this.packageJson = config.packageJson; - this.projectId = options.projectId || PROJECT_ID_TOKEN; - this.projectIdRequired = config.projectIdRequired !== false; - this.Promise = options.promise || Promise; var isCloudFunctionEnv = !!process.env.FUNCTION_NAME; diff --git a/packages/common/src/util.js b/packages/common/src/util.js index 84fa5e58061..286847aa084 100644 --- a/packages/common/src/util.js +++ b/packages/common/src/util.js @@ -346,14 +346,33 @@ function makeAuthenticatedRequestFactory(config) { } function onAuthenticated(err, authenticatedReqOpts) { - if (!err) { + var autoAuthFailed = + err && + err.message.indexOf('Could not load the default credentials') > -1; + + if (autoAuthFailed) { + // Even though authentication failed, the API might not actually care. + authenticatedReqOpts = reqOpts; + } + + if (!err || autoAuthFailed) { + var projectId = authClient.projectId; + + if (config.projectId && config.projectId !== '{{projectId}}') { + projectId = config.projectId; + } + try { authenticatedReqOpts = util.decorateRequest( authenticatedReqOpts, - extend({ projectId: authClient.projectId }, config) + projectId ); + err = null; } catch(e) { - err = e; + // A projectId was required, but we don't have one. + // Re-use the "Could not load the default credentials error" if auto + // auth failed. + err = err || e; } } @@ -473,12 +492,10 @@ util.makeRequest = makeRequest; * Decorate the options about to be made in a request. * * @param {object} reqOpts - The options to be passed to `request`. - * @param {object} config - Service config. + * @param {string} projectId - The project ID. * @return {object} reqOpts - The decorated reqOpts. */ -function decorateRequest(reqOpts, config) { - config = config || {}; - +function decorateRequest(reqOpts, projectId) { delete reqOpts.autoPaginate; delete reqOpts.autoPaginateVal; delete reqOpts.objectMode; @@ -486,16 +503,16 @@ function decorateRequest(reqOpts, config) { if (is.object(reqOpts.qs)) { delete reqOpts.qs.autoPaginate; delete reqOpts.qs.autoPaginateVal; - reqOpts.qs = util.replaceProjectIdToken(reqOpts.qs, config.projectId); + reqOpts.qs = util.replaceProjectIdToken(reqOpts.qs, projectId); } if (is.object(reqOpts.json)) { delete reqOpts.json.autoPaginate; delete reqOpts.json.autoPaginateVal; - reqOpts.json = util.replaceProjectIdToken(reqOpts.json, config.projectId); + reqOpts.json = util.replaceProjectIdToken(reqOpts.json, projectId); } - reqOpts.uri = util.replaceProjectIdToken(reqOpts.uri, config.projectId); + reqOpts.uri = util.replaceProjectIdToken(reqOpts.uri, projectId); return reqOpts; } @@ -528,7 +545,7 @@ function replaceProjectIdToken(value, projectId) { } if (is.string(value) && value.indexOf('{{projectId}}') > -1) { - if (!projectId) { + if (!projectId || projectId === '{{projectId}}') { throw util.missingProjectIdError; } value = value.replace(/{{projectId}}/g, projectId); diff --git a/packages/common/test/service.js b/packages/common/test/service.js index c80aecd513d..9822817d4af 100644 --- a/packages/common/test/service.js +++ b/packages/common/test/service.js @@ -78,7 +78,9 @@ describe('Service', function() { var expectedConfig = extend({}, CONFIG, { credentials: OPTIONS.credentials, keyFile: OPTIONS.keyFilename, - email: OPTIONS.email + email: OPTIONS.email, + projectIdRequired: CONFIG.projectIdRequired, + projectId: OPTIONS.projectId }); assert.deepEqual(config, expectedConfig); diff --git a/packages/common/test/util.js b/packages/common/test/util.js index c49feea5230..0a5c9f70c6f 100644 --- a/packages/common/test/util.js +++ b/packages/common/test/util.js @@ -709,7 +709,7 @@ describe('common/util', function() { var config = { customEndpoint: true }; - var expectedConfig = extend({ projectId: authClient.projectId }, config); + var expectedProjectId = authClient.projectId; beforeEach(function() { makeAuthenticatedRequest = util.makeAuthenticatedRequestFactory(config); @@ -719,9 +719,9 @@ describe('common/util', function() { var reqOpts = { a: 'b', c: 'd' }; var decoratedRequest = {}; - utilOverrides.decorateRequest = function(reqOpts_, config_) { + utilOverrides.decorateRequest = function(reqOpts_, projectId) { assert.strictEqual(reqOpts_, reqOpts); - assert.deepEqual(config_, expectedConfig); + assert.deepEqual(projectId, expectedProjectId); return decoratedRequest; }; @@ -793,6 +793,43 @@ describe('common/util', function() { assert(makeAuthenticatedRequest({}) instanceof stream.Stream); }); + describe('projectId', function() { + it('should default to authClient projectId', function(done) { + authClient.projectId = 'authclient-project-id'; + + utilOverrides.decorateRequest = function(reqOpts, projectId) { + assert.strictEqual(projectId, authClient.projectId); + done(); + }; + + var makeAuthenticatedRequest = util.makeAuthenticatedRequestFactory({ + customEndpoint: true + }); + + makeAuthenticatedRequest({}); + }); + + it('should use user-provided projectId', function(done) { + authClient.projectId = 'authclient-project-id'; + + var config = { + customEndpoint: true, + projectId: 'project-id' + }; + + utilOverrides.decorateRequest = function(reqOpts, projectId) { + assert.strictEqual(projectId, config.projectId); + done(); + }; + + var makeAuthenticatedRequest = util.makeAuthenticatedRequestFactory( + config + ); + + makeAuthenticatedRequest({}); + }); + }); + describe('authentication errors', function() { var error = new Error('Error.'); @@ -804,6 +841,45 @@ describe('common/util', function() { }; }); + it('should attempt request anyway', function(done) { + var makeAuthenticatedRequest = util.makeAuthenticatedRequestFactory(); + + var correctReqOpts = {}; + var incorrectReqOpts = {}; + + authClient.authorizeRequest = function(rOpts, callback) { + var error = new Error('Could not load the default credentials'); + callback(error, incorrectReqOpts); + }; + + makeAuthenticatedRequest(correctReqOpts, { + onAuthenticated: function(err, reqOpts) { + assert.ifError(err); + + assert.strictEqual(reqOpts, correctReqOpts); + assert.notStrictEqual(reqOpts, incorrectReqOpts); + + done(); + } + }); + }); + + it('should block decorateRequest error', function(done) { + var decorateRequestError = new Error('Error.'); + utilOverrides.decorateRequest = function(reqOpts_) { + throw decorateRequestError; + }; + + var makeAuthenticatedRequest = util.makeAuthenticatedRequestFactory(); + makeAuthenticatedRequest({}, { + onAuthenticated: function(err) { + assert.notStrictEqual(err, decorateRequestError); + assert.strictEqual(err, error); + done(); + } + }); + }); + it('should invoke the callback with error', function(done) { var makeAuthenticatedRequest = util.makeAuthenticatedRequestFactory(); makeAuthenticatedRequest({}, function(err) { @@ -1281,64 +1357,58 @@ describe('common/util', function() { }); it('should replace project ID tokens for qs object', function() { - var config = { - projectId: 'project-id' - }; + var projectId = 'project-id'; var reqOpts = { uri: 'http://', qs: {} }; var decoratedQs = {}; - utilOverrides.replaceProjectIdToken = function(qs, projectId) { + utilOverrides.replaceProjectIdToken = function(qs, projectId_) { utilOverrides = {}; assert.strictEqual(qs, reqOpts.qs); - assert.strictEqual(projectId, config.projectId); + assert.strictEqual(projectId_, projectId); return decoratedQs; }; - var decoratedRequest = util.decorateRequest(reqOpts, config); + var decoratedRequest = util.decorateRequest(reqOpts, projectId); assert.strictEqual(decoratedRequest.qs, decoratedQs); }); it('should replace project ID tokens for json object', function() { - var config = { - projectId: 'project-id' - }; + var projectId = 'project-id'; var reqOpts = { uri: 'http://', json: {} }; var decoratedJson = {}; - utilOverrides.replaceProjectIdToken = function(json, projectId) { + utilOverrides.replaceProjectIdToken = function(json, projectId_) { utilOverrides = {}; assert.strictEqual(reqOpts.json, json); - assert.strictEqual(projectId, config.projectId); + assert.strictEqual(projectId_, projectId); return decoratedJson; }; - var decoratedRequest = util.decorateRequest(reqOpts, config); + var decoratedRequest = util.decorateRequest(reqOpts, projectId); assert.strictEqual(decoratedRequest.json, decoratedJson); }); it('should decorate the request', function() { - var config = { - projectId: 'project-id' - }; + var projectId = 'project-id'; var reqOpts = { uri: 'http://' }; var decoratedUri = 'http://decorated'; - utilOverrides.replaceProjectIdToken = function(uri, projectId) { + utilOverrides.replaceProjectIdToken = function(uri, projectId_) { assert.strictEqual(uri, reqOpts.uri); - assert.strictEqual(projectId, config.projectId); + assert.strictEqual(projectId_, projectId); return decoratedUri; }; assert.deepEqual( - util.decorateRequest(reqOpts, config), + util.decorateRequest(reqOpts, projectId), { uri: decoratedUri } ); }); From a9bc50c861c128f1e7be3ecf32fb88710e1c5e17 Mon Sep 17 00:00:00 2001 From: Stephen Sawchuk Date: Wed, 1 Nov 2017 16:24:43 -0400 Subject: [PATCH 2/4] Lint. --- packages/common/test/util.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/common/test/util.js b/packages/common/test/util.js index 0a5c9f70c6f..e9c9a928265 100644 --- a/packages/common/test/util.js +++ b/packages/common/test/util.js @@ -866,7 +866,7 @@ describe('common/util', function() { it('should block decorateRequest error', function(done) { var decorateRequestError = new Error('Error.'); - utilOverrides.decorateRequest = function(reqOpts_) { + utilOverrides.decorateRequest = function() { throw decorateRequestError; }; From 9d1bbce5b7f0420893c145bc14aeff46343d7486 Mon Sep 17 00:00:00 2001 From: Stephen Sawchuk Date: Wed, 1 Nov 2017 16:28:00 -0400 Subject: [PATCH 3/4] Test fix. --- packages/common/test/util.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/common/test/util.js b/packages/common/test/util.js index e9c9a928265..2a6624c9f76 100644 --- a/packages/common/test/util.js +++ b/packages/common/test/util.js @@ -806,7 +806,9 @@ describe('common/util', function() { customEndpoint: true }); - makeAuthenticatedRequest({}); + makeAuthenticatedRequest({ + onAuthenticated: assert.ifError + }); }); it('should use user-provided projectId', function(done) { @@ -826,7 +828,9 @@ describe('common/util', function() { config ); - makeAuthenticatedRequest({}); + makeAuthenticatedRequest({ + onAuthenticated: assert.ifError + }); }); }); From 71d24a8be611e06bce4bd1dd22d03506dd2544d4 Mon Sep 17 00:00:00 2001 From: Stephen Sawchuk Date: Wed, 1 Nov 2017 16:48:48 -0400 Subject: [PATCH 4/4] Text fix 2. --- packages/common/test/util.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/common/test/util.js b/packages/common/test/util.js index 2a6624c9f76..af3c23bd015 100644 --- a/packages/common/test/util.js +++ b/packages/common/test/util.js @@ -799,14 +799,14 @@ describe('common/util', function() { utilOverrides.decorateRequest = function(reqOpts, projectId) { assert.strictEqual(projectId, authClient.projectId); - done(); + setImmediate(done); }; var makeAuthenticatedRequest = util.makeAuthenticatedRequestFactory({ customEndpoint: true }); - makeAuthenticatedRequest({ + makeAuthenticatedRequest({}, { onAuthenticated: assert.ifError }); }); @@ -821,14 +821,14 @@ describe('common/util', function() { utilOverrides.decorateRequest = function(reqOpts, projectId) { assert.strictEqual(projectId, config.projectId); - done(); + setImmediate(done); }; var makeAuthenticatedRequest = util.makeAuthenticatedRequestFactory( config ); - makeAuthenticatedRequest({ + makeAuthenticatedRequest({}, { onAuthenticated: assert.ifError }); });