From b330de5d6e64798dafd7915cc440d956c9b55a22 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 11 Dec 2024 13:10:27 +0000 Subject: [PATCH] Factor out crypto setup process into a store (#28675) * Factor out crypto setup process into a store To make components pure and avoid react 18 dev mode problems due to components making requests when mounted. * fix test * test for the store * Add comment --- src/@types/global.d.ts | 2 + src/components/structures/MatrixChat.tsx | 16 +- src/components/structures/auth/E2eSetup.tsx | 11 +- .../security/InitialCryptoSetupDialog.tsx | 54 ++----- src/stores/InitialCryptoSetupStore.ts | 140 ++++++++++++++++++ src/stores/SetupEncryptionStore.ts | 5 + .../InitialCryptoSetupDialog-test.tsx | 114 +++----------- .../stores/InitialCryptoSetupStore-test.ts | 85 +++++++++++ 8 files changed, 274 insertions(+), 153 deletions(-) create mode 100644 src/stores/InitialCryptoSetupStore.ts create mode 100644 test/unit-tests/stores/InitialCryptoSetupStore-test.ts diff --git a/src/@types/global.d.ts b/src/@types/global.d.ts index be36c5b689f..f921cd291f3 100644 --- a/src/@types/global.d.ts +++ b/src/@types/global.d.ts @@ -44,6 +44,7 @@ import { IConfigOptions } from "../IConfigOptions"; import { MatrixDispatcher } from "../dispatcher/dispatcher"; import { DeepReadonly } from "./common"; import MatrixChat from "../components/structures/MatrixChat"; +import { InitialCryptoSetupStore } from "../stores/InitialCryptoSetupStore"; /* eslint-disable @typescript-eslint/naming-convention */ @@ -117,6 +118,7 @@ declare global { mxPerformanceEntryNames: any; mxUIStore: UIStore; mxSetupEncryptionStore?: SetupEncryptionStore; + mxInitialCryptoStore?: InitialCryptoSetupStore; mxRoomScrollStateStore?: RoomScrollStateStore; mxActiveWidgetStore?: ActiveWidgetStore; mxOnRecaptchaLoaded?: () => void; diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index 548dbff983c..ee120c430ae 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -132,6 +132,7 @@ import { SessionLockStolenView } from "./auth/SessionLockStolenView"; import { ConfirmSessionLockTheftView } from "./auth/ConfirmSessionLockTheftView"; import { LoginSplashView } from "./auth/LoginSplashView"; import { cleanUpDraftsIfRequired } from "../../DraftCleaner"; +import { InitialCryptoSetupStore } from "../../stores/InitialCryptoSetupStore"; // legacy export export { default as Views } from "../../Views"; @@ -428,6 +429,12 @@ export default class MatrixChat extends React.PureComponent { !(await shouldSkipSetupEncryption(cli)) ) { // if cross-signing is not yet set up, do so now if possible. + InitialCryptoSetupStore.sharedInstance().startInitialCryptoSetup( + cli, + Boolean(this.tokenLogin), + this.stores, + this.onCompleteSecurityE2eSetupFinished, + ); this.setStateForNewView({ view: Views.E2E_SETUP }); } else { this.onLoggedIn(); @@ -2073,14 +2080,7 @@ export default class MatrixChat extends React.PureComponent { } else if (this.state.view === Views.COMPLETE_SECURITY) { view = ; } else if (this.state.view === Views.E2E_SETUP) { - view = ( - - ); + view = ; } else if (this.state.view === Views.LOGGED_IN) { // `ready` and `view==LOGGED_IN` may be set before `page_type` (because the // latter is set via the dispatcher). If we don't yet have a `page_type`, diff --git a/src/components/structures/auth/E2eSetup.tsx b/src/components/structures/auth/E2eSetup.tsx index 265905db107..3b064d61346 100644 --- a/src/components/structures/auth/E2eSetup.tsx +++ b/src/components/structures/auth/E2eSetup.tsx @@ -7,17 +7,13 @@ Please see LICENSE files in the repository root for full details. */ import React from "react"; -import { MatrixClient } from "matrix-js-sdk/src/matrix"; import AuthPage from "../../views/auth/AuthPage"; import CompleteSecurityBody from "../../views/auth/CompleteSecurityBody"; import { InitialCryptoSetupDialog } from "../../views/dialogs/security/InitialCryptoSetupDialog"; interface IProps { - matrixClient: MatrixClient; onFinished: () => void; - accountPassword?: string; - tokenLogin: boolean; } export default class E2eSetup extends React.Component { @@ -25,12 +21,7 @@ export default class E2eSetup extends React.Component { return ( - + ); diff --git a/src/components/views/dialogs/security/InitialCryptoSetupDialog.tsx b/src/components/views/dialogs/security/InitialCryptoSetupDialog.tsx index 4ee69f17a48..22635662ce5 100644 --- a/src/components/views/dialogs/security/InitialCryptoSetupDialog.tsx +++ b/src/components/views/dialogs/security/InitialCryptoSetupDialog.tsx @@ -7,20 +7,15 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only Please see LICENSE files in the repository root for full details. */ -import React, { useCallback, useEffect, useState } from "react"; -import { logger } from "matrix-js-sdk/src/logger"; -import { MatrixClient } from "matrix-js-sdk/src/matrix"; +import React, { useCallback } from "react"; import { _t } from "../../../../languageHandler"; import DialogButtons from "../../elements/DialogButtons"; import BaseDialog from "../BaseDialog"; import Spinner from "../../elements/Spinner"; -import { createCrossSigning } from "../../../../CreateCrossSigning"; +import { InitialCryptoSetupStore, useInitialCryptoSetupStatus } from "../../../../stores/InitialCryptoSetupStore"; interface Props { - matrixClient: MatrixClient; - accountPassword?: string; - tokenLogin: boolean; onFinished: (success?: boolean) => void; } @@ -29,54 +24,27 @@ interface Props { * In most cases, only a spinner is shown, but for more * complex auth like SSO, the user may need to complete some steps to proceed. */ -export const InitialCryptoSetupDialog: React.FC = ({ - matrixClient, - accountPassword, - tokenLogin, - onFinished, -}) => { - const [error, setError] = useState(false); +export const InitialCryptoSetupDialog: React.FC = ({ onFinished }) => { + const onRetryClick = useCallback(() => { + InitialCryptoSetupStore.sharedInstance().retry(); + }, []); - const doSetup = useCallback(async () => { - const cryptoApi = matrixClient.getCrypto(); - if (!cryptoApi) return; - - setError(false); - - try { - await createCrossSigning(matrixClient, tokenLogin, accountPassword); - - onFinished(true); - } catch (e) { - if (tokenLogin) { - // ignore any failures, we are relying on grace period here - onFinished(false); - return; - } - - setError(true); - logger.error("Error bootstrapping cross-signing", e); - } - }, [matrixClient, tokenLogin, accountPassword, onFinished]); - - const onCancel = useCallback(() => { + const onCancelClick = useCallback(() => { onFinished(false); }, [onFinished]); - useEffect(() => { - doSetup(); - }, [doSetup]); + const status = useInitialCryptoSetupStatus(InitialCryptoSetupStore.sharedInstance()); let content; - if (error) { + if (status === "error") { content = (

{_t("encryption|unable_to_setup_keys_error")}

diff --git a/src/stores/InitialCryptoSetupStore.ts b/src/stores/InitialCryptoSetupStore.ts new file mode 100644 index 00000000000..0c2e49f5caf --- /dev/null +++ b/src/stores/InitialCryptoSetupStore.ts @@ -0,0 +1,140 @@ +/* +Copyright 2024 New Vector Ltd. + +SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only +Please see LICENSE files in the repository root for full details. +*/ + +import EventEmitter from "events"; +import { MatrixClient } from "matrix-js-sdk/src/matrix"; +import { logger } from "matrix-js-sdk/src/logger"; +import { useEffect, useState } from "react"; + +import { createCrossSigning } from "../CreateCrossSigning"; +import { SdkContextClass } from "../contexts/SDKContext"; + +type Status = "in_progress" | "complete" | "error" | undefined; + +export const useInitialCryptoSetupStatus = (store: InitialCryptoSetupStore): Status => { + const [status, setStatus] = useState(store.getStatus()); + + useEffect(() => { + const update = (): void => { + setStatus(store.getStatus()); + }; + + store.on("update", update); + + return () => { + store.off("update", update); + }; + }, [store]); + + return status; +}; + +/** + * Logic for setting up crypto state that's done immediately after + * a user registers. Should be transparent to the user, not requiring + * interaction in most cases. + * As distinct from SetupEncryptionStore which is for setting up + * 4S or verifying the device, will always require interaction + * from the user in some form. + */ +export class InitialCryptoSetupStore extends EventEmitter { + private status: Status = undefined; + + private client?: MatrixClient; + private isTokenLogin?: boolean; + private stores?: SdkContextClass; + private onFinished?: (success: boolean) => void; + + public static sharedInstance(): InitialCryptoSetupStore { + if (!window.mxInitialCryptoStore) window.mxInitialCryptoStore = new InitialCryptoSetupStore(); + return window.mxInitialCryptoStore; + } + + public getStatus(): Status { + return this.status; + } + + /** + * Start the initial crypto setup process. + * + * @param {MatrixClient} client The client to use for the setup + * @param {boolean} isTokenLogin True if the user logged in via a token login, otherwise false + * @param {SdkContextClass} stores The stores to use for the setup + */ + public startInitialCryptoSetup( + client: MatrixClient, + isTokenLogin: boolean, + stores: SdkContextClass, + onFinished: (success: boolean) => void, + ): void { + this.client = client; + this.isTokenLogin = isTokenLogin; + this.stores = stores; + this.onFinished = onFinished; + + // We just start this process: it's progress is tracked by the events rather + // than returning a promise, so we don't bother. + this.doSetup().catch(() => logger.error("Initial crypto setup failed")); + } + + /** + * Retry the initial crypto setup process. + * + * If no crypto setup is currently in process, this will return false. + * + * @returns {boolean} True if a retry was initiated, otherwise false + */ + public retry(): boolean { + if (this.client === undefined || this.isTokenLogin === undefined || this.stores == undefined) return false; + + this.doSetup().catch(() => logger.error("Initial crypto setup failed")); + + return true; + } + + private reset(): void { + this.client = undefined; + this.isTokenLogin = undefined; + this.stores = undefined; + } + + private async doSetup(): Promise { + if (this.client === undefined || this.isTokenLogin === undefined || this.stores == undefined) { + throw new Error("No setup is in progress"); + } + + const cryptoApi = this.client.getCrypto(); + if (!cryptoApi) throw new Error("No crypto module found!"); + + this.status = "in_progress"; + this.emit("update"); + + try { + await createCrossSigning(this.client, this.isTokenLogin, this.stores.accountPasswordStore.getPassword()); + + this.reset(); + + this.status = "complete"; + this.emit("update"); + this.onFinished?.(true); + } catch (e) { + if (this.isTokenLogin) { + // ignore any failures, we are relying on grace period here + this.reset(); + + this.status = "complete"; + this.emit("update"); + this.onFinished?.(true); + + return; + } + logger.error("Error bootstrapping cross-signing", e); + this.status = "error"; + this.emit("update"); + } + } +} diff --git a/src/stores/SetupEncryptionStore.ts b/src/stores/SetupEncryptionStore.ts index 70c721b1ca3..a13ba26f722 100644 --- a/src/stores/SetupEncryptionStore.ts +++ b/src/stores/SetupEncryptionStore.ts @@ -33,6 +33,11 @@ export enum Phase { ConfirmReset = 6, } +/** + * Logic for setting up 4S and/or verifying the user's device: a process requiring + * ongoing interaction with the user, as distinct from InitialCryptoSetupStore which + * a (usually) non-interactive process that happens immediately after registration. + */ export class SetupEncryptionStore extends EventEmitter { private started?: boolean; public phase?: Phase; diff --git a/test/components/views/dialogs/security/InitialCryptoSetupDialog-test.tsx b/test/components/views/dialogs/security/InitialCryptoSetupDialog-test.tsx index 4d3d495a38d..a589b55289b 100644 --- a/test/components/views/dialogs/security/InitialCryptoSetupDialog-test.tsx +++ b/test/components/views/dialogs/security/InitialCryptoSetupDialog-test.tsx @@ -7,31 +7,22 @@ Please see LICENSE files in the repository root for full details. */ import React from "react"; -import { render, screen, waitFor } from "jest-matrix-react"; -import { mocked } from "jest-mock"; -import { MatrixClient } from "matrix-js-sdk/src/matrix"; +import { render, screen } from "jest-matrix-react"; +import userEvent from "@testing-library/user-event"; -import { createCrossSigning } from "../../../../../src/CreateCrossSigning"; import { InitialCryptoSetupDialog } from "../../../../../src/components/views/dialogs/security/InitialCryptoSetupDialog"; -import { createTestClient } from "../../../../test-utils"; - -jest.mock("../../../../../src/CreateCrossSigning", () => ({ - createCrossSigning: jest.fn(), -})); +import { InitialCryptoSetupStore } from "../../../../../src/stores/InitialCryptoSetupStore"; describe("InitialCryptoSetupDialog", () => { - let client: MatrixClient; - let createCrossSigningResolve: () => void; - let createCrossSigningReject: (e: Error) => void; + const storeMock = { + getStatus: jest.fn(), + retry: jest.fn(), + on: jest.fn(), + off: jest.fn(), + }; beforeEach(() => { - client = createTestClient(); - mocked(createCrossSigning).mockImplementation(() => { - return new Promise((resolve, reject) => { - createCrossSigningResolve = resolve; - createCrossSigningReject = reject; - }); - }); + jest.spyOn(InitialCryptoSetupStore, "sharedInstance").mockReturnValue(storeMock as any); }); afterEach(() => { @@ -39,93 +30,32 @@ describe("InitialCryptoSetupDialog", () => { jest.restoreAllMocks(); }); - it("should call createCrossSigning and show a spinner while it runs", async () => { + it("should show a spinner while the setup is in progress", async () => { const onFinished = jest.fn(); - render( - , - ); - - expect(createCrossSigning).toHaveBeenCalledWith(client, false, "hunter2"); - expect(screen.getByTestId("spinner")).toBeInTheDocument(); + storeMock.getStatus.mockReturnValue("in_progress"); - createCrossSigningResolve!(); + render(); - await waitFor(() => expect(onFinished).toHaveBeenCalledWith(true)); + expect(screen.getByTestId("spinner")).toBeInTheDocument(); }); - it("should display an error if createCrossSigning fails", async () => { - render( - , - ); + it("should display an error if setup has failed", async () => { + storeMock.getStatus.mockReturnValue("error"); - createCrossSigningReject!(new Error("generic error message")); + render(); await expect(await screen.findByRole("button", { name: "Retry" })).toBeInTheDocument(); }); - it("ignores failures when tokenLogin is true", async () => { - const onFinished = jest.fn(); - - render( - , - ); - - createCrossSigningReject!(new Error("generic error message")); - - await waitFor(() => expect(onFinished).toHaveBeenCalledWith(false)); - }); - - it("cancels the dialog when the cancel button is clicked", async () => { + it("calls retry when retry button pressed", async () => { const onFinished = jest.fn(); + storeMock.getStatus.mockReturnValue("error"); - render( - , - ); - - createCrossSigningReject!(new Error("generic error message")); - - const cancelButton = await screen.findByRole("button", { name: "Cancel" }); - cancelButton.click(); - - expect(onFinished).toHaveBeenCalledWith(false); - }); - - it("should retry when the retry button is clicked", async () => { - render( - , - ); - - createCrossSigningReject!(new Error("generic error message")); + render(); - const retryButton = await screen.findByRole("button", { name: "Retry" }); - retryButton.click(); + await userEvent.click(await screen.findByRole("button", { name: "Retry" })); - expect(createCrossSigning).toHaveBeenCalledTimes(2); + expect(storeMock.retry).toHaveBeenCalled(); }); }); diff --git a/test/unit-tests/stores/InitialCryptoSetupStore-test.ts b/test/unit-tests/stores/InitialCryptoSetupStore-test.ts new file mode 100644 index 00000000000..64b81bade22 --- /dev/null +++ b/test/unit-tests/stores/InitialCryptoSetupStore-test.ts @@ -0,0 +1,85 @@ +/* +Copyright 2024 New Vector Ltd. + +SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only +Please see LICENSE files in the repository root for full details. +*/ + +import { mocked } from "jest-mock"; +import { MatrixClient } from "matrix-js-sdk/src/matrix"; +import { waitFor } from "jest-matrix-react"; + +import { createCrossSigning } from "../../../src/CreateCrossSigning"; +import { InitialCryptoSetupStore } from "../../../src/stores/InitialCryptoSetupStore"; +import { SdkContextClass } from "../../../src/contexts/SDKContext"; +import { createTestClient } from "../../test-utils"; +import { AccountPasswordStore } from "../../../src/stores/AccountPasswordStore"; + +jest.mock("../../../src/CreateCrossSigning", () => ({ + createCrossSigning: jest.fn(), +})); + +describe("InitialCryptoSetupStore", () => { + let testStore: InitialCryptoSetupStore; + let client: MatrixClient; + let stores: SdkContextClass; + + let createCrossSigningResolve: () => void; + let createCrossSigningReject: (e: Error) => void; + + beforeEach(() => { + testStore = new InitialCryptoSetupStore(); + client = createTestClient(); + stores = { + accountPasswordStore: { + getPassword: jest.fn(), + } as unknown as AccountPasswordStore, + } as unknown as SdkContextClass; + + mocked(createCrossSigning).mockImplementation(() => { + return new Promise((resolve, reject) => { + createCrossSigningResolve = resolve; + createCrossSigningReject = reject; + }); + }); + }); + + it("should call createCrossSigning when startInitialCryptoSetup is called", async () => { + testStore.startInitialCryptoSetup(client, false, stores, jest.fn()); + + await waitFor(() => expect(createCrossSigning).toHaveBeenCalled()); + }); + + it("emits an update event when createCrossSigning resolves", async () => { + const updateSpy = jest.fn(); + testStore.on("update", updateSpy); + + testStore.startInitialCryptoSetup(client, false, stores, jest.fn()); + createCrossSigningResolve(); + + await waitFor(() => expect(updateSpy).toHaveBeenCalled()); + expect(testStore.getStatus()).toBe("complete"); + }); + + it("emits an update event when createCrossSigning rejects", async () => { + const updateSpy = jest.fn(); + testStore.on("update", updateSpy); + + testStore.startInitialCryptoSetup(client, false, stores, jest.fn()); + createCrossSigningReject(new Error("Test error")); + + await waitFor(() => expect(updateSpy).toHaveBeenCalled()); + expect(testStore.getStatus()).toBe("error"); + }); + + it("should ignore failures if tokenLogin is true", async () => { + const updateSpy = jest.fn(); + testStore.on("update", updateSpy); + + testStore.startInitialCryptoSetup(client, true, stores, jest.fn()); + createCrossSigningReject(new Error("Test error")); + + await waitFor(() => expect(updateSpy).toHaveBeenCalled()); + expect(testStore.getStatus()).toBe("complete"); + }); +});