From c17d71a90bbcc42242be2a4c66383dff7f768c78 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Thu, 4 Sep 2025 10:15:08 +0100 Subject: [PATCH] Block change recovery key button while a change is ongoing. (#30664) * Block change recovery key button while a change is ongoing. * Add disable check * lint * Ensure we test that spamming the button doesn't break it. * Mock out modals * lint * add two more clicks * lint --------- Co-authored-by: Andy Balaam --- .../e2e/settings/encryption-user-tab/index.ts | 5 +- .../settings/encryption/ChangeRecoveryKey.tsx | 16 +++-- .../encryption/ChangeRecoveryKey-test.tsx | 60 ++++++++++++++++--- 3 files changed, 68 insertions(+), 13 deletions(-) diff --git a/playwright/e2e/settings/encryption-user-tab/index.ts b/playwright/e2e/settings/encryption-user-tab/index.ts index a7351fd2b4..3ff89864e2 100644 --- a/playwright/e2e/settings/encryption-user-tab/index.ts +++ b/playwright/e2e/settings/encryption-user-tab/index.ts @@ -104,7 +104,10 @@ class Helpers { const clipboardContent = await this.app.getClipboard(); await dialog.getByRole("textbox").fill(clipboardContent); - await dialog.getByRole("button", { name: confirmButtonLabel }).click(); + const button = dialog.getByRole("button", { name: confirmButtonLabel }); + await button.click(); + // Button should disable immediately after clicking. + await expect(button).toBeDisabled(); await expect(this.getEncryptionRecoverySection()).toMatchScreenshot("default-recovery.png"); } } diff --git a/src/components/views/settings/encryption/ChangeRecoveryKey.tsx b/src/components/views/settings/encryption/ChangeRecoveryKey.tsx index 3eb9875df6..ae568efc62 100644 --- a/src/components/views/settings/encryption/ChangeRecoveryKey.tsx +++ b/src/components/views/settings/encryption/ChangeRecoveryKey.tsx @@ -5,7 +5,7 @@ * Please see LICENSE files in the repository root for full details. */ -import React, { type FormEventHandler, type JSX, type MouseEventHandler, useState } from "react"; +import React, { type JSX, type MouseEventHandler, useState } from "react"; import { Breadcrumb, Button, @@ -310,7 +310,7 @@ interface KeyFormProps { /** * Called when the form is submitted. */ - onSubmit: FormEventHandler; + onSubmit: () => Promise; /** * The recovery key to confirm. */ @@ -329,6 +329,7 @@ interface KeyFormProps { function KeyForm({ onCancelClick, onSubmit, recoveryKey, submitButtonLabel }: KeyFormProps): JSX.Element { // Undefined by default, as the key is not filled yet const [isKeyValid, setIsKeyValid] = useState(); + const [isKeyChangeInProgress, setIsKeyChangeInProgress] = useState(false); const isKeyInvalidAndFilled = isKeyValid === false; return ( @@ -336,7 +337,14 @@ function KeyForm({ onCancelClick, onSubmit, recoveryKey, submitButtonLabel }: Ke className="mx_KeyForm" onSubmit={(evt) => { evt.preventDefault(); - onSubmit(evt); + if (isKeyChangeInProgress) { + // Don't allow repeated attempts. + return; + } + setIsKeyChangeInProgress(true); + onSubmit().finally(() => { + setIsKeyChangeInProgress(false); + }); }} onChange={async (evt) => { evt.preventDefault(); @@ -360,7 +368,7 @@ function KeyForm({ onCancelClick, onSubmit, recoveryKey, submitButtonLabel }: Ke )} - + diff --git a/test/unit-tests/components/views/settings/encryption/ChangeRecoveryKey-test.tsx b/test/unit-tests/components/views/settings/encryption/ChangeRecoveryKey-test.tsx index a058c6c887..3514914e7b 100644 --- a/test/unit-tests/components/views/settings/encryption/ChangeRecoveryKey-test.tsx +++ b/test/unit-tests/components/views/settings/encryption/ChangeRecoveryKey-test.tsx @@ -14,6 +14,8 @@ import { mocked } from "jest-mock"; import { ChangeRecoveryKey } from "../../../../../../src/components/views/settings/encryption/ChangeRecoveryKey"; import { createTestClient, withClientContextRenderOptions } from "../../../../../test-utils"; import { copyPlaintext } from "../../../../../../src/utils/strings"; +import Modal from "../../../../../../src/Modal"; +import ErrorDialog from "../../../../../../src/components/views/dialogs/ErrorDialog"; jest.mock("../../../../../../src/utils/strings", () => ({ copyPlaintext: jest.fn(), @@ -120,27 +122,69 @@ describe("", () => { mocked(matrixClient.getCrypto()!).bootstrapSecretStorage.mockRejectedValue(new Error("can't bootstrap")); const user = userEvent.setup(); - renderComponent(false); + const { getByRole, getByText, getByTitle } = renderComponent(false); // Display the recovery key to save - await waitFor(() => user.click(screen.getByRole("button", { name: "Continue" }))); + await waitFor(() => user.click(getByRole("button", { name: "Continue" }))); // Display the form to confirm the recovery key - await waitFor(() => user.click(screen.getByRole("button", { name: "Continue" }))); + await waitFor(() => user.click(getByRole("button", { name: "Continue" }))); - await waitFor(() => expect(screen.getByText("Enter your recovery key to confirm")).toBeInTheDocument()); + await waitFor(() => expect(getByText("Enter your recovery key to confirm")).toBeInTheDocument()); - const finishButton = screen.getByRole("button", { name: "Finish set up" }); - const input = screen.getByTitle("Enter recovery key"); + const finishButton = getByRole("button", { name: "Finish set up" }); + const input = getByTitle("Enter recovery key"); await userEvent.type(input, "encoded private key"); + + await waitFor(() => { + expect(finishButton).not.toBeDisabled(); + }); + + jest.spyOn(Modal, "createDialog"); + await user.click(finishButton); - await screen.findByText("Failed to set up secret storage"); - await screen.findByText("Error: can't bootstrap"); + expect(Modal.createDialog).toHaveBeenCalledWith(ErrorDialog, { + title: "Failed to set up secret storage", + description: "Error: can't bootstrap", + }); + await waitFor(() => user.click(getByRole("button", { name: "OK" }))); + expect(consoleErrorSpy).toHaveBeenCalledWith( "Failed to set up secret storage:", new Error("can't bootstrap"), ); }); + + it("should disallow repeated attempts to change the recovery key", async () => { + const mockFn = mocked(matrixClient.getCrypto()!).bootstrapSecretStorage.mockImplementation(() => { + // Pretend to do some work. + return new Promise((r) => setTimeout(r, 200)); + }); + + const user = userEvent.setup(); + const { getByRole, getByText, getByTitle } = renderComponent(false); + + // Display the recovery key to save + await waitFor(() => user.click(getByRole("button", { name: "Continue" }))); + // Display the form to confirm the recovery key + await waitFor(() => user.click(getByRole("button", { name: "Continue" }))); + + await waitFor(() => expect(getByText("Enter your recovery key to confirm")).toBeInTheDocument()); + + const finishButton = getByRole("button", { name: "Finish set up" }); + const input = getByTitle("Enter recovery key"); + await userEvent.type(input, "encoded private key"); + + await waitFor(() => { + expect(finishButton).not.toBeDisabled(); + }); + + await user.click(finishButton); + await user.click(finishButton); + await user.click(finishButton); + + await waitFor(() => expect(mockFn).toHaveBeenCalledTimes(1)); + }); }); describe("flow to change the recovery key", () => {