apply changes from review, and other fixes/improvements

src/BasePlatform.ts

@@ -26,7 +26,7 @@ import {CheckUpdatesPayload} from "./dispatcher/payloads/CheckUpdatesPayload";
 import {Action} from "./dispatcher/actions";
 import {hideToast as hideUpdateToast} from "./toasts/UpdateToast";
 import {MatrixClientPeg} from "./MatrixClientPeg";
-import {idbLoad, idbSave, idbDelete} from "./IndexedDB";
+import {idbLoad, idbSave, idbDelete} from "./utils/StorageManager";
 
 export const SSO_HOMESERVER_URL_KEY = "mx_sso_hs_url";
 export const SSO_ID_SERVER_URL_KEY = "mx_sso_is_url";
@@ -344,7 +344,11 @@ export default abstract class BasePlatform {
             {name: "AES-GCM", iv, additionalData}, cryptoKey, randomArray,
         );
 
-        await idbSave("pickleKey", [userId, deviceId], {encrypted, iv, cryptoKey});
+        try {
+            await idbSave("pickleKey", [userId, deviceId], {encrypted, iv, cryptoKey});
+        } catch (e) {
+            return null;
+        }
         return encodeUnpaddedBase64(randomArray);
     }
 

src/IndexedDB.ts

@@ -1,92 +0,0 @@
-/*
-Copyright 2020 The Matrix.org Foundation C.I.C.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-/* Simple wrapper around IndexedDB.
- */
-
-let idb = null;
-
-async function idbInit(): Promise<void> {
-    idb = await new Promise((resolve, reject) => {
-        const request = window.indexedDB.open("element", 1);
-        request.onerror = reject;
-        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-        // @ts-ignore: TS thinks target.result doesn't exist
-        request.onsuccess = (event) => { resolve(event.target.result); };
-        request.onupgradeneeded = (event) => {
-            // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-            // @ts-ignore: TS thinks target.result doesn't exist
-            const db = event.target.result;
-            db.createObjectStore("pickleKey");
-            db.createObjectStore("account");
-        };
-    });
-}
-
-export async function idbLoad(
-    table: string,
-    key: string | string[],
-): Promise<any> {
-    if (!idb) {
-        await idbInit();
-    }
-    return new Promise((resolve, reject) => {
-        const txn = idb.transaction([table], "readonly");
-        txn.onerror = reject;
-
-        const objectStore = txn.objectStore(table);
-        const request = objectStore.get(key);
-        request.onerror = reject;
-        request.onsuccess = (event) => { resolve(request.result); };
-    });
-}
-
-export async function idbSave(
-    table: string,
-    key: string | string[],
-    data: any,
-): Promise<void> {
-    if (!idb) {
-        await idbInit();
-    }
-    return new Promise((resolve, reject) => {
-        const txn = idb.transaction([table], "readwrite");
-        txn.onerror = reject;
-
-        const objectStore = txn.objectStore(table);
-        const request = objectStore.put(data, key);
-        request.onerror = reject;
-        request.onsuccess = (event) => { resolve(); };
-    });
-}
-
-export async function idbDelete(
-    table: string,
-    key: string | string[],
-): Promise<void> {
-    if (!idb) {
-        await idbInit();
-    }
-    return new Promise((resolve, reject) => {
-        const txn = idb.transaction([table], "readwrite");
-        txn.onerror = reject;
-
-        const objectStore = txn.objectStore(table);
-        const request = objectStore.delete(key);
-        request.onerror = reject;
-        request.onsuccess = (event) => { resolve(); };
-    });
-}

src/Lifecycle.ts

@@ -51,7 +51,6 @@ import ThreepidInviteStore from "./stores/ThreepidInviteStore";
 import CountlyAnalytics from "./CountlyAnalytics";
 import CallHandler from './CallHandler';
 import LifecycleCustomisations from "./customisations/Lifecycle";
-import {idbLoad, idbSave, idbDelete} from "./IndexedDB";
 
 const HOMESERVER_URL_KEY = "mx_hs_url";
 const ID_SERVER_URL_KEY = "mx_is_url";
