fix(config): Move account tokens to system keychain

Summary:
This diff also adds an account version number to the config so that the AccountStore can tell whether it should reload accounts (depending on whether it was the instance making tthe changes.)

This diff also fixes a tiny issue where un-opened composers threw an exception if you changed accounts.

Test Plan: New tests

Reviewers: evan, drew, juan

Reviewed By: juan

Subscribers: juan

Differential Revision: https://phab.nylas.com/D2726

build/resources/linux/debian/control.in

@@ -1,7 +1,6 @@
 Package: <%= name %>
 Version: <%= version %>
-Depends: git, gconf2, gconf-service, libgtk2.0-0, libudev0 | libudev1, libgcrypt11 | libgcrypt20, libnotify4, libxtst6, libnss3, python, gvfs-bin, xdg-utils
-Suggests: libgnome-keyring0, gir1.2-gnomekeyring-1.0
+Depends: libgnome-keyring0, gir1.2-gnomekeyring-1.0, git, gconf2, gconf-service, libgtk2.0-0, libudev0 | libudev1, libgcrypt11 | libgcrypt20, libnotify4, libxtst6, libnss3, python, gvfs-bin, xdg-utils
 Section: <%= section %>
 Priority: optional
 Architecture: <%= arch %>

build/resources/linux/redhat/nylas.spec.in

@@ -2,10 +2,12 @@ Name:           <%= name %>
 Version:        <%= version %>
 Release:        0.1%{?dist}
 Summary:        <%= description %>
-License:        Proprietary
+License:        GPLv3
 URL:            https://nylas.com/N1
 AutoReqProv:    no # Avoid libchromiumcontent.so missing dependency
 
+requires:       libgnome-keyring0, gir1.2-gnomekeyring-1.0
+
 %description
 <%= description %>
 

internal_packages/composer/lib/composer-view.cjsx

@@ -561,6 +561,7 @@ class ComposerView extends React.Component
   # When the account store changes, the From field may or may not still
   # be in scope. We need to make sure to update our enabled fields.
   _onAccountStoreChanged: =>
+    return unless @_proxy
     accounts = @_getAccountsForSend()
     enabledFields = if @_shouldShowFromField(@_proxy?.draft())
       @state.enabledFields.concat [Fields.From]

package.json

@@ -37,6 +37,7 @@
     "jasmine-tagged": "^1.1.2",
     "jquery": "^2.1.1",
     "juice": "^1.4",
+    "keytar": "^3.0.0",
     "less-cache": "0.21",
     "marked": "^0.3",
     "mkdirp": "^0.5",

spec/stores/account-store-spec.coffee

@@ -1,4 +1,5 @@
 _ = require 'underscore'
+keytar = require 'keytar'
 AccountStore = require '../../src/flux/stores/account-store'
 Account = require '../../src/flux/models/account'
 Actions = require '../../src/flux/actions'
@@ -7,37 +8,71 @@ describe "AccountStore", ->
   beforeEach ->
     @instance = null
     @constructor = AccountStore.constructor
+    @keys = {}
+    spyOn(keytar, 'getPassword').andCallFake (service, account) =>
+      @keys[account]
+    spyOn(keytar, 'deletePassword').andCallFake (service, account) =>
+      delete @keys[account]
+    spyOn(keytar, 'replacePassword').andCallFake (service, account, pass) =>
+      @keys[account] = pass
 
   afterEach ->
     @instance.stopListeningToAll()
 
-  it "should initialize using data saved in config", ->
-    accounts =
-      [{
-        "id": "123",
-        "client_id" : 'local-4f9d476a-c173',
-        "server_id" : '123',
-        "email_address":"bengotow@gmail.com",
-        "object":"account"
-        "organization_unit": "label"
-      },{
-        "id": "1234",
-        "client_id" : 'local-4f9d476a-c175',
-        "server_id" : '1234',
-        "email_address":"ben@nylas.com",
-        "object":"account"
-        "organization_unit": "label"
-      }]
+  describe "initialization", ->
+    beforeEach ->
+      @configTokens = null
+      @configVersion = 1
+      @configAccounts =
+        [{
+          "id": "A",
+          "client_id" : 'local-4f9d476a-c173',
+          "server_id" : 'A',
+          "email_address":"bengotow@gmail.com",
+          "object":"account"
+          "organization_unit": "label"
+        },{
+          "id": "B",
+          "client_id" : 'local-4f9d476a-c175',
+          "server_id" : 'B',
+          "email_address":"ben@nylas.com",
+          "object":"account"
+          "organization_unit": "label"
+        }]
 
-    spyOn(NylasEnv.config, 'get').andCallFake (key) ->
-      return accounts if key is 'nylas.accounts'
-      return null
-    @instance = new @constructor
+      spyOn(NylasEnv.config, 'set')
+      spyOn(NylasEnv.config, 'get').andCallFake (key) =>
+        return @configAccounts if key is 'nylas.accounts'
+        return @configVersion if key is 'nylas.accountsVersion'
+        return @configTokens if key is 'nylas.accountTokens'
+        return null
 
