Avoid using accessSecretStorage to create 4S (#30244)

* remove resetCrossSigning flag, which is no longer in use

* drop unnecessary check for cross-signing

The only place where verifyUser is called already checks that cross-signing is
set up.  (The function name is also incorrect, since it checks for the
cross-signing key, and not for 4S.)

* avoid calling accessSecretStorage to set up cross-signing or 4S

Send the user to the Encryption settings tab instead

* only create secret storage when specifically asked to

* deprecate using accessSecretStorage to create new 4S

* also remove the obsolete snapshot

* add tests

* Tweak comment

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

---------

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
This commit is contained in:
Hubert Chathi
2025-07-03 09:34:05 -04:00
committed by GitHub
parent 66d7c6a100
commit 9095ebdb1b
17 changed files with 87 additions and 651 deletions

View File

@@ -67,6 +67,17 @@ describe("SecurityManager", () => {
await accessSecretStorage(jest.fn());
}).rejects.toThrow("End-to-end encryption is disabled - unable to access secret storage");
});
it("throws if there is no 4S", async () => {
// Given a client with no default 4S key ID
stubClient();
// When I run accessSecretStorage
// Then we throw an error
await expect(async () => {
await accessSecretStorage(jest.fn());
}).rejects.toThrow("Secret storage has not been created yet");
});
});
it("should show CreateSecretStorageDialog if forceReset=true", async () => {

View File

@@ -9,7 +9,9 @@ import React from "react";
import { render, screen, fireEvent, waitFor } from "jest-matrix-react";
import RecoveryMethodRemovedDialog from "../../../../../src/async-components/views/dialogs/security/RecoveryMethodRemovedDialog";
import Modal from "../../../../../src/Modal.tsx";
import dispatch from "../../../../../src/dispatcher/dispatcher";
import { Action } from "../../../../../src/dispatcher/actions";
import { UserTab } from "../../../../../src/components/views/dialogs/UserTab";
describe("<RecoveryMethodRemovedDialog />", () => {
afterEach(() => {
@@ -18,16 +20,15 @@ describe("<RecoveryMethodRemovedDialog />", () => {
it("should open CreateKeyBackupDialog on primary action click", async () => {
const onFinished = jest.fn();
const spy = jest.spyOn(Modal, "createDialog");
jest.mock("../../../../../src/async-components/views/dialogs/security/CreateKeyBackupDialog", () => ({
__test: true,
__esModule: true,
default: () => <span>mocked dialog</span>,
}));
jest.spyOn(dispatch, "dispatch");
render(<RecoveryMethodRemovedDialog onFinished={onFinished} />);
fireEvent.click(screen.getByRole("button", { name: "Set up Secure Messages" }));
await waitFor(() => expect(spy).toHaveBeenCalledTimes(1));
expect((spy.mock.lastCall![0] as any)._payload._result).toEqual(expect.objectContaining({ __test: true }));
await waitFor(() =>
expect(dispatch.dispatch).toHaveBeenCalledWith({
action: Action.ViewUserSettings,
initialTabId: UserTab.Encryption,
}),
);
});
});

View File

@@ -10,10 +10,13 @@ import React from "react";
import { mocked, type MockedObject } from "jest-mock";
import { type MatrixClient } from "matrix-js-sdk/src/matrix";
import { type CryptoApi, type KeyBackupInfo } from "matrix-js-sdk/src/crypto-api";
import { fireEvent, render, type RenderResult, screen } from "jest-matrix-react";
import { fireEvent, render, type RenderResult, screen, waitFor } from "jest-matrix-react";
import { filterConsole, getMockClientWithEventEmitter, mockClientMethodsCrypto } from "../../../../test-utils";
import LogoutDialog from "../../../../../src/components/views/dialogs/LogoutDialog";
import dispatch from "../../../../../src/dispatcher/dispatcher";
import { Action } from "../../../../../src/dispatcher/actions";
import { UserTab } from "../../../../../src/components/views/dialogs/UserTab";
describe("LogoutDialog", () => {
let mockClient: MockedObject<MatrixClient>;
@@ -56,17 +59,26 @@ describe("LogoutDialog", () => {
await rendered.findByText("You'll lose access to your encrypted messages");
});
it("Prompts user to connect backup if there is a backup on the server", async () => {
it("Prompts user to go to settings if there is a backup on the server", async () => {
mockCrypto.getKeyBackupInfo.mockResolvedValue({} as KeyBackupInfo);
const rendered = renderComponent();
await rendered.findByText("Connect this session to Key Backup");
await rendered.findByText("Go to Settings");
expect(rendered.container).toMatchSnapshot();
jest.spyOn(dispatch, "dispatch");
fireEvent.click(await screen.findByRole("button", { name: "Go to Settings" }));
await waitFor(() =>
expect(dispatch.dispatch).toHaveBeenCalledWith({
action: Action.ViewUserSettings,
initialTabId: UserTab.Encryption,
}),
);
});
it("Prompts user to set up backup if there is no backup on the server", async () => {
it("Prompts user to go to settings if there is no backup on the server", async () => {
mockCrypto.getKeyBackupInfo.mockResolvedValue(null);
const rendered = renderComponent();
await rendered.findByText("Start using Key Backup");
await rendered.findByText("Go to Settings");
expect(rendered.container).toMatchSnapshot();
fireEvent.click(await screen.findByRole("button", { name: "Manually export keys" }));
@@ -75,12 +87,12 @@ describe("LogoutDialog", () => {
describe("when there is an error fetching backups", () => {
filterConsole("Unable to fetch key backup status");
it("prompts user to set up backup", async () => {
it("prompts user to go to settings", async () => {
mockCrypto.getKeyBackupInfo.mockImplementation(async () => {
throw new Error("beep");
});
const rendered = renderComponent();
await rendered.findByText("Start using Key Backup");
await rendered.findByText("Go to Settings");
});
});
});

View File

@@ -1,6 +1,6 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`LogoutDialog Prompts user to connect backup if there is a backup on the server 1`] = `
exports[`LogoutDialog Prompts user to go to settings if there is a backup on the server 1`] = `
<div>
<div
data-focus-guard="true"
@@ -55,7 +55,7 @@ exports[`LogoutDialog Prompts user to connect backup if there is a backup on the
data-testid="dialog-primary-button"
type="button"
>
Connect this session to Key Backup
Go to Settings
</button>
</span>
</div>
@@ -87,7 +87,7 @@ exports[`LogoutDialog Prompts user to connect backup if there is a backup on the
</div>
`;
exports[`LogoutDialog Prompts user to set up backup if there is no backup on the server 1`] = `
exports[`LogoutDialog Prompts user to go to settings if there is no backup on the server 1`] = `
<div>
<div
data-focus-guard="true"
@@ -142,7 +142,7 @@ exports[`LogoutDialog Prompts user to set up backup if there is no backup on the
data-testid="dialog-primary-button"
type="button"
>
Start using Key Backup
Go to Settings
</button>
</span>
</div>

View File

@@ -1,77 +0,0 @@
/*
* Copyright 2024 New Vector Ltd.
* Copyright 2023 The Matrix.org Foundation C.I.C.
*
* SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
* Please see LICENSE files in the repository root for full details.
*/
import { render, screen, waitFor } from "jest-matrix-react";
import React from "react";
import { mocked } from "jest-mock";
import CreateKeyBackupDialog from "../../../../../../src/async-components/views/dialogs/security/CreateKeyBackupDialog";
import { createTestClient } from "../../../../../test-utils";
import { MatrixClientPeg } from "../../../../../../src/MatrixClientPeg";
jest.mock("../../../../../../src/SecurityManager", () => ({
accessSecretStorage: jest.fn().mockResolvedValue(undefined),
withSecretStorageKeyCache: jest.fn().mockImplementation((fn) => fn()),
}));
describe("CreateKeyBackupDialog", () => {
beforeEach(() => {
MatrixClientPeg.safeGet = MatrixClientPeg.get = () => createTestClient();
});
it("should display the spinner when creating backup", () => {
const { asFragment } = render(<CreateKeyBackupDialog onFinished={jest.fn()} />);
// Check if the spinner is displayed
expect(screen.getByTestId("spinner")).toBeDefined();
expect(asFragment()).toMatchSnapshot();
});
it("should display an error message when backup creation failed", async () => {
const matrixClient = createTestClient();
jest.spyOn(matrixClient.secretStorage, "hasKey").mockResolvedValue(true);
mocked(matrixClient.getCrypto()!.resetKeyBackup).mockImplementation(() => {
throw new Error("failed");
});
MatrixClientPeg.safeGet = MatrixClientPeg.get = () => matrixClient;
const { asFragment } = render(<CreateKeyBackupDialog onFinished={jest.fn()} />);
// Check if the error message is displayed
await waitFor(() => expect(screen.getByText("Unable to create key backup")).toBeDefined());
expect(asFragment()).toMatchSnapshot();
});
it("should display an error message when there is no Crypto available", async () => {
const matrixClient = createTestClient();
jest.spyOn(matrixClient.secretStorage, "hasKey").mockResolvedValue(true);
mocked(matrixClient.getCrypto).mockReturnValue(undefined);
MatrixClientPeg.safeGet = MatrixClientPeg.get = () => matrixClient;
render(<CreateKeyBackupDialog onFinished={jest.fn()} />);
// Check if the error message is displayed
await waitFor(() => expect(screen.getByText("Unable to create key backup")).toBeDefined());
});
it("should display the success dialog when the key backup is finished", async () => {
const onFinished = jest.fn();
const { asFragment } = render(<CreateKeyBackupDialog onFinished={onFinished} />);
await waitFor(() =>
expect(
screen.getByText("Your keys are being backed up (the first backup could take a few minutes)."),
).toBeDefined(),
);
expect(asFragment()).toMatchSnapshot();
// Click on the OK button
screen.getByRole("button", { name: "OK" }).click();
expect(onFinished).toHaveBeenCalledWith(true);
});
});

View File

@@ -13,7 +13,7 @@ import { mocked, type MockedObject } from "jest-mock";
import { type MatrixClient, MatrixError } from "matrix-js-sdk/src/matrix";
import { sleep } from "matrix-js-sdk/src/utils";
import { filterConsole, flushPromises, stubClient } from "../../../../../test-utils";
import { filterConsole, stubClient } from "../../../../../test-utils";
import CreateSecretStorageDialog from "../../../../../../src/async-components/views/dialogs/security/CreateSecretStorageDialog";
describe("CreateSecretStorageDialog", () => {
@@ -97,39 +97,4 @@ describe("CreateSecretStorageDialog", () => {
await screen.findByText("Your keys are now being backed up from this device.");
});
});
it("resets keys in the right order when resetting secret storage and cross-signing", async () => {
const result = renderComponent({ forceReset: true, resetCrossSigning: true });
await result.findByText(/Set up Secure Backup/);
jest.spyOn(mockClient.getCrypto()!, "createRecoveryKeyFromPassphrase").mockResolvedValue({
privateKey: new Uint8Array(),
encodedPrivateKey: "abcd efgh ijkl",
});
result.getByRole("button", { name: "Continue" }).click();
await result.findByText(/Save your Recovery Key/);
result.getByRole("button", { name: "Copy" }).click();
// Resetting should reset secret storage, cross signing, and key
// backup. We make sure that all three are reset, and done in the
// right order.
const resetFunctionCallLog: string[] = [];
jest.spyOn(mockClient.getCrypto()!, "bootstrapSecretStorage").mockImplementation(async () => {
resetFunctionCallLog.push("bootstrapSecretStorage");
});
jest.spyOn(mockClient.getCrypto()!, "bootstrapCrossSigning").mockImplementation(async () => {
resetFunctionCallLog.push("bootstrapCrossSigning");
});
jest.spyOn(mockClient.getCrypto()!, "resetKeyBackup").mockImplementation(async () => {
resetFunctionCallLog.push("resetKeyBackup");
});
await flushPromises();
result.getByRole("button", { name: "Continue" }).click();
await result.findByText("Your keys are now being backed up from this device.");
expect(resetFunctionCallLog).toEqual(["bootstrapSecretStorage", "bootstrapCrossSigning", "resetKeyBackup"]);
});
});

View File

@@ -1,168 +0,0 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`CreateKeyBackupDialog should display an error message when backup creation failed 1`] = `
<DocumentFragment>
<div
data-focus-guard="true"
style="width: 1px; height: 0px; padding: 0px; overflow: hidden; position: fixed; top: 1px; left: 1px;"
tabindex="0"
/>
<div
aria-labelledby="mx_BaseDialog_title"
class="mx_CreateKeyBackupDialog mx_Dialog_fixedWidth"
data-focus-lock-disabled="false"
role="dialog"
>
<div
class="mx_Dialog_header"
>
<h1
class="mx_Heading_h3 mx_Dialog_title"
id="mx_BaseDialog_title"
>
Starting backup…
</h1>
</div>
<div>
<div>
<p>
Unable to create key backup
</p>
<div
class="mx_Dialog_buttons"
>
<span
class="mx_Dialog_buttons_row"
>
<button
data-testid="dialog-cancel-button"
type="button"
>
Cancel
</button>
<button
class="mx_Dialog_primary"
data-testid="dialog-primary-button"
type="button"
>
Retry
</button>
</span>
</div>
</div>
</div>
</div>
<div
data-focus-guard="true"
style="width: 1px; height: 0px; padding: 0px; overflow: hidden; position: fixed; top: 1px; left: 1px;"
tabindex="0"
/>
</DocumentFragment>
`;
exports[`CreateKeyBackupDialog should display the spinner when creating backup 1`] = `
<DocumentFragment>
<div
data-focus-guard="true"
style="width: 1px; height: 0px; padding: 0px; overflow: hidden; position: fixed; top: 1px; left: 1px;"
tabindex="0"
/>
<div
aria-labelledby="mx_BaseDialog_title"
class="mx_CreateKeyBackupDialog mx_Dialog_fixedWidth"
data-focus-lock-disabled="false"
role="dialog"
>
<div
class="mx_Dialog_header"
>
<h1
class="mx_Heading_h3 mx_Dialog_title"
id="mx_BaseDialog_title"
>
Starting backup…
</h1>
</div>
<div>
<div>
<div
class="mx_Spinner"
>
<div
aria-label="Loading…"
class="mx_Spinner_icon"
data-testid="spinner"
role="progressbar"
style="width: 32px; height: 32px;"
/>
</div>
</div>
</div>
</div>
<div
data-focus-guard="true"
style="width: 1px; height: 0px; padding: 0px; overflow: hidden; position: fixed; top: 1px; left: 1px;"
tabindex="0"
/>
</DocumentFragment>
`;
exports[`CreateKeyBackupDialog should display the success dialog when the key backup is finished 1`] = `
<DocumentFragment>
<div
data-focus-guard="true"
style="width: 1px; height: 0px; padding: 0px; overflow: hidden; position: fixed; top: 1px; left: 1px;"
tabindex="0"
/>
<div
aria-labelledby="mx_BaseDialog_title"
class="mx_CreateKeyBackupDialog mx_Dialog_fixedWidth"
data-focus-lock-disabled="false"
role="dialog"
>
<div
class="mx_Dialog_header"
>
<h1
class="mx_Heading_h3 mx_Dialog_title"
id="mx_BaseDialog_title"
>
Success!
</h1>
</div>
<div>
<div>
<p>
Your keys are being backed up (the first backup could take a few minutes).
</p>
<div
class="mx_Dialog_buttons"
>
<span
class="mx_Dialog_buttons_row"
>
<button
class="mx_Dialog_primary"
data-testid="dialog-primary-button"
type="button"
>
OK
</button>
</span>
</div>
</div>
</div>
<div
aria-label="Close dialog"
class="mx_AccessibleButton mx_Dialog_cancelButton"
role="button"
tabindex="0"
/>
</div>
<div
data-focus-guard="true"
style="width: 1px; height: 0px; padding: 0px; overflow: hidden; position: fixed; top: 1px; left: 1px;"
tabindex="0"
/>
</DocumentFragment>
`;