Fix the StorageManger detecting a false positive consistency check when manually migrating to rust from labs

package.json

@@ -151,6 +151,7 @@
         "@babel/preset-typescript": "^7.12.7",
         "@babel/register": "^7.12.10",
         "@casualbot/jest-sonar-reporter": "2.2.7",
+        "fake-indexeddb": "^5.0.2",
         "@peculiar/webcrypto": "^1.4.3",
         "@playwright/test": "^1.40.1",
         "@testing-library/dom": "^9.0.0",

playwright/e2e/crypto/staged-rollout.spec.ts

@@ -16,6 +16,7 @@ limitations under the License.
 
 import { test, expect } from "../../element-web-test";
 import { logIntoElement } from "./utils";
+import { SettingLevel } from "../../../src/settings/SettingLevel";
 
 test.describe("Adoption of rust stack", () => {
     test("Test migration of existing logins when rollout is 100%", async ({
@@ -30,6 +31,7 @@ test.describe("Adoption of rust stack", () => {
             "No need to test this on Rust Crypto as we override the config manually",
         );
         await page.goto("/#/login");
+        test.slow();
 
         let featureRustCrypto = false;
         let stagedRolloutPercent = 0;
@@ -150,4 +152,46 @@ test.describe("Adoption of rust stack", () => {
         await app.settings.openUserSettings("Help & About");
         await expect(page.getByText("Crypto version: Olm")).toBeVisible();
     });
+
+    test("Migrate using labflag should work", async ({ page, context, app, credentials, homeserver }, workerInfo) => {
+        test.skip(
+            workerInfo.project.name === "Rust Crypto",
+            "No need to test this on Rust Crypto as we override the config manually",
+        );
+
+        await page.goto("/#/login");
+
+        // In the project.name = "Legacy crypto" it will be olm crypto
+        await logIntoElement(page, homeserver, credentials);
+
+        await app.settings.openUserSettings("Help & About");
+        await expect(page.getByText("Crypto version: Olm")).toBeVisible();
+
+        // We need to enable devtools for this test
+        await app.settings.setValue("developerMode", null, SettingLevel.ACCOUNT, true);
+
+        // Now simulate a refresh with `feature_rust_crypto` enabled but ensure no automatic migration
+        await context.route(`http://localhost:8080/config.json*`, async (route) => {
+            const json = {};
+            json["features"] = {
+                feature_rust_crypto: true,
+            };
+            json["setting_defaults"] = {
+                "RustCrypto.staged_rollout_percent": 0,
+            };
+            await route.fulfill({ json });
+        });
+
+        await page.reload();
+
+        // Go to the labs flag and enable the migration
+        await app.settings.openUserSettings("Labs");
+        await page.getByRole("switch", { name: "Rust cryptography implementation" }).click();
+
+        // Fixes a bug where a missing session data was shown
+        // https://github.com/element-hq/element-web/issues/26970
+
+        await app.settings.openUserSettings("Help & About");
+        await expect(page.getByText("Crypto version: Rust SDK")).toBeVisible();
+    });
 });

src/utils/StorageManager.ts

@@ -19,29 +19,24 @@
 
 import SettingsStore from "../settings/SettingsStore";
 import { Features } from "../settings/Settings";
+import { MigrationState } from "../../../matrix-js-sdk/src/crypto/store/base";
 
 const localStorage = window.localStorage;
 
-// just *accessing* indexedDB throws an exception in firefox with
-// indexeddb disabled.
-let indexedDB: IDBFactory;
-try {
-    indexedDB = window.indexedDB;
-} catch (e) {}
+// make this lazy in order to make testing easier
+function getIndexedDb(): IDBFactory | undefined {
+    // just *accessing* _indexedDB throws an exception in firefox with
+    // indexeddb disabled.
+    try {
+        return window.indexedDB;
+    } catch (e) {}
+}
 
 // The JS SDK will add a prefix of "matrix-js-sdk:" to the sync store name.
 const SYNC_STORE_NAME = "riot-web-sync";
 const LEGACY_CRYPTO_STORE_NAME = "matrix-js-sdk:crypto";
 const RUST_CRYPTO_STORE_NAME = "matrix-js-sdk::matrix-sdk-crypto";
 
-function cryptoStoreName(): string {
-    if (SettingsStore.getValue(Features.RustCrypto)) {
-        return RUST_CRYPTO_STORE_NAME;
-    } else {
-        return LEGACY_CRYPTO_STORE_NAME;
-    }
-}
-
 function log(msg: string): void {
     logger.log(`StorageManager: ${msg}`);
 }
@@ -74,7 +69,7 @@
 }> {
     log("Checking storage consistency");
     log(`Local storage supported? ${!!localStorage}`);
-    log(`IndexedDB supported? ${!!indexedDB}`);
+    log(`IndexedDB supported? ${!!getIndexedDb()}`);
 
     let dataInLocalStorage = false;
     let dataInCryptoStore = false;
@@ -92,7 +87,7 @@
         error("Local storage cannot be used on this browser");
     }
 
-    if (indexedDB && localStorage) {
+    if (getIndexedDb() && localStorage) {
         const results = await checkSyncStore();
         if (!results.healthy) {
             healthy = false;
@@ -102,7 +97,7 @@
         error("Sync store cannot be used on this browser");
     }
 
-    if (indexedDB) {
+    if (getIndexedDb()) {
         const results = await checkCryptoStore();
         dataInCryptoStore = results.exists;
         if (!results.healthy) {
@@ -144,7 +139,7 @@
 async function checkSyncStore(): Promise<StoreCheck> {
     let exists = false;
     try {
-        exists = await IndexedDBStore.exists(indexedDB, SYNC_STORE_NAME);
+        exists = await IndexedDBStore.exists(getIndexedDb()!, SYNC_STORE_NAME);
         log(`Sync store using IndexedDB contains data? ${exists}`);
         return { exists, healthy: true };
     } catch (e) {
@@ -155,23 +150,102 @@
 }
 
 async function checkCryptoStore(): Promise<StoreCheck> {
-    let exists = false;
-    try {
-        exists = await IndexedDBCryptoStore.exists(indexedDB, cryptoStoreName());
-        log(`Crypto store using IndexedDB contains data? ${exists}`);
-        return { exists, healthy: true };
-    } catch (e) {
-        error("Crypto store using IndexedDB inaccessible", e);
-    }
-    try {
-        exists = LocalStorageCryptoStore.exists(localStorage);
-        log(`Crypto store using local storage contains data? ${exists}`);
-        return { exists, healthy: true };
-    } catch (e) {
-        error("Crypto store using local storage inaccessible", e);
+    if (await SettingsStore.getValue(Features.RustCrypto)) {
+        // check first if there is a rust crypto store
+        try {
+            const rustDbExists = await IndexedDBCryptoStore.exists(getIndexedDb()!, RUST_CRYPTO_STORE_NAME);
+            log(`Rust Crypto store using IndexedDB contains data? ${rustDbExists}`);
+
+            if (rustDbExists) {
+                // There was an existing rust database, so consider it healthy.
+                return { exists: true, healthy: true };
+            } else {
+                // No rust store, so let's check if there is a legacy store not yet migrated.
+                try {
+                    const legacyIdbExists = await legacyCryptoExistsAndIsNotMigrated(getIndexedDb()!);
+                    log(`Legacy Crypto store using IndexedDB contains non migrated data? ${legacyIdbExists}`);
+                    return { exists: legacyIdbExists, healthy: true };
+                } catch (e) {
+                    error("Legacy crypto store using IndexedDB inaccessible", e);
+                }
+
+                // No need to check local storage or memory as rust stack doesn't support them.
+                // Given that rust stack requires indexeddb, set healthy to false.
+                return { exists: false, healthy: false };
+            }
+        } catch (e) {
+            error("Rust crypto store using IndexedDB inaccessible", e);
+            return { exists: false, healthy: false };
+        }
+    } else {
+        let exists = false;
+        // legacy checks
+        try {
+            exists = await IndexedDBCryptoStore.exists(getIndexedDb()!, LEGACY_CRYPTO_STORE_NAME);
+            log(`Crypto store using IndexedDB contains data? ${exists}`);
+            return { exists, healthy: true };
+        } catch (e) {
+            error("Crypto store using IndexedDB inaccessible", e);
+        }
+        try {
+            exists = LocalStorageCryptoStore.exists(localStorage);
+            log(`Crypto store using local storage contains data? ${exists}`);
+            return { exists, healthy: true };
+        } catch (e) {
+            error("Crypto store using local storage inaccessible", e);
+        }
+        log("Crypto store using memory only");
+        return { exists, healthy: false };
     }
-    log("Crypto store using memory only");
-    return { exists, healthy: false };
+}
+
+/**
+ * There are some cases where the StorageManager is doing the check after the rust crypto feature
+ * has been enabled and before it has been migrated (via lab flag). In this case, we need to check if the legacy
+ * store is present and not migrated to do proper consistency check. If it is, we should return true.
+ *
+ * @param indexedDB - The `IDBFactory` interface
+ */
+function legacyCryptoExistsAndIsNotMigrated(indexedDB: IDBFactory): Promise<boolean> {
+    return new Promise<boolean>((resolve, reject) => {
+        let exists = true;
+        const openDBRequest = indexedDB.open(LEGACY_CRYPTO_STORE_NAME);
+        openDBRequest.onupgradeneeded = (): void => {
+            // Since we did not provide an explicit version when opening, this event
+            // should only fire if the DB did not exist before at any version.
+            exists = false;
+        };
+        openDBRequest.onblocked = (): void => reject(openDBRequest.error);
+        openDBRequest.onsuccess = (): void => {
+            const db = openDBRequest.result;
+            if (!exists) {
+                db.close();
+                // The DB did not exist before, but has been created as part of this
+                // existence check. Delete it now to restore previous state. Delete can
+                // actually take a while to complete in some browsers, so don't wait for
+                // it. This won't block future open calls that a store might issue next to
+                // properly set up the DB.
+                indexedDB.deleteDatabase(LEGACY_CRYPTO_STORE_NAME);
+                resolve(false);
+            } else {
+                const tx = db.transaction([IndexedDBCryptoStore.STORE_ACCOUNT], "readonly");
+                const objectStore = tx.objectStore(IndexedDBCryptoStore.STORE_ACCOUNT);
+                const getReq = objectStore.get("migrationState");
+
+                getReq.onsuccess = (): void => {
+                    const migrationState = getReq.result ?? MigrationState.NOT_STARTED;
+                    resolve(migrationState === MigrationState.NOT_STARTED);
+                };
+
+                getReq.onerror = (): void => {
+                    reject(getReq.error);
+                };
+
+                db.close();
+            }
+        };
+        openDBRequest.onerror = (): void => reject(openDBRequest.error);
+    });
 }
 
 /**
@@ -194,11 +268,11 @@
 let idb: IDBDatabase | null = null;
 
 async function idbInit(): Promise<void> {
-    if (!indexedDB) {
+    if (!getIndexedDb()) {
         throw new Error("IndexedDB not available");
     }
     idb = await new Promise((resolve, reject) => {
-        const request = indexedDB.open("matrix-react-sdk", 1);
+        const request = getIndexedDb()!.open("matrix-react-sdk", 1);
         request.onerror = reject;
         request.onsuccess = (): void => {
             resolve(request.result);