-    expect(@instance.accounts()).toEqual([
-      (new Account).fromJSON(accounts[0]),
-      (new Account).fromJSON(accounts[1])
-    ])
+    it "should initialize the accounts and version from config", ->
+      @instance = new @constructor
+      expect(@instance._version).toEqual(@configVersion)
+      expect(@instance.accounts()).toEqual([
+        (new Account).fromJSON(@configAccounts[0]),
+        (new Account).fromJSON(@configAccounts[1])
+      ])
+
+    it "should initialize tokens from config, if present, save them to keytar, and remove them from config", ->
+      @configTokens = {'A': 'A-TOKEN'}
+      @instance = new @constructor
+      expect(@instance.tokenForAccountId('A')).toEqual('A-TOKEN')
+      expect(@instance.tokenForAccountId('B')).toEqual(undefined)
+      expect(keytar.replacePassword).toHaveBeenCalledWith('Nylas', 'bengotow@gmail.com', 'A-TOKEN')
+      expect(NylasEnv.config.set).toHaveBeenCalledWith('nylas.accountTokens', null)
+
+    it "should initialize tokens from keytar", ->
+      @configTokens = null
+      jasmine.unspy(keytar, 'getPassword')
+      spyOn(keytar, 'getPassword').andCallFake (service, account) =>
+        return 'A-TOKEN' if account is 'bengotow@gmail.com'
+        return 'B-TOKEN' if account is 'ben@nylas.com'
+        return null
+      @instance = new @constructor
+      expect(@instance.tokenForAccountId('A')).toEqual('A-TOKEN')
+      expect(@instance.tokenForAccountId('B')).toEqual('B-TOKEN')
 
   describe "accountForEmail", ->
     beforeEach ->
@@ -58,33 +93,32 @@ describe "AccountStore", ->
     beforeEach ->
       spyOn(NylasEnv.config, "set")
       @json =
-        "id": "1234",
+        "id": "B",
         "client_id" : 'local-4f9d476a-c175',
-        "server_id" : '1234',
+        "server_id" : 'B',
         "email_address":"ben@nylas.com",
         "provider":"gmail",
         "object":"account"
-        "auth_token": "auth-123"
+        "auth_token": "B-NEW-TOKEN"
         "organization_unit": "label"
       @instance = new @constructor
       spyOn(Actions, 'focusDefaultMailboxPerspectiveForAccounts')
       spyOn(@instance, "trigger")
       @instance.addAccountFromJSON(@json)
 
-    it "sets the tokens", ->
-      expect(@instance._tokens["1234"]).toBe "auth-123"
+    it "saves the token to keytar and to the loaded tokens cache", ->
+      expect(@instance._tokens["B"]).toBe("B-NEW-TOKEN")
+      expect(keytar.replacePassword).toHaveBeenCalledWith("Nylas", "ben@nylas.com", "B-NEW-TOKEN")
 
-    it "sets the accounts", ->
+    it "saves the account to the accounts cache and saves", ->
       account = (new Account).fromJSON(@json)
       expect(@instance._accounts.length).toBe 1
       expect(@instance._accounts[0]).toEqual account
-
-    it "saves the config", ->
       expect(NylasEnv.config.save).toHaveBeenCalled()
       expect(NylasEnv.config.set.calls.length).toBe 2
 
     it "selects the account", ->
-      expect(Actions.focusDefaultMailboxPerspectiveForAccounts).toHaveBeenCalledWith(["1234"])
+      expect(Actions.focusDefaultMailboxPerspectiveForAccounts).toHaveBeenCalledWith(["B"])
 
     it "triggers", ->
       expect(@instance.trigger).toHaveBeenCalled()

src/flux/actions.coffee

