Skip to content
This repository was archived by the owner on Sep 11, 2024. It is now read-only.

Commit cdfcd37

Browse files
authored
Fix the StorageManger detecting a false positive consistency check when manually migrating to rust from labs (#12225)
* Fix StorageManager checks for rust migration manual opt-in * fix restricted imports * Moved utility to check db internals into js-sdk * fix typos and test names * more timeout for migration test
1 parent b14fa36 commit cdfcd37

File tree

5 files changed

+291
-36
lines changed

5 files changed

+291
-36
lines changed

package.json

+1
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@
151151
"@babel/preset-typescript": "^7.12.7",
152152
"@babel/register": "^7.12.10",
153153
"@casualbot/jest-sonar-reporter": "2.2.7",
154+
"fake-indexeddb": "^5.0.2",
154155
"@peculiar/webcrypto": "^1.4.3",
155156
"@playwright/test": "^1.40.1",
156157
"@testing-library/dom": "^9.0.0",

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

+47
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ limitations under the License.
1616

1717
import { test, expect } from "../../element-web-test";
1818
import { logIntoElement } from "./utils";
19+
import { SettingLevel } from "../../../src/settings/SettingLevel";
1920

2021
test.describe("Adoption of rust stack", () => {
2122
test("Test migration of existing logins when rollout is 100%", async ({
@@ -30,6 +31,7 @@ test.describe("Adoption of rust stack", () => {
3031
"No need to test this on Rust Crypto as we override the config manually",
3132
);
3233
await page.goto("/#/login");
34+
test.slow();
3335

3436
let featureRustCrypto = false;
3537
let stagedRolloutPercent = 0;
@@ -86,6 +88,7 @@ test.describe("Adoption of rust stack", () => {
8688
workerInfo.project.name === "Rust Crypto",
8789
"No need to test this on Rust Crypto as we override the config manually",
8890
);
91+
test.slow();
8992
await page.goto("/#/login");
9093

9194
await context.route(`http://localhost:8080/config.json*`, async (route) => {
@@ -123,6 +126,7 @@ test.describe("Adoption of rust stack", () => {
123126
workerInfo.project.name === "Rust Crypto",
124127
"No need to test this on Rust Crypto as we override the config manually",
125128
);
129+
test.slow();
126130

127131
await page.goto("/#/login");
128132

@@ -150,4 +154,47 @@ test.describe("Adoption of rust stack", () => {
150154
await app.settings.openUserSettings("Help & About");
151155
await expect(page.getByText("Crypto version: Olm")).toBeVisible();
152156
});
157+
158+
test("Migrate using labflag should work", async ({ page, context, app, credentials, homeserver }, workerInfo) => {
159+
test.skip(
160+
workerInfo.project.name === "Rust Crypto",
161+
"No need to test this on Rust Crypto as we override the config manually",
162+
);
163+
test.slow();
164+
165+
await page.goto("/#/login");
166+
167+
// In the project.name = "Legacy crypto" it will be olm crypto
168+
await logIntoElement(page, homeserver, credentials);
169+
170+
await app.settings.openUserSettings("Help & About");
171+
await expect(page.getByText("Crypto version: Olm")).toBeVisible();
172+
173+
// We need to enable devtools for this test
174+
await app.settings.setValue("developerMode", null, SettingLevel.ACCOUNT, true);
175+
176+
// Now simulate a refresh with `feature_rust_crypto` enabled but ensure no automatic migration
177+
await context.route(`http://localhost:8080/config.json*`, async (route) => {
178+
const json = {};
179+
json["features"] = {
180+
feature_rust_crypto: true,
181+
};
182+
json["setting_defaults"] = {
183+
"RustCrypto.staged_rollout_percent": 0,
184+
};
185+
await route.fulfill({ json });
186+
});
187+
188+
await page.reload();
189+
190+
// Go to the labs flag and enable the migration
191+
await app.settings.openUserSettings("Labs");
192+
await page.getByRole("switch", { name: "Rust cryptography implementation" }).click();
193+
194+
// Fixes a bug where a missing session data was shown
195+
// https://github.com/element-hq/element-web/issues/26970
196+
197+
await app.settings.openUserSettings("Help & About");
198+
await expect(page.getByText("Crypto version: Rust SDK")).toBeVisible();
199+
});
153200
});

src/utils/StorageManager.ts

+63-36
Original file line numberDiff line numberDiff line change
@@ -22,26 +22,20 @@ import { Features } from "../settings/Settings";
2222

2323
const localStorage = window.localStorage;
2424

25-
// just *accessing* indexedDB throws an exception in firefox with
26-
// indexeddb disabled.
27-
let indexedDB: IDBFactory;
28-
try {
29-
indexedDB = window.indexedDB;
30-
} catch (e) {}
25+
// make this lazy in order to make testing easier
26+
function getIndexedDb(): IDBFactory | undefined {
27+
// just *accessing* _indexedDB throws an exception in firefox with
28+
// indexeddb disabled.
29+
try {
30+
return window.indexedDB;
31+
} catch (e) {}
32+
}
3133

3234
// The JS SDK will add a prefix of "matrix-js-sdk:" to the sync store name.
3335
const SYNC_STORE_NAME = "riot-web-sync";
3436
const LEGACY_CRYPTO_STORE_NAME = "matrix-js-sdk:crypto";
3537
const RUST_CRYPTO_STORE_NAME = "matrix-js-sdk::matrix-sdk-crypto";
3638

37-
function cryptoStoreName(): string {
38-
if (SettingsStore.getValue(Features.RustCrypto)) {
39-
return RUST_CRYPTO_STORE_NAME;
40-
} else {
41-
return LEGACY_CRYPTO_STORE_NAME;
42-
}
43-
}
44-
4539
function log(msg: string): void {
4640
logger.log(`StorageManager: ${msg}`);
4741
}
@@ -74,7 +68,7 @@ export async function checkConsistency(): Promise<{
7468
}> {
7569
log("Checking storage consistency");
7670
log(`Local storage supported? ${!!localStorage}`);
77-
log(`IndexedDB supported? ${!!indexedDB}`);
71+
log(`IndexedDB supported? ${!!getIndexedDb()}`);
7872

7973
let dataInLocalStorage = false;
8074
let dataInCryptoStore = false;
@@ -92,7 +86,7 @@ export async function checkConsistency(): Promise<{
9286
error("Local storage cannot be used on this browser");
9387
}
9488

95-
if (indexedDB && localStorage) {
89+
if (getIndexedDb() && localStorage) {
9690
const results = await checkSyncStore();
9791
if (!results.healthy) {
9892
healthy = false;
@@ -102,7 +96,7 @@ export async function checkConsistency(): Promise<{
10296
error("Sync store cannot be used on this browser");
10397
}
10498

105-
if (indexedDB) {
99+
if (getIndexedDb()) {
106100
const results = await checkCryptoStore();
107101
dataInCryptoStore = results.exists;
108102
if (!results.healthy) {
@@ -144,7 +138,7 @@ interface StoreCheck {
144138
async function checkSyncStore(): Promise<StoreCheck> {
145139
let exists = false;
146140
try {
147-
exists = await IndexedDBStore.exists(indexedDB, SYNC_STORE_NAME);
141+
exists = await IndexedDBStore.exists(getIndexedDb()!, SYNC_STORE_NAME);
148142
log(`Sync store using IndexedDB contains data? ${exists}`);
149143
return { exists, healthy: true };
150144
} catch (e) {
@@ -155,23 +149,56 @@ async function checkSyncStore(): Promise<StoreCheck> {
155149
}
156150

157151
async function checkCryptoStore(): Promise<StoreCheck> {
158-
let exists = false;
159-
try {
160-
exists = await IndexedDBCryptoStore.exists(indexedDB, cryptoStoreName());
161-
log(`Crypto store using IndexedDB contains data? ${exists}`);
162-
return { exists, healthy: true };
163-
} catch (e) {
164-
error("Crypto store using IndexedDB inaccessible", e);
165-
}
166-
try {
167-
exists = LocalStorageCryptoStore.exists(localStorage);
168-
log(`Crypto store using local storage contains data? ${exists}`);
169-
return { exists, healthy: true };
170-
} catch (e) {
171-
error("Crypto store using local storage inaccessible", e);
152+
if (await SettingsStore.getValue(Features.RustCrypto)) {
153+
// check first if there is a rust crypto store
154+
try {
155+
const rustDbExists = await IndexedDBCryptoStore.exists(getIndexedDb()!, RUST_CRYPTO_STORE_NAME);
156+
log(`Rust Crypto store using IndexedDB contains data? ${rustDbExists}`);
157+
158+
if (rustDbExists) {
159+
// There was an existing rust database, so consider it healthy.
160+
return { exists: true, healthy: true };
161+
} else {
162+
// No rust store, so let's check if there is a legacy store not yet migrated.
163+
try {
164+
const legacyIdbExists = await IndexedDBCryptoStore.existsAndIsNotMigrated(
165+
getIndexedDb()!,
166+
LEGACY_CRYPTO_STORE_NAME,
167+
);
168+
log(`Legacy Crypto store using IndexedDB contains non migrated data? ${legacyIdbExists}`);
169+
return { exists: legacyIdbExists, healthy: true };
170+
} catch (e) {
171+
error("Legacy crypto store using IndexedDB inaccessible", e);
172+
}
173+
174+
// No need to check local storage or memory as rust stack doesn't support them.
175+
// Given that rust stack requires indexeddb, set healthy to false.
176+
return { exists: false, healthy: false };
177+
}
178+
} catch (e) {
179+
error("Rust crypto store using IndexedDB inaccessible", e);
180+
return { exists: false, healthy: false };
181+
}
182+
} else {
183+
let exists = false;
184+
// legacy checks
185+
try {
186+
exists = await IndexedDBCryptoStore.exists(getIndexedDb()!, LEGACY_CRYPTO_STORE_NAME);
187+
log(`Crypto store using IndexedDB contains data? ${exists}`);
188+
return { exists, healthy: true };
189+
} catch (e) {
190+
error("Crypto store using IndexedDB inaccessible", e);
191+
}
192+
try {
193+
exists = LocalStorageCryptoStore.exists(localStorage);
194+
log(`Crypto store using local storage contains data? ${exists}`);
195+
return { exists, healthy: true };
196+
} catch (e) {
197+
error("Crypto store using local storage inaccessible", e);
198+
}
199+
log("Crypto store using memory only");
200+
return { exists, healthy: false };
172201
}
173-
log("Crypto store using memory only");
174-
return { exists, healthy: false };
175202
}
176203

177204
/**
@@ -194,11 +221,11 @@ export function setCryptoInitialised(cryptoInited: boolean): void {
194221
let idb: IDBDatabase | null = null;
195222

196223
async function idbInit(): Promise<void> {
197-
if (!indexedDB) {
224+
if (!getIndexedDb()) {
198225
throw new Error("IndexedDB not available");
199226
}
200227
idb = await new Promise((resolve, reject) => {
201-
const request = indexedDB.open("matrix-react-sdk", 1);
228+
const request = getIndexedDb()!.open("matrix-react-sdk", 1);
202229
request.onerror = reject;
203230
request.onsuccess = (): void => {
204231
resolve(request.result);

0 commit comments

Comments
 (0)