Skip to content
This repository has been archived by the owner on Apr 3, 2019. It is now read-only.

Commit

Permalink
fix(client): Ensure a down OAuth server does not cause an undefined…
Browse files Browse the repository at this point in the history 
… error.

Lots of changes under the hood:

* oauth-client.js: Convert jQuery promises to rest of the world promises to ensure compatibility.
* oauth-client.js: Convert server error responses to OAuthError based errors.
* oauth-client.js: Non-reachable OAuth server returns a SERVICE_UNAVAILABLE error
* oauth-errors.js: Add all possible oauth error codes, whether they are used or not.
* service-mixin.js: Work with OAuthError's instead of raw JSON responses.

fixes #1344
  • Loading branch information
@zaach
Shane Tomlinson authored and zaach committed Jul 21, 2014
1 parent 1acb5ff commit 7224d95
Show file tree
Hide file tree
Showing 5 changed files with 142 additions and 46 deletions.
6 changes: 2 additions & 4 deletions app/scripts/lib/auth-errors.js
View file
Expand Up @@ -31,13 +31,11 @@ function () {
MISSING_CONTENT_LENGTH_HEADER: 112,
REQUEST_TOO_LARGE: 113,
THROTTLED: 114,
// If the auth server is unavailable, it will not respond.
// Use a client side only code.
SERVICE_UNAVAILABLE: 998,
SERVER_BUSY: 201,
ENDPOINT_NOT_SUPPORTED: 116,

// local only error codes
SERVICE_UNAVAILABLE: 998,
USER_CANCELED_LOGIN: 1001,
SESSION_EXPIRED: 1002,

Expand Down Expand Up @@ -72,11 +70,11 @@ function () {
112: t('Missing content-length header'),
113: t('Request body too large'),
114: t('Attempt limit exceeded'),
998: t('System unavailable, try again soon'),
201: t('Server busy, try again soon'),
116: t('This endpoint is no longer supported'),

// local only error messages
998: t('System unavailable, try again soon'),
1002: t('Session expired. Sign in to continue.'),
1003: t('Cookies are still disabled'),
1004: t('Passwords do not match'),
Expand Down
19 changes: 17 additions & 2 deletions app/scripts/lib/oauth-client.js
View file
Expand Up @@ -8,9 +8,10 @@ define([
'jquery',
'lib/promise',
'lib/session',
'lib/config-loader'
'lib/config-loader',
'lib/oauth-errors'
],
function ($, p, Session, ConfigLoader) {
function ($, p, Session, ConfigLoader, OAuthErrors) {
var oauthUrl;

var GET_CLIENT = '/v1/client/';
Expand All @@ -25,6 +26,20 @@ function ($, p, Session, ConfigLoader) {
}
}

function transformError(xhr) {
if (!xhr || xhr.status === 0) {
return OAuthErrors.toError('SERVICE_UNAVAILABLE');
}

var errObj = xhr.responseJSON;

if (!errObj) {
return OAuthErrors.toError('UNEXPECTED_ERROR');
}

return OAuthErrors.toError(errObj.errno);
}

OAuthClient.prototype = {
_getOauthUrl: function _getOauthUrl() {
var configLoader = new ConfigLoader();
Expand Down
22 changes: 18 additions & 4 deletions app/scripts/lib/oauth-errors.js
View file
Expand Up @@ -16,22 +16,36 @@ function (_, AuthErrors) {
};

var ERROR_TO_CODE = {
UNEXPECTED_ERROR: 999,
UNKNOWN_CLIENT: 101,
INCORRECT_CLIENT_SECRET: 102,
INCORRECT_REDIRECT: 103,
INVALID_ASSERTION: 104,
UNKNOWN_CODE: 105,
INCORRECT_CODE: 106,
EXPIRED_CODE: 107,
INVALID_PARAMETER: 108,
INVALID_REQUEST_SIGNATURE: 109
INVALID_REQUEST_SIGNATURE: 109,

// local only errors.
SERVICE_UNAVAILABLE: 998,
UNEXPECTED_ERROR: 999
};

var CODE_TO_MESSAGES = {
// errors returned by the oauth server
999: t('Unexpected error'),
101: t('Unknown client'),
102: t('Incorrect client secret'),
103: t('Incorrect redirect_uri'),
104: t('Invalid assertion'),
105: t('Unknown code'),
106: t('Incorrect code'),
107: t('Expired code'),
108: t('Invalid parameter in request body: %(param)s'),
109: t('Invalid request signature')
109: t('Invalid request signature'),

// local only errors.
998: t('System unavailable, try again soon'),
999: t('Unexpected error')
};

return _.extend({}, AuthErrors, {
Expand Down
8 changes: 4 additions & 4 deletions app/scripts/views/mixins/service-mixin.js
View file
Expand Up @@ -61,8 +61,8 @@ define([
self.serviceName = clientInfo.name;
self.serviceRedirectURI = clientInfo.redirect_uri;
})
.fail(function(xhr) {
self.displayError(xhr.responseJSON, OAuthErrors);
.fail(function(err) {
self.displayError(err, OAuthErrors);
});
},

Expand All @@ -86,9 +86,9 @@ define([
self.window.location.href = result.redirect;
return { pageNavigation: true };
})
.fail(function(xhr) {
.fail(function(err) {
Session.clear('oauth');
self.displayError(xhr.responseJSON, OAuthErrors);
self.displayError(err, OAuthErrors);
});
},

Expand Down
133 changes: 101 additions & 32 deletions app/tests/spec/lib/oauth-client.js
View file
Expand Up @@ -44,43 +44,112 @@ function (chai, $, testHelpers, sinon,
});

describe('oauth-client', function () {
it('calls getCode', function () {
/* jshint camelcase: false */
var redirect = RP_URL + '?code=code&state=state';

server.respondWith('POST', OAUTH_URL + '/v1/authorization',
[200, { 'Content-Type': 'application/json' },
'{ "redirect": "' + redirect + '" }']);

var params = {
assertion: 'assertion',
client_id: 'deadbeef',
redirect_uri: 'http://example.com',
scope: 'profile',
state: 'state'
};

return client.getCode(params)
.then(function (result) {
assert.ok(result);
assert.equal(result.redirect, redirect);
});
describe('getCode', function () {
it('normally responds with a redirect', function () {
/* jshint camelcase: false */
var redirect = RP_URL + '?code=code&state=state';

server.respondWith('POST', OAUTH_URL + '/v1/authorization',
[200, { 'Content-Type': 'application/json' },
'{ "redirect": "' + redirect + '" }']);

var params = {
assertion: 'assertion',
client_id: 'deadbeef',
redirect_uri: 'http://example.com',
scope: 'profile',
state: 'state'
};

return client.getCode(params)
.then(function (result) {
assert.ok(result);
assert.equal(result.redirect, redirect);
});
});

it('responds with a SERVICE_UNAVAILABLE error if the service is unavailable', function () {
var clientId = 'clientId';

server.respondWith('GET', OAUTH_URL + '/v1/client/' + clientId,
[0, {}, '']);

return client.getClientInfo(clientId)
.then(function (result) {
assert.fail('unexpected success');
}, function (err) {
assert.isTrue(OAuthErrors.is(err, 'SERVICE_UNAVAILABLE'));
});
});

it('converts returned errors to OAuth error objects', function () {
var clientId = 'clientId';

server.respondWith('GET', OAUTH_URL + '/v1/client/' + clientId,
[400, { 'Content-Type': 'application/json' },
JSON.stringify({
errno: OAuthErrors.toCode('INCORRECT_REDIRECT'),
code: 400
})]);


return client.getClientInfo(clientId)
.then(function (result) {
assert.fail('unexpected success');
}, function (err) {
assert.isTrue(OAuthErrors.is(err, 'INCORRECT_REDIRECT'));
});
});
});

it('calls getClientInfo', function () {
describe('getClientInfo', function () {
var clientId = 'clientId';

server.respondWith('GET', OAUTH_URL + '/v1/client/' + clientId,
[200, { 'Content-Type': 'application/json' },
'{ "name": "MozRP", "imageUri": "https://mozilla.org/firefox.png" }']);

return client.getClientInfo(clientId)
.then(function (result) {
assert.ok(result);
assert.equal(result.name, 'MozRP');
});
it('normally response with a name and imageUri', function () {
server.respondWith('GET', OAUTH_URL + '/v1/client/' + clientId,
[200, { 'Content-Type': 'application/json' },
'{ "name": "MozRP", "imageUri": "https://mozilla.org/firefox.png" }']);

return client.getClientInfo(clientId)
.then(function (result) {
assert.ok(result);
assert.equal(result.name, 'MozRP');
});
})

it('responds with a SERVICE_UNAVAILABLE error if the service is unavailable', function () {
var clientId = 'clientId';

server.respondWith('GET', OAUTH_URL + '/v1/client/' + clientId,
[0, {}, '']);

return client.getClientInfo(clientId)
.then(function (result) {
assert.fail('unexpected success');
}, function (err) {
assert.isTrue(OAuthErrors.is(err, 'SERVICE_UNAVAILABLE'));
});
});

it('converts returned errors to OAuth error objects', function () {
var clientId = 'clientId';

server.respondWith('GET', OAUTH_URL + '/v1/client/' + clientId,
[400, { 'Content-Type': 'application/json' },
JSON.stringify({
errno: OAuthErrors.toCode('EXPIRED_CODE'),
code: 400
})]);


return client.getClientInfo(clientId)
.then(function (result) {
assert.fail('unexpected success');
}, function (err) {
assert.isTrue(OAuthErrors.is(err, 'EXPIRED_CODE'));
});
});
});

});

});
Expand Down

0 comments on commit 7224d95

Please sign in to comment.