From f99c5175d37f5437b411e563e9dfec409e6e7bfc Mon Sep 17 00:00:00 2001 From: Ben Gotow Date: Tue, 8 Aug 2017 10:00:07 -0700 Subject: [PATCH] Refactor secrets management to encrypt imap+smtp credentials as well --- .../onboarding/lib/onboarding-helpers.es6 | 2 +- .../onboarding/lib/onboarding-store.es6 | 16 ++--- .../src/flux/stores/account-store.es6 | 17 ------ packages/client-app/src/key-manager.es6 | 33 ++++++++++- packages/client-app/src/mailsync-process.es6 | 58 ++++++++++++++++--- 5 files changed, 89 insertions(+), 37 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 4b0766ae7..65be3a832 100644 --- a/packages/client-app/internal_packages/onboarding/lib/onboarding-helpers.es6 +++ b/packages/client-app/internal_packages/onboarding/lib/onboarding-helpers.es6 @@ -113,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, IdentityStore.identity()); + const proc = new MailsyncProcess(NylasEnv.getLoadSettings(), IdentityStore.identity(), data); const {account} = await proc.test(); delete data.id; diff --git a/packages/client-app/internal_packages/onboarding/lib/onboarding-store.es6 b/packages/client-app/internal_packages/onboarding/lib/onboarding-store.es6 index 7c86f23bc..73a90fccf 100644 --- a/packages/client-app/internal_packages/onboarding/lib/onboarding-store.es6 +++ b/packages/client-app/internal_packages/onboarding/lib/onboarding-store.es6 @@ -1,4 +1,4 @@ -import {AccountStore, Actions, IdentityStore, FolderSyncProgressStore} from 'nylas-exports'; +import {AccountStore, Actions, IdentityStore, FolderSyncProgressStore, KeyManager} from 'nylas-exports'; import {ipcRenderer} from 'electron'; import NylasStore from 'nylas-store'; @@ -142,20 +142,20 @@ class OnboardingStore extends NylasStore { _onAccountJSONReceived = async (json, cloudToken) => { try { const isFirstAccount = AccountStore.accounts().length === 0; - - AccountStore.addAccountFromJSON(json, cloudToken); - this._accountFromAuth = AccountStore.accountForEmail(json.emailAddress); + const clean = KeyManager.extractAccountSecrets(json, cloudToken); + AccountStore.addAccountFromJSON(clean); Actions.recordUserEvent('Email Account Auth Succeeded', { - provider: this._accountFromAuth.provider, + provider: json.provider, }); + ipcRenderer.send('new-account-added'); NylasEnv.displayWindow(); if (isFirstAccount) { this._onMoveToPage('initial-preferences'); Actions.recordUserEvent('First Account Linked', { - provider: this._accountFromAuth.provider, + provider: json.provider, }); } else { await FolderSyncProgressStore.whenCategoryListSynced(json.id) @@ -178,10 +178,6 @@ class OnboardingStore extends NylasStore { accountInfo() { return this._accountInfo; } - - accountFromAuth() { - return this._accountFromAuth; - } } export default new OnboardingStore(); diff --git a/packages/client-app/src/flux/stores/account-store.es6 b/packages/client-app/src/flux/stores/account-store.es6 index 1914a2493..2b8ff029b 100644 --- a/packages/client-app/src/flux/stores/account-store.es6 +++ b/packages/client-app/src/flux/stores/account-store.es6 @@ -72,28 +72,15 @@ class AccountStore extends NylasStore { try { this._caches = {} this._version = NylasEnv.config.get(configVersionKey) || 0 - - const oldAccountIds = this._accounts ? this._accounts.map(a => a.id) : []; this._accounts = [] for (const json of NylasEnv.config.get(configAccountsKey) || []) { this._accounts.push((new Account()).fromJSON(json)) } - const accountIds = this._accounts.map(a => a.id) - - // Loading passwords from the KeyManager is expensive so only do it if - // 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)); // 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) { - 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) } @@ -221,10 +208,6 @@ class AccountStore extends NylasStore { this._loadAccounts() - 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 ); diff --git a/packages/client-app/src/key-manager.es6 b/packages/client-app/src/key-manager.es6 index be8c78934..7b2dd724c 100644 --- a/packages/client-app/src/key-manager.es6 +++ b/packages/client-app/src/key-manager.es6 @@ -23,11 +23,35 @@ class KeyManager { this.KEY_NAME = "Nylas Mail Keys" } + extractAccountSecrets(accountJSON) { + const next = Object.assign({}, accountJSON); + this._try(() => { + const keys = this._getKeyHash(); + keys[`${accountJSON.emailAddress}-imap`] = next.settings.imap_password; + delete next.settings.imap_password; + keys[`${accountJSON.emailAddress}-smtp`] = next.settings.smtp_password; + delete next.settings.smtp_password; + keys[`${accountJSON.emailAddress}-cloud`] = next.cloudToken; + delete next.cloudToken; + return this._writeKeyHash(keys); + }); + return next; + } + + insertAccountSecrets(accountJSON) { + const next = Object.assign({}, accountJSON); + const keys = this._getKeyHash(); + next.settings.imap_password = keys[`${accountJSON.emailAddress}-imap`]; + next.settings.smtp_password = keys[`${accountJSON.emailAddress}-smtp`]; + next.cloudToken = keys[`${accountJSON.emailAddress}-cloud`]; + return next; + } + replacePassword(keyName, newVal) { this._try(() => { const keys = this._getKeyHash(); keys[keyName] = newVal; - return keytar.replacePassword(this.SERVICE_NAME, this.KEY_NAME, JSON.stringify(keys)) + return this._writeKeyHash(keys); }) } @@ -35,7 +59,7 @@ class KeyManager { this._try(() => { const keys = this._getKeyHash(); delete keys[keyName]; - return keytar.replacePassword(this.SERVICE_NAME, this.KEY_NAME, JSON.stringify(keys)) + return this._writeKeyHash(keys); }) } @@ -53,6 +77,11 @@ class KeyManager { } } + _writeKeyHash(keys) { + // returns true if successful + return keytar.replacePassword(this.SERVICE_NAME, this.KEY_NAME, JSON.stringify(keys)); + } + _try(fn) { const ERR_MSG = "We couldn't store your password securely! For more information, visit https://support.nylas.com/hc/en-us/articles/223790028"; try { diff --git a/packages/client-app/src/mailsync-process.es6 b/packages/client-app/src/mailsync-process.es6 index 3c01a72c5..aaa0aaf98 100644 --- a/packages/client-app/src/mailsync-process.es6 +++ b/packages/client-app/src/mailsync-process.es6 @@ -3,9 +3,11 @@ /* Warning! This file is imported from the main process as well as the renderer process */ -import { spawn } from 'child_process'; +import { spawn, exec } from 'child_process'; import path from 'path'; +import os from 'os'; import { EventEmitter } from 'events'; +import fs from 'fs'; let Utils = null; @@ -31,7 +33,7 @@ const LocalizedErrorStrings = { }; export default class MailsyncProcess extends EventEmitter { - constructor({configDirPath, resourcePath}, account, identity) { + constructor({configDirPath, resourcePath}, identity, account) { super(); this.configDirPath = configDirPath; this.account = account; @@ -55,8 +57,7 @@ export default class MailsyncProcess extends EventEmitter { 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`); + this._proc.stdin.write(`${JSON.stringify(this.account)}\n${JSON.stringify(this.identity)}\n`); }); } } @@ -75,7 +76,6 @@ export default class MailsyncProcess extends EventEmitter { reject(err); }); this._proc.on('close', (code) => { - console.log(`SyncWorker exited mode ${mode} with code ${code}`); try { const lastLine = buffer.toString('UTF-8').split('\n').pop(); const response = JSON.parse(lastLine); @@ -98,6 +98,8 @@ export default class MailsyncProcess extends EventEmitter { sync() { this._spawnProcess('sync'); let buffer = ""; + let lastStderr = null; + this._proc.stdout.on('data', (data) => { const added = data.toString(); buffer += added; @@ -109,14 +111,32 @@ export default class MailsyncProcess extends EventEmitter { } }); this._proc.stderr.on('data', (data) => { - console.log(`Sync worker wrote to stderr: ${data.toString()}`); + lastStderr = data.toString(); }); this._proc.on('error', (err) => { console.log(`Sync worker exited with ${err}`); this.emit('error', err); }); this._proc.on('close', (code) => { - this.emit('close', code); + let error = null; + + if (buffer.length) { + let lastJSON = null; + try { + lastJSON = JSON.parse(buffer); + } finally { + if (lastJSON && lastJSON.error) { + error = new Error(lastJSON.error); + } else { + this.emit('deltas', buffer); + } + } + } + + if (lastStderr) { + error = new Error(lastStderr); + } + this.emit('close', {code, error}); }); } @@ -135,4 +155,28 @@ export default class MailsyncProcess extends EventEmitter { test() { return this._spawnAndWait('test'); } + + attachToXcode() { + const tmppath = path.join(os.tmpdir(), 'attach.applescript'); + fs.writeFileSync(tmppath, ` +tell application "Xcode" + activate +end tell + +tell application "System Events" + tell application process "Xcode" + click (menu item "Attach to Process by PID or Name…" of menu 1 of menu bar item "Debug" of menu bar 1) + end tell + tell application process "Xcode" + set value of text field 1 of sheet 1 of window 1 to "${this._proc.pid}" + end tell + delay 0.5 + tell application process "Xcode" + click button "Attach" of sheet 1 of window 1 + end tell + +end tell + `); + exec(`osascript ${tmppath}`); + } }