[Fixes #4998 ] Handle decryption failures in realm

Signed-off-by: Andrea Maria Piana <andrea.maria.piana@gmail.com>
2018-07-09 09:19:40 +02:00 · 2018-07-09 09:19:40 +02:00 · a2ee06bc9d
parent 1ad6f40f57
commit a2ee06bc9d
10 changed files with 173 additions and 182 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ### Fixed
 - Fixed mailservers connectivity issue
 - Clear chat action correctly clear the unread messages counter
+- Gracefully handle realm decryption failures by showing a pop up asking the user to reset the data

 ### Changed
 - Downgraded React Native to 0.53.3 for improved performance and decreased battery consumption
--- a/src/status_im/data_store/core.cljs
+++ b/src/status_im/data_store/core.cljs
@ -1,6 +1,7 @@
 (ns status-im.data-store.core
  (:require [cljs.core.async :as async]
            [re-frame.core :as re-frame]
+            [taoensso.timbre :as log]
            [status-im.data-store.realm.core :as data-source]
            status-im.data-store.chats
            status-im.data-store.messages
@ -14,11 +15,11 @@

 (defn init [encryption-key]
  (when-not @data-source/base-realm
-    (data-source/open-base-realm encryption-key))
-  (data-source/reset-account-realm encryption-key))
+    (data-source/open-base-realm encryption-key)))

-(defn change-account [address new-account? encryption-key handler]
-  (data-source/change-account address new-account? encryption-key handler))
+(defn change-account [address encryption-key]
+  (log/debug "changing account to: " address)
+  (data-source/change-account address encryption-key))

 (defn- perform-transactions [raw-transactions realm]
  (let [success-events (keep :success-event raw-transactions)
--- a/src/status_im/data_store/realm/core.cljs
+++ b/src/status_im/data_store/realm/core.cljs
@ -11,8 +11,6 @@
            [status-im.react-native.js-dependencies :as rn-dependencies]
            [status-im.utils.utils :as utils]))

-(def new-account-filename "new-account")
-
 (defn to-buffer [key]
  (when key
    (let [length (.-length key)
@ -31,9 +29,8 @@
  [options file-name encryption-key]
  (log/debug "Opening realm at " file-name "...")
  (let [options-js (clj->js (assoc options :path file-name))]
-    (when encryption-key
-      (log/debug "Using encryption key...")
-      (set! (.-encryptionKey options-js) (to-buffer encryption-key)))
+    (log/debug "Using encryption key...")
+    (set! (.-encryptionKey options-js) (to-buffer encryption-key))
    (when (exists? js/window)
      (rn-dependencies/realm. options-js))))

@ -62,12 +59,6 @@
  (when realm
    (.close realm)))

-(defn reset-realm
-  "Delete realm & open a new database using encryption key"
-  [file-name schemas encryption-key]
-  (delete-realm file-name)
-  (open-realm (last schemas) file-name encryption-key))
-
 (defn- migrate-schemas
  "Apply migrations in sequence and open database with the last schema"
  [file-name schemas encryption-key current-version]
@ -110,33 +101,9 @@
  (reset! base-realm (open-migrated-realm (.-defaultPath rn-dependencies/realm) base/schemas encryption-key))
  (log/debug "Created @base-realm"))

-(defn reset-account-realm [encryption-key]
-  (log/debug "Resetting account realm...")
-  (when @account-realm
-    (close @account-realm))
-  (reset! account-realm (open-migrated-realm new-account-filename account/schemas encryption-key))
-  (.write @account-realm #(.deleteAll @account-realm))
-  (log/debug "Created @account-realm"))
-
-(defn move-file-handler [address encryption-key err handler]
-  (log/debug "Moved file with error: " err address)
-  (if err
-    (log/error "Error moving account realm: " (.-message err))
-    (reset! account-realm (open-migrated-realm address account/schemas encryption-key)))
-  (handler err))
-
-(defn change-account [address new-account? encryption-key handler]
-  (let [path (.-path @account-realm)]
-    (log/debug "closing account realm: " path)
-    (close-account-realm)
-    (log/debug "is new account? " new-account?)
-    (if new-account?
-      (let [new-path (string/replace path new-account-filename address)]
-        (log/debug "Moving file " path " to " new-path)
-        (fs/move-file path new-path #(move-file-handler address encryption-key % handler)))
-      (do
-        (reset! account-realm (open-migrated-realm address account/schemas encryption-key))
-        (handler nil)))))
+(defn change-account [address encryption-key]
+  (close-account-realm)
+  (reset! account-realm (open-migrated-realm address account/schemas encryption-key)))

 (declare realm-obj->clj)

@ -153,19 +120,6 @@
                                                    [schema-name :properties])))]
     (.create realm (name schema-name) (clj->js obj-to-save) update?))))

