From b2601b172868bfabfd45c7d926ee7217503f286d Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 9 Dec 2024 13:58:31 +0000 Subject: [PATCH] Enable key backup by default When we set up cross signing, so the key backup key will be stored locally along with the cross signing keys until the user sets up recovery (4s). This will mean that a user can restore their backup if they log in on a new device as long as they verify with the one they registered on. Replaces https://github.com/element-hq/element-web/pull/28267 --- src/CreateCrossSigning.ts | 8 +++--- src/DeviceListener.ts | 28 ++++++++++++------- src/i18n/strings/en_EN.json | 3 ++ src/stores/InitialCryptoSetupStore.ts | 5 ++++ src/toasts/SetupEncryptionToast.ts | 24 ++++++++++++++-- test/unit-tests/DeviceListener-test.ts | 4 +-- .../components/structures/MatrixChat-test.tsx | 1 + .../toasts/SetupEncryptionToast-test.tsx | 24 ++++++++++++++++ 8 files changed, 78 insertions(+), 19 deletions(-) create mode 100644 test/unit-tests/toasts/SetupEncryptionToast-test.tsx diff --git a/src/CreateCrossSigning.ts b/src/CreateCrossSigning.ts index e67e030f60..8c2d4488cf 100644 --- a/src/CreateCrossSigning.ts +++ b/src/CreateCrossSigning.ts @@ -23,11 +23,11 @@ import InteractiveAuthDialog from "./components/views/dialogs/InteractiveAuthDia async function canUploadKeysWithPasswordOnly(cli: MatrixClient): Promise { try { await cli.uploadDeviceSigningKeys(undefined, {} as CrossSigningKeys); - // We should never get here: the server should always require - // UI auth to upload device signing keys. If we do, we upload - // no keys which would be a no-op. + // If we get here, it's because the server is allowing us to upload keys without + // auth the first time due to MSC3967. Therefore, yes, we can upload keys + // (with or without password, technically, but that's fine). logger.log("uploadDeviceSigningKeys unexpectedly succeeded without UI auth!"); - return false; + return true; } catch (error) { if (!(error instanceof MatrixError) || !error.data || !error.data.flows) { logger.log("uploadDeviceSigningKeys advertised no flows!"); diff --git a/src/DeviceListener.ts b/src/DeviceListener.ts index 84d83827da..e34af95962 100644 --- a/src/DeviceListener.ts +++ b/src/DeviceListener.ts @@ -295,21 +295,29 @@ export default class DeviceListener { await crypto.getUserDeviceInfo([cli.getSafeUserId()]); // cross signing isn't enabled - nag to enable it - // There are 2 different toasts for: + // There are 3 different toasts for: if (!(await crypto.getCrossSigningKeyId()) && (await crypto.userHasCrossSigningKeys())) { - // Cross-signing on account but this device doesn't trust the master key (verify this session) + // Toast 1. Cross-signing on account but this device doesn't trust the master key (verify this session) showSetupEncryptionToast(SetupKind.VERIFY_THIS_SESSION); this.checkKeyBackupStatus(); } else { - // No cross-signing or key backup on account (set up encryption) - await cli.waitForClientWellKnown(); - if (isSecureBackupRequired(cli) && isLoggedIn()) { - // If we're meant to set up, and Secure Backup is required, - // trigger the flow directly without a toast once logged in. - hideSetupEncryptionToast(); - accessSecretStorage(); + const backupInfo = await this.getKeyBackupInfo(); + if (backupInfo) { + // Toast 2: Key backup is enabled but recovery (4S) is not set up: prompt user to set up recovery. + // Since we now enable key backup at registration time, this will be the common case for + // new users. + showSetupEncryptionToast(SetupKind.SET_UP_RECOVERY); } else { - showSetupEncryptionToast(SetupKind.SET_UP_ENCRYPTION); + // Toast 3: No cross-signing or key backup on account (set up encryption) + await cli.waitForClientWellKnown(); + if (isSecureBackupRequired(cli) && isLoggedIn()) { + // If we're meant to set up, and Secure Backup is required, + // trigger the flow directly without a toast once logged in. + hideSetupEncryptionToast(); + accessSecretStorage(); + } else { + showSetupEncryptionToast(SetupKind.SET_UP_ENCRYPTION); + } } } } diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index e9ac73b48b..f3c514fca3 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -914,6 +914,9 @@ "warning": "If you didn't remove the recovery method, an attacker may be trying to access your account. Change your account password and set a new recovery method immediately in Settings." }, "reset_all_button": "Forgotten or lost all recovery methods? Reset all", + "set_up_recovery": "Set up recovery", + "set_up_recovery_later": "Not now", + "set_up_recovery_toast_description": "Generate a recovery key that can be used to restore your encrypted message history in case you lose access to your devices.", "set_up_toast_description": "Safeguard against losing access to encrypted messages & data", "set_up_toast_title": "Set up Secure Backup", "setup_secure_backup": { diff --git a/src/stores/InitialCryptoSetupStore.ts b/src/stores/InitialCryptoSetupStore.ts index 0c2e49f5ca..dd6b1f0461 100644 --- a/src/stores/InitialCryptoSetupStore.ts +++ b/src/stores/InitialCryptoSetupStore.ts @@ -116,6 +116,11 @@ export class InitialCryptoSetupStore extends EventEmitter { try { await createCrossSigning(this.client, this.isTokenLogin, this.stores.accountPasswordStore.getPassword()); + const backupInfo = await cryptoApi.getKeyBackupInfo(); + if (backupInfo === null) { + await cryptoApi.resetKeyBackup(); + } + this.reset(); this.status = "complete"; diff --git a/src/toasts/SetupEncryptionToast.ts b/src/toasts/SetupEncryptionToast.ts index 0dd54bb18f..1d6cbe7490 100644 --- a/src/toasts/SetupEncryptionToast.ts +++ b/src/toasts/SetupEncryptionToast.ts @@ -23,15 +23,19 @@ const getTitle = (kind: Kind): string => { switch (kind) { case Kind.SET_UP_ENCRYPTION: return _t("encryption|set_up_toast_title"); + case Kind.SET_UP_RECOVERY: + return _t("encryption|set_up_recovery"); case Kind.VERIFY_THIS_SESSION: return _t("encryption|verify_toast_title"); } }; -const getIcon = (kind: Kind): string => { +const getIcon = (kind: Kind): string | undefined => { switch (kind) { case Kind.SET_UP_ENCRYPTION: return "secure_backup"; + case Kind.SET_UP_RECOVERY: + return undefined; case Kind.VERIFY_THIS_SESSION: return "verification_warning"; } @@ -41,15 +45,29 @@ const getSetupCaption = (kind: Kind): string => { switch (kind) { case Kind.SET_UP_ENCRYPTION: return _t("action|continue"); + case Kind.SET_UP_RECOVERY: + return _t("action|continue"); case Kind.VERIFY_THIS_SESSION: return _t("action|verify"); } }; +const getSecondaryButtonLabel = (kind: Kind): string => { + switch (kind) { + case Kind.SET_UP_RECOVERY: + return _t("encryption|set_up_recovery_later"); + case Kind.SET_UP_ENCRYPTION: + case Kind.VERIFY_THIS_SESSION: + return _t("encryption|verification|unverified_sessions_toast_reject"); + } +}; + const getDescription = (kind: Kind): string => { switch (kind) { case Kind.SET_UP_ENCRYPTION: return _t("encryption|set_up_toast_description"); + case Kind.SET_UP_RECOVERY: + return _t("encryption|set_up_recovery_toast_description"); case Kind.VERIFY_THIS_SESSION: return _t("encryption|verify_toast_description"); } @@ -57,6 +75,7 @@ const getDescription = (kind: Kind): string => { export enum Kind { SET_UP_ENCRYPTION = "set_up_encryption", + SET_UP_RECOVERY = "set_up_recovery", VERIFY_THIS_SESSION = "verify_this_session", } @@ -101,9 +120,8 @@ export const showToast = (kind: Kind): void => { description: getDescription(kind), primaryLabel: getSetupCaption(kind), onPrimaryClick: onAccept, - secondaryLabel: _t("encryption|verification|unverified_sessions_toast_reject"), + secondaryLabel: getSecondaryButtonLabel(kind), onSecondaryClick: onReject, - destructive: "secondary", }, component: GenericToast, priority: kind === Kind.VERIFY_THIS_SESSION ? 95 : 40, diff --git a/test/unit-tests/DeviceListener-test.ts b/test/unit-tests/DeviceListener-test.ts index ad7f14e119..1c8fe1a1c7 100644 --- a/test/unit-tests/DeviceListener-test.ts +++ b/test/unit-tests/DeviceListener-test.ts @@ -352,13 +352,13 @@ describe("DeviceListener", () => { mockCrypto!.getCrossSigningKeyId.mockResolvedValue("abc"); }); - it("shows set up encryption toast when user has a key backup available", async () => { + it("shows set up recovery toast when user has a key backup available", async () => { // non falsy response mockCrypto.getKeyBackupInfo.mockResolvedValue({} as unknown as KeyBackupInfo); await createAndStart(); expect(SetupEncryptionToast.showToast).toHaveBeenCalledWith( - SetupEncryptionToast.Kind.SET_UP_ENCRYPTION, + SetupEncryptionToast.Kind.SET_UP_RECOVERY, ); }); }); diff --git a/test/unit-tests/components/structures/MatrixChat-test.tsx b/test/unit-tests/components/structures/MatrixChat-test.tsx index fd17ccf583..bc6bb0f9ff 100644 --- a/test/unit-tests/components/structures/MatrixChat-test.tsx +++ b/test/unit-tests/components/structures/MatrixChat-test.tsx @@ -1003,6 +1003,7 @@ describe("", () => { userHasCrossSigningKeys: jest.fn().mockResolvedValue(false), // This needs to not finish immediately because we need to test the screen appears bootstrapCrossSigning: jest.fn().mockImplementation(() => bootstrapDeferred.promise), + resetKeyBackup: jest.fn(), isEncryptionEnabledInRoom: jest.fn().mockResolvedValue(false), }; loginClient.getCrypto.mockReturnValue(mockCrypto as any); diff --git a/test/unit-tests/toasts/SetupEncryptionToast-test.tsx b/test/unit-tests/toasts/SetupEncryptionToast-test.tsx new file mode 100644 index 0000000000..5ce3fab9ae --- /dev/null +++ b/test/unit-tests/toasts/SetupEncryptionToast-test.tsx @@ -0,0 +1,24 @@ +/* +Copyright 2024 New Vector Ltd. + +SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only +Please see LICENSE files in the repository root for full details. +*/ + +import React from "react"; +import { render, screen } from "jest-matrix-react"; + +import ToastContainer from "../../../src/components/structures/ToastContainer"; +import { Kind, showToast } from "../../../src/toasts/SetupEncryptionToast"; + +describe("SetupEncryptionToast", () => { + beforeEach(() => { + render(); + }); + + it("should render the se up recovery toast", async () => { + showToast(Kind.SET_UP_RECOVERY); + + await expect(screen.findByText("Set up recovery")).resolves.toBeInTheDocument(); + }); +});