From 19ef3267c084809d37bc5bd03ee6d953eb8ad241 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 22 Oct 2024 12:42:07 +0100 Subject: [PATCH] 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 --------- Co-authored-by: Andy Balaam --- src/CreateCrossSigning.ts | 118 ++++++++++ src/components/structures/MatrixChat.tsx | 1 + src/components/structures/auth/E2eSetup.tsx | 5 +- .../security/CreateCrossSigningDialog.tsx | 212 +++++------------- test/CreateCrossSigning-test.ts | 93 ++++++++ .../CreateCrossSigningDialog-test.tsx | 131 +++++++++++ test/test-utils/test-utils.ts | 1 + .../components/structures/MatrixChat-test.tsx | 2 - 8 files changed, 406 insertions(+), 157 deletions(-) create mode 100644 src/CreateCrossSigning.ts create mode 100644 test/CreateCrossSigning-test.ts create mode 100644 test/components/views/dialogs/security/CreateCrossSigningDialog-test.tsx diff --git a/src/CreateCrossSigning.ts b/src/CreateCrossSigning.ts new file mode 100644 index 0000000000..e67e030f60 --- /dev/null +++ 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 { + 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 { + const cryptoApi = cli.getCrypto(); + if (!cryptoApi) { + throw new Error("No crypto API found!"); + } + + const doBootstrapUIAuth = async ( + makeRequest: (authData: AuthDict) => Promise>, + ): Promise => { + 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, + }); +} diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index c5ef3975a5..d0edcccd4f 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -2088,6 +2088,7 @@ export default class MatrixChat extends React.PureComponent { } else if (this.state.view === Views.E2E_SETUP) { view = ( void; accountPassword?: string; - tokenLogin?: boolean; + tokenLogin: boolean; } export default class E2eSetup extends React.Component { @@ -24,6 +26,7 @@ export default class E2eSetup extends React.Component { 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 { - public constructor(props: IProps) { - super(props); +const CreateCrossSigningDialog: React.FC = ({ 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 { - 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>, - ): Promise => { - 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 => { - 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 = ( -
-

{_t("encryption|unable_to_setup_keys_error")}

-
- -
+ useEffect(() => { + bootstrapCrossSigning(); + }, [bootstrapCrossSigning]); + + let content; + if (error) { + content = ( +
+

{_t("encryption|unable_to_setup_keys_error")}

+
+
- ); - } else { - content = ( -
- -
- ); - } - - return ( - -
{content}
- +
+ ); + } else { + content = ( +
+ +
); } -} + + return ( + +
{content}
+ + ); +}; + +export default CreateCrossSigningDialog; diff --git a/test/CreateCrossSigning-test.ts b/test/CreateCrossSigning-test.ts new file mode 100644 index 0000000000..e1762bb504 --- /dev/null +++ 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(); + }); +}); diff --git a/test/components/views/dialogs/security/CreateCrossSigningDialog-test.tsx b/test/components/views/dialogs/security/CreateCrossSigningDialog-test.tsx new file mode 100644 index 0000000000..3e5dc4eb94 --- /dev/null +++ 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( + , + ); + + 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( + , + ); + + 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( + , + ); + + 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( + , + ); + + 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")); + + const retryButton = await screen.findByRole("button", { name: "Retry" }); + retryButton.click(); + + expect(createCrossSigning).toHaveBeenCalledTimes(2); + }); +}); diff --git a/test/test-utils/test-utils.ts b/test/test-utils/test-utils.ts index 6dc9533ac9..73ff714ae0 100644 --- 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), }), diff --git a/test/unit-tests/components/structures/MatrixChat-test.tsx b/test/unit-tests/components/structures/MatrixChat-test.tsx index af0453af2a..7f565d682f 100644 --- a/test/unit-tests/components/structures/MatrixChat-test.tsx +++ b/test/unit-tests/components/structures/MatrixChat-test.tsx @@ -1112,8 +1112,6 @@ describe("", () => { expect(loginClient.getCrypto()!.userHasCrossSigningKeys).toHaveBeenCalled(); - await flushPromises(); - // set up keys screen is rendered expect(screen.getByText("Setting up keys")).toBeInTheDocument(); });