@@ -171,7 +171,7 @@ class Actions
   Actions.updateAccount(account.id, {accountName: 'new'})
   ```
   ###
-  @updateAccount: ActionScopeGlobal
+  @updateAccount: ActionScopeWindow
 
   ###
   Public: Re-order the provided account in the account list.

src/flux/stores/account-store.coffee

@@ -4,10 +4,12 @@ Actions = require '../actions'
 Account = require '../models/account'
 Utils = require '../models/utils'
 DatabaseStore = require './database-store'
+keytar = require 'keytar'
 
-
-saveObjectsKey = "nylas.accounts"
-saveTokensKey = "nylas.accountTokens"
+configAccountsKey = "nylas.accounts"
+configVersionKey = "nylas.accountsVersion"
+configTokensKey = "nylas.accountTokens"
+keytarServiceName = 'Nylas'
 
 ###
 Public: The AccountStore listens to changes to the available accounts in
@@ -18,31 +20,45 @@ Section: Stores
 class AccountStore extends NylasStore
 
   constructor: ->
-    @_load()
+    @_loadAccounts()
     @listenTo Actions.removeAccount, @_onRemoveAccount
     @listenTo Actions.updateAccount, @_onUpdateAccount
     @listenTo Actions.reorderAccount, @_onReorderAccount
 
-    @_caches = {}
+    NylasEnv.config.onDidChange configVersionKey, (change) =>
+      # If we already have this version of the accounts config, it means we
+      # are the ones who saved the change, and we don't need to reload.
+      return if @_version / 1 is change.newValue / 1
+      oldAccountIds = _.pluck(@_accounts, 'id')
+      @_loadAccounts()
+      newAccountIds = _.pluck(@_accounts, 'id')
+      newAccountIds = _.without(newAccountIds, oldAccountIds)
 
-    NylasEnv.config.onDidChange saveTokensKey, (change) =>
-      updatedTokens = change.newValue
-      return if _.isEqual(updatedTokens, @_tokens)
-      newAccountIds = _.keys(_.omit(updatedTokens, _.keys(@_tokens)))
-      @_load()
       if newAccountIds.length > 0
         Actions.focusDefaultMailboxPerspectiveForAccounts([newAccountIds[0]])
 
-    if NylasEnv.isComposerWindow() or NylasEnv.isWorkWindow()
-      NylasEnv.config.onDidChange saveObjectsKey, => @_load()
-
-  _load: =>
+  _loadAccounts: =>
     @_caches = {}
+    @_version = NylasEnv.config.get(configVersionKey) || 0
+
     @_accounts = []
-    for json in NylasEnv.config.get(saveObjectsKey) || []
+    for json in NylasEnv.config.get(configAccountsKey) || []
       @_accounts.push((new Account).fromJSON(json))
 
-    @_tokens = NylasEnv.config.get(saveTokensKey) || {}
+    # Load tokens using the old config method and save them into the keychain
+    oldTokens = NylasEnv.config.get(configTokensKey)
+    if oldTokens
+      for key, val of oldTokens
+        account = @accountForId(key)
+        continue unless account
+        keytar.replacePassword(keytarServiceName, account.emailAddress, val)
+      NylasEnv.config.set(configTokensKey, null)
+
+    # Load tokens using the new keytar method
+    @_tokens = {}
+    for account in @_accounts
+      @_tokens[account.id] = keytar.getPassword(keytarServiceName, account.emailAddress)
+
     @_trigger()
 
   _trigger: ->
@@ -54,9 +70,11 @@ class AccountStore extends NylasStore
     @trigger()
 
   _save: =>
-    NylasEnv.config.set(saveObjectsKey, @_accounts)
-    NylasEnv.config.set(saveTokensKey, @_tokens)
+    @_version += 1
+    NylasEnv.config.set(configVersionKey, @_version)
+    NylasEnv.config.set(configAccountsKey, @_accounts)
     NylasEnv.config.save()
+    @_trigger()
 
   # Inbound Events
 
@@ -68,23 +86,21 @@ class AccountStore extends NylasStore
     account = _.extend(account, updated)
     @_caches = {}
     @_accounts[idx] = account
-    NylasEnv.config.set(saveObjectsKey, @_accounts)
-    @_trigger()
+    @_save()
 
   _onRemoveAccount: (id) =>
-    idx = _.findIndex @_accounts, (a) -> a.id is id
-    return if idx is -1
+    account = _.findWhere(@_accounts, {id})
+    return unless account
+    keytar.deletePassword(keytarServiceName, account.emailAddress)
 
-    delete @_tokens[id]
     @_caches = {}
-    @_accounts.splice(idx, 1)
+    @_accounts = _.without(@_accounts, account)
     @_save()
 
     if @_accounts.length is 0
       ipc = require('electron').ipcRenderer
       ipc.send('command', 'application:reset-config-and-relaunch')
     else
-      @_trigger()
       Actions.focusDefaultMailboxPerspectiveForAccounts(@_accounts)
 
   _onReorderAccount: (id, newIdx) =>
@@ -95,7 +111,6 @@ class AccountStore extends NylasStore
     @_accounts.splice(existingIdx, 1)
     @_accounts.splice(newIdx, 0, account)
     @_save()
-    @_trigger()
 
   addAccountFromJSON: (json) =>
     if not json.email_address or not json.provider
@@ -103,10 +118,11 @@ class AccountStore extends NylasStore
       console.log JSON.stringify(json)
       throw new Error("Returned account data is invalid")
 
-    @_load()
-    @_tokens[json.id] = json.auth_token
+    @_loadAccounts()
+
+    @_tokens[json.id] = json.auth_token
+    keytar.replacePassword(keytarServiceName, json.email_address, json.auth_token)
 
-    @_caches = {}
     existingIdx = _.findIndex @_accounts, (a) -> a.id is json.id
     if existingIdx is -1
       account = (new Account).fromJSON(json)
@@ -116,8 +132,6 @@ class AccountStore extends NylasStore
       account.fromJSON(json)
 
     @_save()
-
-    @_trigger()
     Actions.focusDefaultMailboxPerspectiveForAccounts([account.id])
 
   # Exposed Data