Refactor CreateCrossSigningDialog (#28218)

* Refactor CreateCrossSigningDialog * Converts CreateCrossSigningDialog to a functional component * Pulls logic out to its own class * Updates usage of deprecated cross signing bootstrap method on client to be on the crypto object and updates test to match Moved from https://github.com/element-hq/matrix-react-sdk/pull/131 * Add mock here too * Use the right mock * Remove duplicate mock * Stray jest mock line * Un-move mocks * tsdoc * Typo Co-authored-by: Andy Balaam <andy.balaam@matrix.org> --------- Co-authored-by: Andy Balaam <andy.balaam@matrix.org>
2024-11-21 08:45:40 +03:00 · 2024-10-22 12:42:07 +01:00 · 2024-10-22 12:42:07 +01:00 · 19ef3267c0
commit 19ef3267c0
parent 539025cf8c
8 changed files with 406 additions and 157 deletions
--- a/src/CreateCrossSigning.ts
+++ b/src/CreateCrossSigning.ts
@ -0,0 +1,118 @@
+/*
+Copyright 2024 New Vector Ltd.
+Copyright 2019, 2020 The Matrix.org Foundation C.I.C.
+Copyright 2018, 2019 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 { logger } from "matrix-js-sdk/src/logger";
+import { AuthDict, CrossSigningKeys, MatrixClient, MatrixError, UIAFlow, UIAResponse } from "matrix-js-sdk/src/matrix";
+
+import { SSOAuthEntry } from "./components/views/auth/InteractiveAuthEntryComponents";
+import Modal from "./Modal";
+import { _t } from "./languageHandler";
+import InteractiveAuthDialog from "./components/views/dialogs/InteractiveAuthDialog";
+
+/**
+ * Determine if the homeserver allows uploading device keys with only password auth.
+ * @param cli The Matrix Client to use
+ * @returns True if the homeserver allows uploading device keys with only password auth, otherwise false
+ */
+async function canUploadKeysWithPasswordOnly(cli: MatrixClient): Promise<boolean> {
+    try {
+        await cli.uploadDeviceSigningKeys(undefined, {} as CrossSigningKeys);
+        // We should never get here: the server should always require
+        // UI auth to upload device signing keys. If we do, we upload
+        // no keys which would be a no-op.
+        logger.log("uploadDeviceSigningKeys unexpectedly succeeded without UI auth!");
+        return false;
+    } catch (error) {
+        if (!(error instanceof MatrixError) || !error.data || !error.data.flows) {
+            logger.log("uploadDeviceSigningKeys advertised no flows!");
+            return false;
+        }
+        const canUploadKeysWithPasswordOnly = error.data.flows.some((f: UIAFlow) => {
+            return f.stages.length === 1 && f.stages[0] === "m.login.password";
+        });
+        return canUploadKeysWithPasswordOnly;
+    }
+}
+
+/**
+ * Ensures that cross signing keys are created and uploaded for the user.
+ * The homeserver may require user-interactive auth to upload the keys, in
+ * which case the user will be prompted to authenticate. If the homeserver
+ * allows uploading keys with just an account password and one is provided,
+ * the keys will be uploaded without user interaction.
+ *
+ * This function does not set up backups of the created cross-signing keys
+ * (or message keys): the cross-signing keys are stored locally and will be
+ * lost requiring a crypto reset, if the user logs out or loses their session.
+ *
+ * @param cli The Matrix Client to use
+ * @param isTokenLogin True if the user logged in via a token login, otherwise false
+ * @param accountPassword The password that the user logged in with
+ */
+export async function createCrossSigning(
+    cli: MatrixClient,
+    isTokenLogin: boolean,
+    accountPassword?: string,
+): Promise<void> {
+    const cryptoApi = cli.getCrypto();
+    if (!cryptoApi) {
+        throw new Error("No crypto API found!");
+    }
+
+    const doBootstrapUIAuth = async (
+        makeRequest: (authData: AuthDict) => Promise<UIAResponse<void>>,
+    ): Promise<void> => {
+        if (accountPassword && (await canUploadKeysWithPasswordOnly(cli))) {
+            await makeRequest({
+                type: "m.login.password",
+                identifier: {
+                    type: "m.id.user",
+                    user: cli.getUserId(),
+                },
+                password: accountPassword,
+            });
+        } else if (isTokenLogin) {
+            // We are hoping the grace period is active
+            await makeRequest({});
+        } else {
+            const dialogAesthetics = {
+                [SSOAuthEntry.PHASE_PREAUTH]: {
+                    title: _t("auth|uia|sso_title"),
+                    body: _t("auth|uia|sso_preauth_body"),
+                    continueText: _t("auth|sso"),
+                    continueKind: "primary",
+                },
+                [SSOAuthEntry.PHASE_POSTAUTH]: {
+                    title: _t("encryption|confirm_encryption_setup_title"),
+                    body: _t("encryption|confirm_encryption_setup_body"),
+                    continueText: _t("action|confirm"),
+                    continueKind: "primary",
+                },
+            };
+
+            const { finished } = Modal.createDialog(InteractiveAuthDialog, {
+                title: _t("encryption|bootstrap_title"),
+                matrixClient: cli,
+                makeRequest,
+                aestheticsForStagePhases: {
+                    [SSOAuthEntry.LOGIN_TYPE]: dialogAesthetics,
+                    [SSOAuthEntry.UNSTABLE_LOGIN_TYPE]: dialogAesthetics,
+                },
+            });
+            const [confirmed] = await finished;
+            if (!confirmed) {
+                throw new Error("Cross-signing key upload auth canceled");
+            }
+        }
+    };
+
+    await cryptoApi.bootstrapCrossSigning({
+        authUploadDeviceSigningKeys: doBootstrapUIAuth,
+    });
+}
--- a/src/components/structures/MatrixChat.tsx
+++ b/src/components/structures/MatrixChat.tsx
@ -2088,6 +2088,7 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
        } else if (this.state.view === Views.E2E_SETUP) {
            view = (
                <E2eSetup
+                    matrixClient={MatrixClientPeg.safeGet()}
                    onFinished={this.onCompleteSecurityE2eSetupFinished}
                    accountPassword={this.stores.accountPasswordStore.getPassword()}
                    tokenLogin={!!this.tokenLogin}
--- a/src/components/structures/auth/E2eSetup.tsx
+++ b/src/components/structures/auth/E2eSetup.tsx
@ -7,15 +7,17 @@ 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 CreateCrossSigningDialog from "../../views/dialogs/security/CreateCrossSigningDialog";

 interface IProps {
+    matrixClient: MatrixClient;
    onFinished: () => void;
    accountPassword?: string;
-    tokenLogin?: boolean;
+    tokenLogin: boolean;
 }

 export default class E2eSetup extends React.Component<IProps> {
@ -24,6 +26,7 @@ export default class E2eSetup extends React.Component<IProps> {
            <AuthPage>
                <CompleteSecurityBody>
                    <CreateCrossSigningDialog
+                        matrixClient={this.props.matrixClient}
                        onFinished={this.props.onFinished}
                        accountPassword={this.props.accountPassword}
                        tokenLogin={this.props.tokenLogin}
--- a/src/components/views/dialogs/security/CreateCrossSigningDialog.tsx
+++ b/src/components/views/dialogs/security/CreateCrossSigningDialog.tsx
@ -7,189 +7,93 @@ 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 from "react";
-import { CrossSigningKeys, AuthDict, MatrixError, UIAFlow, UIAResponse } from "matrix-js-sdk/src/matrix";
+import React, { useCallback, useEffect, useState } from "react";
 import { logger } from "matrix-js-sdk/src/logger";
+import { MatrixClient } from "matrix-js-sdk/src/matrix";

-import { MatrixClientPeg } from "../../../../MatrixClientPeg";
 import { _t } from "../../../../languageHandler";
-import Modal from "../../../../Modal";
-import { SSOAuthEntry } from "../../auth/InteractiveAuthEntryComponents";
 import DialogButtons from "../../elements/DialogButtons";
 import BaseDialog from "../BaseDialog";
 import Spinner from "../../elements/Spinner";
-import InteractiveAuthDialog from "../InteractiveAuthDialog";
+import { createCrossSigning } from "../../../../CreateCrossSigning";

-interface IProps {
+interface Props {
+    matrixClient: MatrixClient;
    accountPassword?: string;
-    tokenLogin?: boolean;
+    tokenLogin: boolean;
    onFinished: (success?: boolean) => void;
 }

-interface IState {
-    error: boolean;
-    canUploadKeysWithPasswordOnly: boolean | null;
-    accountPassword: string;
-}
-
 /*
 * Walks the user through the process of creating a cross-signing keys. 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 default class CreateCrossSigningDialog extends React.PureComponent<IProps, IState> {
-    public constructor(props: IProps) {
-        super(props);
+const CreateCrossSigningDialog: React.FC<Props> = ({ matrixClient, accountPassword, tokenLogin, onFinished }) => {
+    const [error, setError] = useState(false);

-        this.state = {
-            error: false,
-            // Does the server offer a UI auth flow with just m.login.password
-            // for /keys/device_signing/upload?
-            // If we have an account password in memory, let's simplify and
-            // assume it means password auth is also supported for device
-            // signing key upload as well. This avoids hitting the server to
-            // test auth flows, which may be slow under high load.
-            canUploadKeysWithPasswordOnly: props.accountPassword ? true : null,
-            accountPassword: props.accountPassword || "",
-        };
+    const bootstrapCrossSigning = useCallback(async () => {
+        const cryptoApi = matrixClient.getCrypto();
+        if (!cryptoApi) return;

-        if (!this.state.accountPassword) {
-            this.queryKeyUploadAuth();
-        }
-    }
-
-    public componentDidMount(): void {
-        this.bootstrapCrossSigning();
-    }
-
-    private async queryKeyUploadAuth(): Promise<void> {
-        try {
-            await MatrixClientPeg.safeGet().uploadDeviceSigningKeys(undefined, {} as CrossSigningKeys);
-            // We should never get here: the server should always require
-            // UI auth to upload device signing keys. If we do, we upload
-            // no keys which would be a no-op.
-            logger.log("uploadDeviceSigningKeys unexpectedly succeeded without UI auth!");
-        } catch (error) {
-            if (!(error instanceof MatrixError) || !error.data || !error.data.flows) {
-                logger.log("uploadDeviceSigningKeys advertised no flows!");
-                return;
-            }
-            const canUploadKeysWithPasswordOnly = error.data.flows.some((f: UIAFlow) => {
-                return f.stages.length === 1 && f.stages[0] === "m.login.password";
-            });
-            this.setState({
-                canUploadKeysWithPasswordOnly,
-            });
-        }
-    }
-
-    private doBootstrapUIAuth = async (
-        makeRequest: (authData: AuthDict) => Promise<UIAResponse<void>>,
-    ): Promise<void> => {
-        if (this.state.canUploadKeysWithPasswordOnly && this.state.accountPassword) {
-            await makeRequest({
-                type: "m.login.password",
-                identifier: {
-                    type: "m.id.user",
-                    user: MatrixClientPeg.safeGet().getUserId(),
-                },
-                password: this.state.accountPassword,
-            });
-        } else if (this.props.tokenLogin) {
-            // We are hoping the grace period is active
-            await makeRequest({});
-        } else {
-            const dialogAesthetics = {
-                [SSOAuthEntry.PHASE_PREAUTH]: {
-                    title: _t("auth|uia|sso_title"),
-                    body: _t("auth|uia|sso_preauth_body"),
-                    continueText: _t("auth|sso"),
-                    continueKind: "primary",
-                },
-                [SSOAuthEntry.PHASE_POSTAUTH]: {
-                    title: _t("encryption|confirm_encryption_setup_title"),
-                    body: _t("encryption|confirm_encryption_setup_body"),
-                    continueText: _t("action|confirm"),
-                    continueKind: "primary",
-                },
-            };
-
-            const { finished } = Modal.createDialog(InteractiveAuthDialog, {
-                title: _t("encryption|bootstrap_title"),
-                matrixClient: MatrixClientPeg.safeGet(),
-                makeRequest,
-                aestheticsForStagePhases: {
-                    [SSOAuthEntry.LOGIN_TYPE]: dialogAesthetics,
-                    [SSOAuthEntry.UNSTABLE_LOGIN_TYPE]: dialogAesthetics,
-                },
-            });
-            const [confirmed] = await finished;
-            if (!confirmed) {
-                throw new Error("Cross-signing key upload auth canceled");
-            }
-        }
-    };
-
-    private bootstrapCrossSigning = async (): Promise<void> => {
-        this.setState({
-            error: false,
-        });
+        setError(false);

        try {
-            const cli = MatrixClientPeg.safeGet();
-            await cli.getCrypto()?.bootstrapCrossSigning({
-                authUploadDeviceSigningKeys: this.doBootstrapUIAuth,
-            });
-            this.props.onFinished(true);
+            await createCrossSigning(matrixClient, tokenLogin, accountPassword);
+            onFinished(true);
        } catch (e) {
-            if (this.props.tokenLogin) {
+            if (tokenLogin) {
                // ignore any failures, we are relying on grace period here
-                this.props.onFinished(false);
+                onFinished(false);
                return;
            }

-            this.setState({ error: true });
+            setError(true);
            logger.error("Error bootstrapping cross-signing", e);
        }
-    };
+    }, [matrixClient, tokenLogin, accountPassword, onFinished]);

-    private onCancel = (): void => {
-        this.props.onFinished(false);
-    };
+    const onCancel = useCallback(() => {
+        onFinished(false);
+    }, [onFinished]);

-    public render(): React.ReactNode {
-        let content;
-        if (this.state.error) {
-            content = (
-                <div>
-                    <p>{_t("encryption|unable_to_setup_keys_error")}</p>
-                    <div className="mx_Dialog_buttons">
-                        <DialogButtons
-                            primaryButton={_t("action|retry")}
-                            onPrimaryButtonClick={this.bootstrapCrossSigning}
-                            onCancel={this.onCancel}
-                        />
-                    </div>
+    useEffect(() => {
+        bootstrapCrossSigning();
+    }, [bootstrapCrossSigning]);
+
+    let content;
+    if (error) {
+        content = (
+            <div>
+                <p>{_t("encryption|unable_to_setup_keys_error")}</p>
+                <div className="mx_Dialog_buttons">
+                    <DialogButtons
+                        primaryButton={_t("action|retry")}
+                        onPrimaryButtonClick={bootstrapCrossSigning}
+                        onCancel={onCancel}
+                    />
                </div>
-            );
-        } else {
-            content = (
-                <div>
-                    <Spinner />
-                </div>
-            );
-        }
-
-        return (
-            <BaseDialog
-                className="mx_CreateCrossSigningDialog"
-                onFinished={this.props.onFinished}
-                title={_t("encryption|bootstrap_title")}
-                hasCancel={false}
-                fixedWidth={false}
-            >
-                <div>{content}</div>
-            </BaseDialog>
+            </div>
+        );
+    } else {
+        content = (
+            <div>
+                <Spinner />
+            </div>
        );
    }
-}
+
+    return (
+        <BaseDialog
+            className="mx_CreateCrossSigningDialog"
+            onFinished={onFinished}
+            title={_t("encryption|bootstrap_title")}
+            hasCancel={false}
+            fixedWidth={false}
+        >
+            <div>{content}</div>
+        </BaseDialog>
+    );
+};
+
+export default CreateCrossSigningDialog;
--- a/test/CreateCrossSigning-test.ts
+++ b/test/CreateCrossSigning-test.ts
@ -0,0 +1,93 @@
+/*
+Copyright 2024 New Vector Ltd.
+Copyright 2018-2022 The Matrix.org Foundation C.I.C.
+
+SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only
+Please see LICENSE files in the repository root for full details.
+*/
+
+import { MatrixClient, MatrixError } from "matrix-js-sdk/src/matrix";
+import { mocked } from "jest-mock";
+
+import { createCrossSigning } from "../src/CreateCrossSigning";
+import { createTestClient } from "./test-utils";
+import Modal from "../src/Modal";
+
+describe("CreateCrossSigning", () => {
+    let client: MatrixClient;
+
+    beforeEach(() => {
+        client = createTestClient();
+    });
+
+    it("should call bootstrapCrossSigning with an authUploadDeviceSigningKeys function", async () => {
+        await createCrossSigning(client, false, "password");
+
+        expect(client.getCrypto()?.bootstrapCrossSigning).toHaveBeenCalledWith({
+            authUploadDeviceSigningKeys: expect.any(Function),
+        });
+    });
+
+    it("should upload with password auth if possible", async () => {
+        client.uploadDeviceSigningKeys = jest.fn().mockRejectedValueOnce(
+            new MatrixError({
+                flows: [
+                    {
+                        stages: ["m.login.password"],
+                    },
+                ],
+            }),
+        );
+
+        await createCrossSigning(client, false, "password");
+
+        const { authUploadDeviceSigningKeys } = mocked(client.getCrypto()!).bootstrapCrossSigning.mock.calls[0][0];
+
+        const makeRequest = jest.fn();
+        await authUploadDeviceSigningKeys!(makeRequest);
+        expect(makeRequest).toHaveBeenCalledWith({
+            type: "m.login.password",
+            identifier: {
+                type: "m.id.user",
+                user: client.getUserId(),
+            },
+            password: "password",
+        });
+    });
+
+    it("should attempt to upload keys without auth if using token login", async () => {
+        await createCrossSigning(client, true, undefined);
+
+        const { authUploadDeviceSigningKeys } = mocked(client.getCrypto()!).bootstrapCrossSigning.mock.calls[0][0];
+
+        const makeRequest = jest.fn();
+        await authUploadDeviceSigningKeys!(makeRequest);
+        expect(makeRequest).toHaveBeenCalledWith({});
+    });
+
+    it("should prompt user if password upload not possible", async () => {
+        const createDialog = jest.spyOn(Modal, "createDialog").mockReturnValue({
+            finished: Promise.resolve([true]),
+            close: jest.fn(),
+        });
+
+        client.uploadDeviceSigningKeys = jest.fn().mockRejectedValueOnce(
+            new MatrixError({
+                flows: [
+                    {
+                        stages: ["dummy.mystery_flow_nobody_knows"],
+                    },
+                ],
+            }),
+        );
+
+        await createCrossSigning(client, false, "password");
+
+        const { authUploadDeviceSigningKeys } = mocked(client.getCrypto()!).bootstrapCrossSigning.mock.calls[0][0];
+
+        const makeRequest = jest.fn();
+        await authUploadDeviceSigningKeys!(makeRequest);
+        expect(makeRequest).not.toHaveBeenCalledWith();
+        expect(createDialog).toHaveBeenCalled();
+    });
+});
--- a/test/components/views/dialogs/security/CreateCrossSigningDialog-test.tsx
+++ b/test/components/views/dialogs/security/CreateCrossSigningDialog-test.tsx
@ -0,0 +1,131 @@
+/*
+Copyright 2024 New Vector Ltd.
+Copyright 2018-2022 The Matrix.org Foundation C.I.C.
+
+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 from "react";
+import { render, screen, waitFor } from "jest-matrix-react";
+import { mocked } from "jest-mock";
+import { MatrixClient } from "matrix-js-sdk/src/matrix";
+
+import { createCrossSigning } from "../../../../../src/CreateCrossSigning";
+import CreateCrossSigningDialog from "../../../../../src/components/views/dialogs/security/CreateCrossSigningDialog";
+import { createTestClient } from "../../../../test-utils";
+
+jest.mock("../../../../../src/CreateCrossSigning", () => ({
+    createCrossSigning: jest.fn(),
+}));
+
+describe("CreateCrossSigningDialog", () => {
+    let client: MatrixClient;
+    let createCrossSigningResolve: () => void;
+    let createCrossSigningReject: (e: Error) => void;
+
+    beforeEach(() => {
+        client = createTestClient();
+        mocked(createCrossSigning).mockImplementation(() => {
+            return new Promise((resolve, reject) => {
+                createCrossSigningResolve = resolve;
+                createCrossSigningReject = reject;
+            });
+        });
+    });
+
+    afterEach(() => {
+        jest.resetAllMocks();
+        jest.restoreAllMocks();
+    });
+
+    it("should call createCrossSigning and show a spinner while it runs", async () => {
+        const onFinished = jest.fn();
+
+        render(
+            <CreateCrossSigningDialog
+                matrixClient={client}
+                accountPassword="hunter2"
+                tokenLogin={false}
+                onFinished={onFinished}
+            />,
+        );
+
+        expect(createCrossSigning).toHaveBeenCalledWith(client, false, "hunter2");
+        expect(screen.getByTestId("spinner")).toBeInTheDocument();
+
+        createCrossSigningResolve!();
+
+        await waitFor(() => expect(onFinished).toHaveBeenCalledWith(true));
+    });
+
+    it("should display an error if createCrossSigning fails", async () => {
+        render(
+            <CreateCrossSigningDialog
+                matrixClient={client}
+                accountPassword="hunter2"
+                tokenLogin={false}
+                onFinished={jest.fn()}
+            />,
+        );
+
+        createCrossSigningReject!(new Error("generic error message"));
+
+        await expect(await screen.findByRole("button", { name: "Retry" })).toBeInTheDocument();
+    });
+
+    it("ignores failures when tokenLogin is true", async () => {
+        const onFinished = jest.fn();
+
+        render(
+            <CreateCrossSigningDialog
+                matrixClient={client}
+                accountPassword="hunter2"
+                tokenLogin={true}
+                onFinished={onFinished}
+            />,
+        );
+
+        createCrossSigningReject!(new Error("generic error message"));
+
+        await waitFor(() => expect(onFinished).toHaveBeenCalledWith(false));
+    });
+
+    it("cancels the dialog when the cancel button is clicked", async () => {
+        const onFinished = jest.fn();
+
+        render(
+            <CreateCrossSigningDialog
+                matrixClient={client}
+                accountPassword="hunter2"
+                tokenLogin={false}
+                onFinished={onFinished}
+            />,
+        );
+
+        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(
+            <CreateCrossSigningDialog
+                matrixClient={client}
+                accountPassword="hunter2"
+                tokenLogin={false}
+                onFinished={jest.fn()}
+            />,
+        );
+
+        createCrossSigningReject!(new Error("generic error message"));
+
+        const retryButton = await screen.findByRole("button", { name: "Retry" });
+        retryButton.click();
+
+        expect(createCrossSigning).toHaveBeenCalledTimes(2);
+    });
+});
--- a/test/test-utils/test-utils.ts
+++ b/test/test-utils/test-utils.ts
@ -125,6 +125,7 @@ export function createTestClient(): MatrixClient {
            getVerificationRequestsToDeviceInProgress: jest.fn().mockReturnValue([]),
            setDeviceIsolationMode: jest.fn(),
            prepareToEncrypt: jest.fn(),
+            bootstrapCrossSigning: jest.fn(),
            getActiveSessionBackupVersion: jest.fn().mockResolvedValue(null),
        }),

--- a/test/unit-tests/components/structures/MatrixChat-test.tsx
+++ b/test/unit-tests/components/structures/MatrixChat-test.tsx
@ -1112,8 +1112,6 @@ describe("<MatrixChat />", () => {

                expect(loginClient.getCrypto()!.userHasCrossSigningKeys).toHaveBeenCalled();

-                await flushPromises();
-
                // set up keys screen is rendered
                expect(screen.getByText("Setting up keys")).toBeInTheDocument();
            });