@@ -154,8 +153,8 @@ export async function loadSession(opts: ILoadSessionOpts = {}): Promise<boolean>
  *     return [null, null].
  */
 export async function getStoredSessionOwner(): Promise<[string, boolean]> {
-    const {hsUrl, userId, accessToken, isGuest} = await getLocalStorageSessionVars();
+    const {hsUrl, userId, hasAccessToken, isGuest} = await getStoredSessionVars();
-    return hsUrl && userId && accessToken ? [userId, isGuest] : [null, null];
+    return hsUrl && userId && hasAccessToken ? [userId, isGuest] : [null, null];
 }
 
 /**
@@ -193,7 +192,7 @@ export function attemptTokenLogin(
     ).then(function(creds) {
         console.log("Logged in with token");
         return clearStorage().then(async () => {
-            await persistCredentialsToLocalStorage(creds);
+            await persistCredentials(creds);
             // remember that we just logged in
             sessionStorage.setItem("mx_fresh_login", String(true));
             return true;
@@ -271,31 +270,42 @@ function registerAsGuest(
     });
 }
 
-export interface ILocalStorageSession {
+export interface IStoredSession {
     hsUrl: string;
     isUrl: string;
-    accessToken: string;
+    hasAccessToken: boolean;
+    accessToken: string | object;
     userId: string;
     deviceId: string;
     isGuest: boolean;
 }
 
 /**
- * Retrieves information about the stored session in localstorage. The session
+ * Retrieves information about the stored session from the browser's storage. The session
  * may not be valid, as it is not tested for consistency here.
  * @returns {Object} Information about the session - see implementation for variables.
  */
-export async function getLocalStorageSessionVars(): Promise<ILocalStorageSession> {
+export async function getStoredSessionVars(): Promise<IStoredSession> {
     const hsUrl = localStorage.getItem(HOMESERVER_URL_KEY);
     const isUrl = localStorage.getItem(ID_SERVER_URL_KEY);
-    let accessToken = await idbLoad("account", "mx_access_token");
+    let accessToken;
+    try {
+        accessToken = await StorageManager.idbLoad("account", "mx_access_token");
+    } catch (e) {}
     if (!accessToken) {
         accessToken = localStorage.getItem("mx_access_token");
         if (accessToken) {
-            await idbSave("account", "mx_access_token", accessToken);
+            try {
-            localStorage.removeItem("mx_access_token");
+                // try to migrate access token to IndexedDB if we can
+                await StorageManager.idbSave("account", "mx_access_token", accessToken);
+                localStorage.removeItem("mx_access_token");
+            } catch (e) {}
         }
     }
+    // if we pre-date storing "mx_has_access_token", but we retrieved an access
+    // token, then we should say we have an access token
+    const hasAccessToken =
+        (localStorage.getItem("mx_has_access_token") === "true") || !!accessToken;
     const userId = localStorage.getItem("mx_user_id");
     const deviceId = localStorage.getItem("mx_device_id");
 
@@ -307,7 +317,7 @@ export async function getLocalStorageSessionVars(): Promise<ILocalStorageSession
         isGuest = localStorage.getItem("matrix-is-guest") === "true";
     }
 
-    return {hsUrl, isUrl, accessToken, userId, deviceId, isGuest};
+    return {hsUrl, isUrl, hasAccessToken, accessToken, userId, deviceId, isGuest};
 }
 
 // The pickle key is a string of unspecified length and format.  For AES, we
@@ -334,6 +344,18 @@ async function pickleKeyToAesKey(pickleKey: string): Promise<Uint8Array> {
     ));
 }
 
+async function abortLogin() {
+    const signOut = await showStorageEvictedDialog();
+    if (signOut) {
+        await clearStorage();
+        // This error feels a bit clunky, but we want to make sure we don't go any
+        // further and instead head back to sign in.
+        throw new AbortLoginAndRebuildStorage(
+            "Aborting login in progress because of storage inconsistency",
+        );
+    }
+}
+
 // returns a promise which resolves to true if a session is found in
 // localstorage
 //
