moparisthebest/mail
1
0
Fork 0
mirror of https://github.com/moparisthebest/mail synced 2024-11-22 17:02:17 -05:00
Code Issues Releases Wiki Activity

[WO-643] Refactor initialization workflow

Browse Source 
* Move initialization pre-flight checks to app-controller
* Refresh cached public keys for user during incomplete setups
* Reorder redirect checks in login ctrl from most specific (pubkey + privkey) to most generic (no keys)
* Add overridePermission flag to KeychainDAO.refreshKeyForUserId to refresh w/o asking for user permission
This commit is contained in:
Felix Hammerl 2014-11-06 14:28:14 +01:00
parent 4722af1457
commit 7959be55a7
11 changed files with 281 additions and 165 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

91
src/js/app-controller.js
View File

@ -7,8 +7,9 @@
var axe = require('axe-logger'),
Auth = require('./bo/auth'),
PGP = require('./crypto/pgp'),
PgpMailer = require('pgpmailer'),
OAuth = require('./util/oauth'),
PgpMailer = require('pgpmailer'),
util = require('crypto-lib').util,
PgpBuilder = require('pgpbuilder'),
OutboxBO = require('./bo/outbox'),
mailreader = require('mailreader'),
@ -121,29 +122,9 @@ ctrl.checkForUpdate = function() {
};
/**
* Instanciate the mail email data access object and its dependencies. Login to imap on init.
* Fire up the database, retrieve the available keys for the user and initialize the email data access object
*/
ctrl.init = function(options, callback) {
// init user's local database
ctrl._userStorage.init(options.emailAddress, function(err) {
if (err) {
callback(err);
return;
}
// Migrate the databases if necessary
ctrl._updateHandler.update(onUpdate);
});
function onUpdate(err) {
if (err) {
callback({
errMsg: 'Update failed, please reinstall the app.',
err: err
});
return;
}
// account information for the email dao
var account = {
realname: options.realname,
@ -151,16 +132,70 @@ ctrl.init = function(options, callback) {
asymKeySize: config.asymKeySize
};
// init email dao
ctrl._emailDao.init({
account: account
}, function(err, keypair) {
// Pre-Flight check: don't even start to initialize stuff if the email address is not valid
if (!util.validateEmailAddress(options.emailAddress)) {
return callback(new Error('The user email address is invalid!'));
}
prepareDatabase();
// Pre-Flight check: initialize and prepare user's local database
function prepareDatabase() {
ctrl._userStorage.init(options.emailAddress, function(err) {
if (err) {
callback(err);
return callback(err);
}
// Migrate the databases if necessary
ctrl._updateHandler.update(function(err) {
if (err) {
return callback(new Error('Updating the internal database failed. Please reinstall the app! Reason: ' + err.message));
}
prepareKeys();
});
});
}
// retrieve keypair fom devicestorage/cloud, refresh public key if signup was incomplete before
function prepareKeys() {
ctrl._keychain.getUserKeyPair(options.emailAddress, function(err, keys) {
if (err) {
return callback(err);
}
// this is either a first start on a new device, OR a subsequent start without completing the signup,
// since we can't differenciate those cases here, do a public key refresh because it might be outdated
if (keys && keys.publicKey && !keys.privateKey) {
ctrl._keychain.refreshKeyForUserId({
userId: options.emailAddress,
overridePermission: true
}, function(err, publicKey) {
if (err) {
return callback(err);
}
initEmailDao({
publicKey: publicKey
});
});
return;
}
callback(null, keypair);
// either signup was complete or no pubkey is available, so we're good here.
initEmailDao(keys);
});
}
function initEmailDao(keys) {
ctrl._emailDao.init({
account: account
}, function(err) {
if (err) {
return callback(err);
}
callback(null, keys);
});
}
};

45
src/js/controller/login.js
View File

@ -49,12 +49,26 @@ var LoginCtrl = function($scope, $location) {
}
function redirect(availableKeys) {
// redirect if needed
if (typeof availableKeys === 'undefined') {
// no public key available, start onboarding process
goTo('/login-initial');
if (availableKeys && availableKeys.publicKey && availableKeys.privateKey) {
// public and private key available, try empty passphrase
appController._emailDao.unlock({
keypair: availableKeys,
passphrase: undefined
}, function(err) {
if (err) {
goTo('/login-existing');
return;
}
} else if (availableKeys && !availableKeys.privateKey) {
appController._auth.storeCredentials(function(err) {
if (err) {
return $scope.onError(err);
}
goTo('/desktop');
});
});
} else if (availableKeys && availableKeys.publicKey && !availableKeys.privateKey) {
// check if private key is synced
appController._keychain.requestPrivateKeyDownload({
userId: availableKeys.publicKey.userId,
@ -74,26 +88,9 @@ var LoginCtrl = function($scope, $location) {
// no private key, import key file
goTo('/login-new-device');
});
} else {
// public and private key available, try empty passphrase
appController._emailDao.unlock({
keypair: availableKeys,
passphrase: undefined
}, function(err) {
if (err) {
goTo('/login-existing');
return;
}
appController._auth.storeCredentials(function(err) {
if (err) {
return $scope.onError(err);
}
goTo('/desktop');
});
});
// no public key available, start onboarding process
goTo('/login-initial');
}
}

4
src/js/controller/mail-list.js
View File

@ -71,7 +71,9 @@ var MailListCtrl = function($scope, $routeParams) {
}
firstSelect = false;
keychainDao.refreshKeyForUserId(email.from[0].address, onKeyRefreshed);
keychainDao.refreshKeyForUserId({
userId: email.from[0].address
}, onKeyRefreshed);
function onKeyRefreshed(err) {
if (err) {

4
src/js/controller/write.js
View File

@ -229,7 +229,9 @@ var WriteCtrl = function($scope, $filter, $q) {
if (keychainDao) {
// check if to address is contained in known public keys
// when we write an email, we always need to work with the latest keys available
keychainDao.refreshKeyForUserId(recipient.address, function(err, key) {
keychainDao.refreshKeyForUserId({
userId: recipient.address
}, function(err, key) {
if (err) {
$scope.onError(err);
return;

58
src/js/dao/email-dao.js
View File

@ -1,7 +1,6 @@
'use strict';
var util = require('crypto-lib').util,
config = require('../app-config').config,
var config = require('../app-config').config,
str = require('../app-config').string;
//
@ -65,60 +64,21 @@ var EmailDAO = function(keychain, pgp, devicestorage, pgpbuilder, mailreader) {
/**
* Initializes the email dao:
* - validates the email address
* - retrieves the user's key pair (if available)
* - assigns _account
* - initializes _account.folders with the content from memory
*
* @param {Object} options.account The account
* @param {String} options.account.emailAddress The user's id
* @param {String} options.account.realname The user's id
* @param {Function} callback(error, keypair) Invoked with the keypair or error information when the email dao is initialized
*/
EmailDAO.prototype.init = function(options, callback) {
var self = this,
keypair;
this._account = options.account;
this._account.busy = 0; // > 0 triggers the spinner
this._account.online = false;
this._account.loggingIn = false;
self._account = options.account;
self._account.busy = 0; // triggers the spinner
self._account.online = false;
self._account.loggingIn = false;
// validate email address
var emailAddress = self._account.emailAddress;
if (!util.validateEmailAddress(emailAddress)) {
callback({
errMsg: 'The user email address must be specified!'
});
return;
}
// init keychain and then crypto module
initKeychain();
function initKeychain() {
// call getUserKeyPair to read/sync keypair with devicestorage/cloud
self._keychain.getUserKeyPair(emailAddress, function(err, storedKeypair) {
if (err) {
callback(err);
return;
}
keypair = storedKeypair;
initFolders();
});
}
function initFolders() {
// try init folders from memory, since imap client not initiated yet
self._initFoldersFromDisk(function(err) {
// dont handle offline case this time
if (err && err.code !== 42) {
callback(err);
return;
}
callback(null, keypair);
});
}
// init folders from memory
this._initFoldersFromDisk(callback);
};
/**

22
src/js/dao/keychain-dao.js
View File

@ -81,11 +81,14 @@ KeychainDAO.prototype.getPublicKeys = function(ids, callback) {
/**
* Checks for public key updates of a given user id
* @param {String} userId The user id (email address) for which to check the key
* @param {String} options.userId The user id (email address) for which to check the key
* @param {String} options.overridePermission (optional) Indicates if the update should happen automatically (true) or with the user being queried (false). Defaults to false
* @param {Function} callback(error, key) Invoked when the key has been updated or an error occurred
*/
KeychainDAO.prototype.refreshKeyForUserId = function(userId, callback) {
var self = this;
KeychainDAO.prototype.refreshKeyForUserId = function(options, callback) {
var self = this,
userId = options.userId,
overridePermission = options.overridePermission;
// get the public key corresponding to the userId
self.getReceiverPublicKey(userId, function(err, localKey) {
@ -146,10 +149,18 @@ KeychainDAO.prototype.refreshKeyForUserId = function(userId, callback) {
}
// the public key has changed, we need to ask for permission to update the key
if (overridePermission) {
// don't query the user, update the public key right away
onPermissionReceived(true);
} else {
// query the user if the public key should be updated
self.requestPermissionForKeyUpdate({
userId: userId,
newKey: newKey
}, function(granted) {
}, onPermissionReceived);
}
function onPermissionReceived(granted) {
if (!granted) {
// permission was not given to update the key, so don't overwrite the old one!
callback(null, localKey);
@ -169,8 +180,7 @@ KeychainDAO.prototype.refreshKeyForUserId = function(userId, callback) {
callback(err, err ? undefined : newKey);
});
});
});
}
});
}
};

96
test/unit/app-controller-test.js
View File

@ -5,10 +5,12 @@ var controller = require('../../src/js/app-controller'),
OutboxBO = require('../../src/js/bo/outbox'),
DeviceStorageDAO = require('../../src/js/dao/devicestorage-dao'),
UpdateHandler = require('../../src/js/util/update/update-handler'),
KeychainDAO = require('../../src/js/dao/keychain-dao'),
config = require('../../src/js/app-config').config,
Auth = require('../../src/js/bo/auth');
describe('App Controller unit tests', function() {
var emailDaoStub, outboxStub, updateHandlerStub, appConfigStoreStub, devicestorageStub, isOnlineStub, authStub;
var emailDaoStub, outboxStub, updateHandlerStub, appConfigStoreStub, devicestorageStub, isOnlineStub, authStub, keychainStub;
beforeEach(function() {
controller._emailDao = emailDaoStub = sinon.createStubInstance(EmailDAO);
@ -17,6 +19,7 @@ describe('App Controller unit tests', function() {
controller._userStorage = devicestorageStub = sinon.createStubInstance(DeviceStorageDAO);
controller._updateHandler = updateHandlerStub = sinon.createStubInstance(UpdateHandler);
controller._auth = authStub = sinon.createStubInstance(Auth);
controller._keychain = keychainStub = sinon.createStubInstance(KeychainDAO);
isOnlineStub = sinon.stub(controller, 'isOnline');
});
@ -133,10 +136,13 @@ describe('App Controller unit tests', function() {
});
describe('init', function() {
var onConnectStub, emailAddress;
var onConnectStub, emailAddress, keysWithPubKey;
beforeEach(function() {
emailAddress = 'alice@bob.com';
keysWithPubKey = {
publicKey: {}
};
// onConnect
onConnectStub = sinon.stub(controller, 'onConnect');
@ -146,10 +152,22 @@ describe('App Controller unit tests', function() {
onConnectStub.restore();
});
it('should fail due to error in storage initialization', function(done) {
devicestorageStub.init.withArgs(undefined).yields({});
it('should fail due to malformed email address', function(done) {
controller.init({
emailAddress: 'ishallfail'
}, function(err, keypair) {
expect(err).to.exist;
expect(keypair).to.not.exist;
done();
});
});
controller.init({}, function(err, keypair) {
it('should fail due to error in storage initialization', function(done) {
devicestorageStub.init.withArgs(emailAddress).yields(new Error());
controller.init({
emailAddress: emailAddress
}, function(err, keypair) {
expect(err).to.exist;
expect(keypair).to.not.exist;
expect(devicestorageStub.init.calledOnce).to.be.true;
@ -160,7 +178,7 @@ describe('App Controller unit tests', function() {
it('should fail due to error in update handler', function(done) {
devicestorageStub.init.yields();
updateHandlerStub.update.yields({});
updateHandlerStub.update.yields(new Error());
controller.init({
emailAddress: emailAddress
@ -173,10 +191,48 @@ describe('App Controller unit tests', function() {
});
});
it('should fail due to error in getUserKeyPair', function(done) {
devicestorageStub.init.yields();
updateHandlerStub.update.yields();
keychainStub.getUserKeyPair.yields(new Error());
controller.init({
emailAddress: emailAddress
}, function(err, keypair) {
expect(err).to.exist;
expect(keypair).to.not.exist;
expect(updateHandlerStub.update.calledOnce).to.be.true;
expect(devicestorageStub.init.calledOnce).to.be.true;
expect(keychainStub.getUserKeyPair.calledOnce).to.be.true;
done();
});
});
it('should fail due to error in refreshKeyForUserId', function(done) {
devicestorageStub.init.yields();
updateHandlerStub.update.yields();
keychainStub.getUserKeyPair.yields(null, keysWithPubKey);
keychainStub.refreshKeyForUserId.yields(new Error());
controller.init({
emailAddress: emailAddress
}, function(err, keypair) {
expect(err).to.exist;
expect(keypair).to.not.exist;
expect(updateHandlerStub.update.calledOnce).to.be.true;
expect(devicestorageStub.init.calledOnce).to.be.true;
expect(keychainStub.getUserKeyPair.calledOnce).to.be.true;
expect(keychainStub.refreshKeyForUserId.calledOnce).to.be.true;
done();
});
});
it('should fail due to error in emailDao.init', function(done) {
devicestorageStub.init.yields();
updateHandlerStub.update.yields();
emailDaoStub.init.yields({});
keychainStub.getUserKeyPair.yields(null, keysWithPubKey);
keychainStub.refreshKeyForUserId.yields();
emailDaoStub.init.yields(new Error());
controller.init({
emailAddress: emailAddress
@ -184,25 +240,41 @@ describe('App Controller unit tests', function() {
expect(err).to.exist;
expect(keypair).to.not.exist;
expect(updateHandlerStub.update.calledOnce).to.be.true;
expect(emailDaoStub.init.calledOnce).to.be.true;
expect(devicestorageStub.init.calledOnce).to.be.true;
expect(keychainStub.getUserKeyPair.calledOnce).to.be.true;
expect(keychainStub.refreshKeyForUserId.calledOnce).to.be.true;
expect(emailDaoStub.init.calledOnce).to.be.true;
done();
});
});
it('should work and return a keypair', function(done) {
it('should work and not return a keypair', function(done) {
devicestorageStub.init.withArgs(emailAddress).yields();
emailDaoStub.init.yields(null, {});
updateHandlerStub.update.yields();
keychainStub.getUserKeyPair.withArgs(emailAddress).yields(null, keysWithPubKey);
keychainStub.refreshKeyForUserId.withArgs({
userId: emailAddress,
overridePermission: true
}).yields();
emailDaoStub.init.withArgs({
account: {
realname: undefined,
emailAddress: emailAddress,
asymKeySize: config.asymKeySize
}
}).yields();
controller.init({
emailAddress: emailAddress
}, function(err, keypair) {
expect(err).to.not.exist;
expect(keypair).to.exist;
expect(keypair.publicKey).to.not.exist;
expect(updateHandlerStub.update.calledOnce).to.be.true;
expect(emailDaoStub.init.calledOnce).to.be.true;
expect(devicestorageStub.init.calledOnce).to.be.true;
expect(keychainStub.getUserKeyPair.calledOnce).to.be.true;
expect(keychainStub.refreshKeyForUserId.calledOnce).to.be.true;
expect(emailDaoStub.init.calledOnce).to.be.true;
done();
});
});

22
test/unit/email-dao-test.js
View File

@ -148,36 +148,18 @@ describe('Email DAO unit tests', function() {
initFoldersStub = sinon.stub(dao, '_initFoldersFromDisk');
});
it('should initialize folders and return keypair', function(done) {
keychainStub.getUserKeyPair.withArgs(emailAddress).yieldsAsync(null, mockKeyPair);
it('should initialize folders', function(done) {
initFoldersStub.yieldsAsync();
dao.init({
account: account
}, function(err, keypair) {
}, function(err) {
expect(err).to.not.exist;
expect(keypair).to.exist;
expect(keychainStub.getUserKeyPair.calledOnce).to.be.true;
expect(initFoldersStub.calledOnce).to.be.true;
done();
});
});
it('should fail when keychain errors', function(done) {
keychainStub.getUserKeyPair.yieldsAsync({});
dao.init({
account: account
}, function(err, keypair) {
expect(err).to.exist;
expect(keypair).to.not.exist;
expect(keychainStub.getUserKeyPair.calledOnce).to.be.true;
expect(initFoldersStub.called).to.be.false;
done();
});
});
});
describe('#unlock', function() {

72
test/unit/keychain-dao-test.js
View File

@ -85,7 +85,9 @@ describe('Keychain DAO unit tests', function() {
it('should not find a key', function(done) {
getPubKeyStub.yields();
keychainDao.refreshKeyForUserId(testUser, function(err, key) {
keychainDao.refreshKeyForUserId({
userId: testUser
}, function(err, key) {
expect(err).to.not.exist;
expect(key).to.not.exist;
@ -97,7 +99,9 @@ describe('Keychain DAO unit tests', function() {
getPubKeyStub.yields(null, oldKey);
pubkeyDaoStub.get.withArgs(oldKey._id).yields(null, oldKey);
keychainDao.refreshKeyForUserId(testUser, function(err, key) {
keychainDao.refreshKeyForUserId({
userId: testUser
}, function(err, key) {
expect(err).to.not.exist;
expect(key).to.to.equal(oldKey);
@ -121,7 +125,33 @@ describe('Keychain DAO unit tests', function() {
lawnchairDaoStub.remove.withArgs('publickey_' + oldKey._id).yields();
lawnchairDaoStub.persist.withArgs('publickey_' + newKey._id, newKey).yields();
keychainDao.refreshKeyForUserId(testUser, function(err, key) {
keychainDao.refreshKeyForUserId({
userId: testUser
}, function(err, key) {
expect(err).to.not.exist;
expect(key).to.equal(newKey);
expect(getPubKeyStub.calledOnce).to.be.true;
expect(pubkeyDaoStub.get.calledOnce).to.be.true;
expect(pubkeyDaoStub.getByUserId.calledOnce).to.be.true;
expect(lawnchairDaoStub.remove.calledOnce).to.be.true;
expect(lawnchairDaoStub.persist.calledOnce).to.be.true;
done();
});
});
it('should update key without approval', function(done) {
getPubKeyStub.yields(null, oldKey);
pubkeyDaoStub.get.withArgs(oldKey._id).yields();
pubkeyDaoStub.getByUserId.withArgs(testUser).yields(null, newKey);
lawnchairDaoStub.remove.withArgs('publickey_' + oldKey._id).yields();
lawnchairDaoStub.persist.withArgs('publickey_' + newKey._id, newKey).yields();
keychainDao.refreshKeyForUserId({
userId: testUser,
overridePermission: true
}, function(err, key) {
expect(err).to.not.exist;
expect(key).to.equal(newKey);
@ -146,7 +176,9 @@ describe('Keychain DAO unit tests', function() {
};
lawnchairDaoStub.remove.withArgs('publickey_' + oldKey._id).yields();
keychainDao.refreshKeyForUserId(testUser, function(err, key) {
keychainDao.refreshKeyForUserId({
userId: testUser
}, function(err, key) {
expect(err).to.not.exist;
expect(key).to.not.exist;
@ -167,7 +199,9 @@ describe('Keychain DAO unit tests', function() {
code: 42
});
keychainDao.refreshKeyForUserId(testUser, function(err, key) {
keychainDao.refreshKeyForUserId({
userId: testUser
}, function(err, key) {
expect(err).to.not.exist;
expect(key).to.to.equal(oldKey);
@ -191,7 +225,9 @@ describe('Keychain DAO unit tests', function() {
cb(false);
};
keychainDao.refreshKeyForUserId(testUser, function(err, key) {
keychainDao.refreshKeyForUserId({
userId: testUser
}, function(err, key) {
expect(err).to.not.exist;
expect(key).to.equal(oldKey);
@ -208,7 +244,9 @@ describe('Keychain DAO unit tests', function() {
it('should not remove manually imported key', function(done) {
getPubKeyStub.yields(null, importedKey);
keychainDao.refreshKeyForUserId(testUser, function(err, key) {
keychainDao.refreshKeyForUserId({
userId: testUser
}, function(err, key) {
expect(err).to.not.exist;
expect(key).to.equal(importedKey);
@ -225,7 +263,9 @@ describe('Keychain DAO unit tests', function() {
code: 42
});
keychainDao.refreshKeyForUserId(testUser, function(err, key) {
keychainDao.refreshKeyForUserId({
userId: testUser
}, function(err, key) {
expect(err).to.not.exist;
expect(key).to.to.equal(oldKey);
@ -251,7 +291,9 @@ describe('Keychain DAO unit tests', function() {
lawnchairDaoStub.remove.withArgs('publickey_' + oldKey._id).yields();
lawnchairDaoStub.persist.yields({});
keychainDao.refreshKeyForUserId(testUser, function(err, key) {
keychainDao.refreshKeyForUserId({
userId: testUser
}, function(err, key) {
expect(err).to.exist;
expect(key).to.not.exist;
@ -275,7 +317,9 @@ describe('Keychain DAO unit tests', function() {
};
lawnchairDaoStub.remove.yields({});
keychainDao.refreshKeyForUserId(testUser, function(err, key) {
keychainDao.refreshKeyForUserId({
userId: testUser
}, function(err, key) {
expect(err).to.exist;
expect(key).to.not.exist;
@ -301,7 +345,9 @@ describe('Keychain DAO unit tests', function() {
lawnchairDaoStub.remove.withArgs('publickey_' + oldKey._id).yields();
lawnchairDaoStub.persist.yields({});
keychainDao.refreshKeyForUserId(testUser, function(err, key) {
keychainDao.refreshKeyForUserId({
userId: testUser
}, function(err, key) {
expect(err).to.exist;
expect(key).to.not.exist;
@ -319,7 +365,9 @@ describe('Keychain DAO unit tests', function() {
getPubKeyStub.yields(null, oldKey);
pubkeyDaoStub.get.withArgs(oldKey._id).yields({});
keychainDao.refreshKeyForUserId(testUser, function(err, key) {
keychainDao.refreshKeyForUserId({
userId: testUser
}, function(err, key) {
expect(err).to.exist;
expect(key).to.not.exist;

6
test/unit/mail-list-ctrl-test.js
View File

@ -364,7 +364,9 @@ describe('Mail List controller unit test', function() {
}
};
keychainMock.refreshKeyForUserId.withArgs(mail.from[0].address).yields();
keychainMock.refreshKeyForUserId.withArgs({
userId: mail.from[0].address
}).yields();
scope.select(mail);
@ -397,7 +399,7 @@ describe('Mail List controller unit test', function() {
}
};
keychainMock.refreshKeyForUserId.withArgs(mail.from[0].address).yields();
keychainMock.refreshKeyForUserId.withArgs({userId: mail.from[0].address}).yields();
scope.select(mail);

12
test/unit/write-ctrl-test.js
View File

@ -192,7 +192,9 @@ describe('Write controller unit test', function() {
address: 'asds@example.com'
};
keychainMock.refreshKeyForUserId.withArgs(recipient.address).yields({
keychainMock.refreshKeyForUserId.withArgs({
userId: recipient.address
}).yields({
errMsg: '404 not found yadda yadda'
});
@ -212,7 +214,9 @@ describe('Write controller unit test', function() {
address: 'asdf@example.com'
};
keychainMock.refreshKeyForUserId.withArgs(recipient.address).yields(null, {
keychainMock.refreshKeyForUserId.withArgs({
userId: recipient.address
}).yields(null, {
userId: 'asdf@example.com'
});
@ -240,7 +244,9 @@ describe('Write controller unit test', function() {
}]
};
keychainMock.refreshKeyForUserId.withArgs(recipient.address).yields(null, key);
keychainMock.refreshKeyForUserId.withArgs({
userId: recipient.address
}).yields(null, key);
scope.$digest = function() {
expect(recipient.key).to.deep.equal(key);