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 <andy.balaam@matrix.org>
This commit is contained in:
Will Hunt
2025-09-04 10:15:08 +01:00
committed by GitHub
parent 07c253d11f
commit c17d71a90b
3 changed files with 68 additions and 13 deletions

View File

@@ -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");
}
}

View File

@@ -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<void>;
/**
* 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<boolean>();
const [isKeyChangeInProgress, setIsKeyChangeInProgress] = useState<boolean>(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
)}
</Field>
<EncryptionCardButtons>
<Button disabled={!isKeyValid}>{submitButtonLabel}</Button>
<Button disabled={!isKeyValid || isKeyChangeInProgress}>{submitButtonLabel}</Button>
<Button kind="tertiary" onClick={onCancelClick}>
{_t("action|cancel")}
</Button>

View File

@@ -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("<ChangeRecoveryKey />", () => {
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", () => {