diff --git a/playwright/e2e/crypto/backups.spec.ts b/playwright/e2e/crypto/backups.spec.ts index da162474fa7..4e0877a2bd8 100644 --- a/playwright/e2e/crypto/backups.spec.ts +++ b/playwright/e2e/crypto/backups.spec.ts @@ -9,6 +9,8 @@ Please see LICENSE files in the repository root for full details. import { type Page } from "@playwright/test"; import { test, expect } from "../../element-web-test"; +import { test as masTest, registerAccountMas } from "../oidc"; +import { isDendrite } from "../../plugins/homeserver/dendrite"; async function expectBackupVersionToBe(page: Page, version: string) { await expect(page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(5) td")).toHaveText( @@ -18,6 +20,19 @@ async function expectBackupVersionToBe(page: Page, version: string) { await expect(page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(6) td")).toHaveText(version); } +masTest.describe("Key backup is enabled by default after registration", () => { + masTest.skip(isDendrite, "does not yet support MAS"); + + masTest("Key backup is enabled by default after registration", async ({ page, mailhog, app }) => { + await page.goto("/#/login"); + await page.getByRole("button", { name: "Continue" }).click(); + await registerAccountMas(page, mailhog.api, "alice", "alice@email.com", "Pa$sW0rD!"); + + await app.settings.openUserSettings("Security & Privacy"); + expect(page.getByText("This session is backing up your keys.")).toBeVisible(); + }); +}); + test.describe("Backups", () => { test.use({ displayName: "Hanako", diff --git a/src/CreateCrossSigning.ts b/src/CreateCrossSigning.ts index e67e030f60e..8c2d4488cf9 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 84d83827da5..e34af95962c 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/components/views/dialogs/LogoutDialog.tsx b/src/components/views/dialogs/LogoutDialog.tsx index 4731c593bc3..ee16bb726fe 100644 --- a/src/components/views/dialogs/LogoutDialog.tsx +++ b/src/components/views/dialogs/LogoutDialog.tsx @@ -38,6 +38,9 @@ enum BackupStatus { /** there is a backup on the server but we are not backing up to it */ SERVER_BACKUP_BUT_DISABLED, + /** Key backup is set up but recovery (4s) is not */ + BACKUP_NO_RECOVERY, + /** backup is not set up locally and there is no backup on the server */ NO_BACKUP, @@ -104,7 +107,11 @@ export default class LogoutDialog extends React.Component { } if ((await crypto.getActiveSessionBackupVersion()) !== null) { - this.setState({ backupStatus: BackupStatus.BACKUP_ACTIVE }); + if (await crypto.isSecretStorageReady()) { + this.setState({ backupStatus: BackupStatus.BACKUP_ACTIVE }); + } else { + this.setState({ backupStatus: BackupStatus.BACKUP_NO_RECOVERY }); + } return; } @@ -254,6 +261,7 @@ export default class LogoutDialog extends React.Component { case BackupStatus.NO_BACKUP: case BackupStatus.SERVER_BACKUP_BUT_DISABLED: case BackupStatus.ERROR: + case BackupStatus.BACKUP_NO_RECOVERY: return this.renderSetupBackupDialog(); } } diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index e9ac73b48b4..f3c514fca38 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 0c2e49f5caf..dd6b1f04619 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 0dd54bb18fd..1d6cbe74906 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 ad7f14e1190..1c8fe1a1c73 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 fd17ccf5838..0838bf33f03 100644 --- a/test/unit-tests/components/structures/MatrixChat-test.tsx +++ b/test/unit-tests/components/structures/MatrixChat-test.tsx @@ -1003,7 +1003,9 @@ 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), + getKeyBackupInfo: jest.fn().mockResolvedValue(null), }; loginClient.getCrypto.mockReturnValue(mockCrypto as any); }); diff --git a/test/unit-tests/components/views/dialogs/LogoutDialog-test.tsx b/test/unit-tests/components/views/dialogs/LogoutDialog-test.tsx index 0557e538d0e..98f758ebbd6 100644 --- a/test/unit-tests/components/views/dialogs/LogoutDialog-test.tsx +++ b/test/unit-tests/components/views/dialogs/LogoutDialog-test.tsx @@ -42,12 +42,20 @@ describe("LogoutDialog", () => { expect(rendered.container).toMatchSnapshot(); }); - it("shows a regular dialog if backups are working", async () => { + it("shows a regular dialog if backups and recovery are working", async () => { mockCrypto.getActiveSessionBackupVersion.mockResolvedValue("1"); + mockCrypto.isSecretStorageReady.mockResolvedValue(true); const rendered = renderComponent(); await rendered.findByText("Are you sure you want to sign out?"); }); + it("prompts user to set up recovery if backups are enabled but recovery isn't", async () => { + mockCrypto.getActiveSessionBackupVersion.mockResolvedValue("1"); + mockCrypto.isSecretStorageReady.mockResolvedValue(false); + const rendered = renderComponent(); + await rendered.findByText("You'll lose access to your encrypted messages"); + }); + it("Prompts user to connect backup if there is a backup on the server", async () => { mockCrypto.getKeyBackupInfo.mockResolvedValue({} as KeyBackupInfo); const rendered = renderComponent(); diff --git a/test/unit-tests/toasts/SetupEncryptionToast-test.tsx b/test/unit-tests/toasts/SetupEncryptionToast-test.tsx new file mode 100644 index 00000000000..5ce3fab9aeb --- /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(); + }); +});