@@ -351,7 +373,11 @@ async function restoreFromLocalStorage(opts?: { ignoreGuest?: boolean }): Promis
         return false;
     }
 
-    const {hsUrl, isUrl, accessToken, userId, deviceId, isGuest} = await getLocalStorageSessionVars();
+    const {hsUrl, isUrl, hasAccessToken, accessToken, userId, deviceId, isGuest} = await getStoredSessionVars();
+
+    if (hasAccessToken && !accessToken) {
+        abortLogin();
+    }
 
     if (accessToken && userId && hsUrl) {
         if (ignoreGuest && isGuest) {
@@ -379,7 +405,7 @@ async function restoreFromLocalStorage(opts?: { ignoreGuest?: boolean }): Promis
         await doSetLoggedIn({
             userId: userId,
             deviceId: deviceId,
-            accessToken: decryptedAccessToken,
+            accessToken: decryptedAccessToken as string,
             homeserverUrl: hsUrl,
             identityServerUrl: isUrl,
             guest: isGuest,
@@ -518,15 +544,7 @@ async function doSetLoggedIn(
     // crypto store, we'll be generally confused when handling encrypted data.
     // Show a modal recommending a full reset of storage.
     if (results.dataInLocalStorage && results.cryptoInited && !results.dataInCryptoStore) {
-        const signOut = await showStorageEvictedDialog();
+        await abortLogin();
-        if (signOut) {
-            await clearStorage();
-            // This error feels a bit clunky, but we want to make sure we don't go any
-            // further and instead head back to sign in.
-            throw new AbortLoginAndRebuildStorage(
-                "Aborting login in progress because of storage inconsistency",
-            );
-        }
     }
 
     Analytics.setLoggedIn(credentials.guest, credentials.homeserverUrl);
@@ -548,7 +566,7 @@ async function doSetLoggedIn(
 
     if (localStorage) {
         try {
-            await persistCredentialsToLocalStorage(credentials);
+            await persistCredentials(credentials);
             // make sure we don't think that it's a fresh login any more
             sessionStorage.removeItem("mx_fresh_login");
         } catch (e) {
@@ -577,7 +595,7 @@ function showStorageEvictedDialog(): Promise<boolean> {
 // `instanceof`. Babel 7 supports this natively in their class handling.
 class AbortLoginAndRebuildStorage extends Error { }
 
-async function persistCredentialsToLocalStorage(credentials: IMatrixClientCreds): Promise<void> {
+async function persistCredentials(credentials: IMatrixClientCreds): Promise<void> {
     localStorage.setItem(HOMESERVER_URL_KEY, credentials.homeserverUrl);
     if (credentials.identityServerUrl) {
         localStorage.setItem(ID_SERVER_URL_KEY, credentials.identityServerUrl);
@@ -585,14 +603,47 @@ async function persistCredentialsToLocalStorage(credentials: IMatrixClientCreds)
     localStorage.setItem("mx_user_id", credentials.userId);
     localStorage.setItem("mx_is_guest", JSON.stringify(credentials.guest));
 
+    // store whether we expect to find an access token, to detect the case
+    // where IndexedDB is blown away
+    if (credentials.accessToken) {
+        localStorage.setItem("mx_has_access_token", "true");
+    } else {
+        localStorage.deleteItem("mx_has_access_token");
+    }
+
     if (credentials.pickleKey) {
-        const encrKey = await pickleKeyToAesKey(credentials.pickleKey);
+        let encryptedAccessToken;
-        const encryptedAccessToken = await encryptAES(credentials.accessToken, encrKey, "access_token");
+        try {
-        encrKey.fill(0);
+            // try to encrypt the access token using the pickle key
-        await idbSave("account", "mx_access_token", encryptedAccessToken);
+            const encrKey = await pickleKeyToAesKey(credentials.pickleKey);
+            encryptedAccessToken = await encryptAES(credentials.accessToken, encrKey, "access_token");
+            encrKey.fill(0);
+        } catch (e) {
+            console.warn("Could not encrypt access token", e);
+        }
+        try {
+            // save either the encrypted access token, or the plain access
+            // token if we were unable to encrypt (e.g. if the browser doesn't
+            // have WebCrypto).
+            await StorageManager.idbSave(
+                "account", "mx_access_token",
+                encryptedAccessToken || credentials.accessToken,
+            );
+        } catch (e) {
+            // if we couldn't save to indexedDB, fall back to localStorage.  We
+            // store the access token unencrypted since localStorage only saves
+            // strings.
+            localStorage.setItem("mx_access_token", credentials.accessToken);
+        }
         localStorage.setItem("mx_has_pickle_key", String(true));
     } else {
-        await idbSave("account", "mx_access_token", credentials.accessToken);
+        try {
+            await StorageManager.idbSave(
+                "account", "mx_access_token", credentials.accessToken,
+            );
+        } catch (e) {
+            localStorage.setItem("mx_access_token", credentials.accessToken);
+        }
         if (localStorage.getItem("mx_has_pickle_key")) {
             console.error("Expected a pickle key, but none provided.  Encryption may not work.");
         }
@@ -769,7 +820,9 @@ async function clearStorage(opts?: { deleteEverything?: boolean }): Promise<void
 
         window.localStorage.clear();
 
-        await idbDelete("account", "mx_access_token");
+        try {
+            await StorageManager.idbDelete("account", "mx_access_token");
+        } catch (e) {}
 
         // now restore those invites
         if (!opts?.deleteEverything) {

src/utils/StorageManager.js

@@ -190,3 +190,79 @@ export function trackStores(client) {
 export function setCryptoInitialised(cryptoInited) {
     localStorage.setItem("mx_crypto_initialised", cryptoInited);
 }
+
+/* Simple wrapper functions around IndexedDB.
+ */
+
+let idb = null;
+
+async function idbInit(): Promise<void> {
+    if (!indexedDB) {
+        throw new Error("IndexedDB not available");
+    }
+    idb = await new Promise((resolve, reject) => {
+        const request = indexedDB.open("matrix-react-sdk", 1);
+        request.onerror = reject;
+        request.onsuccess = (event) => { resolve(request.result); };
+        request.onupgradeneeded = (event) => {
+            const db = request.result;
+            db.createObjectStore("pickleKey");
+            db.createObjectStore("account");
+        };
+    });
+}
+
+export async function idbLoad(
+    table: string,
+    key: string | string[],
+): Promise<any> {
+    if (!idb) {
+        await idbInit();
+    }
+    return new Promise((resolve, reject) => {
+        const txn = idb.transaction([table], "readonly");
+        txn.onerror = reject;
+
+        const objectStore = txn.objectStore(table);
+        const request = objectStore.get(key);
+        request.onerror = reject;
+        request.onsuccess = (event) => { resolve(request.result); };
+    });
+}
+
+export async function idbSave(
+    table: string,
+    key: string | string[],
+    data: any,
+): Promise<void> {
+    if (!idb) {
+        await idbInit();
+    }
+    return new Promise((resolve, reject) => {
+        const txn = idb.transaction([table], "readwrite");
+        txn.onerror = reject;
+
+        const objectStore = txn.objectStore(table);
+        const request = objectStore.put(data, key);
+        request.onerror = reject;
+        request.onsuccess = (event) => { resolve(); };
+    });
+}
+
+export async function idbDelete(
+    table: string,
+    key: string | string[],
+): Promise<void> {
+    if (!idb) {
+        await idbInit();
+    }
+    return new Promise((resolve, reject) => {
+        const txn = idb.transaction([table], "readwrite");
+        txn.onerror = reject;
+
+        const objectStore = txn.objectStore(table);
+        const request = objectStore.delete(key);
+        request.onerror = reject;
+        request.onsuccess = (event) => { resolve(); };
+    });
+}