From 46b28d73b6fa3f7819f2d54b155c421364bc47ed Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Fri, 15 Dec 2023 14:59:36 +0000 Subject: [PATCH 1/2] Element-R: fix repeated requests to enter 4S key during cross-signing reset (#12059) * Remove redundant `forceReset` parameter This was always true, so let's get rid of it. Also some function renames. * Factor out new `withSecretStorageKeyCache` helper ... so that we can use the cache without the whole of `accessSecretStorage`. * Cache secret storage key during cross-signing reset * Playwright test for resetting cross-signing * CrossSigningPanel: Silence annoying react warnings React complains if we don't include an explicit `tbody`. * Simple unit test of reset button (cherry picked from commit de5931d5a8e509aa4b411bc07326bdc01211a793) --- playwright/e2e/crypto/crypto.spec.ts | 37 +++++ src/SecurityManager.ts | 39 ++++- .../views/settings/CrossSigningPanel.tsx | 153 +++++++++--------- .../views/settings/CrossSigningPanel-test.tsx | 21 +++ .../SecurityUserSettingsTab-test.tsx.snap | 122 +++++++------- 5 files changed, 229 insertions(+), 143 deletions(-) diff --git a/playwright/e2e/crypto/crypto.spec.ts b/playwright/e2e/crypto/crypto.spec.ts index 83d1383675f..6bd2f8e27ad 100644 --- a/playwright/e2e/crypto/crypto.spec.ts +++ b/playwright/e2e/crypto/crypto.spec.ts @@ -212,6 +212,43 @@ test.describe("Cryptography", function () { }); } + test("Can reset cross-signing keys", async ({ page, app, user: aliceCredentials }) => { + const secretStorageKey = await enableKeyBackup(app); + + // Fetch the current cross-signing keys + async function fetchMasterKey() { + return await test.step("Fetch master key from server", async () => { + const k = await app.client.evaluate(async (cli) => { + const userId = cli.getUserId(); + const keys = await cli.downloadKeysForUsers([userId]); + return Object.values(keys.master_keys[userId].keys)[0]; + }); + console.log(`fetchMasterKey: ${k}`); + return k; + }); + } + const masterKey1 = await fetchMasterKey(); + + // Find the "reset cross signing" button, and click it + await app.settings.openUserSettings("Security & Privacy"); + await page.locator("div.mx_CrossSigningPanel_buttonRow").getByRole("button", { name: "Reset" }).click(); + + // Confirm + await page.getByRole("button", { name: "Clear cross-signing keys" }).click(); + + // Enter the 4S key + await page.getByPlaceholder("Security Key").fill(secretStorageKey); + await page.getByRole("button", { name: "Continue" }).click(); + + await expect(async () => { + const masterKey2 = await fetchMasterKey(); + expect(masterKey1).not.toEqual(masterKey2); + }).toPass(); + + // The dialog should have gone away + await expect(page.locator(".mx_Dialog")).toHaveCount(1); + }); + test("creating a DM should work, being e2e-encrypted / user verification", async ({ page, app, diff --git a/src/SecurityManager.ts b/src/SecurityManager.ts index 056be7a2a92..f6fff3628e0 100644 --- a/src/SecurityManager.ts +++ b/src/SecurityManager.ts @@ -299,6 +299,28 @@ export async function promptForBackupPassphrase(): Promise { return key; } +/** + * Carry out an operation that may require multiple accesses to secret storage, caching the key. + * + * Use this helper to wrap an operation that may require multiple accesses to secret storage; the user will be prompted + * to enter the 4S key or passphrase on the first access, and the key will be cached for the rest of the operation. + * + * @param func - The operation to be wrapped. + */ +export async function withSecretStorageKeyCache(func: () => Promise): Promise { + secretStorageBeingAccessed = true; + try { + return await func(); + } finally { + // Clear secret storage key cache now that work is complete + secretStorageBeingAccessed = false; + if (!isCachingAllowed()) { + secretStorageKeys = {}; + secretStorageKeyInfo = {}; + } + } +} + /** * This helper should be used whenever you need to access secret storage. It * ensures that secret storage (and also cross-signing since they each depend on @@ -326,7 +348,15 @@ export async function accessSecretStorage( forceReset = false, setupNewKeyBackup = true, ): Promise { - secretStorageBeingAccessed = true; + await withSecretStorageKeyCache(() => doAccessSecretStorage(func, forceReset, setupNewKeyBackup)); +} + +/** Helper for {@link #accessSecretStorage} */ +async function doAccessSecretStorage( + func: () => Promise, + forceReset: boolean, + setupNewKeyBackup: boolean, +): Promise { try { const cli = MatrixClientPeg.safeGet(); if (!(await cli.hasSecretStorageKey()) || forceReset) { @@ -403,13 +433,6 @@ export async function accessSecretStorage( logger.error(e); // Re-throw so that higher level logic can abort as needed throw e; - } finally { - // Clear secret storage key cache now that work is complete - secretStorageBeingAccessed = false; - if (!isCachingAllowed()) { - secretStorageKeys = {}; - secretStorageKeyInfo = {}; - } } } diff --git a/src/components/views/settings/CrossSigningPanel.tsx b/src/components/views/settings/CrossSigningPanel.tsx index 65886902544..91791e63725 100644 --- a/src/components/views/settings/CrossSigningPanel.tsx +++ b/src/components/views/settings/CrossSigningPanel.tsx @@ -26,7 +26,7 @@ import Spinner from "../elements/Spinner"; import InteractiveAuthDialog from "../dialogs/InteractiveAuthDialog"; import ConfirmDestroyCrossSigningDialog from "../dialogs/security/ConfirmDestroyCrossSigningDialog"; import SetupEncryptionDialog from "../dialogs/security/SetupEncryptionDialog"; -import { accessSecretStorage } from "../../../SecurityManager"; +import { accessSecretStorage, withSecretStorageKeyCache } from "../../../SecurityManager"; import AccessibleButton from "../elements/AccessibleButton"; import { SettingsSubsectionText } from "./shared/SettingsSubsection"; @@ -118,31 +118,27 @@ export default class CrossSigningPanel extends React.PureComponent<{}, IState> { } /** - * Bootstrapping cross-signing take one of these paths: - * 1. Create cross-signing keys locally and store in secret storage (if it - * already exists on the account). - * 2. Access existing secret storage by requesting passphrase and accessing - * cross-signing keys as needed. - * 3. All keys are loaded and there's nothing to do. - * @param {bool} [forceReset] Bootstrap again even if keys already present + * Reset the user's cross-signing keys. */ - private bootstrapCrossSigning = async ({ forceReset = false }): Promise => { + private async resetCrossSigning(): Promise { this.setState({ error: false }); try { const cli = MatrixClientPeg.safeGet(); - await cli.bootstrapCrossSigning({ - authUploadDeviceSigningKeys: async (makeRequest): Promise => { - const { finished } = Modal.createDialog(InteractiveAuthDialog, { - title: _t("encryption|bootstrap_title"), - matrixClient: cli, - makeRequest, - }); - const [confirmed] = await finished; - if (!confirmed) { - throw new Error("Cross-signing key upload auth canceled"); - } - }, - setupNewCrossSigning: forceReset, + await withSecretStorageKeyCache(async () => { + await cli.getCrypto()!.bootstrapCrossSigning({ + authUploadDeviceSigningKeys: async (makeRequest): Promise => { + const { finished } = Modal.createDialog(InteractiveAuthDialog, { + title: _t("encryption|bootstrap_title"), + matrixClient: cli, + makeRequest, + }); + const [confirmed] = await finished; + if (!confirmed) { + throw new Error("Cross-signing key upload auth canceled"); + } + }, + setupNewCrossSigning: true, + }); }); } catch (e) { this.setState({ error: true }); @@ -150,13 +146,18 @@ export default class CrossSigningPanel extends React.PureComponent<{}, IState> { } if (this.unmounted) return; this.getUpdatedStatus(); - }; + } - private resetCrossSigning = (): void => { + /** + * Callback for when the user clicks the "reset cross signing" button. + * + * Shows a confirmation dialog, and then does the reset if confirmed. + */ + private onResetCrossSigningClick = (): void => { Modal.createDialog(ConfirmDestroyCrossSigningDialog, { - onFinished: (act) => { + onFinished: async (act) => { if (!act) return; - this.bootstrapCrossSigning({ forceReset: true }); + this.resetCrossSigning(); }, }); }; @@ -243,7 +244,7 @@ export default class CrossSigningPanel extends React.PureComponent<{}, IState> { if (keysExistAnywhere) { actions.push( - + {_t("action|reset")} , ); @@ -260,54 +261,56 @@ export default class CrossSigningPanel extends React.PureComponent<{}, IState> {
{_t("common|advanced")} - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + +
{_t("settings|security|cross_signing_public_keys")} - {crossSigningPublicKeysOnDevice - ? _t("settings|security|cross_signing_in_memory") - : _t("settings|security|cross_signing_not_found")} -
{_t("settings|security|cross_signing_private_keys")} - {crossSigningPrivateKeysInStorage - ? _t("settings|security|cross_signing_in_4s") - : _t("settings|security|cross_signing_not_in_4s")} -
{_t("settings|security|cross_signing_master_private_Key")} - {masterPrivateKeyCached - ? _t("settings|security|cross_signing_cached") - : _t("settings|security|cross_signing_not_cached")} -
{_t("settings|security|cross_signing_self_signing_private_key")} - {selfSigningPrivateKeyCached - ? _t("settings|security|cross_signing_cached") - : _t("settings|security|cross_signing_not_cached")} -
{_t("settings|security|cross_signing_user_signing_private_key")} - {userSigningPrivateKeyCached - ? _t("settings|security|cross_signing_cached") - : _t("settings|security|cross_signing_not_cached")} -
{_t("settings|security|cross_signing_homeserver_support")} - {homeserverSupportsCrossSigning - ? _t("settings|security|cross_signing_homeserver_support_exists") - : _t("settings|security|cross_signing_not_found")} -
{_t("settings|security|cross_signing_public_keys")} + {crossSigningPublicKeysOnDevice + ? _t("settings|security|cross_signing_in_memory") + : _t("settings|security|cross_signing_not_found")} +
{_t("settings|security|cross_signing_private_keys")} + {crossSigningPrivateKeysInStorage + ? _t("settings|security|cross_signing_in_4s") + : _t("settings|security|cross_signing_not_in_4s")} +
{_t("settings|security|cross_signing_master_private_Key")} + {masterPrivateKeyCached + ? _t("settings|security|cross_signing_cached") + : _t("settings|security|cross_signing_not_cached")} +
{_t("settings|security|cross_signing_self_signing_private_key")} + {selfSigningPrivateKeyCached + ? _t("settings|security|cross_signing_cached") + : _t("settings|security|cross_signing_not_cached")} +
{_t("settings|security|cross_signing_user_signing_private_key")} + {userSigningPrivateKeyCached + ? _t("settings|security|cross_signing_cached") + : _t("settings|security|cross_signing_not_cached")} +
{_t("settings|security|cross_signing_homeserver_support")} + {homeserverSupportsCrossSigning + ? _t("settings|security|cross_signing_homeserver_support_exists") + : _t("settings|security|cross_signing_not_found")} +
{errorSection} diff --git a/test/components/views/settings/CrossSigningPanel-test.tsx b/test/components/views/settings/CrossSigningPanel-test.tsx index 96c65fd5c61..501b0fe901e 100644 --- a/test/components/views/settings/CrossSigningPanel-test.tsx +++ b/test/components/views/settings/CrossSigningPanel-test.tsx @@ -26,6 +26,8 @@ import { mockClientMethodsCrypto, mockClientMethodsUser, } from "../../../test-utils"; +import Modal from "../../../../src/Modal"; +import ConfirmDestroyCrossSigningDialog from "../../../../src/components/views/dialogs/security/ConfirmDestroyCrossSigningDialog"; describe("", () => { const userId = "@alice:server.org"; @@ -43,6 +45,10 @@ describe("", () => { mockClient.isCrossSigningReady.mockResolvedValue(false); }); + afterEach(() => { + jest.restoreAllMocks(); + }); + it("should render a spinner while loading", () => { getComponent(); @@ -85,6 +91,21 @@ describe("", () => { expect(screen.getByTestId("summarised-status").innerHTML).toEqual("✅ Cross-signing is ready for use."); expect(screen.getByText("Cross-signing private keys:").parentElement!).toMatchSnapshot(); }); + + it("should allow reset of cross-signing", async () => { + mockClient.getCrypto()!.bootstrapCrossSigning = jest.fn().mockResolvedValue(undefined); + getComponent(); + await flushPromises(); + + const modalSpy = jest.spyOn(Modal, "createDialog"); + + screen.getByRole("button", { name: "Reset" }).click(); + expect(modalSpy).toHaveBeenCalledWith(ConfirmDestroyCrossSigningDialog, expect.any(Object)); + modalSpy.mock.lastCall![1]!.onFinished(true); + expect(mockClient.getCrypto()!.bootstrapCrossSigning).toHaveBeenCalledWith( + expect.objectContaining({ setupNewCrossSigning: true }), + ); + }); }); describe("when cross signing is not ready", () => { diff --git a/test/components/views/settings/tabs/user/__snapshots__/SecurityUserSettingsTab-test.tsx.snap b/test/components/views/settings/tabs/user/__snapshots__/SecurityUserSettingsTab-test.tsx.snap index 562d8c3952d..ea531925421 100644 --- a/test/components/views/settings/tabs/user/__snapshots__/SecurityUserSettingsTab-test.tsx.snap +++ b/test/components/views/settings/tabs/user/__snapshots__/SecurityUserSettingsTab-test.tsx.snap @@ -175,66 +175,68 @@ exports[` renders security section 1`] = ` - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + +
- Cross-signing public keys: - - not found -
- Cross-signing private keys: - - not found in storage -
- Master private key: - - not found locally -
- Self signing private key: - - not found locally -
- User signing private key: - - not found locally -
- Homeserver feature support: - - not found -
+ Cross-signing public keys: + + not found +
+ Cross-signing private keys: + + not found in storage +
+ Master private key: + + not found locally +
+ Self signing private key: + + not found locally +
+ User signing private key: + + not found locally +
+ Homeserver feature support: + + not found +
From e072c36c9cb2536020a4d619cb153573c76188bd Mon Sep 17 00:00:00 2001 From: Valere Date: Thu, 4 Jan 2024 14:11:28 +0100 Subject: [PATCH 2/2] Revert "Set up key backup using non-deprecated APIs (#12005)" (#12102) (cherry picked from commit 61070698cc6a758d9bcbbd783d5c0500af157fa7) --- playwright/e2e/crypto/backups.spec.ts | 57 ----------------- playwright/e2e/crypto/utils.ts | 27 +++++++- playwright/e2e/crypto/verification.spec.ts | 23 +++++-- playwright/element-web-test.ts | 4 -- playwright/pages/ElementAppPage.ts | 13 ---- src/SecurityManager.ts | 25 ++------ .../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 - 11 files changed, 79 insertions(+), 211 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/e2e/crypto/utils.ts b/playwright/e2e/crypto/utils.ts index 070e615e874..f1e1f811903 100644 --- a/playwright/e2e/crypto/utils.ts +++ b/playwright/e2e/crypto/utils.ts @@ -103,12 +103,35 @@ 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 + 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"); + } + + await expect(page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(5) td")).toHaveText( + expectedBackupVersion + " (Algorithm: m.megolm_backup.v1.curve25519-aes-sha2)", + ); + + 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 fc499f3f726..55d65a9b087 100644 --- a/playwright/e2e/crypto/verification.spec.ts +++ b/playwright/e2e/crypto/verification.spec.ts @@ -33,6 +33,9 @@ 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 }) => { // Visit the login page of the app, to load the matrix sdk await page.goto("/#/login"); @@ -49,9 +52,13 @@ test.describe("Device verification", () => { bootstrapSecretStorage: true, }); aliceBotClient.setCredentials(credentials); - await aliceBotClient.prepareClient(); + const mxClientHandle = await aliceBotClient.prepareClient(); await page.waitForTimeout(20000); + + expectedBackupVersion = await mxClientHandle.evaluate(async (mxClient) => { + return await mxClient.getCrypto()!.getActiveSessionBackupVersion(); + }); }); // Click the "Verify with another device" button, and have the bot client auto-accept it. @@ -87,7 +94,9 @@ test.describe("Device verification", () => { await checkDeviceIsCrossSigned(app); // Check that the current device is connected to key backup - await checkDeviceIsConnectedKeyBackup(page); + // 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 }) => { @@ -130,7 +139,9 @@ test.describe("Device verification", () => { await checkDeviceIsCrossSigned(app); // Check that the current device is connected to key backup - await checkDeviceIsConnectedKeyBackup(page); + // 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 }) => { @@ -150,7 +161,8 @@ test.describe("Device verification", () => { await checkDeviceIsCrossSigned(app); // Check that the current device is connected to key backup - await checkDeviceIsConnectedKeyBackup(page); + // 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 }) => { @@ -172,7 +184,8 @@ test.describe("Device verification", () => { await checkDeviceIsCrossSigned(app); // Check that the current device is connected to key backup - await checkDeviceIsConnectedKeyBackup(page); + // 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 }) => { diff --git a/playwright/element-web-test.ts b/playwright/element-web-test.ts index ecea2963325..e3ee8200231 100644 --- a/playwright/element-web-test.ts +++ b/playwright/element-web-test.ts @@ -260,7 +260,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 4e065a4b176..bf6ec155e87 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..42b6c740998 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] 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)); +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 { +export async function doAccessSecretStorage(func = async (): Promise => {}, forceReset = false): Promise { try { const cli = MatrixClientPeg.safeGet(); if (!(await cli.hasSecretStorageKey()) || forceReset) { @@ -387,12 +378,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 +391,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`] = `