From d8b1559d848e842fea31a2032d0edf543bc794ba Mon Sep 17 00:00:00 2001 From: Mo Bitar Date: Wed, 3 Jan 2018 00:56:31 -0600 Subject: [PATCH] Better handling for errorDecrypting --- .../app/frontend/models/local/itemParams.js | 29 ++++++++++++------- .../javascripts/app/services/authManager.js | 3 +- .../services/directives/views/accountMenu.js | 1 + .../services/encryption/encryptionHelper.js | 9 +++++- .../javascripts/app/services/modelManager.js | 9 +++++- .../javascripts/app/services/syncManager.js | 23 ++++++++++++++- .../directives/account-menu.html.haml | 3 -- 7 files changed, 59 insertions(+), 18 deletions(-) diff --git a/app/assets/javascripts/app/frontend/models/local/itemParams.js b/app/assets/javascripts/app/frontend/models/local/itemParams.js index 7cb25214c..d5a488ae7 100644 --- a/app/assets/javascripts/app/frontend/models/local/itemParams.js +++ b/app/assets/javascripts/app/frontend/models/local/itemParams.js @@ -31,20 +31,27 @@ class ItemParams { console.assert(!this.item.dummy, "Item is dummy, should not have gotten here.", this.item.dummy) var params = {uuid: this.item.uuid, content_type: this.item.content_type, deleted: this.item.deleted, created_at: this.item.created_at}; - if(this.keys && !this.item.doNotEncrypt()) { - var encryptedParams = EncryptionHelper.encryptItem(this.item, this.keys, this.version); - _.merge(params, encryptedParams); + if(!this.item.errorDecrypting) { + if(this.keys && !this.item.doNotEncrypt()) { + var encryptedParams = EncryptionHelper.encryptItem(this.item, this.keys, this.version); + _.merge(params, encryptedParams); - if(this.version !== "001") { - params.auth_hash = null; + if(this.version !== "001") { + params.auth_hash = null; + } } - } - else { - params.content = this.forExportFile ? this.item.createContentJSONFromProperties() : "000" + Neeto.crypto.base64(JSON.stringify(this.item.createContentJSONFromProperties())); - if(!this.forExportFile) { - params.enc_item_key = null; - params.auth_hash = null; + else { + params.content = this.forExportFile ? this.item.createContentJSONFromProperties() : "000" + Neeto.crypto.base64(JSON.stringify(this.item.createContentJSONFromProperties())); + if(!this.forExportFile) { + params.enc_item_key = null; + params.auth_hash = null; + } } + } else { + // Error decrypting, keep "content" and related fields as is (and do not try to encrypt, otherwise that would be undefined behavior) + params.content = this.item.content; + params.enc_item_key = this.item.enc_item_key; + params.auth_hash = this.item.auth_hash; } if(this.additionalFields) { diff --git a/app/assets/javascripts/app/services/authManager.js b/app/assets/javascripts/app/services/authManager.js index cf78c6000..46011d36f 100644 --- a/app/assets/javascripts/app/services/authManager.js +++ b/app/assets/javascripts/app/services/authManager.js @@ -197,7 +197,8 @@ angular.module('app.frontend') this.saveKeys = function(keys) { this._keys = keys; - storageManager.setItem("pw", keys.pw); + // Doesn't need to be saved. + // storageManager.setItem("pw", keys.pw); storageManager.setItem("mk", keys.mk); storageManager.setItem("ak", keys.ak); } diff --git a/app/assets/javascripts/app/services/directives/views/accountMenu.js b/app/assets/javascripts/app/services/directives/views/accountMenu.js index 21ddfe81d..4c9d977ee 100644 --- a/app/assets/javascripts/app/services/directives/views/accountMenu.js +++ b/app/assets/javascripts/app/services/directives/views/accountMenu.js @@ -149,6 +149,7 @@ class AccountMenu { var block = function() { $timeout(function(){ $scope.onSuccessfulAuth()(); + syncManager.refreshErroredItems(); syncManager.sync(); }) } diff --git a/app/assets/javascripts/app/services/encryption/encryptionHelper.js b/app/assets/javascripts/app/services/encryption/encryptionHelper.js index 6ec830665..7b20a9616 100644 --- a/app/assets/javascripts/app/services/encryption/encryptionHelper.js +++ b/app/assets/javascripts/app/services/encryption/encryptionHelper.js @@ -92,6 +92,7 @@ class EncryptionHelper { // return if uuid in auth hash does not match item uuid. Signs of tampering. if(keyParams.uuid && keyParams.uuid !== item.uuid) { + if(!item.errorDecrypting) { item.errorDecryptingValueChanged = true;} item.errorDecrypting = true; return; } @@ -99,6 +100,7 @@ class EncryptionHelper { var item_key = Neeto.crypto.decryptText(keyParams, requiresAuth); if(!item_key) { + if(!item.errorDecrypting) { item.errorDecryptingValueChanged = true;} item.errorDecrypting = true; return; } @@ -110,6 +112,7 @@ class EncryptionHelper { // return if uuid in auth hash does not match item uuid. Signs of tampering. if(itemParams.uuid && itemParams.uuid !== item.uuid) { + if(!item.errorDecrypting) { item.errorDecryptingValueChanged = true;} item.errorDecrypting = true; return; } @@ -121,11 +124,14 @@ class EncryptionHelper { var content = Neeto.crypto.decryptText(itemParams, true); if(!content) { + if(!item.errorDecrypting) { item.errorDecryptingValueChanged = true;} item.errorDecrypting = true; } else { + if(item.errorDecrypting == true) { item.errorDecryptingValueChanged = true;} + // Content should only be set if it was successfully decrypted, and should otherwise remain unchanged. item.errorDecrypting = false; + item.content = content; } - item.content = content; } static decryptMultipleItems(items, keys, throws) { @@ -139,6 +145,7 @@ class EncryptionHelper { try { this.decryptItem(item, keys); } catch (e) { + if(!item.errorDecrypting) { item.errorDecryptingValueChanged = true;} item.errorDecrypting = true; if(throws) { throw e; diff --git a/app/assets/javascripts/app/services/modelManager.js b/app/assets/javascripts/app/services/modelManager.js index 831b184b8..43cdbe11c 100644 --- a/app/assets/javascripts/app/services/modelManager.js +++ b/app/assets/javascripts/app/services/modelManager.js @@ -118,7 +118,14 @@ class ModelManager { continue; } - json_obj = _.omit(json_obj, omitFields || []) + // Lodash's _.omit, which was previously used, seems to cause unexpected behavior + // when json_obj is an ES6 item class. So we instead manually omit each key. + if(Array.isArray(omitFields)) { + for(var key of omitFields) { + delete json_obj[key]; + } + } + var item = this.findItem(json_obj.uuid); if(item) { diff --git a/app/assets/javascripts/app/services/syncManager.js b/app/assets/javascripts/app/services/syncManager.js index 8e5bc90e6..d11fe51c3 100644 --- a/app/assets/javascripts/app/services/syncManager.js +++ b/app/assets/javascripts/app/services/syncManager.js @@ -77,7 +77,7 @@ class SyncManager { markAllItemsDirtyAndSaveOffline(callback, alternateUUIDs) { // use a copy, as alternating uuid will affect array - var originalItems = this.modelManager.allItems.slice(); + var originalItems = this.modelManager.allItems.filter((item) => {return !item.errorDecrypting}).slice(); var block = () => { var allItems = this.modelManager.allItems; @@ -359,9 +359,30 @@ class SyncManager { var keys = this.authManager.keys() || this.passcodeManager.keys(); EncryptionHelper.decryptMultipleItems(responseItems, keys); var items = this.modelManager.mapResponseItemsToLocalModelsOmittingFields(responseItems, omitFields, source); + + // During the decryption process, items may be marked as "errorDecrypting". If so, we want to be sure + // to persist this new state by writing these items back to local storage. When an item's "errorDecrypting" + // flag is changed, its "errorDecryptingValueChanged" flag will be set, so we can find these items by filtering (then unsetting) below: + var itemsWithErrorStatusChange = items.filter((item) => { + var valueChanged = item.errorDecryptingValueChanged; + // unset after consuming value + item.errorDecryptingValueChanged = false; + return valueChanged; + }); + if(itemsWithErrorStatusChange.length > 0) { + this.writeItemsToLocalStorage(itemsWithErrorStatusChange, false, null); + } + return items; } + refreshErroredItems() { + var erroredItems = this.modelManager.allItems.filter((item) => {return item.errorDecrypting == true}); + if(erroredItems.length > 0) { + this.handleItemsResponse(erroredItems, null, ModelManager.MappingSourceLocalRetrieved); + } + } + handleUnsavedItemsResponse(unsaved) { if(unsaved.length == 0) { return; diff --git a/app/assets/templates/frontend/directives/account-menu.html.haml b/app/assets/templates/frontend/directives/account-menu.html.haml index 87ca42432..1537cbda5 100644 --- a/app/assets/templates/frontend/directives/account-menu.html.haml +++ b/app/assets/templates/frontend/directives/account-menu.html.haml @@ -89,9 +89,6 @@ %label.block Encryption key: .wrap.normal.mt-1.selectable {{encryptionKey()}} - %label.block.mt-5.mb-0 - Server password: - .wrap.normal.mt-1.selectable {{serverPassword() ? serverPassword() : 'Not available. Sign out then sign back in to compute.'}} %label.block.mt-5.mb-0 Authentication key: .wrap.normal.mt-1.selectable {{authKey() ? authKey() : 'Not available. Sign out then sign back in to compute.'}}