From 386b782f2aba6b33bb2caac35103a5619d0d5468 Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Wed, 30 Oct 2024 12:22:05 +0100 Subject: [PATCH] Remove "Upgrade your encryption" flow in `CreateSecretStorageDialog` (#28290) * Remove "Upgrade your encryption" flow * Rename and remove tests * Remove `BackupTrustInfo` * Get keybackup when bootstraping the secret storage. * Update src/async-components/views/dialogs/security/CreateSecretStorageDialog.tsx 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> --- .../security/CreateSecretStorageDialog.tsx | 222 ++-------- src/i18n/strings/en_EN.json | 6 - test/test-utils/test-utils.ts | 5 + .../CreateSecretStorageDialog-test.tsx | 213 +++------- .../CreateSecretStorageDialog-test.tsx.snap | 394 +++--------------- 5 files changed, 146 insertions(+), 694 deletions(-) diff --git a/src/async-components/views/dialogs/security/CreateSecretStorageDialog.tsx b/src/async-components/views/dialogs/security/CreateSecretStorageDialog.tsx index 2278fb3806..3022aa6b8d 100644 --- a/src/async-components/views/dialogs/security/CreateSecretStorageDialog.tsx +++ b/src/async-components/views/dialogs/security/CreateSecretStorageDialog.tsx @@ -11,7 +11,7 @@ import React, { createRef } from "react"; import FileSaver from "file-saver"; import { logger } from "matrix-js-sdk/src/logger"; import { AuthDict, CrossSigningKeys, MatrixError, UIAFlow, UIAResponse } from "matrix-js-sdk/src/matrix"; -import { CryptoEvent, BackupTrustInfo, GeneratedSecretStorageKey, KeyBackupInfo } from "matrix-js-sdk/src/crypto-api"; +import { GeneratedSecretStorageKey } from "matrix-js-sdk/src/crypto-api"; import classNames from "classnames"; import CheckmarkIcon from "@vector-im/compound-design-tokens/assets/web/icons/check"; @@ -25,7 +25,6 @@ import StyledRadioButton from "../../../../components/views/elements/StyledRadio import AccessibleButton from "../../../../components/views/elements/AccessibleButton"; import DialogButtons from "../../../../components/views/elements/DialogButtons"; import InlineSpinner from "../../../../components/views/elements/InlineSpinner"; -import RestoreKeyBackupDialog from "../../../../components/views/dialogs/security/RestoreKeyBackupDialog"; import { getSecureBackupSetupMethods, isSecureBackupRequired, @@ -45,7 +44,6 @@ enum Phase { Loading = "loading", LoadError = "load_error", ChooseKeyPassphrase = "choose_key_passphrase", - Migrate = "migrate", Passphrase = "passphrase", PassphraseConfirm = "passphrase_confirm", ShowKey = "show_key", @@ -72,24 +70,6 @@ interface IState { downloaded: boolean; setPassphrase: boolean; - /** Information on the current key backup version, as returned by the server. - * - * `null` could mean any of: - * * we haven't yet requested the data from the server. - * * we were unable to reach the server. - * * the server returned key backup version data we didn't understand or was malformed. - * * there is actually no backup on the server. - */ - backupInfo: KeyBackupInfo | null; - - /** - * Information on whether the backup in `backupInfo` is correctly signed, and whether we have the right key to - * decrypt it. - * - * `undefined` if `backupInfo` is null, or if crypto is not enabled in the client. - */ - backupTrustInfo: BackupTrustInfo | undefined; - // does the server offer a UI auth flow with just m.login.password // for /keys/device_signing/upload? canUploadKeysWithPasswordOnly: boolean | null; @@ -141,16 +121,17 @@ export default class CreateSecretStorageDialog extends React.PureComponent { - try { - const cli = MatrixClientPeg.safeGet(); - const backupInfo = await cli.getKeyBackupVersion(); - const backupTrustInfo = - // we may not have started crypto yet, in which case we definitely don't trust the backup - backupInfo ? await cli.getCrypto()?.isKeyBackupTrusted(backupInfo) : undefined; - - const { forceReset } = this.props; - const phase = backupInfo && !forceReset ? Phase.Migrate : Phase.ChooseKeyPassphrase; - - this.setState({ - phase, - backupInfo, - backupTrustInfo, - }); - - return backupTrustInfo; - } catch (e) { - console.error("Error fetching backup data from server", e); - this.setState({ phase: Phase.LoadError }); - return undefined; - } + private initExtension(keyFromCustomisations: Uint8Array): void { + logger.log("CryptoSetupExtension: Created key via extension, jumping to bootstrap step"); + this.recoveryKey = { + privateKey: keyFromCustomisations, + }; + this.bootstrapSecretStorage(); } private async queryKeyUploadAuth(): Promise { @@ -237,10 +173,6 @@ export default class CreateSecretStorageDialog extends React.PureComponent { - if (this.state.phase === Phase.Migrate) this.fetchBackupInfo(); - }; - private onKeyPassphraseChange = (e: React.ChangeEvent): void => { this.setState({ passPhraseKeySelected: e.target.value, @@ -265,15 +197,6 @@ export default class CreateSecretStorageDialog extends React.PureComponent { - e.preventDefault(); - if (this.state.backupTrustInfo?.trusted) { - this.bootstrapSecretStorage(); - } else { - this.restoreBackup(); - } - }; - private onCopyClick = (): void => { const successful = copyNode(this.recoveryKeyNode.current); if (successful) { @@ -340,16 +263,28 @@ export default class CreateSecretStorageDialog extends React.PureComponent => { + const cli = MatrixClientPeg.safeGet(); + const crypto = cli.getCrypto()!; + const { forceReset } = this.props; + + let backupInfo; + // First, unless we know we want to do a reset, we see if there is an existing key backup + if (!forceReset) { + try { + this.setState({ phase: Phase.Loading }); + backupInfo = await cli.getKeyBackupVersion(); + } catch (e) { + logger.error("Error fetching backup data from server", e); + this.setState({ phase: Phase.LoadError }); + return; + } + } + this.setState({ phase: Phase.Storing, error: undefined, }); - const cli = MatrixClientPeg.safeGet(); - const crypto = cli.getCrypto()!; - - const { forceReset } = this.props; - try { if (forceReset) { logger.log("Forcing secret storage reset"); @@ -371,8 +306,7 @@ export default class CreateSecretStorageDialog extends React.PureComponent this.recoveryKey!, - keyBackupInfo: this.state.backupInfo!, - setupNewKeyBackup: !this.state.backupInfo, + setupNewKeyBackup: !backupInfo, }); } await initialiseDehydration(true); @@ -381,20 +315,7 @@ export default class CreateSecretStorageDialog extends React.PureComponent => { - const { finished } = Modal.createDialog( - RestoreKeyBackupDialog, - { - showSummary: false, - }, - undefined, - /* priority = */ false, - /* static = */ false, - ); - - await finished; - const backupTrustInfo = await this.fetchBackupInfo(); - if (backupTrustInfo?.trusted && this.state.canUploadKeysWithPasswordOnly && this.state.accountPassword) { - this.bootstrapSecretStorage(); - } - }; - private onLoadRetryClick = (): void => { - this.setState({ phase: Phase.Loading }); - this.fetchBackupInfo(); + this.bootstrapSecretStorage(); }; private onShowKeyContinueClick = (): void => { @@ -495,12 +397,6 @@ export default class CreateSecretStorageDialog extends React.PureComponent): void => { - this.setState({ - accountPassword: e.target.value, - }); - }; - private renderOptionKey(): JSX.Element { return ( -
{_t("settings|key_backup|setup_secure_backup|requires_password_confirmation")}
-
- -
- - ); - } else if (!this.state.backupTrustInfo?.trusted) { - authPrompt = ( -
-
{_t("settings|key_backup|setup_secure_backup|requires_key_restore")}
-
- ); - nextCaption = _t("action|restore"); - } else { - authPrompt =

{_t("settings|key_backup|setup_secure_backup|requires_server_authentication")}

; - } - - return ( -
-

{_t("settings|key_backup|setup_secure_backup|session_upgrade_description")}

-
{authPrompt}
- - - -
- ); - } - private renderPhasePassPhrase(): JSX.Element { return (
@@ -829,8 +676,6 @@ export default class CreateSecretStorageDialog extends React.PureComponent { let mockClient: MockedObject; - let mockCrypto: MockedObject; beforeEach(() => { - mockClient = getMockClientWithEventEmitter({ - ...mockClientMethodsServer(), - ...mockClientMethodsCrypto(), - uploadDeviceSigningKeys: jest.fn().mockImplementation(async () => { - await sleep(0); // CreateSecretStorageDialog doesn't expect this to resolve immediately - throw new MatrixError({ flows: [] }); - }), - }); - - mockCrypto = mocked(mockClient.getCrypto()!); - Object.assign(mockCrypto, { - isKeyBackupTrusted: jest.fn(), - isDehydrationSupported: jest.fn(() => false), - bootstrapCrossSigning: jest.fn(), - bootstrapSecretStorage: jest.fn(), + mockClient = mocked(stubClient()); + mockClient.uploadDeviceSigningKeys.mockImplementation(async () => { + await sleep(0); // CreateSecretStorageDialog doesn't expect this to resolve immediately + throw new MatrixError({ flows: [] }); }); + // Mock the clipboard API + document.execCommand = jest.fn().mockReturnValue(true); }); afterEach(() => { @@ -59,11 +40,37 @@ describe("CreateSecretStorageDialog", () => { return render(); } - it("shows a loading spinner initially", async () => { - const { container } = renderComponent(); - expect(screen.getByTestId("spinner")).toBeDefined(); - expect(container).toMatchSnapshot(); - await flushPromises(); + it("handles the happy path", async () => { + const result = renderComponent(); + await result.findByText( + "Safeguard against losing access to encrypted messages & data by backing up encryption keys on your server.", + ); + expect(result.container).toMatchSnapshot(); + await userEvent.click(result.getByRole("button", { name: "Continue" })); + + await screen.findByText("Save your Security Key"); + expect(result.container).toMatchSnapshot(); + // Copy the key to enable the continue button + await userEvent.click(screen.getByRole("button", { name: "Copy" })); + expect(result.queryByText("Copied!")).not.toBeNull(); + await userEvent.click(screen.getByRole("button", { name: "Continue" })); + + await screen.findByText("Your keys are now being backed up from this device."); + }); + + it("when there is an error when bootstraping the secret storage, it shows an error", async () => { + jest.spyOn(mockClient.getCrypto()!, "bootstrapSecretStorage").mockRejectedValue(new Error("error")); + + renderComponent(); + await screen.findByText( + "Safeguard against losing access to encrypted messages & data by backing up encryption keys on your server.", + ); + await userEvent.click(screen.getByRole("button", { name: "Continue" })); + await screen.findByText("Save your Security Key"); + await userEvent.click(screen.getByRole("button", { name: "Copy" })); + await userEvent.click(screen.getByRole("button", { name: "Continue" })); + + await screen.findByText("Unable to set up secret storage"); }); describe("when there is an error fetching the backup version", () => { @@ -75,139 +82,19 @@ describe("CreateSecretStorageDialog", () => { }); const result = renderComponent(); + // We go though the dialog until we have to get the key backup + await userEvent.click(result.getByRole("button", { name: "Continue" })); + await userEvent.click(screen.getByRole("button", { name: "Copy" })); + await userEvent.click(screen.getByRole("button", { name: "Continue" })); + // XXX the error message is... misleading. - await result.findByText("Unable to query secret storage status"); - expect(result.container).toMatchSnapshot(); - }); - }); - - it("shows 'Generate a Security Key' text if no key backup is present", async () => { - const result = renderComponent(); - await flushPromises(); - expect(result.container).toMatchSnapshot(); - result.getByText("Generate a Security Key"); - }); - - describe("when canUploadKeysWithPasswordOnly", () => { - // spy on Modal.createDialog - let modalSpy: jest.SpyInstance; - - // deferred which should be resolved to indicate that the created dialog has completed - let restoreDialogFinishedDefer: IDeferred<[done?: boolean]>; - - beforeEach(() => { - mockClient.getKeyBackupVersion.mockResolvedValue({} as KeyBackupInfo); - mockClient.uploadDeviceSigningKeys.mockImplementation(async () => { - await sleep(0); - throw new MatrixError({ - flows: [{ stages: ["m.login.password"] }], - }); - }); - - restoreDialogFinishedDefer = defer<[done?: boolean]>(); - modalSpy = jest.spyOn(Modal, "createDialog").mockReturnValue({ - finished: restoreDialogFinishedDefer.promise, - close: jest.fn(), - }); - }); - - it("prompts for a password and then shows RestoreKeyBackupDialog", async () => { - const result = renderComponent(); - await result.findByText(/Enter your account password to confirm the upgrade/); + await screen.findByText("Unable to query secret storage status"); expect(result.container).toMatchSnapshot(); - await userEvent.type(result.getByPlaceholderText("Password"), "my pass"); - result.getByRole("button", { name: "Next" }).click(); - - expect(modalSpy).toHaveBeenCalledWith( - RestoreKeyBackupDialog, - { - showSummary: false, - }, - undefined, - false, - false, - ); - - restoreDialogFinishedDefer.resolve([]); - }); - - it("calls bootstrapSecretStorage once keys are restored if the backup is now trusted", async () => { - const result = renderComponent(); - await result.findByText(/Enter your account password to confirm the upgrade/); - expect(result.container).toMatchSnapshot(); - - await userEvent.type(result.getByPlaceholderText("Password"), "my pass"); - result.getByRole("button", { name: "Next" }).click(); - - expect(modalSpy).toHaveBeenCalled(); - - // While we restore the key backup, its signature becomes accepted - mockCrypto.isKeyBackupTrusted.mockResolvedValue({ trusted: true } as BackupTrustInfo); - - restoreDialogFinishedDefer.resolve([]); - await flushPromises(); - - // XXX no idea why this is a sensible thing to do. I just work here. - expect(mockCrypto.bootstrapCrossSigning).toHaveBeenCalled(); - expect(mockCrypto.bootstrapSecretStorage).toHaveBeenCalled(); - - await result.findByText("Your keys are now being backed up from this device."); - }); - - describe("when there is an error fetching the backup version after RestoreKeyBackupDialog", () => { - filterConsole("Error fetching backup data from server"); - - it("handles the error sensibly", async () => { - const result = renderComponent(); - await result.findByText(/Enter your account password to confirm the upgrade/); - expect(result.container).toMatchSnapshot(); - - await userEvent.type(result.getByPlaceholderText("Password"), "my pass"); - result.getByRole("button", { name: "Next" }).click(); - - expect(modalSpy).toHaveBeenCalled(); - - mockClient.getKeyBackupVersion.mockImplementation(async () => { - throw new Error("bleh bleh"); - }); - restoreDialogFinishedDefer.resolve([]); - await result.findByText("Unable to query secret storage status"); - }); - }); - }); - - describe("when backup is present but not trusted", () => { - beforeEach(() => { - mockClient.getKeyBackupVersion.mockResolvedValue({} as KeyBackupInfo); - }); - - it("shows migrate text, then 'RestoreKeyBackupDialog' if 'Restore' is clicked", async () => { - const result = renderComponent(); - await result.findByText("Restore your key backup to upgrade your encryption"); - expect(result.container).toMatchSnapshot(); - - // before we click "Restore", set up a spy on createDialog - const restoreDialogFinishedDefer = defer<[done?: boolean]>(); - const modalSpy = jest.spyOn(Modal, "createDialog").mockReturnValue({ - finished: restoreDialogFinishedDefer.promise, - close: jest.fn(), - }); - - result.getByRole("button", { name: "Restore" }).click(); - - expect(modalSpy).toHaveBeenCalledWith( - RestoreKeyBackupDialog, - { - showSummary: false, - }, - undefined, - false, - false, - ); - - // simulate RestoreKeyBackupDialog completing, to run that code path - restoreDialogFinishedDefer.resolve([]); + // Now we can get the backup and we retry + mockClient.getKeyBackupVersion.mockRestore(); + await userEvent.click(screen.getByRole("button", { name: "Retry" })); + await screen.findByText("Your keys are now being backed up from this device."); }); }); }); diff --git a/test/unit-tests/components/views/dialogs/security/__snapshots__/CreateSecretStorageDialog-test.tsx.snap b/test/unit-tests/components/views/dialogs/security/__snapshots__/CreateSecretStorageDialog-test.tsx.snap index 3ba1018ae1..5ad3590422 100644 --- a/test/unit-tests/components/views/dialogs/security/__snapshots__/CreateSecretStorageDialog-test.tsx.snap +++ b/test/unit-tests/components/views/dialogs/security/__snapshots__/CreateSecretStorageDialog-test.tsx.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`CreateSecretStorageDialog shows 'Generate a Security Key' text if no key backup is present 1`] = ` +exports[`CreateSecretStorageDialog handles the happy path 1`] = `
`; -exports[`CreateSecretStorageDialog shows a loading spinner initially 1`] = ` +exports[`CreateSecretStorageDialog handles the happy path 2`] = `
+ > +

+ Save your Security Key +

+
+

+ Store your Security Key somewhere safe, like a password manager or a safe, as it's used to safeguard your encrypted data. +

+ class="mx_CreateSecretStorageDialog_recoveryKeyContainer" + > +
+ +
+
+
+ Download +
+ + or + +
+ Copy +
+
+
+
+
+ + +
@@ -168,331 +217,6 @@ exports[`CreateSecretStorageDialog shows a loading spinner initially 1`] = `
`; -exports[`CreateSecretStorageDialog when backup is present but not trusted shows migrate text, then 'RestoreKeyBackupDialog' if 'Restore' is clicked 1`] = ` -
-
- -
-
-`; - -exports[`CreateSecretStorageDialog when canUploadKeysWithPasswordOnly calls bootstrapSecretStorage once keys are restored if the backup is now trusted 1`] = ` -
-
- -
-
-`; - -exports[`CreateSecretStorageDialog when canUploadKeysWithPasswordOnly prompts for a password and then shows RestoreKeyBackupDialog 1`] = ` -
-
- -
-
-`; - -exports[`CreateSecretStorageDialog when canUploadKeysWithPasswordOnly when there is an error fetching the backup version after RestoreKeyBackupDialog handles the error sensibly 1`] = ` -
-
- -
-
-`; - exports[`CreateSecretStorageDialog when there is an error fetching the backup version shows an error 1`] = `