-(defn save
-  ([realm schema-name obj]
-   (save realm schema-name obj false))
-  ([realm schema-name obj update?]
-   (write realm #(create realm schema-name obj update?))))
-
-(defn save-all
-  ([realm schema-name objs]
-   (save-all realm schema-name objs false))
-  ([realm schema-name objs update?]
-   (write realm (fn []
-                  (mapv #(save realm schema-name % update?) objs)))))
-
 (defn delete [realm obj]
  (.delete realm obj))

--- a/src/status_im/translations/en.cljs
+++ b/src/status_im/translations/en.cljs
@ -686,6 +686,12 @@
   :invalid-key-content                  "To protect yourself, you need to create new account and erase your old data by tapping “Apply”. If you have an existing account and would like to save your seed phrase then choose “Cancel”, back it up, and restart the app. We strongly recommend creating new account because the old one is stored unencrypted."
   :invalid-key-confirm                  "Apply"

+   ;; decryption-failed
+
+   :decryption-failed-title              "We were not able to decrypt your data"
+   :decryption-failed-content            "We were not able to decrypt your data, you might need to create new account and erase your old data by tapping “Apply”. Clicking on “Cancel”, will try again"
+   :decryption-failed-confirm            "Apply"
+
   ;; browser
   :browser                              "Browser"
   :enter-dapp-url                       "Enter a ÐApp URL"
--- a/src/status_im/ui/screens/accounts/events.cljs
+++ b/src/status_im/ui/screens/accounts/events.cljs
@ -88,28 +88,12 @@
       (-> (add-account db account)
           (assoc :dispatch [:login-account normalized-address password]))))))

-(handlers/register-handler-fx
- :load-accounts
- [(re-frame/inject-cofx :data-store/get-all-accounts)]
- (fn [{:keys [db all-accounts]} _]
-   (let [accounts (->> all-accounts
-                       (map (fn [{:keys [address] :as account}]
-                              [address account]))
-                       (into {}))
-         ;;workaround for realm bug, migrating account v4
-         events   (mapv #(when (empty? (:networks %)) [:account-update-networks (:address %)]) (vals accounts))]
-     (merge
-      {:db (assoc db :accounts/accounts accounts)}
-      (when-not (empty? events)
-        {:dispatch-n events})))))
-
-(handlers/register-handler-fx
- :account-update-networks
- (fn [{{:accounts/keys [accounts] :networks/keys [networks] :as db} :db} [_ id]]
-   (let [current-account (get accounts id)
-         new-account     (assoc current-account :networks networks)]
-     {:db                 (assoc-in db [:accounts/accounts id] new-account)
-      :data-store/base-tx [(accounts-store/save-account-tx new-account)]})))
+(defn load-accounts [{:keys [db all-accounts]}]
+  (let [accounts (->> all-accounts
+                      (map (fn [{:keys [address] :as account}]
+                             [address account]))
+                      (into {}))]
+    {:db (assoc db :accounts/accounts accounts)}))

 (defn update-settings
  ([settings cofx] (update-settings settings nil cofx))
--- a/src/status_im/ui/screens/accounts/login/events.cljs
+++ b/src/status_im/ui/screens/accounts/login/events.cljs
@ -1,6 +1,6 @@
 (ns status-im.ui.screens.accounts.login.events
  (:require status-im.ui.screens.accounts.login.navigation
-            [re-frame.core :refer [dispatch reg-fx]]
+            [re-frame.core :as re-frame]
            [status-im.utils.handlers :refer [register-handler-db register-handler-fx]]
            [taoensso.timbre :as log]
            [status-im.utils.types :refer [json->clj]]
@ -12,39 +12,34 @@

 ;;;; FX

-(reg-fx ::stop-node (fn [] (status/stop-node)))
+(re-frame/reg-fx ::stop-node (fn [] (status/stop-node)))

-(reg-fx
+(re-frame/reg-fx
 ::login
 (fn [[address password]]
-   (status/login address password #(dispatch [:login-handler % address]))))
+   (status/login address password #(re-frame/dispatch [:login-handler % address]))))

-(reg-fx
+(re-frame/reg-fx
 ::clear-web-data
 (fn []
   (status/clear-web-data)))

-(defn change-account [address encryption-key]
-  (let [change-account-fn (fn [] (data-store/change-account address
-                                                            false
-                                                            encryption-key
-                                                            #(dispatch [:change-account-handler % address])))]
-    (if config/stub-status-go?
-      (utils/set-timeout change-account-fn
-                         300)
-      (change-account-fn))))
+(defn change-account! [address encryption-key]
+  (data-store/change-account address encryption-key)
+  (re-frame/dispatch [:change-account-handler address]))

-(reg-fx
+(defn handle-change-account [address]
+  ;; No matter what is the keychain we use, as checks are done on decrypting base
+  (.. (keychain/safe-get-encryption-key)
+      (then (partial change-account! address))
+      (catch (fn [error]
+               ;; If all else fails we fallback to showing initial error
+               (re-frame/dispatch [:initialize-app "" :decryption-failed])))))
+
+(re-frame/reg-fx
 ::change-account
 (fn [[address]]
-   ;; if we don't add delay when running app without status-go
-   ;; "null is not an object (evaluating 'realm.schema')" error appears
-   (.. (keychain/get-encryption-key)
-       (then (partial change-account address))
-       (catch (fn [{:keys [error key]}]
-                ;; no need of further error handling as already taken care
-                ;; when starting the app
-                (change-account address (or key "")))))))
+   (handle-change-account address)))

 ;;;; Handlers

@ -154,12 +149,10 @@

 (register-handler-fx
 :change-account-handler
- (fn [{{:keys [view-id] :as db} :db} [_ error address]]
-   (if (nil? error)
-     {:db       (cond-> (dissoc db :accounts/login)
-                  (= view-id :create-account)
-                  (assoc-in [:accounts/create :step] :enter-name))
-      :dispatch [:initialize-account address
-                 (when (not= view-id :create-account)
-                   [[:navigate-to-clean :home]])]}
-     (log/debug "Error changing acount: " error))))
+ (fn [{{:keys [view-id] :as db} :db} [_ address]]
+   {:db       (cond-> (dissoc db :accounts/login)
+                (= view-id :create-account)
+                (assoc-in [:accounts/create :step] :enter-name))
+    :dispatch [:initialize-account address
+               (when (not= view-id :create-account)
+                 [[:navigate-to-clean :home]])]}))
--- a/src/status_im/ui/screens/events.cljs
+++ b/src/status_im/ui/screens/events.cljs
@ -6,7 +6,7 @@
            status-im.network.events
            [status-im.transport.handlers :as transport.handlers]
            status-im.protocol.handlers
-            status-im.ui.screens.accounts.events
+            [status-im.ui.screens.accounts.events :as accounts.events]
            status-im.ui.screens.accounts.login.events
            status-im.ui.screens.accounts.recover.events
            [status-im.ui.screens.contacts.events :as contacts]
@ -139,25 +139,23 @@
   (doseq [call calls]
     (http-get call))))

+;; Try to decrypt the database, move on if successful otherwise go back to
+;; initial state
 (re-frame/reg-fx
 ::init-store
 (fn [encryption-key]
-   (data-store/init encryption-key)))
-
-(defn move-to-internal-storage [config]
-  (status/move-to-internal-storage
-   #(status/start-node config)))
+   (try
+     (do
+       (data-store/init encryption-key)
+       (re-frame/dispatch [:after-decryption]))
+     (catch js/Error error
+       (log/warn "Could not decrypt database" error)
+       (re-frame/dispatch [:initialize-app encryption-key :decryption-failed])))))

 (re-frame/reg-fx
 :initialize-geth-fx
 (fn [config]
-    ;;TODO get rid of this, because we don't need this anymore
-   (status/should-move-to-internal-storage?
-    (fn [should-move?]
-      (if should-move?
-        (re-frame/dispatch [:request-permissions {:permissions [:read-external-storage]
-                                                  :on-allowed  #(move-to-internal-storage config)}])
-        (status/start-node (types/clj->json config)))))))
+   (status/start-node (types/clj->json config))))

 (re-frame/reg-fx
 ::status-module-initialized-fx
@ -224,16 +222,16 @@
 (fn [db [_ path v]]
   (assoc-in db path v)))

-(handlers/register-handler-fx
- :initialize-keychain
- (fn [_ _]
-   {:get-encryption-key [:initialize-app]}))
-
 (defn- reset-keychain []
  (.. (keychain/reset)
      (then
       #(re-frame/dispatch [:initialize-keychain]))))

+(defn- handle-reset-data  []
+  (.. (realm/delete-realms)
+      (then reset-keychain)
+      (catch reset-keychain)))
+
 (defn handle-invalid-key-parameters [encryption-key]
  {:title               (i18n/label :invalid-key-title)
   :content             (i18n/label :invalid-key-content)
@ -243,45 +241,38 @@
   :on-cancel           #(do
                           (log/warn "initializing app with invalid key")
                           (re-frame/dispatch [:initialize-app encryption-key]))
-   :on-accept (fn []
-                (.. (realm/delete-realms)
-                    (then reset-keychain)
-                    (catch reset-keychain)))})
+   :on-accept           handle-reset-data})

-(handlers/register-handler-fx
- :initialize-app
- (fn [_ [_ encryption-key error]]
-   (if (= :invalid-key error)
-     {:show-confirmation (handle-invalid-key-parameters encryption-key)}
-     {::init-device-UUID nil
-      ::testfairy-alert  nil
-      :dispatch-n        [[:initialize-db encryption-key]
-                          [:load-accounts]
-                          [:initialize-views]
-                          [:listen-to-network-status]
-                          [:initialize-geth]]})))
+(defn handle-decryption-failed-parameters [encryption-key]
+  {:title               (i18n/label :decryption-failed-title)
+   :content             (i18n/label :decryption-failed-content)
+   :confirm-button-text (i18n/label :decryption-failed-confirm)
+   ;; On cancel we initialize the app with the same key, in case the error was
+   ;; not related/fs error
+   :on-cancel           #(do
+                           (log/warn "initializing app with same key after decryption failed")
+                           (re-frame/dispatch [:initialize-app encryption-key]))
+   :on-accept           handle-reset-data})

-(handlers/register-handler-fx
- :logout
- (fn [{:keys [db] :as cofx} [this-event encryption-key]]
-   (if encryption-key
-     (let [{:transport/keys [chats]} db]
-       (handlers-macro/merge-fx cofx
-                                {:dispatch-n [[:initialize-db encryption-key]
-                                              [:load-accounts]
-                                              [:listen-to-network-status]
-                                              [:navigate-to :accounts]]}
-                                (navigation/navigate-to-clean nil)
-                                (transport/stop-whisper)))
-     {:get-encryption-key [this-event]})))
+(defn initialize-views [{{:accounts/keys [accounts] :as db} :db}]
+  {:db (if (empty? accounts)
+         (assoc db :view-id :intro :navigation-stack (list :intro))
+         (let [{:keys [address photo-path name]} (first (sort-by :last-sign-in > (vals accounts)))]
+           (-> db
+               (assoc :view-id :login
+                      :navigation-stack (list :login))
+               (update :accounts/login assoc
+                       :address address
+                       :photo-path photo-path
+                       :name name))))})

-(defn initialize-db [encryption-key
-                     {{:universal-links/keys [url]
-                       :keys                 [status-module-initialized? status-node-started?
-                                              network-status network peers-count peers-summary device-UUID]
-                       :or                   {network (get app-db :network)}} :db}]
-  {::init-store encryption-key
-   :db          (assoc app-db
+(defn initialize-db
+  "Initialize db to the initial state"
+  [{{:universal-links/keys [url]
+     :keys                 [status-module-initialized? status-node-started?
+                            network-status network peers-count peers-summary device-UUID]
+     :or                   {network (get app-db :network)}} :db}]
+  {:db          (assoc app-db
                       :contacts/contacts {}
                       :network-status network-status
                       :peers-count (or peers-count 0)
@ -292,10 +283,51 @@
                       :universal-links/url url
                       :device-UUID device-UUID)})

+;; Entrypoint, fetches the key from the keychain and initialize the app
 (handlers/register-handler-fx
- :initialize-db
- (fn [cofx [_ encryption-key]]
-   (initialize-db encryption-key cofx)))
+ :initialize-keychain
+ (fn [_ _]
+   {:get-encryption-key [:initialize-app]}))
+
+;; Check the key is valid, shows options if not, otherwise continues loading
+;; the database
+(handlers/register-handler-fx
+ :initialize-app
+ (fn [cofx [_ encryption-key error]]
+   (cond
+     (= :invalid-key error)
+     {:show-confirmation (handle-invalid-key-parameters encryption-key)}
+
+     (= :decryption-failed error)
+     {:show-confirmation (handle-decryption-failed-parameters encryption-key)}
+
+     :else
+     (handlers-macro/merge-fx cofx
+                              {::init-device-UUID nil
+                               ::init-store       encryption-key
+                               ::testfairy-alert  nil}
+                              (initialize-db)))))
+
+;; DB has been decrypted, load accounts, initialize geth, etc
+(handlers/register-handler-fx
+ :after-decryption
+ [(re-frame/inject-cofx :data-store/get-all-accounts)]
+ (fn [cofx _]
+   (handlers-macro/merge-fx cofx
+                            {:dispatch-n
+                             [[:listen-to-network-status]
+                              [:initialize-geth]]}
+                            (accounts.events/load-accounts)
+                            (initialize-views))))
+
+(handlers/register-handler-fx
+ :logout
+ (fn [{:keys [db] :as cofx} _]
+   (let [{:transport/keys [chats]} db]
+     (handlers-macro/merge-fx cofx
+                              {:dispatch [:initialize-keychain]}
+                              (navigation/navigate-to-clean nil)
+                              (transport/stop-whisper)))))

 (handlers/register-handler-db
 :initialize-account-db
@ -344,20 +376,6 @@
                         (universal-links/stored-url-event cofx)]
                  (seq events-after) (into events-after))}))

-(handlers/register-handler-fx
- :initialize-views
- (fn [{{:accounts/keys [accounts] :as db} :db} _]
-   {:db (if (empty? accounts)
-          (assoc db :view-id :intro :navigation-stack (list :intro))
-          (let [{:keys [address photo-path name]} (first (sort-by :last-sign-in > (vals accounts)))]
-            (-> db
-                (assoc :view-id :login
-                       :navigation-stack (list :login))
-                (update :accounts/login assoc
-                        :address address
-                        :photo-path photo-path
-                        :name name))))}))
-
 (handlers/register-handler-fx
 :initialize-geth
 (fn [{db :db} _]
--- a/src/status_im/utils/keychain/core.cljs
+++ b/src/status_im/utils/keychain/core.cljs
@ -61,6 +61,15 @@
           (handle-found res)
           (handle-not-found))))))

+(defn safe-get-encryption-key
+  "Return encryption key or empty string in case invalid/empty"
+  []
+  (log/debug "initializing realm encryption key...")
+  (.. (get-encryption-key)
+      (catch (fn [{:keys [_ key]}]
+               (log/warn "key is invalid, continuing")
+               (or key "")))))
+
 (defn reset []
  (log/debug "resetting key...")
  (.resetGenericPassword rn/keychain))
--- a/test/cljs/status_im/test/ui/screens/events.cljs
+++ b/test/cljs/status_im/test/ui/screens/events.cljs
@ -4,7 +4,6 @@

 (deftest initialize-db
  (testing "it preserves universal-links/url"
-    (is (= "some-url" (get-in (events/initialize-db "blah"
-                                                    {:db
+    (is (= "some-url" (get-in (events/initialize-db {:db
                                                     {:universal-links/url "some-url"}})
                              [:db :universal-links/url])))))
--- a/test/cljs/status_im/test/utils/keychain/core.cljs
+++ b/test/cljs/status_im/test/utils/keychain/core.cljs
@ -50,3 +50,29 @@
                    (is (= :weak-key error))
                    (is (= weak-key (js->clj key)))
                    (done))))))))
+
+(deftest safe-key-is-not-valid
+  (async
+   done
+   (with-redefs [rn/keychain #js {:getGenericPassword (constantly (.resolve js/Promise #js {:password (key->json weak-key)}))}]
+     (testing "it returns a valid key"
+       (.. (keychain/safe-get-encryption-key)
+           (then (fn [k]
+                   (is (= weak-key (js->clj k)))
+                   (done)))
+           (catch (fn [err]
+                    (is (not err))
+                    (done))))))))
+
+(deftest safe-key-is-nil
+  (async
+   done
+   (with-redefs [rn/keychain #js {:getGenericPassword (constantly (.resolve js/Promise #js {:password nil}))}]
+     (testing "it returns a valid key"
+       (.. (keychain/safe-get-encryption-key)
+           (then (fn [k]
+                   (is (= "" (js->clj k)))
+                   (done)))
+           (catch (fn [err]
+                    (is (not err))
+                    (done))))))))