Skip to content
This repository has been archived by the owner on Jan 19, 2024. It is now read-only.

Updates to 401 Handling #64

Merged
merged 2 commits into from
Nov 5, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 11 additions & 6 deletions lib/fuel-rest.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ FuelRest.prototype._processRequest = function(options, callback) {
this.AuthClient.getAccessToken(clone(options.auth), function(err, authResponse) {
var authOptions;
var localError;
var retry;
var retry = false;
var consolidatedOpts = {};

if(err) {
Expand All @@ -97,18 +97,22 @@ FuelRest.prototype._processRequest = function(options, callback) {
return;
}

retry = options.retry || false;
authOptions = clone(options.auth);

delete options.retry;
delete options.auth;

options.uri = helpers.resolveUri(this.origin, options.uri);
options.headers = merge({}, this.defaultHeaders, options.headers);
options.headers.Authorization = options.headers.Authorization || 'Bearer ' + authResponse.accessToken;

if(!options.headers.Authorization) {
options.headers.Authorization = 'Bearer ' + authResponse.accessToken;
retry = options.retry || false;
Copy link
Contributor

Choose a reason for hiding this comment

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

So only retry if they don't provide custom auth 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.

correct, thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

At first I wondered, but now I think it's fine. Since the auth header shouldn't really diff from the one provided (just there for an extra option), this is totally cool. 👍

}

delete options.retry;
delete options.auth;

consolidatedOpts.req = options;
consolidatedOpts.auth = authOptions;
consolidatedOpts.accessToken = authResponse.accessToken;
consolidatedOpts.retry = retry;

this._makeRequest(consolidatedOpts, callback);
Expand All @@ -129,6 +133,7 @@ FuelRest.prototype._makeRequest = function(consolidatedOpts, callback) {

// check if we should retry req
if(helpers.isValid401(res) && consolidatedOpts.retry) {
this.AuthClient.invalidateToken(consolidatedOpts.accessToken);
requestOptions.auth = consolidatedOpts.auth;
this.apiRequest(requestOptions, callback);
return;
Expand Down
61 changes: 61 additions & 0 deletions test/specs/fn-apiRequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,38 @@ describe('apiRequest method', function() {
}, true);
});

it('should skip retry when Authorization header is provided and request 401s', function(done) {
var requestSpy;
var RestClient;

requestSpy = sinon.spy(FuelRest.prototype, 'apiRequest');

sinon.stub(FuelAuth.prototype, 'getAccessToken', function(options, callback) {
callback(null, { accessToken: 'testing', expiresIn: 3600 });
});

RestClient = new FuelRest(initOptions);

requestOptions.uri = routes.invalidToken;
requestOptions.retry = true;
requestOptions.auth = {
force: true
};
requestOptions.headers = {
Authorization: 'Bearer SomeToken'
};

RestClient.apiRequest(requestOptions, function() {
// error should be passed, and data should be null
expect(requestSpy.calledTwice).to.be.false;

FuelRest.prototype.apiRequest.restore();
FuelAuth.prototype.getAccessToken.restore();
// finish async test
done();
}, true);
});

it('should use a full URI if provided', function(done) {
requestOptions.uri = localhost + routes.get;

Expand All @@ -256,4 +288,33 @@ describe('apiRequest method', function() {
done();
});
});

describe('invalidating token', function() {
it('should tell auth client to invalide it\'s token', function(done) {
var invalidateSpy = sinon.stub(FuelAuth.prototype, 'invalidateToken');
var RestClient;

sinon.stub(FuelAuth.prototype, 'getAccessToken', function(options, callback) {
callback(null, { accessToken: 'testing', expiresIn: 3600 });
});

RestClient = new FuelRest(initOptions);

requestOptions.uri = routes.invalidToken;
requestOptions.retry = true;
requestOptions.auth = {
force: true
};

RestClient.apiRequest(requestOptions, function() {
expect(invalidateSpy.callCount).to.equal(1);

FuelAuth.prototype.getAccessToken.restore();
FuelAuth.prototype.invalidateToken.restore();

// finish async test
done();
}, true);
});
});
});