Merge pull request #12230 from matrix-org/backport-12225-to-staging

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

Author: dbkr

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;
@@ -86,6 +88,7 @@ test.describe("Adoption of rust stack", () => {
             workerInfo.project.name === "Rust Crypto",
             "No need to test this on Rust Crypto as we override the config manually",
         );
+        test.slow();
         await page.goto("/#/login");
 
         await context.route(`http://localhost:8080/config.json*`, async (route) => {
@@ -123,6 +126,7 @@ test.describe("Adoption of rust stack", () => {
             workerInfo.project.name === "Rust Crypto",
             "No need to test this on Rust Crypto as we override the config manually",
         );
+        test.slow();
 
         await page.goto("/#/login");
 
@@ -150,4 +154,47 @@ 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",
+        );
+        test.slow();
+
+        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

@@ -22,26 +22,20 @@ import { Features } from "../settings/Settings";
 
 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 +68,7 @@ export async function checkConsistency(): Promise<{
 }> {
     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 +86,7 @@ export async function checkConsistency(): Promise<{
         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 +96,7 @@ export async function checkConsistency(): Promise<{
         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 +138,7 @@ interface StoreCheck {
 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 +149,56 @@ async function checkSyncStore(): Promise<StoreCheck> {
 }
 
 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 IndexedDBCryptoStore.existsAndIsNotMigrated(
+                        getIndexedDb()!,
+                        LEGACY_CRYPTO_STORE_NAME,
+                    );
+                    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 };
 }
 
 /**
@@ -194,11 +221,11 @@ export function setCryptoInitialised(cryptoInited: boolean): void {
 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);