Refactor secrets management to encrypt imap+smtp credentials as well

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;

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();

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
     );

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 {

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}`);
+  }
 }