From fa6285b8648d12d0532fe9ed29bafee281cecfdd Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 3 Jan 2024 16:43:37 +0100 Subject: [PATCH 1/6] Revert "Set up key backup using non-deprecated APIs (#12005)" This reverts commit df11b90fd658167003410d8df4c60fb54a059c91. --- playwright/e2e/crypto/backups.spec.ts | 57 ----------------- playwright/element-web-test.ts | 4 -- playwright/pages/ElementAppPage.ts | 13 ---- src/SecurityManager.ts | 27 ++------ .../security/CreateKeyBackupDialog.tsx | 37 ++++++----- test/SecurityManager-test.ts | 62 ------------------- .../security/CreateKeyBackupDialog-test.tsx | 39 +++--------- .../CreateKeyBackupDialog-test.tsx.snap | 2 +- test/test-utils/test-utils.ts | 1 - 9 files changed, 35 insertions(+), 207 deletions(-) delete mode 100644 playwright/e2e/crypto/backups.spec.ts delete mode 100644 test/SecurityManager-test.ts diff --git a/playwright/e2e/crypto/backups.spec.ts b/playwright/e2e/crypto/backups.spec.ts deleted file mode 100644 index 847784bff22..00000000000 --- a/playwright/e2e/crypto/backups.spec.ts +++ /dev/null @@ -1,57 +0,0 @@ -/* -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 { test, expect } from "../../element-web-test"; - -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); - 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(); - 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(); - - // Delete it - await app.settings.openUserSettings("Security & Privacy"); - await expect(tab.getByRole("heading", { name: "Secure Backup" })).toBeVisible(); - await tab.getByRole("button", { name: "Delete Backup", exact: true }).click(); - await dialog.getByTestId("dialog-primary-button").click(); // Click "Delete Backup" - - // Create another - await tab.getByRole("button", { name: "Set up", exact: true }).click(); - 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(); - }); -}); 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"], -}); diff --git a/playwright/pages/ElementAppPage.ts b/playwright/pages/ElementAppPage.ts index 31170f81f4f..9a5de3b1480 100644 --- a/playwright/pages/ElementAppPage.ts +++ b/playwright/pages/ElementAppPage.ts @@ -51,19 +51,6 @@ 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 f6fff3628e0..58a381aeceb 100644 --- a/src/SecurityManager.ts +++ b/src/SecurityManager.ts @@ -341,22 +341,9 @@ 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 */ -export async function accessSecretStorage( - func = async (): Promise => {}, - forceReset = false, - setupNewKeyBackup = true, -): Promise { - await withSecretStorageKeyCache(() => doAccessSecretStorage(func, forceReset, setupNewKeyBackup)); -} - -/** Helper for {@link #accessSecretStorage} */ -async function doAccessSecretStorage( - func: () => Promise, - forceReset: boolean, - setupNewKeyBackup: boolean, -): Promise { +export async function accessSecretStorage(func = async (): Promise => {}, forceReset = false): Promise { + secretStorageBeingAccessed = true; try { const cli = MatrixClientPeg.safeGet(); if (!(await cli.hasSecretStorageKey()) || forceReset) { @@ -387,12 +374,7 @@ async function doAccessSecretStorage( throw new Error("Secret storage creation canceled"); } } else { - const crypto = cli.getCrypto(); - if (!crypto) { - throw new Error("End-to-end encryption is disabled - unable to access secret storage."); - } - - await crypto.bootstrapCrossSigning({ + await cli.bootstrapCrossSigning({ authUploadDeviceSigningKeys: async (makeRequest): Promise => { const { finished } = Modal.createDialog(InteractiveAuthDialog, { title: _t("encryption|bootstrap_title"), @@ -405,9 +387,8 @@ async function doAccessSecretStorage( } }, }); - await crypto.bootstrapSecretStorage({ + await cli.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..453a9dc84c4 100644 --- a/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx +++ b/src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx @@ -17,6 +17,7 @@ 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"; @@ -74,25 +75,24 @@ 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."); - } - await crypto.resetKeyBackup(); - }, - forceReset, - setupNewKeyBackup, - ); + await accessSecretStorage(async (): Promise => { + // `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, + }); + info = await cli.createKeyBackupVersion(info); + }); + await cli.scheduleAllGroupSessionsForBackup(); this.setState({ phase: Phase.Done, }); @@ -102,6 +102,9 @@ 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 faa466e3f5b..b0d461b8f8d 100644 --- a/test/components/views/dialogs/security/CreateKeyBackupDialog-test.tsx +++ b/test/components/views/dialogs/security/CreateKeyBackupDialog-test.tsx @@ -19,13 +19,11 @@ import React from "react"; import { mocked } from "jest-mock"; import CreateKeyBackupDialog from "../../../../../src/async-components/views/dialogs/security/CreateKeyBackupDialog"; -import { createTestClient, filterConsole } from "../../../../test-utils"; +import { createTestClient } from "../../../../test-utils"; import { MatrixClientPeg } from "../../../../../src/MatrixClientPeg"; jest.mock("../../../../../src/SecurityManager", () => ({ - accessSecretStorage: async (func = async () => Promise) => { - await func(); - }, + accessSecretStorage: jest.fn().mockResolvedValue(undefined), })); describe("CreateKeyBackupDialog", () => { @@ -41,33 +39,16 @@ describe("CreateKeyBackupDialog", () => { expect(asFragment()).toMatchSnapshot(); }); - describe("expecting failure", () => { - filterConsole("Error creating key backup"); + it("should display the error message when backup creation failed", async () => { + const matrixClient = createTestClient(); + mocked(matrixClient.scheduleAllGroupSessionsForBackup).mockRejectedValue("my error"); + MatrixClientPeg.safeGet = MatrixClientPeg.get = () => matrixClient; - it("should display an error message when backup creation failed", async () => { - const matrixClient = createTestClient(); - mocked(matrixClient.getCrypto()!.resetKeyBackup).mockImplementation(() => { - throw new Error("failed"); - }); - MatrixClientPeg.safeGet = MatrixClientPeg.get = () => matrixClient; - - const { asFragment } = render(); - - // 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.getCrypto).mockReturnValue(undefined); - MatrixClientPeg.safeGet = MatrixClientPeg.get = () => matrixClient; - - render(); + const { asFragment } = 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()); + expect(asFragment()).toMatchSnapshot(); }); 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 dd82b840f1c..9d62384e3c1 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 expecting failure should display an error message when backup creation failed 1`] = ` +exports[`CreateKeyBackupDialog should display the error message when backup creation failed 1`] = `
Date: Wed, 3 Jan 2024 16:46:54 +0100 Subject: [PATCH 2/6] fix secretStorageBeingAccessed --- src/SecurityManager.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/SecurityManager.ts b/src/SecurityManager.ts index 58a381aeceb..87e203a323b 100644 --- a/src/SecurityManager.ts +++ b/src/SecurityManager.ts @@ -414,6 +414,8 @@ export async function accessSecretStorage(func = async (): Promise => {}, logger.error(e); // Re-throw so that higher level logic can abort as needed throw e; + } finally { + secretStorageBeingAccessed = false; } } From 35806b5bc793b233011ceeccbf8cd09fba6745a5 Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 3 Jan 2024 17:04:15 +0100 Subject: [PATCH 3/6] Maintain the change introduced in #12059 --- src/SecurityManager.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/SecurityManager.ts b/src/SecurityManager.ts index 87e203a323b..42b6c740998 100644 --- a/src/SecurityManager.ts +++ b/src/SecurityManager.ts @@ -343,7 +343,11 @@ export async function withSecretStorageKeyCache(func: () => Promise): Prom * @param {bool} [forceReset] Reset secret storage even if it's already set up */ export async function accessSecretStorage(func = async (): Promise => {}, forceReset = false): Promise { - secretStorageBeingAccessed = true; + await withSecretStorageKeyCache(() => doAccessSecretStorage(func, forceReset)); +} + +/** Helper for {@link #accessSecretStorage} */ +export async function doAccessSecretStorage(func = async (): Promise => {}, forceReset = false): Promise { try { const cli = MatrixClientPeg.safeGet(); if (!(await cli.hasSecretStorageKey()) || forceReset) { @@ -414,8 +418,6 @@ export async function accessSecretStorage(func = async (): Promise => {}, logger.error(e); // Re-throw so that higher level logic can abort as needed throw e; - } finally { - secretStorageBeingAccessed = false; } } From 1b27a961b31647fe315926a4ab57e2f3d7454024 Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 3 Jan 2024 18:33:44 +0100 Subject: [PATCH 4/6] add regression test --- playwright/e2e/crypto/utils.ts | 21 +++++++++++++++++++-- playwright/e2e/crypto/verification.spec.ts | 14 +++++++++----- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/playwright/e2e/crypto/utils.ts b/playwright/e2e/crypto/utils.ts index 070e615e874..907aa94a28a 100644 --- a/playwright/e2e/crypto/utils.ts +++ b/playwright/e2e/crypto/utils.ts @@ -103,12 +103,29 @@ export async function checkDeviceIsCrossSigned(app: ElementAppPage): Promise { 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_SecureBackupPanel_advanced"); + await summary.locator("..").click(); + + const cacheDecryptionKeyStatusElement = page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(2) td"); + await expect(cacheDecryptionKeyStatusElement).toHaveText("cached locally, well formed"); + + const serverVersion = await page.locator(".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_SecureBackupPanel_statusList tr:nth-child(6) td").textContent(); + expect(activeVersion.trim()).toBe(expectedBackupVersion); } /** diff --git a/playwright/e2e/crypto/verification.spec.ts b/playwright/e2e/crypto/verification.spec.ts index fc499f3f726..dbe0ace981a 100644 --- a/playwright/e2e/crypto/verification.spec.ts +++ b/playwright/e2e/crypto/verification.spec.ts @@ -32,6 +32,7 @@ import { Bot } from "../../pages/bot"; test.describe("Device verification", () => { let aliceBotClient: Bot; + let expectedBackupVersion: string; test.beforeEach(async ({ page, homeserver, credentials }) => { // Visit the login page of the app, to load the matrix sdk @@ -49,7 +50,10 @@ test.describe("Device verification", () => { bootstrapSecretStorage: true, }); aliceBotClient.setCredentials(credentials); - await aliceBotClient.prepareClient(); + const mxClientHandle = await aliceBotClient.prepareClient(); + expectedBackupVersion = await mxClientHandle.evaluate(async (mxClient) => { + return await mxClient.getCrypto()!.getActiveSessionBackupVersion(); + }); await page.waitForTimeout(20000); }); @@ -87,7 +91,7 @@ test.describe("Device verification", () => { await checkDeviceIsCrossSigned(app); // Check that the current device is connected to key backup - await checkDeviceIsConnectedKeyBackup(page); + await checkDeviceIsConnectedKeyBackup(page, expectedBackupVersion); }); test("Verify device with QR code during login", async ({ page, app, credentials, homeserver }) => { @@ -130,7 +134,7 @@ test.describe("Device verification", () => { await checkDeviceIsCrossSigned(app); // Check that the current device is connected to key backup - await checkDeviceIsConnectedKeyBackup(page); + await checkDeviceIsConnectedKeyBackup(page, expectedBackupVersion); }); test("Verify device with Security Phrase during login", async ({ page, app, credentials, homeserver }) => { @@ -150,7 +154,7 @@ test.describe("Device verification", () => { await checkDeviceIsCrossSigned(app); // Check that the current device is connected to key backup - await checkDeviceIsConnectedKeyBackup(page); + await checkDeviceIsConnectedKeyBackup(page, expectedBackupVersion); }); test("Verify device with Security Key during login", async ({ page, app, credentials, homeserver }) => { @@ -172,7 +176,7 @@ test.describe("Device verification", () => { await checkDeviceIsCrossSigned(app); // Check that the current device is connected to key backup - await checkDeviceIsConnectedKeyBackup(page); + await checkDeviceIsConnectedKeyBackup(page, expectedBackupVersion); }); test("Handle incoming verification request with SAS", async ({ page, credentials, homeserver, toasts }) => { From 2bbd86cc144730a9ba750263c47e890c183d1cf7 Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 3 Jan 2024 20:28:22 +0100 Subject: [PATCH 5/6] fix test a bit flaky --- playwright/e2e/crypto/utils.ts | 13 ++++++++++--- playwright/e2e/crypto/verification.spec.ts | 19 +++++++++++++------ 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/playwright/e2e/crypto/utils.ts b/playwright/e2e/crypto/utils.ts index 907aa94a28a..fbac38b4ac2 100644 --- a/playwright/e2e/crypto/utils.ts +++ b/playwright/e2e/crypto/utils.ts @@ -108,8 +108,13 @@ export async function checkDeviceIsCrossSigned(app: ElementAppPage): Promise { +export async function checkDeviceIsConnectedKeyBackup( + page: Page, + expectedBackupVersion: string, + checkBackupKeyInCache: boolean, +): Promise { 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(); @@ -118,8 +123,10 @@ export async function checkDeviceIsConnectedKeyBackup(page: Page, expectedBackup const summary = page.locator(".mx_SecureBackupPanel_advanced"); await summary.locator("..").click(); - const cacheDecryptionKeyStatusElement = page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(2) td"); - await expect(cacheDecryptionKeyStatusElement).toHaveText("cached locally, well formed"); + if (checkBackupKeyInCache) { + const cacheDecryptionKeyStatusElement = page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(2) td"); + await expect(cacheDecryptionKeyStatusElement).toHaveText("cached locally, well formed"); + } const serverVersion = await page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(5) td").textContent(); expect(serverVersion.trim()).toBe(expectedBackupVersion + " (Algorithm: m.megolm_backup.v1.curve25519-aes-sha2)"); diff --git a/playwright/e2e/crypto/verification.spec.ts b/playwright/e2e/crypto/verification.spec.ts index dbe0ace981a..796f575df95 100644 --- a/playwright/e2e/crypto/verification.spec.ts +++ b/playwright/e2e/crypto/verification.spec.ts @@ -51,11 +51,12 @@ test.describe("Device verification", () => { }); aliceBotClient.setCredentials(credentials); const mxClientHandle = await aliceBotClient.prepareClient(); + + await page.waitForTimeout(20000); + expectedBackupVersion = await mxClientHandle.evaluate(async (mxClient) => { return await mxClient.getCrypto()!.getActiveSessionBackupVersion(); }); - - await page.waitForTimeout(20000); }); // Click the "Verify with another device" button, and have the bot client auto-accept it. @@ -91,7 +92,9 @@ test.describe("Device verification", () => { await checkDeviceIsCrossSigned(app); // Check that the current device is connected to key backup - await checkDeviceIsConnectedKeyBackup(page, expectedBackupVersion); + // For now we don't check that the backup key is in cache because it's a bit flaky, + // as we need to wait for the secret gossiping to happen and the settings dialog doesn't refresh automatically. + await checkDeviceIsConnectedKeyBackup(page, expectedBackupVersion, false); }); test("Verify device with QR code during login", async ({ page, app, credentials, homeserver }) => { @@ -134,7 +137,9 @@ test.describe("Device verification", () => { await checkDeviceIsCrossSigned(app); // Check that the current device is connected to key backup - await checkDeviceIsConnectedKeyBackup(page, expectedBackupVersion); + // For now we don't check that the backup key is in cache because it's a bit flaky, + // as we need to wait for the secret gossiping to happen and the settings dialog doesn't refresh automatically. + await checkDeviceIsConnectedKeyBackup(page, expectedBackupVersion, false); }); test("Verify device with Security Phrase during login", async ({ page, app, credentials, homeserver }) => { @@ -154,7 +159,8 @@ test.describe("Device verification", () => { await checkDeviceIsCrossSigned(app); // Check that the current device is connected to key backup - await checkDeviceIsConnectedKeyBackup(page, expectedBackupVersion); + // The backup decryption key should be in cache also, as we got it directly from the 4S + await checkDeviceIsConnectedKeyBackup(page, expectedBackupVersion, true); }); test("Verify device with Security Key during login", async ({ page, app, credentials, homeserver }) => { @@ -176,7 +182,8 @@ test.describe("Device verification", () => { await checkDeviceIsCrossSigned(app); // Check that the current device is connected to key backup - await checkDeviceIsConnectedKeyBackup(page, expectedBackupVersion); + // The backup decryption key should be in cache also, as we got it directly from the 4S + await checkDeviceIsConnectedKeyBackup(page, expectedBackupVersion, true); }); test("Handle incoming verification request with SAS", async ({ page, credentials, homeserver, toasts }) => { From 2d38b02db164bb3d8105a446e71fe9a262dc708f Mon Sep 17 00:00:00 2001 From: Valere Date: Thu, 4 Jan 2024 09:58:38 +0100 Subject: [PATCH 6/6] code review --- playwright/e2e/crypto/utils.ts | 11 +++++------ playwright/e2e/crypto/verification.spec.ts | 2 ++ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/playwright/e2e/crypto/utils.ts b/playwright/e2e/crypto/utils.ts index fbac38b4ac2..f1e1f811903 100644 --- a/playwright/e2e/crypto/utils.ts +++ b/playwright/e2e/crypto/utils.ts @@ -120,19 +120,18 @@ export async function checkDeviceIsConnectedKeyBackup( 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_SecureBackupPanel_advanced"); - await summary.locator("..").click(); + await page.locator(".mx_SecureBackupPanel_advanced").locator("..").click(); if (checkBackupKeyInCache) { const cacheDecryptionKeyStatusElement = page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(2) td"); await expect(cacheDecryptionKeyStatusElement).toHaveText("cached locally, well formed"); } - const serverVersion = await page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(5) td").textContent(); - expect(serverVersion.trim()).toBe(expectedBackupVersion + " (Algorithm: m.megolm_backup.v1.curve25519-aes-sha2)"); + await expect(page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(5) td")).toHaveText( + expectedBackupVersion + " (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(expectedBackupVersion); + await expect(page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(6) td")).toHaveText(expectedBackupVersion); } /** diff --git a/playwright/e2e/crypto/verification.spec.ts b/playwright/e2e/crypto/verification.spec.ts index 796f575df95..55d65a9b087 100644 --- a/playwright/e2e/crypto/verification.spec.ts +++ b/playwright/e2e/crypto/verification.spec.ts @@ -32,6 +32,8 @@ import { Bot } from "../../pages/bot"; test.describe("Device verification", () => { let aliceBotClient: Bot; + + /** The backup version that was set up by the bot client. */ let expectedBackupVersion: string; test.beforeEach(async ({ page, homeserver, credentials }) => {