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

Commit

Permalink
fix(accounts): only fetch access token if verified
Browse files Browse the repository at this point in the history
  • Loading branch information
@zaach
zaach committed Nov 14, 2014
1 parent d4f0d58 commit d316885
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 50 deletions.
1 change: 1 addition & 0 deletions app/scripts/lib/app-start.js
View file
Expand Up @@ -271,6 +271,7 @@ function (
oAuthClientId: this._config.oauthClientId,
profileClient: this._profileClient,
oAuthClient: this._oAuthClient,
fxaClient: this._fxaClient,
assertion: this._assertionLibrary
});
}
Expand Down
62 changes: 33 additions & 29 deletions app/scripts/models/account.js
View file
Expand Up @@ -45,28 +45,41 @@ define([
self._oAuthClient = options.oAuthClient;
self._assertion = options.assertion;
self._profileClient = options.profileClient;
self._fxaClient = options.fxaClient;
},

// Hydrate the account
fetch: function () {
var self = this;
var promise = p();

// upgrade the credentials with an accessToken
if (self._needsAccessToken()) {
return self._getAccessTokenFromSessionToken(self.get('sessionToken'))
.then(function (accessToken) {
self.set('accessToken', accessToken);
// if we can sign a cert, we must be verified
self.set('verified', true);
}, function (err) {
if (AuthErrors.is(err, 'UNVERIFIED_ACCOUNT')) {
self.set('verified', false);
return;
}
if (! self.get('sessionToken')) {
return promise;
}

// upgrade the credentials with verified state
if (! self.get('verified')) {
promise = self.isVerified()
.then(function (verified) {
self.set('verified', verified);
}, function () {
// Ignore errors; we'll just fetch again when needed
});
} else {
return p();
}

// upgrade the credentials with an accessToken
promise = promise.then(function () {
if (self._needsAccessToken()) {
return self._getAccessTokenFromSessionToken(self.get('sessionToken'))
.then(function (accessToken) {
self.set('accessToken', accessToken);
}, function () {
// Ignore errors; we'll just fetch again when needed
});
}
});

return promise;
},

profileClient: function () {
Expand All @@ -89,13 +102,10 @@ define([
});
},

// If the account doesn't have an accessToken (or isn't verified),
// we should attempt to fetch an access token. That will determine if
// the account has since been verified and retrieve said access token.
// The sessionToken is needed to perform the fetch.
// If we're verified and don't have an accessToken, we should
// go ahead and get one.
_needsAccessToken: function () {
return this.get('sessionToken') &&
(! this.get('accessToken') || ! this.get('verified'));
return this.get('verified') && ! this.get('accessToken');
},

_getAccessTokenFromSessionToken: function (sessionToken) {
Expand All @@ -117,15 +127,9 @@ define([
},

isVerified: function () {
return this._getAccessTokenFromSessionToken(this.get('sessionToken'))
.then(function () {
return true;
}, function (err) {
if (AuthErrors.is(err, 'UNVERIFIED_ACCOUNT')) {
return false;
}

throw err;
return this._fxaClient.recoveryEmailStatus(this.get('sessionToken'))
.then(function (results) {
return results.verified;
});
}

Expand Down
2 changes: 2 additions & 0 deletions app/scripts/models/user.js
View file
Expand Up @@ -27,6 +27,7 @@ define([
this._oAuthClientId = options.oAuthClientId;
this._oAuthClient = options.oAuthClient;
this._profileClient = options.profileClient;
this._fxaClient = options.fxaClient;
this._assertion = options.assertion;
this._storage = options.storage || new Storage(localStorage);
},
Expand Down Expand Up @@ -67,6 +68,7 @@ define([
assertion: this._assertion,
oAuthClient: this._oAuthClient,
profileClient: this._profileClient,
fxaClient: this._fxaClient,
oAuthClientId: this._oAuthClientId
});
},
Expand Down
73 changes: 52 additions & 21 deletions app/tests/spec/models/account.js
View file
Expand Up @@ -13,32 +13,37 @@ define([
'lib/assertion',
'lib/profile-client',
'lib/oauth-client',
'lib/fxa-client',
'lib/auth-errors',
'models/account'
],
function (chai, sinon, p, Constants, Assertion, ProfileClient,
OAuthClient, AuthErrors, Account) {
OAuthClient, FxaClientWrapper, AuthErrors, Account) {
var assert = chai.assert;

describe('models/account', function () {
var account;
var assertion;
var oAuthClient;
var profileClient;
var fxaClient;
var EMAIL = 'user@example.domain';
var UID = '6d940dd41e636cc156074109b8092f96';
var URL = 'http://127.0.0.1:1112/avatar/example.jpg';
var CLIENT_ID = 'client_id';
var SESSION_TOKEN = 'abc123';

beforeEach(function () {
assertion = new Assertion();
oAuthClient = new OAuthClient();
profileClient = new ProfileClient();
fxaClient = new FxaClientWrapper();

account = new Account({
oAuthClient: oAuthClient,
assertion: assertion,
profileClient: profileClient,
fxaClient: fxaClient,
oAuthClientId: CLIENT_ID,
accountData: {
email: EMAIL,
Expand All @@ -63,7 +68,10 @@ function (chai, sinon, p, Constants, Assertion, ProfileClient,
});

it('fetches access token and sets verified state', function () {
account.set('sessionToken', 'abc123');
account.set('sessionToken', SESSION_TOKEN);
sinon.stub(fxaClient, 'recoveryEmailStatus', function () {
return p({ verified: true });
});
sinon.stub(assertion, 'generate', function () {
return p('assertion');
});
Expand All @@ -73,7 +81,7 @@ function (chai, sinon, p, Constants, Assertion, ProfileClient,

return account.fetch()
.then(function () {
assert.isTrue(assertion.generate.calledWith('abc123'));
assert.isTrue(assertion.generate.calledWith(SESSION_TOKEN));
assert.isTrue(oAuthClient.getToken.calledWith({
'client_id': CLIENT_ID,
assertion: 'assertion',
Expand All @@ -85,10 +93,22 @@ function (chai, sinon, p, Constants, Assertion, ProfileClient,
});
});

it('fails to set verified state with error', function () {
account.set('sessionToken', SESSION_TOKEN);
sinon.stub(fxaClient, 'recoveryEmailStatus', function () {
return p.reject(AuthErrors.toError('UNKNOWN_ACCOUNT'));
});
return account.fetch()
.then(function () {
assert.isTrue(fxaClient.recoveryEmailStatus.calledWith(SESSION_TOKEN));
assert.isUndefined(account.get('verified'));
});
});

it('fails to fetch access token with an unverified account', function () {
account.set('sessionToken', 'abc123');
sinon.stub(assertion, 'generate', function () {
return p.reject(AuthErrors.toError('UNVERIFIED_ACCOUNT'));
account.set('sessionToken', SESSION_TOKEN);
sinon.stub(fxaClient, 'recoveryEmailStatus', function () {
return p({ verified: false });
});

return account.fetch()
Expand All @@ -99,49 +119,57 @@ function (chai, sinon, p, Constants, Assertion, ProfileClient,
});

it('fails to fetch with other errors', function () {
account.set('sessionToken', 'abc123');
account.set('sessionToken', SESSION_TOKEN);
sinon.stub(fxaClient, 'recoveryEmailStatus', function () {
return p({ verified: true });
});
sinon.stub(assertion, 'generate', function () {
return p.reject(AuthErrors.toError('UNKNOWN_ACCOUNT'));
});
return account.fetch()
.then(function () {
assert.isUndefined(account.get('accessToken'));
assert.isUndefined(account.get('verified'));
assert.isTrue(account.get('verified'));
});
});

});

it('isVerified returns false if account is unverified', function () {
account.set('sessionToken', 'abc123');
sinon.stub(assertion, 'generate', function () {
return p.reject(AuthErrors.toError('UNVERIFIED_ACCOUNT'));
account.set('sessionToken', SESSION_TOKEN);
sinon.stub(fxaClient, 'recoveryEmailStatus', function () {
return p({ verified: false });
});

return account.isVerified()
.then(function (isVerified) {
assert.isTrue(fxaClient.recoveryEmailStatus.calledWith(SESSION_TOKEN));
assert.isFalse(isVerified);
});
});

it('isVerified fails if an error occurs', function () {
account.set('sessionToken', 'abc123');
sinon.stub(assertion, 'generate', function () {
account.set('sessionToken', SESSION_TOKEN);
sinon.stub(fxaClient, 'recoveryEmailStatus', function () {
return p.reject(AuthErrors.toError('UNKNOWN_ACCOUNT'));
});

return account.isVerified()
.then(assert.fail, function () {
// success
return;
});
.then(assert.fail,
function (err) {
assert.isTrue(fxaClient.recoveryEmailStatus.calledWith(SESSION_TOKEN));
assert.isTrue(AuthErrors.is(err, 'UNKNOWN_ACCOUNT'));
});
});

describe('with an access token', function () {
var accessToken = 'access token';

beforeEach(function () {
account.set('sessionToken', 'abc123');
account.set('sessionToken', SESSION_TOKEN);
sinon.stub(fxaClient, 'recoveryEmailStatus', function () {
return p({ verified: true });
});
sinon.stub(assertion, 'generate', function () {
return p('assertion');
});
Expand Down Expand Up @@ -238,10 +266,13 @@ function (chai, sinon, p, Constants, Assertion, ProfileClient,

describe('without an access token', function () {
beforeEach(function () {
account.set('sessionToken', 'abc123');
account.set('sessionToken', SESSION_TOKEN);
sinon.stub(assertion, 'generate', function () {
return p('assertion');
});
sinon.stub(fxaClient, 'recoveryEmailStatus', function () {
return p({ verified: true });
});
sinon.stub(oAuthClient, 'getToken', function () {
return p.reject(ProfileClient.Errors.toError('UNVERIFIED_ACCOUNT'));
});
Expand Down Expand Up @@ -276,7 +307,7 @@ function (chai, sinon, p, Constants, Assertion, ProfileClient,
accountData: {
email: EMAIL,
uid: UID,
sessionToken: 'abc123',
sessionToken: SESSION_TOKEN,
foo: 'bar'
}
});
Expand All @@ -296,7 +327,7 @@ function (chai, sinon, p, Constants, Assertion, ProfileClient,
accountData: {
email: EMAIL,
uid: UID,
sessionToken: 'abc123',
sessionToken: SESSION_TOKEN,
foo: 'bar'
}
});
Expand Down

0 comments on commit d316885

Please sign in to comment.