OpenCloud
/
riot-web
mirror of https://github.com/vector-im/riot-web
1
0
Fork 0
Code Issues Releases Wiki Activity

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
pull/28217/head
Valere 2024-02-05 18:57:12 +01:00 committed by GitHub
parent b14fa366dd
commit cdfcd37b94
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 291 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
package.json
View File

@ -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",

47
playwright/e2e/crypto/staged-rollout.spec.ts
View File

@ -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();
});
});

99
src/utils/StorageManager.ts
View File

@ -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);
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 };
}
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 };
}
/**
@ -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);

175
test/utils/StorageManager-test.ts Normal file
View File

@ -0,0 +1,175 @@
/*
Copyright 2024 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.
*/
import "fake-indexeddb/auto";
import { IDBFactory } from "fake-indexeddb";
import { IndexedDBCryptoStore } from "matrix-js-sdk/src/matrix";
import * as StorageManager from "../../src/utils/StorageManager";
import SettingsStore from "../../src/settings/SettingsStore";
const LEGACY_CRYPTO_STORE_NAME = "matrix-js-sdk:crypto";
const RUST_CRYPTO_STORE_NAME = "matrix-js-sdk::matrix-sdk-crypto";
describe("StorageManager", () => {
async function createDB(name: string, withStores: string[] | undefined = undefined): Promise<IDBDatabase> {
const request = indexedDB.open(name);
return new Promise((resolve, reject) => {
request.onupgradeneeded = function (event) {
const db = request.result;
if (withStores) {
withStores.forEach((storeName) => {
db.createObjectStore(storeName);
});
}
};
request.onsuccess = function (event) {
const db = request.result;
resolve(db);
};
request.onerror = function (event) {
reject(event);
};
});
}
async function populateLegacyStore(migrationState: number | undefined) {
const db = await createDB(LEGACY_CRYPTO_STORE_NAME, [IndexedDBCryptoStore.STORE_ACCOUNT]);
if (migrationState) {
const transaction = db.transaction([IndexedDBCryptoStore.STORE_ACCOUNT], "readwrite");
const store = transaction.objectStore(IndexedDBCryptoStore.STORE_ACCOUNT);
store.put(migrationState, "migrationState");
await new Promise((resolve, reject) => {
transaction.oncomplete = resolve;
transaction.onerror = reject;
});
}
}
beforeEach(() => {
global.structuredClone = (v) => JSON.parse(JSON.stringify(v));
});
describe("Crypto store checks", () => {
async function populateHealthySession() {
// Storage manager only check for the existence of the `riot-web-sync` store, so just create one.
await createDB("riot-web-sync");
}
beforeEach(async () => {
await populateHealthySession();
// eslint-disable-next-line no-global-assign
indexedDB = new IDBFactory();
});
describe("with `feature_rust_crypto` enabled", () => {
beforeEach(() => {
jest.spyOn(SettingsStore, "getValue").mockImplementation(async (key) => {
if (key === "feature_rust_crypto") {
return true;
}
throw new Error(`Unknown key ${key}`);
});
});
it("should not be ok if sync store but no crypto store", async () => {
const result = await StorageManager.checkConsistency();
expect(result.healthy).toBe(true);
expect(result.dataInCryptoStore).toBe(false);
});
it("should be ok if sync store and a rust crypto store", async () => {
await createDB(RUST_CRYPTO_STORE_NAME);
const result = await StorageManager.checkConsistency();
expect(result.healthy).toBe(true);
expect(result.dataInCryptoStore).toBe(true);
});
describe("without rust store", () => {
it("should be ok if there is non migrated legacy crypto store", async () => {
await populateLegacyStore(undefined);
const result = await StorageManager.checkConsistency();
expect(result.healthy).toBe(true);
expect(result.dataInCryptoStore).toBe(true);
});
it("should be ok if legacy store in MigrationState `NOT_STARTED`", async () => {
await populateLegacyStore(0 /* MigrationState.NOT_STARTED*/);
const result = await StorageManager.checkConsistency();
expect(result.healthy).toBe(true);
expect(result.dataInCryptoStore).toBe(true);
});
it("should not be ok if MigrationState greater than `NOT_STARTED`", async () => {
await populateLegacyStore(1 /*INITIAL_DATA_MIGRATED*/);
const result = await StorageManager.checkConsistency();
expect(result.healthy).toBe(true);
expect(result.dataInCryptoStore).toBe(false);
});
it("should not be healthy if no indexeddb", async () => {
// eslint-disable-next-line no-global-assign
indexedDB = {} as IDBFactory;
const result = await StorageManager.checkConsistency();
expect(result.healthy).toBe(false);
// eslint-disable-next-line no-global-assign
indexedDB = new IDBFactory();
});
});
});
describe("with `feature_rust_crypto` disabled", () => {
beforeEach(() => {
jest.spyOn(SettingsStore, "getValue").mockImplementation(async (key) => {
if (key === "feature_rust_crypto") {
return false;
}
throw new Error(`Unknown key ${key}`);
});
});
it("should not be ok if sync store but no crypto store", async () => {
const result = await StorageManager.checkConsistency();
expect(result.healthy).toBe(true);
expect(result.dataInCryptoStore).toBe(false);
});
it("should not be ok if sync store but no crypto store and a rust store", async () => {
await createDB(RUST_CRYPTO_STORE_NAME);
const result = await StorageManager.checkConsistency();
expect(result.healthy).toBe(true);
expect(result.dataInCryptoStore).toBe(false);
});
it("should be healthy if sync store and a legacy crypto store", async () => {
await createDB(LEGACY_CRYPTO_STORE_NAME);
const result = await StorageManager.checkConsistency();
expect(result.healthy).toBe(true);
expect(result.dataInCryptoStore).toBe(true);
});
});
});
});

5
yarn.lock
View File

@ -5119,6 +5119,11 @@ extend@^3.0.0:
resolved "https://registry.yarnpkg.com/extend/-/extend-3.0.2.tgz#f8b1136b4071fbd8eb140aff858b1019ec2915fa"
integrity sha512-fjquC59cD7CyW6urNXK0FBufkZcoiGG80wTuPujX590cB5Ttln20E2UB4S/WARVqhXffZl2LNgS+gQdPIIim/g==
fake-indexeddb@^5.0.2:
version "5.0.2"
resolved "https://registry.yarnpkg.com/fake-indexeddb/-/fake-indexeddb-5.0.2.tgz#8e0b6c75c6dc6639cbb50c1aa948772147d7c93e"
integrity sha512-cB507r5T3D55DfclY01GLkninZLfU7HXV/mhVRTnTRm5k2u+fY7Fof2dBkr80p5t7G7dlA/G5dI87QiMdPpMCQ==
fast-deep-equal@^3.1.1, fast-deep-equal@^3.1.3:
version "3.1.3"
resolved "https://registry.yarnpkg.com/fast-deep-equal/-/fast-deep-equal-3.1.3.tgz#3a7d56b559d6cbc3eb512325244e619a65c6c525"