From 0337bd1b0aa20dc5db7d62860a7edf379f75a50a Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 10 Jan 2024 11:34:03 +0100 Subject: [PATCH] Set up key backup using non-deprecated APIs (2nd take) (#12098) * Ensure backup settings in playwright * Fix verification by pass causing backup reset * fix force backup setup by default * fix test * clarify when we need to bootstrap * jslint * post merge fix * post rebase missing files * fix bad merge * update test * Fix import * test user forgot passkey * better usage of locator * fix snapshot * remove getDialogByTitle * Update src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * unneeded permission * code review * cleaning --------- Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- playwright/e2e/crypto/backups.spec.ts | 120 ++++++++++++++++++ playwright/pages/ElementAppPage.ts | 4 + src/SecurityManager.ts | 11 +- .../security/CreateKeyBackupDialog.tsx | 48 ++++--- test/SecurityManager-test.ts | 62 +++++++++ .../security/CreateKeyBackupDialog-test.tsx | 20 ++- .../CreateKeyBackupDialog-test.tsx.snap | 2 +- test/test-utils/test-utils.ts | 5 + 8 files changed, 246 insertions(+), 26 deletions(-) create mode 100644 playwright/e2e/crypto/backups.spec.ts create mode 100644 test/SecurityManager-test.ts diff --git a/playwright/e2e/crypto/backups.spec.ts b/playwright/e2e/crypto/backups.spec.ts new file mode 100644 index 00000000000..75c8e3eed75 --- /dev/null +++ b/playwright/e2e/crypto/backups.spec.ts @@ -0,0 +1,120 @@ +/* +Copyright 2023 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { type Page } from "@playwright/test"; + +import { test, expect } from "../../element-web-test"; + +async function expectBackupVersionToBe(page: Page, version: string) { + await expect(page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(5) td")).toHaveText( + version + " (Algorithm: m.megolm_backup.v1.curve25519-aes-sha2)", + ); + + await expect(page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(6) td")).toHaveText(version); +} + +test.describe("Backups", () => { + test.use({ + displayName: "Hanako", + }); + + test("Create, delete and recreate a keys backup", async ({ page, user, app }, workerInfo) => { + // Create a backup + const securityTab = await app.settings.openUserSettings("Security & Privacy"); + + await expect(securityTab.getByRole("heading", { name: "Secure Backup" })).toBeVisible(); + await securityTab.getByRole("button", { name: "Set up", exact: true }).click(); + + const currentDialogLocator = page.locator(".mx_Dialog"); + + // It's the first time and secure storage is not set up, so it will create one + await expect(currentDialogLocator.getByRole("heading", { name: "Set up Secure Backup" })).toBeVisible(); + await currentDialogLocator.getByRole("button", { name: "Continue", exact: true }).click(); + await expect(currentDialogLocator.getByRole("heading", { name: "Save your Security Key" })).toBeVisible(); + await currentDialogLocator.getByRole("button", { name: "Copy", exact: true }).click(); + // copy the recovery key to use it later + const securityKey = await app.getClipboard(); + await currentDialogLocator.getByRole("button", { name: "Continue", exact: true }).click(); + + await expect(currentDialogLocator.getByRole("heading", { name: "Secure Backup successful" })).toBeVisible(); + await currentDialogLocator.getByRole("button", { name: "Done", exact: true }).click(); + + // Open the settings again + await app.settings.openUserSettings("Security & Privacy"); + await expect(securityTab.getByRole("heading", { name: "Secure Backup" })).toBeVisible(); + + // expand the advanced section to see the active version in the reports + await page + .locator(".mx_Dialog .mx_SettingsSubsection_content details .mx_SecureBackupPanel_advanced") + .locator("..") + .click(); + + await expectBackupVersionToBe(page, "1"); + + await securityTab.getByRole("button", { name: "Delete Backup", exact: true }).click(); + await expect(currentDialogLocator.getByRole("heading", { name: "Delete Backup" })).toBeVisible(); + // Delete it + await currentDialogLocator.getByTestId("dialog-primary-button").click(); // Click "Delete Backup" + + // Create another + await securityTab.getByRole("button", { name: "Set up", exact: true }).click(); + await expect(currentDialogLocator.getByRole("heading", { name: "Security Key" })).toBeVisible(); + await currentDialogLocator.getByLabel("Security Key").fill(securityKey); + await currentDialogLocator.getByRole("button", { name: "Continue", exact: true }).click(); + + // Should be successful + await expect(currentDialogLocator.getByRole("heading", { name: "Success!" })).toBeVisible(); + await currentDialogLocator.getByRole("button", { name: "OK", exact: true }).click(); + + // Open the settings again + await app.settings.openUserSettings("Security & Privacy"); + await expect(securityTab.getByRole("heading", { name: "Secure Backup" })).toBeVisible(); + + // expand the advanced section to see the active version in the reports + await page + .locator(".mx_Dialog .mx_SettingsSubsection_content details .mx_SecureBackupPanel_advanced") + .locator("..") + .click(); + + await expectBackupVersionToBe(page, "2"); + + // == + // Ensure that if you don't have the secret storage passphrase the backup won't be created + // == + + // First delete version 2 + await securityTab.getByRole("button", { name: "Delete Backup", exact: true }).click(); + await expect(currentDialogLocator.getByRole("heading", { name: "Delete Backup" })).toBeVisible(); + // Click "Delete Backup" + await currentDialogLocator.getByTestId("dialog-primary-button").click(); + + // Try to create another + await securityTab.getByRole("button", { name: "Set up", exact: true }).click(); + await expect(currentDialogLocator.getByRole("heading", { name: "Security Key" })).toBeVisible(); + // But cancel the security key dialog, to simulate not having the secret storage passphrase + await currentDialogLocator.getByTestId("dialog-cancel-button").click(); + + await expect(currentDialogLocator.getByRole("heading", { name: "Starting backup…" })).toBeVisible(); + // check that it failed + await expect(currentDialogLocator.getByText("Unable to create key backup")).toBeVisible(); + // cancel + await currentDialogLocator.getByTestId("dialog-cancel-button").click(); + + // go back to the settings to check that no backup was created (the setup button should still be there) + await app.settings.openUserSettings("Security & Privacy"); + await expect(securityTab.getByRole("button", { name: "Set up", exact: true })).toBeVisible(); + }); +}); diff --git a/playwright/pages/ElementAppPage.ts b/playwright/pages/ElementAppPage.ts index 9a5de3b1480..be80cf62808 100644 --- a/playwright/pages/ElementAppPage.ts +++ b/playwright/pages/ElementAppPage.ts @@ -51,6 +51,10 @@ export class ElementAppPage { return this.settings.closeDialog(); } + public async getClipboard(): Promise { + return await this.page.evaluate(() => navigator.clipboard.readText()); + } + /** * Opens the given room by name. The room must be visible in the * room list, but the room list may be folded horizontally, and the diff --git a/src/SecurityManager.ts b/src/SecurityManager.ts index 42b6c740998..ff8946614fd 100644 --- a/src/SecurityManager.ts +++ b/src/SecurityManager.ts @@ -347,7 +347,7 @@ export async function accessSecretStorage(func = async (): Promise => {}, } /** Helper for {@link #accessSecretStorage} */ -export async function doAccessSecretStorage(func = async (): Promise => {}, forceReset = false): Promise { +async function doAccessSecretStorage(func: () => Promise, forceReset: boolean): Promise { try { const cli = MatrixClientPeg.safeGet(); if (!(await cli.hasSecretStorageKey()) || forceReset) { @@ -378,7 +378,12 @@ export async function doAccessSecretStorage(func = async (): Promise => {} throw new Error("Secret storage creation canceled"); } } else { - await cli.bootstrapCrossSigning({ + const crypto = cli.getCrypto(); + if (!crypto) { + throw new Error("End-to-end encryption is disabled - unable to access secret storage."); + } + + await crypto.bootstrapCrossSigning({ authUploadDeviceSigningKeys: async (makeRequest): Promise => { const { finished } = Modal.createDialog(InteractiveAuthDialog, { title: _t("encryption|bootstrap_title"), @@ -391,7 +396,7 @@ export async function doAccessSecretStorage(func = async (): Promise => {} } }, }); - await cli.bootstrapSecretStorage({ + await crypto.bootstrapSecretStorage({ getKeyBackupPassphrase: promptForBackupPassphrase, }); diff --git a/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx b/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx index 453a9dc84c4..1dd280d085b 100644 --- a/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx +++ b/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx @@ -17,11 +17,10 @@ limitations under the License. import React from "react"; import { logger } from "matrix-js-sdk/src/logger"; -import { IKeyBackupInfo } from "matrix-js-sdk/src/crypto/keybackup"; import { MatrixClientPeg } from "../../../../MatrixClientPeg"; import { _t } from "../../../../languageHandler"; -import { accessSecretStorage } from "../../../../SecurityManager"; +import { accessSecretStorage, withSecretStorageKeyCache } from "../../../../SecurityManager"; import Spinner from "../../../../components/views/elements/Spinner"; import BaseDialog from "../../../../components/views/dialogs/BaseDialog"; import DialogButtons from "../../../../components/views/elements/DialogButtons"; @@ -75,24 +74,36 @@ export default class CreateKeyBackupDialog extends React.PureComponent => { - // `accessSecretStorage` will have bootstrapped secret storage if necessary, so we can now - // set up key backup. - // - // XXX: `bootstrapSecretStorage` also sets up key backup as a side effect, so there is a 90% chance - // this is actually redundant. - // - // The only time it would *not* be redundant would be if, for some reason, we had working 4S but no - // working key backup. (For example, if the user clicked "Delete Backup".) - info = await cli.prepareKeyBackupVersion(null /* random key */, { - secureSecretStorage: true, + // Check if 4S already set up + const secretStorageAlreadySetup = await cli.hasSecretStorageKey(); + + if (!secretStorageAlreadySetup) { + // bootstrap secret storage; that will also create a backup version + await accessSecretStorage(async (): Promise => { + // do nothing, all is now set up correctly }); - info = await cli.createKeyBackupVersion(info); - }); - await cli.scheduleAllGroupSessionsForBackup(); + } else { + await withSecretStorageKeyCache(async () => { + const crypto = cli.getCrypto(); + if (!crypto) { + throw new Error("End-to-end encryption is disabled - unable to create backup."); + } + + // Before we reset the backup, let's make sure we can access secret storage, to + // reduce the chance of us getting into a broken state where we have an outdated + // secret in secret storage. + // `SecretStorage.get` will ask the user to enter their passphrase/key if necessary; + // it will then be cached for the actual backup reset operation. + await cli.secretStorage.get("m.megolm_backup.v1"); + + // We now know we can store the new backup key in secret storage, so it is safe to + // go ahead with the reset. + await crypto.resetKeyBackup(); + }); + } + this.setState({ phase: Phase.Done, }); @@ -102,9 +113,6 @@ export default class CreateKeyBackupDialog extends React.PureComponent { + describe("accessSecretStorage", () => { + filterConsole("Not setting dehydration key: no SSSS key found"); + + it("runs the function passed in", async () => { + // Given a client + const crypto = { + bootstrapCrossSigning: () => {}, + bootstrapSecretStorage: () => {}, + } as unknown as CryptoApi; + const client = stubClient(); + mocked(client.hasSecretStorageKey).mockResolvedValue(true); + mocked(client.getCrypto).mockReturnValue(crypto); + + // When I run accessSecretStorage + const func = jest.fn(); + await accessSecretStorage(func); + + // Then we call the passed-in function + expect(func).toHaveBeenCalledTimes(1); + }); + + describe("expecting errors", () => { + filterConsole("End-to-end encryption is disabled - unable to access secret storage"); + + it("throws if crypto is unavailable", async () => { + // Given a client with no crypto + const client = stubClient(); + mocked(client.hasSecretStorageKey).mockResolvedValue(true); + mocked(client.getCrypto).mockReturnValue(undefined); + + // When I run accessSecretStorage + // Then we throw an error + await expect(async () => { + await accessSecretStorage(jest.fn()); + }).rejects.toThrow("End-to-end encryption is disabled - unable to access secret storage"); + }); + }); + }); +}); diff --git a/test/components/views/dialogs/security/CreateKeyBackupDialog-test.tsx b/test/components/views/dialogs/security/CreateKeyBackupDialog-test.tsx index b0d461b8f8d..f5a59b07032 100644 --- a/test/components/views/dialogs/security/CreateKeyBackupDialog-test.tsx +++ b/test/components/views/dialogs/security/CreateKeyBackupDialog-test.tsx @@ -24,6 +24,7 @@ import { MatrixClientPeg } from "../../../../../src/MatrixClientPeg"; jest.mock("../../../../../src/SecurityManager", () => ({ accessSecretStorage: jest.fn().mockResolvedValue(undefined), + withSecretStorageKeyCache: jest.fn().mockImplementation((fn) => fn()), })); describe("CreateKeyBackupDialog", () => { @@ -39,9 +40,12 @@ describe("CreateKeyBackupDialog", () => { expect(asFragment()).toMatchSnapshot(); }); - it("should display the error message when backup creation failed", async () => { + it("should display an error message when backup creation failed", async () => { const matrixClient = createTestClient(); - mocked(matrixClient.scheduleAllGroupSessionsForBackup).mockRejectedValue("my error"); + mocked(matrixClient.hasSecretStorageKey).mockResolvedValue(true); + mocked(matrixClient.getCrypto()!.resetKeyBackup).mockImplementation(() => { + throw new Error("failed"); + }); MatrixClientPeg.safeGet = MatrixClientPeg.get = () => matrixClient; const { asFragment } = render(); @@ -51,6 +55,18 @@ describe("CreateKeyBackupDialog", () => { expect(asFragment()).toMatchSnapshot(); }); + it("should display an error message when there is no Crypto available", async () => { + const matrixClient = createTestClient(); + mocked(matrixClient.hasSecretStorageKey).mockResolvedValue(true); + mocked(matrixClient.getCrypto).mockReturnValue(undefined); + MatrixClientPeg.safeGet = MatrixClientPeg.get = () => matrixClient; + + render(); + + // 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(); diff --git a/test/components/views/dialogs/security/__snapshots__/CreateKeyBackupDialog-test.tsx.snap b/test/components/views/dialogs/security/__snapshots__/CreateKeyBackupDialog-test.tsx.snap index 9d62384e3c1..d3880d875c0 100644 --- a/test/components/views/dialogs/security/__snapshots__/CreateKeyBackupDialog-test.tsx.snap +++ b/test/components/views/dialogs/security/__snapshots__/CreateKeyBackupDialog-test.tsx.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`CreateKeyBackupDialog should display the error message when backup creation failed 1`] = ` +exports[`CreateKeyBackupDialog should display an error message when backup creation failed 1`] = `