From c263b173212b4fcd432628cacd574af2826d2c6d Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 2 Jan 2024 20:03:31 +0100 Subject: [PATCH 01/19] Ensure backup settings in playwright --- playwright/e2e/crypto/utils.ts | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/playwright/e2e/crypto/utils.ts b/playwright/e2e/crypto/utils.ts index 070e615e874..ce630359137 100644 --- a/playwright/e2e/crypto/utils.ts +++ b/playwright/e2e/crypto/utils.ts @@ -109,6 +109,28 @@ export async function checkDeviceIsConnectedKeyBackup(page: Page) { await page.getByRole("button", { name: "User menu" }).click(); await page.locator(".mx_UserMenu_contextMenu").getByRole("menuitem", { name: "Security & Privacy" }).click(); await expect(page.locator(".mx_Dialog").getByRole("button", { name: "Restore from Backup" })).toBeVisible(); + + // expand the advanced section to see the active version in the reports + const summary = page.locator(".mx_Dialog .mx_SettingsSubsection_content details .mx_SecureBackupPanel_advanced"); + await summary.locator("..").click(); + + const decryptionKeyCached = await page + .locator(".mx_Dialog .mx_SecureBackupPanel_statusList tr:nth-child(2) td") + .textContent(); + expect(decryptionKeyCached.trim()).toBe("cached locally, well formed"); + + // when initially setting up the key backup, the server version is 1, + // it should not change after that unless explicitly reset + const expectedBackupVersion = "1"; + const serverVersion = await page + .locator(".mx_Dialog .mx_SecureBackupPanel_statusList tr:nth-child(5) td") + .textContent(); + expect(serverVersion.trim()).toBe(expectedBackupVersion + " (Algorithm: m.megolm_backup.v1.curve25519-aes-sha2)"); + + const activeVersion = await page + .locator(".mx_Dialog .mx_SecureBackupPanel_statusList tr:nth-child(6) td") + .textContent(); + expect(activeVersion.trim()).toBe(expectedBackupVersion); } /** From c0cef0b9ba527ad9519759b0be8b34c7731348e3 Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 2 Jan 2024 20:26:49 +0100 Subject: [PATCH 02/19] Fix verification by pass causing backup reset --- src/SecurityManager.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/SecurityManager.ts b/src/SecurityManager.ts index f6fff3628e0..71fdc826c69 100644 --- a/src/SecurityManager.ts +++ b/src/SecurityManager.ts @@ -341,12 +341,12 @@ export async function withSecretStorageKeyCache(func: () => Promise): Prom * @param {Function} [func] An operation to perform once secret storage has been * bootstrapped. Optional. * @param {bool} [forceReset] Reset secret storage even if it's already set up - * @param {bool} [setupNewKeyBackup] Reset secret storage even if it's already set up + * @param {bool} [setupNewKeyBackup] Force setup a new server side room keys backup even if it's already set up */ export async function accessSecretStorage( func = async (): Promise => {}, forceReset = false, - setupNewKeyBackup = true, + setupNewKeyBackup = false, ): Promise { await withSecretStorageKeyCache(() => doAccessSecretStorage(func, forceReset, setupNewKeyBackup)); } From 1bc286ca8ca74557403ba67d6bf16d54018ae15b Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 3 Jan 2024 14:56:59 +0100 Subject: [PATCH 03/19] fix force backup setup by default --- playwright/e2e/crypto/backups.spec.ts | 47 ++++++++++++++++--- src/SecurityManager.ts | 16 ++----- .../security/CreateKeyBackupDialog.tsx | 22 ++++----- 3 files changed, 54 insertions(+), 31 deletions(-) diff --git a/playwright/e2e/crypto/backups.spec.ts b/playwright/e2e/crypto/backups.spec.ts index 847784bff22..c1c1ade5819 100644 --- a/playwright/e2e/crypto/backups.spec.ts +++ b/playwright/e2e/crypto/backups.spec.ts @@ -14,22 +14,31 @@ 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) { + const serverVersion = await page + .locator(".mx_Dialog .mx_SecureBackupPanel_statusList tr:nth-child(5) td") + .textContent(); + expect(serverVersion.trim()).toBe(version + " (Algorithm: m.megolm_backup.v1.curve25519-aes-sha2)"); + + const activeVersion = await page + .locator(".mx_Dialog .mx_SecureBackupPanel_statusList tr:nth-child(6) td") + .textContent(); + expect(activeVersion.trim()).toBe(version); +} + test.describe("Backups", () => { test.use({ displayName: "Hanako", }); test("Create, delete and recreate a keys backup", async ({ page, user, app }, workerInfo) => { - // skipIfLegacyCrypto - test.skip( - workerInfo.project.name === "Legacy Crypto", - "This test only works with Rust crypto. Deleting the backup seems to fail with legacy crypto.", - ); - // Create a backup const tab = await app.settings.openUserSettings("Security & Privacy"); + await expect(tab.getByRole("heading", { name: "Secure Backup" })).toBeVisible(); await tab.getByRole("button", { name: "Set up", exact: true }).click(); const dialog = await app.getDialogByTitle("Set up Secure Backup", 60000); @@ -41,9 +50,19 @@ test.describe("Backups", () => { await expect(dialog.getByRole("heading", { name: "Secure Backup successful" })).toBeVisible(); await dialog.getByRole("button", { name: "Done", exact: true }).click(); - // Delete it + // Open the settings again await app.settings.openUserSettings("Security & Privacy"); await expect(tab.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"); + + // Delete it await tab.getByRole("button", { name: "Delete Backup", exact: true }).click(); await dialog.getByTestId("dialog-primary-button").click(); // Click "Delete Backup" @@ -53,5 +72,19 @@ test.describe("Backups", () => { await dialog.getByRole("button", { name: "Continue", exact: true }).click(); await expect(dialog.getByRole("heading", { name: "Success!" })).toBeVisible(); await dialog.getByRole("button", { name: "OK", exact: true }).click(); + + // Open the settings again + await app.settings.openUserSettings("Security & Privacy"); + await expect(tab.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"); + + await page.pause(); }); }); diff --git a/src/SecurityManager.ts b/src/SecurityManager.ts index 71fdc826c69..ff8946614fd 100644 --- a/src/SecurityManager.ts +++ b/src/SecurityManager.ts @@ -341,22 +341,13 @@ export async function withSecretStorageKeyCache(func: () => Promise): Prom * @param {Function} [func] An operation to perform once secret storage has been * bootstrapped. Optional. * @param {bool} [forceReset] Reset secret storage even if it's already set up - * @param {bool} [setupNewKeyBackup] Force setup a new server side room keys backup even if it's already set up */ -export async function accessSecretStorage( - func = async (): Promise => {}, - forceReset = false, - setupNewKeyBackup = false, -): Promise { - await withSecretStorageKeyCache(() => doAccessSecretStorage(func, forceReset, setupNewKeyBackup)); +export async function accessSecretStorage(func = async (): Promise => {}, forceReset = false): Promise { + await withSecretStorageKeyCache(() => doAccessSecretStorage(func, forceReset)); } /** Helper for {@link #accessSecretStorage} */ -async function doAccessSecretStorage( - func: () => Promise, - forceReset: boolean, - setupNewKeyBackup: boolean, -): Promise { +async function doAccessSecretStorage(func: () => Promise, forceReset: boolean): Promise { try { const cli = MatrixClientPeg.safeGet(); if (!(await cli.hasSecretStorageKey()) || forceReset) { @@ -407,7 +398,6 @@ async function doAccessSecretStorage( }); await crypto.bootstrapSecretStorage({ getKeyBackupPassphrase: promptForBackupPassphrase, - setupNewKeyBackup, }); const keyId = Object.keys(secretStorageKeys)[0]; diff --git a/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx b/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx index 5a7e317dd28..fb8a50cbb31 100644 --- a/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx +++ b/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx @@ -76,23 +76,23 @@ export default class CreateKeyBackupDialog extends React.PureComponent => { + // Check if 4S already setup + const secretStorageAlreadySetup = await cli.hasSecretStorageKey(); + + await accessSecretStorage(async (): Promise => { + // The first time we create secret storage a backup version is automatically + // created, so we don't need to do anything here. + // If the user has already created the secret storage, we need to create a new + // backup version. + if (secretStorageAlreadySetup) { const crypto = cli.getCrypto(); if (!crypto) { throw new Error("End-to-end encryption is disabled - unable to create backup."); } await crypto.resetKeyBackup(); - }, - forceReset, - setupNewKeyBackup, - ); + } + }, forceReset); this.setState({ phase: Phase.Done, }); From 70091ad227c878b8c55693a7bcb3bbe3f9186550 Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 3 Jan 2024 15:12:34 +0100 Subject: [PATCH 04/19] fix test --- .../views/dialogs/security/CreateKeyBackupDialog-test.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/components/views/dialogs/security/CreateKeyBackupDialog-test.tsx b/test/components/views/dialogs/security/CreateKeyBackupDialog-test.tsx index faa466e3f5b..6cb0909b527 100644 --- a/test/components/views/dialogs/security/CreateKeyBackupDialog-test.tsx +++ b/test/components/views/dialogs/security/CreateKeyBackupDialog-test.tsx @@ -46,6 +46,7 @@ describe("CreateKeyBackupDialog", () => { it("should display an error message when backup creation failed", async () => { const matrixClient = createTestClient(); + mocked(matrixClient.hasSecretStorageKey).mockResolvedValue(true); mocked(matrixClient.getCrypto()!.resetKeyBackup).mockImplementation(() => { throw new Error("failed"); }); @@ -60,6 +61,7 @@ describe("CreateKeyBackupDialog", () => { 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; From 795e21449ca5fd00f54b6f998df9c91f6de38681 Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 3 Jan 2024 18:27:32 +0100 Subject: [PATCH 05/19] clarify when we need to bootstrap --- playwright/e2e/crypto/backups.spec.ts | 4 +-- .../security/CreateKeyBackupDialog.tsx | 35 +++++++++++-------- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/playwright/e2e/crypto/backups.spec.ts b/playwright/e2e/crypto/backups.spec.ts index c1c1ade5819..283b53ac789 100644 --- a/playwright/e2e/crypto/backups.spec.ts +++ b/playwright/e2e/crypto/backups.spec.ts @@ -20,12 +20,12 @@ import { test, expect } from "../../element-web-test"; async function expectBackupVersionToBe(page: Page, version: string) { const serverVersion = await page - .locator(".mx_Dialog .mx_SecureBackupPanel_statusList tr:nth-child(5) td") + .locator(".mx_SecureBackupPanel_statusList tr:nth-child(5) td") .textContent(); expect(serverVersion.trim()).toBe(version + " (Algorithm: m.megolm_backup.v1.curve25519-aes-sha2)"); const activeVersion = await page - .locator(".mx_Dialog .mx_SecureBackupPanel_statusList tr:nth-child(6) td") + .locator(".mx_SecureBackupPanel_statusList tr:nth-child(6) td") .textContent(); expect(activeVersion.trim()).toBe(version); } diff --git a/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx b/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx index fb8a50cbb31..e6723f3a8d9 100644 --- a/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx +++ b/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx @@ -20,7 +20,7 @@ import { logger } from "matrix-js-sdk/src/logger"; 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"; @@ -76,23 +76,28 @@ export default class CreateKeyBackupDialog extends React.PureComponent => { - // The first time we create secret storage a backup version is automatically - // created, so we don't need to do anything here. - // If the user has already created the secret storage, we need to create a new - // backup version. - if (secretStorageAlreadySetup) { - const crypto = cli.getCrypto(); - if (!crypto) { - throw new Error("End-to-end encryption is disabled - unable to create backup."); - } - await crypto.resetKeyBackup(); - } - }, forceReset); + if (!secretStorageAlreadySetup) { + // bootstrap secret storage, that will also create a backup version + await accessSecretStorage(async (): Promise => { + // do nothing, all is now setup correctly + }); + } else { + // Secret storage exists, we need to ensure that we can write to it before + // we create a new backup version. It ensures that we can write to it and keep it in sync. + await withSecretStorageKeyCache(async () => { + // this is the secret that will need to be updated, ensure that we can access it. + // This will ask the user to enter their passphrase/key. + await cli.secretStorage.get("m.megolm_backup.v1"); + // if we get here, we can access the secret storage, so we can create a backup version. + // We don't reset if we can't access the secret storage, as the secret storage will then + // have an outdated secret for backup that future sessions will not be able to use. + await cli.getCrypto()?.resetKeyBackup(); + }); + } + this.setState({ phase: Phase.Done, }); From d5a227ca8621820b062e883cf76a985eff28592b Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 3 Jan 2024 18:52:20 +0100 Subject: [PATCH 06/19] jslint --- playwright/e2e/crypto/backups.spec.ts | 8 ++------ .../views/dialogs/security/CreateKeyBackupDialog.tsx | 4 ++-- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/playwright/e2e/crypto/backups.spec.ts b/playwright/e2e/crypto/backups.spec.ts index 283b53ac789..a8d4016fba8 100644 --- a/playwright/e2e/crypto/backups.spec.ts +++ b/playwright/e2e/crypto/backups.spec.ts @@ -19,14 +19,10 @@ import { type Page } from "@playwright/test"; import { test, expect } from "../../element-web-test"; async function expectBackupVersionToBe(page: Page, version: string) { - const serverVersion = await page - .locator(".mx_SecureBackupPanel_statusList tr:nth-child(5) td") - .textContent(); + const serverVersion = await page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(5) td").textContent(); expect(serverVersion.trim()).toBe(version + " (Algorithm: m.megolm_backup.v1.curve25519-aes-sha2)"); - const activeVersion = await page - .locator(".mx_SecureBackupPanel_statusList tr:nth-child(6) td") - .textContent(); + const activeVersion = await page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(6) td").textContent(); expect(activeVersion.trim()).toBe(version); } diff --git a/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx b/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx index e6723f3a8d9..86980e57a02 100644 --- a/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx +++ b/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx @@ -20,7 +20,7 @@ import { logger } from "matrix-js-sdk/src/logger"; import { MatrixClientPeg } from "../../../../MatrixClientPeg"; import { _t } from "../../../../languageHandler"; -import {accessSecretStorage, withSecretStorageKeyCache} 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"; @@ -76,7 +76,7 @@ export default class CreateKeyBackupDialog extends React.PureComponent Date: Thu, 4 Jan 2024 17:13:34 +0100 Subject: [PATCH 07/19] post merge fix --- playwright/e2e/crypto/backups.spec.ts | 8 ++++---- playwright/pages/ElementAppPage.ts | 13 +++++++++++++ src/SecurityManager.ts | 9 +++++++-- .../dialogs/security/CreateKeyBackupDialog.tsx | 5 ----- 4 files changed, 24 insertions(+), 11 deletions(-) diff --git a/playwright/e2e/crypto/backups.spec.ts b/playwright/e2e/crypto/backups.spec.ts index a8d4016fba8..8e00ec7ba32 100644 --- a/playwright/e2e/crypto/backups.spec.ts +++ b/playwright/e2e/crypto/backups.spec.ts @@ -19,11 +19,11 @@ import { type Page } from "@playwright/test"; import { test, expect } from "../../element-web-test"; async function expectBackupVersionToBe(page: Page, version: string) { - const serverVersion = await page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(5) td").textContent(); - expect(serverVersion.trim()).toBe(version + " (Algorithm: m.megolm_backup.v1.curve25519-aes-sha2)"); + await expect(page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(5) td")).toHaveText( + version + " (Algorithm: m.megolm_backup.v1.curve25519-aes-sha2)", + ); - const activeVersion = await page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(6) td").textContent(); - expect(activeVersion.trim()).toBe(version); + await expect(page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(6) td")).toHaveText(version); } test.describe("Backups", () => { diff --git a/playwright/pages/ElementAppPage.ts b/playwright/pages/ElementAppPage.ts index 9a5de3b1480..31170f81f4f 100644 --- a/playwright/pages/ElementAppPage.ts +++ b/playwright/pages/ElementAppPage.ts @@ -51,6 +51,19 @@ export class ElementAppPage { return this.settings.closeDialog(); } + public async getClipboard(): Promise { + return await this.page.evaluate(() => navigator.clipboard.readText()); + } + + /** + * Find an open dialog by its title + */ + public async getDialogByTitle(title: string, timeout = 5000): Promise { + const dialog = this.page.locator(".mx_Dialog"); + await dialog.getByRole("heading", { name: title }).waitFor({ timeout }); + return dialog; + } + /** * 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 9266a5e297d..ff8946614fd 100644 --- a/src/SecurityManager.ts +++ b/src/SecurityManager.ts @@ -378,7 +378,12 @@ async function doAccessSecretStorage(func: () => Promise, forceReset: bool 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 @@ async function doAccessSecretStorage(func: () => Promise, forceReset: bool } }, }); - 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 2c51762a794..86980e57a02 100644 --- a/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx +++ b/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx @@ -17,7 +17,6 @@ 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"; @@ -75,7 +74,6 @@ export default class CreateKeyBackupDialog extends React.PureComponent Date: Thu, 4 Jan 2024 17:46:47 +0100 Subject: [PATCH 08/19] post rebase missing files --- playwright/element-web-test.ts | 4 +++ test/SecurityManager-test.ts | 62 ++++++++++++++++++++++++++++++++++ test/test-utils/test-utils.ts | 1 + 3 files changed, 67 insertions(+) create mode 100644 test/SecurityManager-test.ts diff --git a/playwright/element-web-test.ts b/playwright/element-web-test.ts index 9a3c7a36da7..5a28518878e 100644 --- a/playwright/element-web-test.ts +++ b/playwright/element-web-test.ts @@ -293,3 +293,7 @@ export const expect = baseExpect.extend({ return { pass: true, message: () => "", name: "toMatchScreenshot" }; }, }); + +test.use({ + permissions: ["clipboard-read"], +}); diff --git a/test/SecurityManager-test.ts b/test/SecurityManager-test.ts new file mode 100644 index 00000000000..15d1eb1deca --- /dev/null +++ b/test/SecurityManager-test.ts @@ -0,0 +1,62 @@ +/* +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 { mocked } from "jest-mock"; +import { CryptoApi } from "matrix-js-sdk/src/crypto-api"; + +import { accessSecretStorage } from "../src/SecurityManager"; +import { filterConsole, stubClient } from "./test-utils"; + +describe("SecurityManager", () => { + 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/test-utils/test-utils.ts b/test/test-utils/test-utils.ts index c0cb061321b..90bd5f63d13 100644 --- a/test/test-utils/test-utils.ts +++ b/test/test-utils/test-utils.ts @@ -132,6 +132,7 @@ export function createTestClient(): MatrixClient { getUserDeviceInfo: jest.fn(), getUserVerificationStatus: jest.fn(), getDeviceVerificationStatus: jest.fn(), + resetKeyBackup: jest.fn(), }), getPushActionsForEvent: jest.fn(), From 602d2b21327e53c54800ceb7ad94f8136a913d88 Mon Sep 17 00:00:00 2001 From: Valere Date: Fri, 5 Jan 2024 14:18:36 +0100 Subject: [PATCH 09/19] fix bad merge --- .../security/CreateKeyBackupDialog-test.tsx | 42 ++++++------- .../CreateKeyBackupDialog-test.tsx.snap | 60 +++++++++++++++++++ 2 files changed, 78 insertions(+), 24 deletions(-) diff --git a/test/components/views/dialogs/security/CreateKeyBackupDialog-test.tsx b/test/components/views/dialogs/security/CreateKeyBackupDialog-test.tsx index 3178782a0d1..5f736da76ef 100644 --- a/test/components/views/dialogs/security/CreateKeyBackupDialog-test.tsx +++ b/test/components/views/dialogs/security/CreateKeyBackupDialog-test.tsx @@ -39,37 +39,31 @@ 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; - it("should display an error message when backup creation failed", async () => { - const matrixClient = createTestClient(); - mocked(matrixClient.hasSecretStorageKey).mockResolvedValue(true); - mocked(matrixClient.getCrypto()!.resetKeyBackup).mockImplementation(() => { - throw new Error("failed"); - }); - MatrixClientPeg.safeGet = MatrixClientPeg.get = () => matrixClient; - - const { asFragment } = render(); + const { asFragment } = render(); - // Check if the error message is displayed - await waitFor(() => expect(screen.getByText("Unable to create key backup")).toBeDefined()); - expect(asFragment()).toMatchSnapshot(); - }); + // 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(); - mocked(matrixClient.hasSecretStorageKey).mockResolvedValue(true); - mocked(matrixClient.getCrypto).mockReturnValue(undefined); - MatrixClientPeg.safeGet = MatrixClientPeg.get = () => matrixClient; + 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(); + render(); - // Check if the error message is displayed - await waitFor(() => expect(screen.getByText("Unable to create key backup")).toBeDefined()); - }); + // 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 () => { 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..db4b3cb6d5c 100644 --- a/test/components/views/dialogs/security/__snapshots__/CreateKeyBackupDialog-test.tsx.snap +++ b/test/components/views/dialogs/security/__snapshots__/CreateKeyBackupDialog-test.tsx.snap @@ -1,5 +1,65 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`CreateKeyBackupDialog should display an error message when backup creation failed 1`] = ` + +
+ +
+ +`; + exports[`CreateKeyBackupDialog should display the error message when backup creation failed 1`] = `
Date: Fri, 5 Jan 2024 15:51:25 +0100 Subject: [PATCH 10/19] update test --- .../views/dialogs/security/CreateKeyBackupDialog.tsx | 7 ++++++- .../views/dialogs/security/CreateKeyBackupDialog-test.tsx | 1 + test/test-utils/test-utils.ts | 5 +++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx b/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx index 86980e57a02..0d5bf097ce4 100644 --- a/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx +++ b/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx @@ -88,13 +88,18 @@ export default class CreateKeyBackupDialog extends React.PureComponent { + const crypto = cli.getCrypto(); + if (!crypto) { + throw new Error("End-to-end encryption is disabled - unable to create backup."); + } + // this is the secret that will need to be updated, ensure that we can access it. // This will ask the user to enter their passphrase/key. await cli.secretStorage.get("m.megolm_backup.v1"); // if we get here, we can access the secret storage, so we can create a backup version. // We don't reset if we can't access the secret storage, as the secret storage will then // have an outdated secret for backup that future sessions will not be able to use. - await cli.getCrypto()?.resetKeyBackup(); + await crypto.resetKeyBackup(); }); } diff --git a/test/components/views/dialogs/security/CreateKeyBackupDialog-test.tsx b/test/components/views/dialogs/security/CreateKeyBackupDialog-test.tsx index 5f736da76ef..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", () => { diff --git a/test/test-utils/test-utils.ts b/test/test-utils/test-utils.ts index 90bd5f63d13..a62b7c2fa9b 100644 --- a/test/test-utils/test-utils.ts +++ b/test/test-utils/test-utils.ts @@ -56,6 +56,7 @@ import { ValidatedServerConfig } from "../../src/utils/ValidatedServerConfig"; import { EnhancedMap } from "../../src/utils/maps"; import { AsyncStoreWithClient } from "../../src/stores/AsyncStoreWithClient"; import MatrixClientBackedSettingsHandler from "../../src/settings/handlers/MatrixClientBackedSettingsHandler"; +import { ServerSideSecretStorage } from "../../../matrix-js-sdk/src/secret-storage"; /** * Stub out the MatrixClient, and configure the MatrixClientPeg object to @@ -116,6 +117,10 @@ export function createTestClient(): MatrixClient { bootstrapCrossSigning: jest.fn(), hasSecretStorageKey: jest.fn(), + secretStorage: { + get: jest.fn(), + } as unknown as ServerSideSecretStorage, + store: { getPendingEvents: jest.fn().mockResolvedValue([]), setPendingEvents: jest.fn().mockResolvedValue(undefined), From 5e5d5ef4a2ed9764931da7f92b9e2f22e2161426 Mon Sep 17 00:00:00 2001 From: Valere Date: Fri, 5 Jan 2024 16:04:12 +0100 Subject: [PATCH 11/19] Fix import --- test/test-utils/test-utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test-utils/test-utils.ts b/test/test-utils/test-utils.ts index a62b7c2fa9b..37e2b9e0ed2 100644 --- a/test/test-utils/test-utils.ts +++ b/test/test-utils/test-utils.ts @@ -49,6 +49,7 @@ import { MapperOpts } from "matrix-js-sdk/src/event-mapper"; import { MatrixRTCSessionManager } from "matrix-js-sdk/src/matrixrtc/MatrixRTCSessionManager"; // eslint-disable-next-line no-restricted-imports import { MatrixRTCSession } from "matrix-js-sdk/src/matrixrtc/MatrixRTCSession"; +import { ServerSideSecretStorage } from "matrix-js-sdk/src/secret-storage"; import type { GroupCall } from "matrix-js-sdk/src/matrix"; import { MatrixClientPeg as peg } from "../../src/MatrixClientPeg"; @@ -56,7 +57,6 @@ import { ValidatedServerConfig } from "../../src/utils/ValidatedServerConfig"; import { EnhancedMap } from "../../src/utils/maps"; import { AsyncStoreWithClient } from "../../src/stores/AsyncStoreWithClient"; import MatrixClientBackedSettingsHandler from "../../src/settings/handlers/MatrixClientBackedSettingsHandler"; -import { ServerSideSecretStorage } from "../../../matrix-js-sdk/src/secret-storage"; /** * Stub out the MatrixClient, and configure the MatrixClientPeg object to From 08da26f94426db409deb682721496952f6779058 Mon Sep 17 00:00:00 2001 From: Valere Date: Fri, 5 Jan 2024 16:58:44 +0100 Subject: [PATCH 12/19] test user forgot passkey --- playwright/e2e/crypto/backups.spec.ts | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/playwright/e2e/crypto/backups.spec.ts b/playwright/e2e/crypto/backups.spec.ts index 8e00ec7ba32..568cbef3060 100644 --- a/playwright/e2e/crypto/backups.spec.ts +++ b/playwright/e2e/crypto/backups.spec.ts @@ -64,7 +64,7 @@ test.describe("Backups", () => { // Create another await tab.getByRole("button", { name: "Set up", exact: true }).click(); - dialog.getByLabel("Security Key").fill(securityKey); + await dialog.getByLabel("Security Key").fill(securityKey); await dialog.getByRole("button", { name: "Continue", exact: true }).click(); await expect(dialog.getByRole("heading", { name: "Success!" })).toBeVisible(); await dialog.getByRole("button", { name: "OK", exact: true }).click(); @@ -81,6 +81,29 @@ test.describe("Backups", () => { await expectBackupVersionToBe(page, "2"); - await page.pause(); + // == + // Ensure that if you don't have the secret storage passphrase the backup won't be created + // == + + // First delete version 2 + await tab.getByRole("button", { name: "Delete Backup", exact: true }).click(); + await dialog.getByTestId("dialog-primary-button").click(); // Click "Delete Backup" + + // Try to create another + await tab.getByRole("button", { name: "Set up", exact: true }).click(); + // But cancel the security key dialog, to simulate not having the secret storage passphrase + await dialog.getByTestId("dialog-cancel-button").click(); + + const dialog2 = await app.getDialogByTitle("Starting backup…", 60000); + // check that it failed + await expect(dialog2.getByText("Unable to create key backup")).toBeVisible(); + + // cancel + await dialog2.getByTestId("dialog-cancel-button").click(); + + // go back to the settings to check that no backup was created (the setup button should still be there) + const tab2 = await app.settings.openUserSettings("Security & Privacy"); + + await expect(tab2.getByRole("button", { name: "Set up", exact: true })).toBeVisible(); }); }); From 6fe094006648230b19c6c4acbd631e96efac193c Mon Sep 17 00:00:00 2001 From: Valere Date: Fri, 5 Jan 2024 17:42:47 +0100 Subject: [PATCH 13/19] better usage of locator --- playwright/e2e/crypto/backups.spec.ts | 63 +++++++++++++++++---------- 1 file changed, 41 insertions(+), 22 deletions(-) diff --git a/playwright/e2e/crypto/backups.spec.ts b/playwright/e2e/crypto/backups.spec.ts index 568cbef3060..adc8458a58e 100644 --- a/playwright/e2e/crypto/backups.spec.ts +++ b/playwright/e2e/crypto/backups.spec.ts @@ -37,12 +37,19 @@ test.describe("Backups", () => { await expect(tab.getByRole("heading", { name: "Secure Backup" })).toBeVisible(); await tab.getByRole("button", { name: "Set up", exact: true }).click(); - const dialog = await app.getDialogByTitle("Set up Secure Backup", 60000); + const dialog = page.locator(".mx_Dialog"); + + // It's the first time and secure storage not setup, so it will create one + await expect(dialog.getByRole("heading", { name: "Set up Secure Backup" })).toBeVisible(); await dialog.getByRole("button", { name: "Continue", exact: true }).click(); + await expect(dialog.getByRole("heading", { name: "Save your Security Key" })).toBeVisible(); await dialog.getByRole("button", { name: "Copy", exact: true }).click(); + + // copy the recovery key to use it later const securityKey = await app.getClipboard(); await dialog.getByRole("button", { name: "Continue", exact: true }).click(); + await expect(dialog.getByRole("heading", { name: "Secure Backup successful" })).toBeVisible(); await dialog.getByRole("button", { name: "Done", exact: true }).click(); @@ -58,16 +65,24 @@ test.describe("Backups", () => { await expectBackupVersionToBe(page, "1"); - // Delete it await tab.getByRole("button", { name: "Delete Backup", exact: true }).click(); - await dialog.getByTestId("dialog-primary-button").click(); // Click "Delete Backup" + { + const dialog = await app.getDialogByTitle("Delete Backup", 60000); + // Delete it + await dialog.getByTestId("dialog-primary-button").click(); // Click "Delete Backup" - // Create another - await tab.getByRole("button", { name: "Set up", exact: true }).click(); - await dialog.getByLabel("Security Key").fill(securityKey); - await dialog.getByRole("button", { name: "Continue", exact: true }).click(); - await expect(dialog.getByRole("heading", { name: "Success!" })).toBeVisible(); - await dialog.getByRole("button", { name: "OK", exact: true }).click(); + // Create another + await tab.getByRole("button", { name: "Set up", exact: true }).click(); + + const dialog2 = await app.getDialogByTitle("Security Key", 60000); + + await dialog2.getByLabel("Security Key").fill(securityKey); + await dialog2.getByRole("button", { name: "Continue", exact: true }).click(); + + const dialog3 = await app.getDialogByTitle("Success!", 60000); + + await dialog3.getByRole("button", { name: "OK", exact: true }).click(); + } // Open the settings again await app.settings.openUserSettings("Security & Privacy"); @@ -87,23 +102,27 @@ test.describe("Backups", () => { // First delete version 2 await tab.getByRole("button", { name: "Delete Backup", exact: true }).click(); - await dialog.getByTestId("dialog-primary-button").click(); // Click "Delete Backup" + { + const dialog = await app.getDialogByTitle("Delete Backup", 60000); + await dialog.getByTestId("dialog-primary-button").click(); // Click "Delete Backup" - // Try to create another - await tab.getByRole("button", { name: "Set up", exact: true }).click(); - // But cancel the security key dialog, to simulate not having the secret storage passphrase - await dialog.getByTestId("dialog-cancel-button").click(); + // Try to create another + await tab.getByRole("button", { name: "Set up", exact: true }).click(); - const dialog2 = await app.getDialogByTitle("Starting backup…", 60000); - // check that it failed - await expect(dialog2.getByText("Unable to create key backup")).toBeVisible(); + const dialog2 = await app.getDialogByTitle("Security Key", 60000); + // But cancel the security key dialog, to simulate not having the secret storage passphrase + await dialog2.getByTestId("dialog-cancel-button").click(); - // cancel - await dialog2.getByTestId("dialog-cancel-button").click(); + const dialog3 = await app.getDialogByTitle("Starting backup…", 60000); + // check that it failed + await expect(dialog3.getByText("Unable to create key backup")).toBeVisible(); - // go back to the settings to check that no backup was created (the setup button should still be there) - const tab2 = await app.settings.openUserSettings("Security & Privacy"); + // cancel + await dialog3.getByTestId("dialog-cancel-button").click(); - await expect(tab2.getByRole("button", { name: "Set up", exact: true })).toBeVisible(); + // go back to the settings to check that no backup was created (the setup button should still be there) + const tab2 = await app.settings.openUserSettings("Security & Privacy"); + await expect(tab2.getByRole("button", { name: "Set up", exact: true })).toBeVisible(); + } }); }); From a726c5dbe272ea3ee4c4c1c783bad9fee8eec2ae Mon Sep 17 00:00:00 2001 From: Valere Date: Mon, 8 Jan 2024 09:29:47 +0100 Subject: [PATCH 14/19] fix snapshot --- .../CreateKeyBackupDialog-test.tsx.snap | 60 ------------------- 1 file changed, 60 deletions(-) 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 db4b3cb6d5c..d3880d875c0 100644 --- a/test/components/views/dialogs/security/__snapshots__/CreateKeyBackupDialog-test.tsx.snap +++ b/test/components/views/dialogs/security/__snapshots__/CreateKeyBackupDialog-test.tsx.snap @@ -60,66 +60,6 @@ exports[`CreateKeyBackupDialog should display an error message when backup creat `; -exports[`CreateKeyBackupDialog should display the error message when backup creation failed 1`] = ` - -
- -
- -`; - exports[`CreateKeyBackupDialog should display the spinner when creating backup 1`] = `
Date: Tue, 9 Jan 2024 10:22:28 +0100 Subject: [PATCH 15/19] remove getDialogByTitle --- playwright/e2e/crypto/backups.spec.ts | 80 ++++++++++++++------------- playwright/pages/ElementAppPage.ts | 9 --- 2 files changed, 42 insertions(+), 47 deletions(-) diff --git a/playwright/e2e/crypto/backups.spec.ts b/playwright/e2e/crypto/backups.spec.ts index adc8458a58e..2b162581dca 100644 --- a/playwright/e2e/crypto/backups.spec.ts +++ b/playwright/e2e/crypto/backups.spec.ts @@ -36,22 +36,26 @@ test.describe("Backups", () => { const tab = await app.settings.openUserSettings("Security & Privacy"); await expect(tab.getByRole("heading", { name: "Secure Backup" })).toBeVisible(); + + let securityKey = ""; + + const currentDialogLocator = page.locator(".mx_Dialog"); + await tab.getByRole("button", { name: "Set up", exact: true }).click(); - const dialog = page.locator(".mx_Dialog"); // It's the first time and secure storage not setup, so it will create one - await expect(dialog.getByRole("heading", { name: "Set up Secure Backup" })).toBeVisible(); - await dialog.getByRole("button", { name: "Continue", exact: true }).click(); + await expect(currentDialogLocator.getByRole("heading", { name: "Set up Secure Backup" })).toBeVisible(); + await currentDialogLocator.getByRole("button", { name: "Continue", exact: true }).click(); - await expect(dialog.getByRole("heading", { name: "Save your Security Key" })).toBeVisible(); - await dialog.getByRole("button", { name: "Copy", 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 dialog.getByRole("button", { name: "Continue", exact: true }).click(); + securityKey = await app.getClipboard(); + await currentDialogLocator.getByRole("button", { name: "Continue", exact: true }).click(); - await expect(dialog.getByRole("heading", { name: "Secure Backup successful" })).toBeVisible(); - await dialog.getByRole("button", { name: "Done", 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"); @@ -66,23 +70,23 @@ test.describe("Backups", () => { await expectBackupVersionToBe(page, "1"); await tab.getByRole("button", { name: "Delete Backup", exact: true }).click(); - { - const dialog = await app.getDialogByTitle("Delete Backup", 60000); - // Delete it - await dialog.getByTestId("dialog-primary-button").click(); // Click "Delete Backup" - // Create another - await tab.getByRole("button", { name: "Set up", exact: true }).click(); + await expect(currentDialogLocator.getByRole("heading", { name: "Delete Backup" })).toBeVisible(); - const dialog2 = await app.getDialogByTitle("Security Key", 60000); + // Delete it + await currentDialogLocator.getByTestId("dialog-primary-button").click(); // Click "Delete Backup" + + // Create another + await tab.getByRole("button", { name: "Set up", exact: true }).click(); - await dialog2.getByLabel("Security Key").fill(securityKey); - await dialog2.getByRole("button", { name: "Continue", exact: true }).click(); + await expect(currentDialogLocator.getByRole("heading", { name: "Security Key" })).toBeVisible(); - const dialog3 = await app.getDialogByTitle("Success!", 60000); + await currentDialogLocator.getByLabel("Security Key").fill(securityKey); + await currentDialogLocator.getByRole("button", { name: "Continue", exact: true }).click(); - await dialog3.getByRole("button", { name: "OK", exact: true }).click(); - } + 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"); @@ -102,27 +106,27 @@ test.describe("Backups", () => { // First delete version 2 await tab.getByRole("button", { name: "Delete Backup", exact: true }).click(); - { - const dialog = await app.getDialogByTitle("Delete Backup", 60000); - await dialog.getByTestId("dialog-primary-button").click(); // Click "Delete Backup" - // Try to create another - await tab.getByRole("button", { name: "Set up", exact: true }).click(); + await expect(currentDialogLocator.getByRole("heading", { name: "Delete Backup" })).toBeVisible(); + await currentDialogLocator.getByTestId("dialog-primary-button").click(); // Click "Delete Backup" + + // Try to create another + await tab.getByRole("button", { name: "Set up", exact: true }).click(); + + await expect(currentDialogLocator.getByRole("heading", { name: "Security Key" })).toBeVisible(); - const dialog2 = await app.getDialogByTitle("Security Key", 60000); - // But cancel the security key dialog, to simulate not having the secret storage passphrase - await dialog2.getByTestId("dialog-cancel-button").click(); + // But cancel the security key dialog, to simulate not having the secret storage passphrase + await currentDialogLocator.getByTestId("dialog-cancel-button").click(); - const dialog3 = await app.getDialogByTitle("Starting backup…", 60000); - // check that it failed - await expect(dialog3.getByText("Unable to create key backup")).toBeVisible(); + 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 dialog3.getByTestId("dialog-cancel-button").click(); + // 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) - const tab2 = await app.settings.openUserSettings("Security & Privacy"); - await expect(tab2.getByRole("button", { name: "Set up", exact: true })).toBeVisible(); - } + // go back to the settings to check that no backup was created (the setup button should still be there) + const tab2 = await app.settings.openUserSettings("Security & Privacy"); + await expect(tab2.getByRole("button", { name: "Set up", exact: true })).toBeVisible(); }); }); diff --git a/playwright/pages/ElementAppPage.ts b/playwright/pages/ElementAppPage.ts index 31170f81f4f..be80cf62808 100644 --- a/playwright/pages/ElementAppPage.ts +++ b/playwright/pages/ElementAppPage.ts @@ -55,15 +55,6 @@ export class ElementAppPage { return await this.page.evaluate(() => navigator.clipboard.readText()); } - /** - * Find an open dialog by its title - */ - public async getDialogByTitle(title: string, timeout = 5000): Promise { - const dialog = this.page.locator(".mx_Dialog"); - await dialog.getByRole("heading", { name: title }).waitFor({ timeout }); - return dialog; - } - /** * 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 From 5114eb4ae3263f31d59ff56091b9cd11041c0246 Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 9 Jan 2024 10:27:23 +0100 Subject: [PATCH 16/19] Update src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- .../dialogs/security/CreateKeyBackupDialog.tsx | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx b/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx index 0d5bf097ce4..f0d5856da5f 100644 --- a/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx +++ b/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx @@ -93,12 +93,15 @@ export default class CreateKeyBackupDialog extends React.PureComponent Date: Tue, 9 Jan 2024 10:40:34 +0100 Subject: [PATCH 17/19] unneeded permission --- playwright/element-web-test.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/playwright/element-web-test.ts b/playwright/element-web-test.ts index 5a28518878e..9a3c7a36da7 100644 --- a/playwright/element-web-test.ts +++ b/playwright/element-web-test.ts @@ -293,7 +293,3 @@ export const expect = baseExpect.extend({ return { pass: true, message: () => "", name: "toMatchScreenshot" }; }, }); - -test.use({ - permissions: ["clipboard-read"], -}); From 0b826470371d7ed6731e7ca32c1066fc2adc5756 Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 9 Jan 2024 15:21:05 +0100 Subject: [PATCH 18/19] code review --- playwright/e2e/crypto/backups.spec.ts | 44 ++++++++++----------------- test/test-utils/test-utils.ts | 3 +- 2 files changed, 17 insertions(+), 30 deletions(-) diff --git a/playwright/e2e/crypto/backups.spec.ts b/playwright/e2e/crypto/backups.spec.ts index 2b162581dca..75c8e3eed75 100644 --- a/playwright/e2e/crypto/backups.spec.ts +++ b/playwright/e2e/crypto/backups.spec.ts @@ -33,25 +33,20 @@ test.describe("Backups", () => { test("Create, delete and recreate a keys backup", async ({ page, user, app }, workerInfo) => { // Create a backup - const tab = await app.settings.openUserSettings("Security & Privacy"); + const securityTab = await app.settings.openUserSettings("Security & Privacy"); - await expect(tab.getByRole("heading", { name: "Secure Backup" })).toBeVisible(); - - let securityKey = ""; + 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"); - await tab.getByRole("button", { name: "Set up", exact: true }).click(); - - // It's the first time and secure storage not setup, so it will create one + // 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 - securityKey = await app.getClipboard(); + const securityKey = await app.getClipboard(); await currentDialogLocator.getByRole("button", { name: "Continue", exact: true }).click(); await expect(currentDialogLocator.getByRole("heading", { name: "Secure Backup successful" })).toBeVisible(); @@ -59,7 +54,7 @@ test.describe("Backups", () => { // Open the settings again await app.settings.openUserSettings("Security & Privacy"); - await expect(tab.getByRole("heading", { name: "Secure Backup" })).toBeVisible(); + await expect(securityTab.getByRole("heading", { name: "Secure Backup" })).toBeVisible(); // expand the advanced section to see the active version in the reports await page @@ -69,28 +64,24 @@ test.describe("Backups", () => { await expectBackupVersionToBe(page, "1"); - await tab.getByRole("button", { name: "Delete Backup", exact: true }).click(); - + 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 tab.getByRole("button", { name: "Set up", exact: true }).click(); - + 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(tab.getByRole("heading", { name: "Secure Backup" })).toBeVisible(); + await expect(securityTab.getByRole("heading", { name: "Secure Backup" })).toBeVisible(); // expand the advanced section to see the active version in the reports await page @@ -105,28 +96,25 @@ test.describe("Backups", () => { // == // First delete version 2 - await tab.getByRole("button", { name: "Delete Backup", exact: true }).click(); - + await securityTab.getByRole("button", { name: "Delete Backup", exact: true }).click(); await expect(currentDialogLocator.getByRole("heading", { name: "Delete Backup" })).toBeVisible(); - await currentDialogLocator.getByTestId("dialog-primary-button").click(); // Click "Delete Backup" + // Click "Delete Backup" + await currentDialogLocator.getByTestId("dialog-primary-button").click(); // Try to create another - await tab.getByRole("button", { name: "Set up", exact: true }).click(); - + 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) - const tab2 = await app.settings.openUserSettings("Security & Privacy"); - await expect(tab2.getByRole("button", { name: "Set up", exact: true })).toBeVisible(); + await app.settings.openUserSettings("Security & Privacy"); + await expect(securityTab.getByRole("button", { name: "Set up", exact: true })).toBeVisible(); }); }); diff --git a/test/test-utils/test-utils.ts b/test/test-utils/test-utils.ts index 37e2b9e0ed2..7d072a31ca9 100644 --- a/test/test-utils/test-utils.ts +++ b/test/test-utils/test-utils.ts @@ -49,7 +49,6 @@ import { MapperOpts } from "matrix-js-sdk/src/event-mapper"; import { MatrixRTCSessionManager } from "matrix-js-sdk/src/matrixrtc/MatrixRTCSessionManager"; // eslint-disable-next-line no-restricted-imports import { MatrixRTCSession } from "matrix-js-sdk/src/matrixrtc/MatrixRTCSession"; -import { ServerSideSecretStorage } from "matrix-js-sdk/src/secret-storage"; import type { GroupCall } from "matrix-js-sdk/src/matrix"; import { MatrixClientPeg as peg } from "../../src/MatrixClientPeg"; @@ -119,7 +118,7 @@ export function createTestClient(): MatrixClient { secretStorage: { get: jest.fn(), - } as unknown as ServerSideSecretStorage, + }, store: { getPendingEvents: jest.fn().mockResolvedValue([]), From 2ae60bccad98e332de9ac4224f4bc25d7929ea78 Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 9 Jan 2024 15:36:31 +0100 Subject: [PATCH 19/19] cleaning --- .../views/dialogs/security/CreateKeyBackupDialog.tsx | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx b/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx index f0d5856da5f..1dd280d085b 100644 --- a/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx +++ b/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx @@ -80,13 +80,11 @@ export default class CreateKeyBackupDialog extends React.PureComponent => { - // do nothing, all is now setup correctly + // do nothing, all is now set up correctly }); } else { - // Secret storage exists, we need to ensure that we can write to it before - // we create a new backup version. It ensures that we can write to it and keep it in sync. await withSecretStorageKeyCache(async () => { const crypto = cli.getCrypto(); if (!crypto) {