From b0353d048bcdd050a1c88e326adc30a8e659e2fd Mon Sep 17 00:00:00 2001 From: James Smith Date: Mon, 28 May 2018 14:04:22 +0100 Subject: [PATCH 1/8] get access token before importing users --- src/management/JobsManager.js | 76 ++++++++++++++++++----------------- 1 file changed, 40 insertions(+), 36 deletions(-) diff --git a/src/management/JobsManager.js b/src/management/JobsManager.js index 9611b1b3d..6d0f41df7 100644 --- a/src/management/JobsManager.js +++ b/src/management/JobsManager.js @@ -100,7 +100,7 @@ JobsManager.prototype.get = function(params, cb) { /** * Given a path to a file and a connection id, create a new job that imports the - * users contained in the file or JSON string and associate them with the given + * users contained in the file or JSON string and associate them with the given * connection. * * @method importUsers @@ -135,42 +135,46 @@ JobsManager.prototype.importUsers = function(data, cb) { var url = options.baseUrl + '/jobs/users-imports'; var method = 'POST'; - var promise = new Promise(function(resolve, reject) { - request( - { - url: url, - method: method, - headers: headers, - formData: { - users: { - value: data.users_json ? Buffer.from(data.users_json) : fs.createReadStream(data.users), - options: { - filename: data.users_json ? 'users.json' : data.users, - } - }, - connection_id: data.connection_id + var promise = options.tokenProvider.getAccessToken().then(function(access_token) { + return new Promise(function(resolve, reject) { + request( + { + url: url, + method: method, + headers: extend({ Authorization: `Bearer ${access_token}` }, headers), + formData: { + users: { + value: data.users_json + ? Buffer.from(data.users_json) + : fs.createReadStream(data.users), + options: { + filename: data.users_json ? 'users.json' : data.users + } + }, + connection_id: data.connection_id + } + }, + function(err, res) { + // `superagent` uses the error parameter in callback on http errors. + // the following code is intended to keep that behaviour (https://github.com/visionmedia/superagent/blob/master/lib/node/response.js#L170) + var type = (res.statusCode / 100) | 0; + var isErrorResponse = 4 === type || 5 === type; + if (isErrorResponse) { + var error = new Error('cannot ' + method + url + ' (' + res.statusCode + ')'); + error.status = res.statusCode; + error.method = method; + error.text = res.text; + reject(error); + } + + if (err) { + reject(err); + } + + resolve(res); } - }, - function(err, res) { - // `superagent` uses the error parameter in callback on http errors. - // the following code is intended to keep that behaviour (https://github.com/visionmedia/superagent/blob/master/lib/node/response.js#L170) - var type = (res.statusCode / 100) | 0; - var isErrorResponse = 4 === type || 5 === type; - if (isErrorResponse) { - var error = new Error('cannot ' + method + url + ' (' + res.statusCode + ')'); - error.status = res.statusCode; - error.method = method; - error.text = res.text; - reject(error); - } - - if (err) { - reject(err); - } - - resolve(res); - } - ); + ); + }); }); // Don't return a promise if a callback was given. From 4359feff280b702e5cee2ccdf449c175c69dc0cf Mon Sep 17 00:00:00 2001 From: James Smith Date: Mon, 28 May 2018 14:48:21 +0100 Subject: [PATCH 2/8] remove the hardcoded auth token in test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit now it’s obtained automatically --- test/management/jobs.tests.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/management/jobs.tests.js b/test/management/jobs.tests.js index f93fef635..5cb4037f2 100644 --- a/test/management/jobs.tests.js +++ b/test/management/jobs.tests.js @@ -12,10 +12,8 @@ var ArgumentError = require('rest-facade').ArgumentError; describe('JobsManager', function() { before(function() { - this.token = 'TOKEN'; this.id = 'testJob'; this.jobs = new JobsManager({ - headers: { authorization: 'Bearer ' + this.token }, baseUrl: API_URL }); }); From 60f44b1af179690bb3b779b485386c1decf1f0bc Mon Sep 17 00:00:00 2001 From: James Smith Date: Mon, 28 May 2018 14:48:52 +0100 Subject: [PATCH 3/8] mock tokenProvider to get an accessToken --- test/management/jobs.tests.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/management/jobs.tests.js b/test/management/jobs.tests.js index 5cb4037f2..31980d122 100644 --- a/test/management/jobs.tests.js +++ b/test/management/jobs.tests.js @@ -14,6 +14,12 @@ describe('JobsManager', function() { before(function() { this.id = 'testJob'; this.jobs = new JobsManager({ + tokenProvider: { + getAccessToken: function() { + return Promise.resolve('TOKEN'); + } + }, + headers: {}, baseUrl: API_URL }); }); From 2512c31b2739394aee0b476e01b604cbbb44e97c Mon Sep 17 00:00:00 2001 From: James Smith Date: Mon, 28 May 2018 17:58:21 +0100 Subject: [PATCH 4/8] update token tests to include the right token --- test/management/jobs.tests.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/management/jobs.tests.js b/test/management/jobs.tests.js index 31980d122..bd8cec968 100644 --- a/test/management/jobs.tests.js +++ b/test/management/jobs.tests.js @@ -16,7 +16,7 @@ describe('JobsManager', function() { this.jobs = new JobsManager({ tokenProvider: { getAccessToken: function() { - return Promise.resolve('TOKEN'); + return Promise.resolve('VALID_TOKEN'); } }, headers: {}, @@ -119,7 +119,7 @@ describe('JobsManager', function() { var request = nock(API_URL) .get('/jobs/' + this.id) - .matchHeader('Authorization', 'Bearer ' + this.token) + .matchHeader('Authorization', 'Bearer VALID_TOKEN') .reply(200); this.jobs.get({ id: this.id }).then(function() { @@ -257,7 +257,7 @@ describe('JobsManager', function() { var request = nock(API_URL) .post('/jobs/users-imports') - .matchHeader('Authorization', 'Bearer ' + this.token) + .matchHeader('Authorization', 'Bearer VALID_TOKEN') .reply(200); this.jobs.importUsers(data).then(function() { @@ -382,7 +382,7 @@ describe('JobsManager', function() { var request = nock(API_URL) .post('/jobs/verification-email') - .matchHeader('Authorization', 'Bearer ' + this.token) + .matchHeader('Authorization', 'Bearer VALID_TOKEN') .reply(200); this.jobs.verifyEmail(data).then(function() { From 44e2a3b5592bcab33a038e3299afe3d13517826c Mon Sep 17 00:00:00 2001 From: James Smith Date: Mon, 28 May 2018 18:05:43 +0100 Subject: [PATCH 5/8] DRY up token value --- test/management/jobs.tests.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/management/jobs.tests.js b/test/management/jobs.tests.js index bd8cec968..b1f4d3234 100644 --- a/test/management/jobs.tests.js +++ b/test/management/jobs.tests.js @@ -10,13 +10,15 @@ var API_URL = 'https://tenant.auth0.com'; var JobsManager = require(SRC_DIR + '/management/JobsManager'); var ArgumentError = require('rest-facade').ArgumentError; +var token = 'TOKEN'; + describe('JobsManager', function() { before(function() { this.id = 'testJob'; this.jobs = new JobsManager({ tokenProvider: { getAccessToken: function() { - return Promise.resolve('VALID_TOKEN'); + return Promise.resolve(token); } }, headers: {}, @@ -119,7 +121,7 @@ describe('JobsManager', function() { var request = nock(API_URL) .get('/jobs/' + this.id) - .matchHeader('Authorization', 'Bearer VALID_TOKEN') + .matchHeader('Authorization', 'Bearer ' + token) .reply(200); this.jobs.get({ id: this.id }).then(function() { @@ -257,7 +259,7 @@ describe('JobsManager', function() { var request = nock(API_URL) .post('/jobs/users-imports') - .matchHeader('Authorization', 'Bearer VALID_TOKEN') + .matchHeader('Authorization', 'Bearer ' + token) .reply(200); this.jobs.importUsers(data).then(function() { @@ -382,7 +384,7 @@ describe('JobsManager', function() { var request = nock(API_URL) .post('/jobs/verification-email') - .matchHeader('Authorization', 'Bearer VALID_TOKEN') + .matchHeader('Authorization', 'Bearer ' + token) .reply(200); this.jobs.verifyEmail(data).then(function() { From a2fec6652e3f39c96b60c5354fdf1536be963564 Mon Sep 17 00:00:00 2001 From: James Smith Date: Thu, 31 May 2018 12:05:43 +0100 Subject: [PATCH 6/8] move error check to start of request handler --- src/management/JobsManager.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/management/JobsManager.js b/src/management/JobsManager.js index 6d0f41df7..f52a63893 100644 --- a/src/management/JobsManager.js +++ b/src/management/JobsManager.js @@ -155,6 +155,10 @@ JobsManager.prototype.importUsers = function(data, cb) { } }, function(err, res) { + if (err) { + reject(err); + return; + } // `superagent` uses the error parameter in callback on http errors. // the following code is intended to keep that behaviour (https://github.com/visionmedia/superagent/blob/master/lib/node/response.js#L170) var type = (res.statusCode / 100) | 0; @@ -166,11 +170,6 @@ JobsManager.prototype.importUsers = function(data, cb) { error.text = res.text; reject(error); } - - if (err) { - reject(err); - } - resolve(res); } ); From e22113033ec9123b76dafba155f92f15db853cfd Mon Sep 17 00:00:00 2001 From: James Smith Date: Thu, 31 May 2018 12:07:16 +0100 Subject: [PATCH 7/8] fix error message spacing and check in test --- src/management/JobsManager.js | 2 +- test/management/jobs.tests.js | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/management/JobsManager.js b/src/management/JobsManager.js index f52a63893..fb26918f5 100644 --- a/src/management/JobsManager.js +++ b/src/management/JobsManager.js @@ -164,7 +164,7 @@ JobsManager.prototype.importUsers = function(data, cb) { var type = (res.statusCode / 100) | 0; var isErrorResponse = 4 === type || 5 === type; if (isErrorResponse) { - var error = new Error('cannot ' + method + url + ' (' + res.statusCode + ')'); + var error = new Error('cannot ' + method + ' ' + url + ' (' + res.statusCode + ')'); error.status = res.statusCode; error.method = method; error.text = res.text; diff --git a/test/management/jobs.tests.js b/test/management/jobs.tests.js index b1f4d3234..432f3d203 100644 --- a/test/management/jobs.tests.js +++ b/test/management/jobs.tests.js @@ -175,7 +175,7 @@ describe('JobsManager', function() { .catch(done.bind(null, null)); }); - it('should pass any errors to the promise catch handler', function(done) { + it('should pass HTTP errors to the promise catch handler', function(done) { nock.cleanAll(); var request = nock(API_URL) @@ -183,7 +183,9 @@ describe('JobsManager', function() { .reply(500); this.jobs.importUsers(data).catch(function(err) { - expect(err).to.exist; + expect(err.message).to.equal( + 'cannot POST https://tenant.auth0.com/jobs/users-imports (500)' + ); done(); }); }); From 429da3bef08086a2d751af1439f0df2a3e009fc9 Mon Sep 17 00:00:00 2001 From: James Smith Date: Thu, 31 May 2018 12:07:51 +0100 Subject: [PATCH 8/8] add check for total request failure --- test/management/jobs.tests.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/management/jobs.tests.js b/test/management/jobs.tests.js index 432f3d203..c273fbfb2 100644 --- a/test/management/jobs.tests.js +++ b/test/management/jobs.tests.js @@ -175,6 +175,19 @@ describe('JobsManager', function() { .catch(done.bind(null, null)); }); + it('should pass request errors to the promise catch handler', function(done) { + nock.cleanAll(); + + var request = nock(API_URL) + .post('/jobs/users-imports') + .replyWithError('printer on fire'); + + this.jobs.importUsers(data).catch(function(err) { + expect(err.message).to.equal('printer on fire'); + done(); + }); + }); + it('should pass HTTP errors to the promise catch handler', function(done) { nock.cleanAll();