From cb7fd5118e62dd1485b78eccdca5e36b172d34f5 Mon Sep 17 00:00:00 2001 From: Valere Date: Fri, 2 Feb 2024 13:20:13 +0100 Subject: [PATCH] SettingsStore: Change feature_rust_crypto to default true (#12203) * Set default crypto stack to rust * Update test for default to rust crypto * Fix labs settings tests * Fix test not working with rust crypto --- playwright/e2e/crypto/staged-rollout.spec.ts | 95 ++++++++++- playwright/element-web-test.ts | 5 +- src/settings/Settings.tsx | 2 +- test/MatrixClientPeg-test.ts | 155 +++++++++++------- .../components/structures/MatrixChat-test.tsx | 17 +- .../tabs/user/LabsUserSettingsTab-test.tsx | 19 +++ 6 files changed, 225 insertions(+), 68 deletions(-) diff --git a/playwright/e2e/crypto/staged-rollout.spec.ts b/playwright/e2e/crypto/staged-rollout.spec.ts index a735444d1e..4395b9c6b0 100644 --- a/playwright/e2e/crypto/staged-rollout.spec.ts +++ b/playwright/e2e/crypto/staged-rollout.spec.ts @@ -17,7 +17,7 @@ limitations under the License. import { test, expect } from "../../element-web-test"; import { logIntoElement } from "./utils"; -test.describe("Migration of existing logins", () => { +test.describe("Adoption of rust stack", () => { test("Test migration of existing logins when rollout is 100%", async ({ page, context, @@ -25,23 +25,36 @@ test.describe("Migration of existing logins", () => { credentials, homeserver, }, workerInfo) => { - test.skip(workerInfo.project.name === "Rust Crypto", "This test only works with Rust crypto."); + 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"); let featureRustCrypto = false; let stagedRolloutPercent = 0; await context.route(`http://localhost:8080/config.json*`, async (route) => { - const json = {}; + const json = { + default_server_config: { + "m.homeserver": { + base_url: "https://server.invalid", + }, + }, + }; json["features"] = { feature_rust_crypto: featureRustCrypto, }; json["setting_defaults"] = { + "language": "en-GB", "RustCrypto.staged_rollout_percent": stagedRolloutPercent, }; await route.fulfill({ json }); }); + // reload to ensure we read the config + await page.reload(); + await logIntoElement(page, homeserver, credentials); await app.settings.openUserSettings("Help & About"); @@ -61,4 +74,80 @@ test.describe("Migration of existing logins", () => { await app.settings.openUserSettings("Help & About"); await expect(page.getByText("Crypto version: Rust SDK")).toBeVisible(); }); + + test("Test new logins by default on rust stack", 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"); + + await context.route(`http://localhost:8080/config.json*`, async (route) => { + const json = { + default_server_config: { + "m.homeserver": { + base_url: "https://server.invalid", + }, + }, + }; + // we only want to test the default + json["features"] = {}; + json["setting_defaults"] = { + language: "en-GB", + }; + await route.fulfill({ json }); + }); + + // reload to get the new config + await page.reload(); + await logIntoElement(page, homeserver, credentials); + + await app.settings.openUserSettings("Help & About"); + await expect(page.getByText("Crypto version: Rust SDK")).toBeVisible(); + }); + + test("Test default is to not rollout existing logins", 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(); + + // Now simulate a refresh with `feature_rust_crypto` enabled but ensure we use the default rollout + await context.route(`http://localhost:8080/config.json*`, async (route) => { + const json = {}; + json["features"] = { + feature_rust_crypto: true, + }; + json["setting_defaults"] = { + // We want to test the default so we don't set this + // "RustCrypto.staged_rollout_percent": 0, + }; + await route.fulfill({ json }); + }); + + await page.reload(); + + await app.settings.openUserSettings("Help & About"); + await expect(page.getByText("Crypto version: Olm")).toBeVisible(); + }); }); diff --git a/playwright/element-web-test.ts b/playwright/element-web-test.ts index aceaa2b51b..4e7d3a5b65 100644 --- a/playwright/element-web-test.ts +++ b/playwright/element-web-test.ts @@ -111,8 +111,9 @@ export const test = base.extend< return obj; }, {}), }; - if (cryptoBackend === "rust") { - json.features.feature_rust_crypto = true; + // the default is to use rust now, so set to `false` if on legacy backend + if (cryptoBackend === "legacy") { + json.features.feature_rust_crypto = false; } await route.fulfill({ json }); }); diff --git a/src/settings/Settings.tsx b/src/settings/Settings.tsx index 3f7013c495..6ec3870ddc 100644 --- a/src/settings/Settings.tsx +++ b/src/settings/Settings.tsx @@ -501,7 +501,7 @@ export const SETTINGS: { [setting: string]: ISetting } = { } }, shouldWarn: true, - default: false, + default: true, controller: new RustCryptoSdkController(), }, // Must be set under `setting_defaults` in config.json. diff --git a/test/MatrixClientPeg-test.ts b/test/MatrixClientPeg-test.ts index 1f937ba4f8..b94ec7cb01 100644 --- a/test/MatrixClientPeg-test.ts +++ b/test/MatrixClientPeg-test.ts @@ -24,6 +24,7 @@ import SettingsStore from "../src/settings/SettingsStore"; import Modal from "../src/Modal"; import PlatformPeg from "../src/PlatformPeg"; import { SettingLevel } from "../src/settings/SettingLevel"; +import { Features } from "../src/settings/Settings"; jest.useFakeTimers(); @@ -91,45 +92,74 @@ describe("MatrixClientPeg", () => { }); }); - it("should initialise client crypto", async () => { - const mockInitCrypto = jest.spyOn(testPeg.safeGet(), "initCrypto").mockResolvedValue(undefined); - const mockSetTrustCrossSignedDevices = jest - .spyOn(testPeg.safeGet(), "setCryptoTrustCrossSignedDevices") - .mockImplementation(() => {}); - const mockStartClient = jest.spyOn(testPeg.safeGet(), "startClient").mockResolvedValue(undefined); + describe("legacy crypto", () => { + beforeEach(() => { + const originalGetValue = SettingsStore.getValue; + jest.spyOn(SettingsStore, "getValue").mockImplementation( + (settingName: string, roomId: string | null = null, excludeDefault = false) => { + if (settingName === "feature_rust_crypto") { + return false; + } + return originalGetValue(settingName, roomId, excludeDefault); + }, + ); + }); - await testPeg.start(); - expect(mockInitCrypto).toHaveBeenCalledTimes(1); - expect(mockSetTrustCrossSignedDevices).toHaveBeenCalledTimes(1); - expect(mockStartClient).toHaveBeenCalledTimes(1); + it("should initialise client crypto", async () => { + const mockInitCrypto = jest.spyOn(testPeg.safeGet(), "initCrypto").mockResolvedValue(undefined); + const mockSetTrustCrossSignedDevices = jest + .spyOn(testPeg.safeGet(), "setCryptoTrustCrossSignedDevices") + .mockImplementation(() => {}); + const mockStartClient = jest.spyOn(testPeg.safeGet(), "startClient").mockResolvedValue(undefined); + + await testPeg.start(); + expect(mockInitCrypto).toHaveBeenCalledTimes(1); + expect(mockSetTrustCrossSignedDevices).toHaveBeenCalledTimes(1); + expect(mockStartClient).toHaveBeenCalledTimes(1); + }); + + it("should carry on regardless if there is an error initialising crypto", async () => { + const e2eError = new Error("nope nope nope"); + const mockInitCrypto = jest.spyOn(testPeg.safeGet(), "initCrypto").mockRejectedValue(e2eError); + const mockSetTrustCrossSignedDevices = jest + .spyOn(testPeg.safeGet(), "setCryptoTrustCrossSignedDevices") + .mockImplementation(() => {}); + const mockStartClient = jest.spyOn(testPeg.safeGet(), "startClient").mockResolvedValue(undefined); + const mockWarning = jest.spyOn(logger, "warn").mockReturnValue(undefined); + + await testPeg.start(); + expect(mockInitCrypto).toHaveBeenCalledTimes(1); + expect(mockSetTrustCrossSignedDevices).not.toHaveBeenCalled(); + expect(mockStartClient).toHaveBeenCalledTimes(1); + expect(mockWarning).toHaveBeenCalledWith(expect.stringMatching("Unable to initialise e2e"), e2eError); + }); + + it("should reload when store database closes for a guest user", async () => { + testPeg.safeGet().isGuest = () => true; + const emitter = new EventEmitter(); + testPeg.safeGet().store.on = emitter.on.bind(emitter); + const platform: any = { reload: jest.fn() }; + PlatformPeg.set(platform); + await testPeg.assign(); + emitter.emit("closed" as any); + expect(platform.reload).toHaveBeenCalled(); + }); + + it("should show error modal when store database closes", async () => { + testPeg.safeGet().isGuest = () => false; + const emitter = new EventEmitter(); + const platform: any = { getHumanReadableName: jest.fn() }; + PlatformPeg.set(platform); + testPeg.safeGet().store.on = emitter.on.bind(emitter); + const spy = jest.spyOn(Modal, "createDialog"); + await testPeg.assign(); + emitter.emit("closed" as any); + expect(spy).toHaveBeenCalled(); + }); }); - it("should carry on regardless if there is an error initialising crypto", async () => { - const e2eError = new Error("nope nope nope"); - const mockInitCrypto = jest.spyOn(testPeg.safeGet(), "initCrypto").mockRejectedValue(e2eError); - const mockSetTrustCrossSignedDevices = jest - .spyOn(testPeg.safeGet(), "setCryptoTrustCrossSignedDevices") - .mockImplementation(() => {}); - const mockStartClient = jest.spyOn(testPeg.safeGet(), "startClient").mockResolvedValue(undefined); - const mockWarning = jest.spyOn(logger, "warn").mockReturnValue(undefined); - - await testPeg.start(); - expect(mockInitCrypto).toHaveBeenCalledTimes(1); - expect(mockSetTrustCrossSignedDevices).not.toHaveBeenCalled(); - expect(mockStartClient).toHaveBeenCalledTimes(1); - expect(mockWarning).toHaveBeenCalledWith(expect.stringMatching("Unable to initialise e2e"), e2eError); - }); - - it("should initialise the rust crypto library, if enabled", async () => { - const originalGetValue = SettingsStore.getValue; - jest.spyOn(SettingsStore, "getValue").mockImplementation( - (settingName: string, roomId: string | null = null, excludeDefault = false) => { - if (settingName === "feature_rust_crypto") { - return true; - } - return originalGetValue(settingName, roomId, excludeDefault); - }, - ); + it("should initialise the rust crypto library by default", async () => { + await SettingsStore.setValue(Features.RustCrypto, null, SettingLevel.DEVICE, null); const mockSetValue = jest.spyOn(SettingsStore, "setValue").mockResolvedValue(undefined); @@ -144,6 +174,32 @@ describe("MatrixClientPeg", () => { expect(mockSetValue).toHaveBeenCalledWith("feature_rust_crypto", null, SettingLevel.DEVICE, true); }); + it("should initialise the legacy crypto library if set", async () => { + await SettingsStore.setValue(Features.RustCrypto, null, SettingLevel.DEVICE, null); + + const originalGetValue = SettingsStore.getValue; + jest.spyOn(SettingsStore, "getValue").mockImplementation( + (settingName: string, roomId: string | null = null, excludeDefault = false) => { + if (settingName === "feature_rust_crypto") { + return false; + } + return originalGetValue(settingName, roomId, excludeDefault); + }, + ); + + const mockSetValue = jest.spyOn(SettingsStore, "setValue").mockResolvedValue(undefined); + + const mockInitCrypto = jest.spyOn(testPeg.safeGet(), "initCrypto").mockResolvedValue(undefined); + const mockInitRustCrypto = jest.spyOn(testPeg.safeGet(), "initRustCrypto").mockResolvedValue(undefined); + + await testPeg.start(); + expect(mockInitCrypto).toHaveBeenCalled(); + expect(mockInitRustCrypto).not.toHaveBeenCalledTimes(1); + + // we should have stashed the setting in the settings store + expect(mockSetValue).toHaveBeenCalledWith("feature_rust_crypto", null, SettingLevel.DEVICE, false); + }); + describe("Rust staged rollout", () => { function mockSettingStore( userIsUsingRust: boolean, @@ -178,10 +234,12 @@ describe("MatrixClientPeg", () => { let mockInitCrypto: jest.SpyInstance; let mockInitRustCrypto: jest.SpyInstance; - beforeEach(() => { + beforeEach(async () => { mockSetValue = jest.spyOn(SettingsStore, "setValue").mockResolvedValue(undefined); mockInitCrypto = jest.spyOn(testPeg.safeGet(), "initCrypto").mockResolvedValue(undefined); mockInitRustCrypto = jest.spyOn(testPeg.safeGet(), "initRustCrypto").mockResolvedValue(undefined); + + await SettingsStore.setValue(Features.RustCrypto, null, SettingLevel.DEVICE, null); }); it("Should not migrate existing login if rollout is 0", async () => { @@ -254,28 +312,5 @@ describe("MatrixClientPeg", () => { expect(mockSetValue).toHaveBeenCalledWith("feature_rust_crypto", null, SettingLevel.DEVICE, false); }); }); - - it("should reload when store database closes for a guest user", async () => { - testPeg.safeGet().isGuest = () => true; - const emitter = new EventEmitter(); - testPeg.safeGet().store.on = emitter.on.bind(emitter); - const platform: any = { reload: jest.fn() }; - PlatformPeg.set(platform); - await testPeg.assign(); - emitter.emit("closed" as any); - expect(platform.reload).toHaveBeenCalled(); - }); - - it("should show error modal when store database closes", async () => { - testPeg.safeGet().isGuest = () => false; - const emitter = new EventEmitter(); - const platform: any = { getHumanReadableName: jest.fn() }; - PlatformPeg.set(platform); - testPeg.safeGet().store.on = emitter.on.bind(emitter); - const spy = jest.spyOn(Modal, "createDialog"); - await testPeg.assign(); - emitter.emit("closed" as any); - expect(spy).toHaveBeenCalled(); - }); }); }); diff --git a/test/components/structures/MatrixChat-test.tsx b/test/components/structures/MatrixChat-test.tsx index 2d6cfa6bf0..797777767c 100644 --- a/test/components/structures/MatrixChat-test.tsx +++ b/test/components/structures/MatrixChat-test.tsx @@ -57,6 +57,7 @@ import * as Lifecycle from "../../../src/Lifecycle"; import { SSO_HOMESERVER_URL_KEY, SSO_ID_SERVER_URL_KEY } from "../../../src/BasePlatform"; import SettingsStore from "../../../src/settings/SettingsStore"; import { SettingLevel } from "../../../src/settings/SettingLevel"; +import { MatrixClientPeg as peg } from "../../../src/MatrixClientPeg"; jest.mock("matrix-js-sdk/src/oidc/authorize", () => ({ completeAuthorizationCodeGrant: jest.fn(), @@ -84,6 +85,7 @@ describe("", () => { getClientWellKnown: jest.fn().mockReturnValue({}), isVersionSupported: jest.fn().mockResolvedValue(false), isCryptoEnabled: jest.fn().mockReturnValue(false), + initRustCrypto: jest.fn(), getRoom: jest.fn(), getMediaHandler: jest.fn().mockReturnValue({ setVideoInput: jest.fn(), @@ -835,10 +837,22 @@ describe("", () => { }); it("should show the soft-logout page", async () => { + // XXX This test is strange, it was working with legacy crypto + // without mocking the following but the initCrypto call was failing + // but as the exception was swallowed, the test was passing (see in `initClientCrypto`). + // There are several uses of the peg in the app, so during all these tests you might end-up + // with a real client instead of the mocked one. Not sure how reliable all these tests are. + const originalReplace = peg.replaceUsingCreds; + peg.replaceUsingCreds = jest.fn().mockResolvedValue(mockClient); + // @ts-ignore - need to mock this for the test + peg.matrixClient = mockClient; + const result = getComponent(); await result.findByText("You're signed out"); expect(result.container).toMatchSnapshot(); + + peg.replaceUsingCreds = originalReplace; }); }); @@ -1336,7 +1350,6 @@ describe("", () => { await populateStorageForSession(); const client = new MockClientWithEventEmitter({ - initCrypto: jest.fn(), ...getMockClientMethods(), }) as unknown as Mocked; jest.spyOn(MatrixJs, "createClient").mockReturnValue(client); @@ -1344,7 +1357,7 @@ describe("", () => { // intercept initCrypto and have it block until we complete the deferred const initCryptoCompleteDefer = defer(); const initCryptoCalled = new Promise((resolve) => { - client.initCrypto.mockImplementation(() => { + client.initRustCrypto.mockImplementation(() => { resolve(); return initCryptoCompleteDefer.promise; }); diff --git a/test/components/views/settings/tabs/user/LabsUserSettingsTab-test.tsx b/test/components/views/settings/tabs/user/LabsUserSettingsTab-test.tsx index 83f489f411..3239c0f875 100644 --- a/test/components/views/settings/tabs/user/LabsUserSettingsTab-test.tsx +++ b/test/components/views/settings/tabs/user/LabsUserSettingsTab-test.tsx @@ -71,6 +71,25 @@ describe("", () => { }); describe("Not enabled in config", () => { + // these tests only works if the feature is not enabled in the config by default? + const copyOfGetValueAt = SettingsStore.getValueAt; + + beforeEach(() => { + SettingsStore.getValueAt = ( + level: SettingLevel, + name: string, + roomId?: string, + isExplicit?: boolean, + ) => { + if (level == SettingLevel.CONFIG && name === "feature_rust_crypto") return false; + return copyOfGetValueAt(level, name, roomId, isExplicit); + }; + }); + + afterEach(() => { + SettingsStore.getValueAt = copyOfGetValueAt; + }); + it("can be turned on if not already", async () => { // By the time the settings panel is shown, `MatrixClientPeg.initClientCrypto` has saved the current // value to the settings store.