Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get access token before importing users #267

Merged
merged 9 commits into from
May 31, 2018
Merged

Get access token before importing users #267

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b0353d0
get access token before importing users
Floppy May 28, 2018
4359fef
remove the hardcoded auth token in test
Floppy May 28, 2018
60f44b1
mock tokenProvider to get an accessToken
Floppy May 28, 2018
2512c31
update token tests to include the right token
Floppy May 28, 2018
44e2a3b
DRY up token value
Floppy May 28, 2018
af160e2
Merge branch 'master' into auth-before-import
luisrudge May 30, 2018
a2fec66
move error check to start of request handler
Floppy May 31, 2018
e221130
fix error message spacing and check in test
Floppy May 31, 2018
429da3b
add check for total request failure
Floppy May 31, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 39 additions & 36 deletions src/management/JobsManager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -135,42 +135,45 @@ 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) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before the existing request promise, we call getAccessToken from the provider we were passed on creation. This is then passed a function that wraps the existing code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is the right way to do it, but I'm not an expert with the various ways this codebase handles authentication, so if it should do something different as well / instead, please let me know.

return new Promise(function(resolve, reject) {
request(
{
url: url,
method: method,
headers: extend({ Authorization: `Bearer ${access_token}` }, headers),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The token is then added to the 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) {
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;
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);
}
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.
Expand Down
35 changes: 28 additions & 7 deletions test/management/jobs.tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,18 @@ 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.token = 'TOKEN';
this.id = 'testJob';
this.jobs = new JobsManager({
headers: { authorization: 'Bearer ' + this.token },
tokenProvider: {
getAccessToken: function() {
return Promise.resolve(token);
}
},
headers: {},
baseUrl: API_URL
});
});
Expand Down Expand Up @@ -115,7 +121,7 @@ describe('JobsManager', function() {

var request = nock(API_URL)
.get('/jobs/' + this.id)
.matchHeader('Authorization', 'Bearer ' + this.token)
.matchHeader('Authorization', 'Bearer ' + token)
.reply(200);

this.jobs.get({ id: this.id }).then(function() {
Expand Down Expand Up @@ -169,15 +175,30 @@ describe('JobsManager', function() {
.catch(done.bind(null, null));
});

it('should pass any errors to the promise catch handler', function(done) {
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();

var request = nock(API_URL)
.post('/jobs/users-imports')
.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();
});
});
Expand Down Expand Up @@ -253,7 +274,7 @@ describe('JobsManager', function() {

var request = nock(API_URL)
.post('/jobs/users-imports')
.matchHeader('Authorization', 'Bearer ' + this.token)
.matchHeader('Authorization', 'Bearer ' + token)
.reply(200);

this.jobs.importUsers(data).then(function() {
Expand Down Expand Up @@ -378,7 +399,7 @@ describe('JobsManager', function() {

var request = nock(API_URL)
.post('/jobs/verification-email')
.matchHeader('Authorization', 'Bearer ' + this.token)
.matchHeader('Authorization', 'Bearer ' + token)
.reply(200);

this.jobs.verifyEmail(data).then(function() {
Expand Down