From b4f89d90d748f53e3cfd10fdc1eaecc44a263640 Mon Sep 17 00:00:00 2001 From: Ben Gotow Date: Sun, 6 Aug 2017 16:18:04 -0500 Subject: [PATCH] Refactor storage of secrets to include imap/smtp pass, pass identity to C++ workers --- .../onboarding/lib/onboarding-helpers.es6 | 3 +- .../lib/search-mailbox-perspective.es6 | 2 +- .../thread-search/lib/thread-search-bar.jsx | 2 +- .../client-app/src/flux/mailsync-bridge.es6 | 12 ++- .../client-app/src/flux/nylas-api-request.es6 | 12 +-- .../src/flux/stores/account-store.es6 | 79 ++++++------------- packages/client-app/src/mailsync-process.es6 | 29 +++++-- packages/client-app/src/n1-cloud-api.es6 | 2 +- packages/client-app/src/sheet-toolbar.jsx | 6 +- 9 files changed, 71 insertions(+), 76 deletions(-) diff --git a/packages/client-app/internal_packages/onboarding/lib/onboarding-helpers.es6 b/packages/client-app/internal_packages/onboarding/lib/onboarding-helpers.es6 index 0bc738957..4b0766ae7 100644 --- a/packages/client-app/internal_packages/onboarding/lib/onboarding-helpers.es6 +++ b/packages/client-app/internal_packages/onboarding/lib/onboarding-helpers.es6 @@ -4,6 +4,7 @@ import crypto from 'crypto'; import {CommonProviderSettings} from 'imap-provider-settings'; import { NylasAPIRequest, + IdentityStore, RegExpUtils, MailsyncProcess, } from 'nylas-exports'; @@ -112,7 +113,7 @@ export async function runAuthValidation(accountInfo) { // Send the form data directly to Nylas to get code // If this succeeds, send the received code to N1 server to register the account // Otherwise process the error message from the server and highlight UI as needed - const proc = new MailsyncProcess(NylasEnv.getLoadSettings(), data); + const proc = new MailsyncProcess(NylasEnv.getLoadSettings(), data, IdentityStore.identity()); const {account} = await proc.test(); delete data.id; diff --git a/packages/client-app/internal_packages/thread-search/lib/search-mailbox-perspective.es6 b/packages/client-app/internal_packages/thread-search/lib/search-mailbox-perspective.es6 index 234f93411..077a230b4 100644 --- a/packages/client-app/internal_packages/thread-search/lib/search-mailbox-perspective.es6 +++ b/packages/client-app/internal_packages/thread-search/lib/search-mailbox-perspective.es6 @@ -55,7 +55,7 @@ class SearchMailboxPerspective extends MailboxPerspective { tasksForRemovingItems(threads) { return TaskFactory.tasksForThreadsByAccountId(threads, (accountThreads, accountId) => { - const account = AccountStore.accountForId(accountId) + const account = AccountStore.accountForId(accountId); const dest = account.preferredRemovalDestination(); if (dest instanceof Folder) { diff --git a/packages/client-app/internal_packages/thread-search/lib/thread-search-bar.jsx b/packages/client-app/internal_packages/thread-search/lib/thread-search-bar.jsx index 82af37500..caa484c33 100644 --- a/packages/client-app/internal_packages/thread-search/lib/thread-search-bar.jsx +++ b/packages/client-app/internal_packages/thread-search/lib/thread-search-bar.jsx @@ -59,7 +59,7 @@ class ThreadSearchBar extends Component { if (this.props.perspective.isInbox()) { return 'Search all email'; } - return `Search ${this.props.perspective.name}`; + return `Search ${this.props.perspective.name || ""}`; } render() { diff --git a/packages/client-app/src/flux/mailsync-bridge.es6 b/packages/client-app/src/flux/mailsync-bridge.es6 index ff12d9c97..3b45dfae8 100644 --- a/packages/client-app/src/flux/mailsync-bridge.es6 +++ b/packages/client-app/src/flux/mailsync-bridge.es6 @@ -6,6 +6,7 @@ import Actions from './actions'; import Utils from './models/utils'; let AccountStore = null; +let IdentityStore = null; let Task = null; export default class MailsyncBridge { @@ -23,8 +24,16 @@ export default class MailsyncBridge { this.clients = {}; Task = require('./tasks/task').default; //eslint-disable-line + + IdentityStore = require('./stores/identity-store').default; + IdentityStore.listen(() => { + Object.values(this.clients).each(c => c.kill()); + this.ensureClients(); + }, this); + AccountStore = require('./stores/account-store').default; //eslint-disable-line AccountStore.listen(this.ensureClients, this); + this.ensureClients(); NylasEnv.onBeforeUnload(this.onBeforeUnload); @@ -33,6 +42,7 @@ export default class MailsyncBridge { ensureClients() { const toLaunch = []; const clientsToStop = Object.assign({}, this.clients); + const identity = IdentityStore.identity(); for (const acct of AccountStore.accounts()) { if (!this.clients[acct.id]) { @@ -47,7 +57,7 @@ export default class MailsyncBridge { } toLaunch.forEach((acct) => { - const client = new MailsyncProcess(NylasEnv.getLoadSettings(), acct); + const client = new MailsyncProcess(NylasEnv.getLoadSettings(), identity, acct); client.sync(); client.on('deltas', this.onIncomingMessages); client.on('close', () => { diff --git a/packages/client-app/src/flux/nylas-api-request.es6 b/packages/client-app/src/flux/nylas-api-request.es6 index b5ab24ca5..cfcc1e604 100644 --- a/packages/client-app/src/flux/nylas-api-request.es6 +++ b/packages/client-app/src/flux/nylas-api-request.es6 @@ -1,5 +1,5 @@ +/* eslint global-require: 0 */ import {APIError} from './errors' -import IdentityStore from './stores/identity-store' // A 0 code is when an error returns without a status code, like "ESOCKETTIMEDOUT" export const TimeoutErrorCodes = [0, 408, "ETIMEDOUT", "ESOCKETTIMEDOUT", "ECONNRESET", "ENETDOWN", "ENETUNREACH"] @@ -7,18 +7,19 @@ export const PermanentErrorCodes = [400, 401, 402, 403, 404, 405, 429, 500, "ENO export const CanceledErrorCodes = [-123, "ECONNABORTED"] export const SampleTemporaryErrorCode = 504 +let IdentityStore = null; + // server option export function rootURLForServer(server) { const env = NylasEnv.config.get('env'); - if (!['development', 'local', 'staging', 'production'].includes(env)) { + if (!['development', 'staging', 'production'].includes(env)) { throw new Error(`rootURLForServer: ${env} is not a valid environment.`); } if (server === 'identity') { return { - local: "http://localhost:5101", development: "http://localhost:5101", staging: "https://id-staging.nylas.com", production: "https://id.nylas.com", @@ -26,7 +27,6 @@ export function rootURLForServer(server) { } if (server === 'accounts') { return { - local: "http://localhost:5100", development: "http://localhost:5100", staging: "https://accounts-staging.nylas.com", production: "https://accounts.nylas.com", @@ -47,7 +47,9 @@ export async function makeRequest(options) { if (!options.auth) { if (options.server === 'identity') { - options.headers.set('Authorization', `Basic ${btoa(`${IdentityStore._identity.token}:`)}`) + IdentityStore = IdentityStore || require('./stores/identity-store').default; + const username = IdentityStore.identity().token; + options.headers.set('Authorization', `Basic ${btoa(`${username}:`)}`) } } diff --git a/packages/client-app/src/flux/stores/account-store.es6 b/packages/client-app/src/flux/stores/account-store.es6 index 2688e8daf..1914a2493 100644 --- a/packages/client-app/src/flux/stores/account-store.es6 +++ b/packages/client-app/src/flux/stores/account-store.es6 @@ -26,7 +26,6 @@ class AccountStore extends NylasStore { this.listenTo(Actions.removeAccount, this._onRemoveAccount) this.listenTo(Actions.updateAccount, this._onUpdateAccount) this.listenTo(Actions.reorderAccount, this._onReorderAccount) - this.listenTo(Actions.apiAuthError, this._onAPIAuthError) NylasEnv.config.onDidChange(configVersionKey, async (change) => { // If we already have this version of the accounts config, it means we @@ -69,29 +68,9 @@ class AccountStore extends NylasStore { return false } - _onAPIAuthError = (apiError, apiOptions) => { - // Prevent /auth errors from presenting auth failure notices - const apiToken = apiOptions.auth.user - if (!apiToken) { - return Promise.resolve() - } - - const account = this.accounts().find((acc) => - this.tokensForAccountId(acc.id) === apiToken - ); - - if (account) { - const n1CloudState = Account.N1_CLOUD_STATE_AUTH_FAILED - this._onUpdateAccount(account.id, {n1CloudState}) - } - - return Promise.resolve() - } - _loadAccounts = () => { try { this._caches = {} - this._tokens = this._tokens || {}; this._version = NylasEnv.config.get(configVersionKey) || 0 const oldAccountIds = this._accounts ? this._accounts.map(a => a.id) : []; @@ -105,24 +84,15 @@ class AccountStore extends NylasStore { // we really have to (i.e. we're loading a new Account) const addedAccountIds = _.difference(accountIds, oldAccountIds); const addedAccounts = this._accounts.filter((a) => addedAccountIds.includes(a.id)); - const removedAccountIds = _.difference(oldAccountIds, accountIds); - const removedAccounts = this._accounts.filter((a) => removedAccountIds.includes(a.id)); // Run a few checks on account consistency. We want to display useful error // messages and these can result in very strange exceptions downstream otherwise. this._enforceAccountsValidity() for (const account of addedAccounts) { - this._tokens[account.emailAddress] = this._tokens[account.id] = KeyManager.getPassword(`${account.emailAddress}`); - } - for (const removedAccount of removedAccounts) { - const {id, emailAddress} = removedAccount - if (this._tokens[id]) { - delete this._tokens[id] - } - if (this._tokens[emailAddress]) { - delete this._tokens[emailAddress] - } + account.settings.imap_password = KeyManager.getPassword(`${account.emailAddress}-imap`); + account.settings.smtp_password = KeyManager.getPassword(`${account.emailAddress}-smtp`); + account.cloudToken = KeyManager.getPassword(`${account.emailAddress}-cloud`); } } catch (error) { NylasEnv.reportError(error) @@ -172,11 +142,16 @@ class AccountStore extends NylasStore { } _save = () => { - this._version += 1 - const configAccounts = this._accounts.map(a => a.toJSON()) - configAccounts.forEach(a => delete a.sync_error) - NylasEnv.config.set(configAccountsKey, configAccounts) - NylasEnv.config.set(configVersionKey, this._version) + this._version += 1; + const configAccounts = this._accounts.map(a => a.toJSON()); + configAccounts.forEach(a => { + delete a.sync_error + delete a.settings.imap_password + delete a.settings.smtp_password + delete a.cloudToken + }); + NylasEnv.config.set(configAccountsKey, configAccounts); + NylasEnv.config.set(configVersionKey, this._version); this._trigger() } @@ -203,7 +178,7 @@ class AccountStore extends NylasStore { _onRemoveAccount = (id) => { const account = this._accounts.find(a => a.id === id); if (!account) return - KeyManager.deletePassword(account.emailAddress) + KeyManager.deletePassword(account.id) this._caches = {} @@ -239,29 +214,28 @@ class AccountStore extends NylasStore { this._save() } - addAccountFromJSON = (json, cloudToken) => { + addAccountFromJSON = (json) => { if (!json.emailAddress || !json.provider) { - console.error("Returned account data is invalid", json) - console.log(JSON.stringify(json)) - throw new Error("Returned account data is invalid") + throw new Error(`Returned account data is invalid: ${JSON.stringify(json)}`) } this._loadAccounts() - this._tokens[json.id] = cloudToken; - KeyManager.replacePassword(`${json.emailAddress}`, cloudToken) + KeyManager.replacePassword(`${json.emailAddress}-cloud`, json.cloudToken); + KeyManager.replacePassword(`${json.emailAddress}-imap`, json.settings.imap_password); + KeyManager.replacePassword(`${json.emailAddress}-smtp`, json.settings.smtp_passwowrd); const existingIdx = this._accounts.findIndex((a) => a.id === json.id || a.emailAddress === json.emailAddress ); if (existingIdx === -1) { - const account = (new Account()).fromJSON(json) - this._accounts.push(account) + const account = (new Account()).fromJSON(json); + this._accounts.push(account); } else { - const account = this._accounts[existingIdx] - account.syncState = Account.SYNC_STATE_RUNNING - account.fromJSON(json) + const account = this._accounts[existingIdx]; + account.syncState = Account.SYNC_STATE_RUNNING; + account.fromJSON(json); // Restart the connection in case account credentials have changed // todo bg } @@ -349,11 +323,6 @@ class AccountStore extends NylasStore { current() { throw new Error("AccountStore.current() has been deprecated.") } - - // Private: This method is going away soon, do not rely on it. - tokenForAccountId(id) { - return this._tokens[id] - } } export default new AccountStore() diff --git a/packages/client-app/src/mailsync-process.es6 b/packages/client-app/src/mailsync-process.es6 index ee3a19dd4..3c01a72c5 100644 --- a/packages/client-app/src/mailsync-process.es6 +++ b/packages/client-app/src/mailsync-process.es6 @@ -1,12 +1,16 @@ /* eslint global-require: 0 */ + +/* +Warning! This file is imported from the main process as well as the renderer process +*/ import { spawn } from 'child_process'; import path from 'path'; -import {EventEmitter} from 'events'; +import { EventEmitter } from 'events'; let Utils = null; const LocalizedErrorStrings = { - ErrorConnection: "Connection Error", + ErrorConnection: "Connection Error - Check that your internet connection is active.", ErrorInvalidAccount: "This account is invalid, or does not have an inbox or all folder.", ErrorTLSNotAvailable: "TLS Not Available", ErrorParse: "Parsing Error", @@ -27,23 +31,32 @@ const LocalizedErrorStrings = { }; export default class MailsyncProcess extends EventEmitter { - constructor({configDirPath, resourcePath}, account) { + constructor({configDirPath, resourcePath}, account, identity) { super(); this.configDirPath = configDirPath; this.account = account; + this.identity = identity; this.binaryPath = path.join(resourcePath, 'MailSync').replace('app.asar', 'app.asar.unpacked'); this._proc = null; } _spawnProcess(mode) { - this._proc = spawn(this.binaryPath, [`--mode`, mode], { - env: { - CONFIG_DIR_PATH: this.configDirPath, - }, - }); + const env = { + CONFIG_DIR_PATH: this.configDirPath, + IDENTITY_SERVER: 'unknown', + ACCOUNTS_SERVER: 'unknown', + }; + if (process.type === 'renderer') { + const rootURLForServer = require('./flux/nylas-api-request').rootURLForServer; + env.IDENTITY_SERVER = rootURLForServer('identity'); + env.ACCOUNTS_SERVER = rootURLForServer('accounts'); + } + + this._proc = spawn(this.binaryPath, [`--mode`, mode], {env}); if (this.account) { this._proc.stdout.once('data', () => { this._proc.stdin.write(`${JSON.stringify(this.account)}\n`); + this._proc.stdin.write(`${JSON.stringify(this.identity)}\n`); }); } } diff --git a/packages/client-app/src/n1-cloud-api.es6 b/packages/client-app/src/n1-cloud-api.es6 index 589fbaff0..895ee8841 100644 --- a/packages/client-app/src/n1-cloud-api.es6 +++ b/packages/client-app/src/n1-cloud-api.es6 @@ -8,7 +8,7 @@ class N1CloudAPI { _onConfigChanged = () => { const env = NylasEnv.config.get('env') - if (['development', 'local'].includes(env)) { + if (env === 'development') { this.APIRoot = "http://lvh.me:5100"; } else if (env === 'staging') { this.APIRoot = "https://n1-staging.nylas.com"; diff --git a/packages/client-app/src/sheet-toolbar.jsx b/packages/client-app/src/sheet-toolbar.jsx index e992f4a5e..c68c0b3ea 100644 --- a/packages/client-app/src/sheet-toolbar.jsx +++ b/packages/client-app/src/sheet-toolbar.jsx @@ -39,14 +39,14 @@ class WindowTitle extends React.Component { } componentDidMount() { - this.unlisten = NylasEnv.onWindowPropsReceived(() => + this.disposable = NylasEnv.onWindowPropsReceived(() => this.setState(NylasEnv.getLoadSettings()) ); } componentWillUnmount() { - if (this.unlisten) { - this.unlisten(); + if (this.disposable) { + this.disposable.dispose(); } }