diff --git a/playwright/e2e/crypto/backups.spec.ts b/playwright/e2e/crypto/backups.spec.ts index da162474fa7..d174cc89e5a 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,32 @@ async function expectBackupVersionToBe(page: Page, version: string) { await expect(page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(6) td")).toHaveText(version); } +masTest.describe("Encryption state after registration", () => { + masTest.skip(isDendrite, "does not yet support MAS"); + + masTest("Key backup is enabled by default", 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(); + }); + + masTest("user is prompted to set up recovery", 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 page.getByRole("button", { name: "Add room" }).click(); + await page.getByRole("menuitem", { name: "New room" }).click(); + await page.getByRole("textbox", { name: "Name" }).fill("test room"); + await page.getByRole("button", { name: "Create room" }).click(); + + await expect(page.getByRole("heading", { name: "Set up recovery" })).toBeVisible(); + }); +}); + test.describe("Backups", () => { test.use({ displayName: "Hanako", diff --git a/src/CreateCrossSigning.ts b/src/CreateCrossSigning.ts index e67e030f60e..c38f1a3dd57 100644 --- a/src/CreateCrossSigning.ts +++ b/src/CreateCrossSigning.ts @@ -16,18 +16,19 @@ import { _t } from "./languageHandler"; import InteractiveAuthDialog from "./components/views/dialogs/InteractiveAuthDialog"; /** - * Determine if the homeserver allows uploading device keys with only password auth. + * Determine if the homeserver allows uploading device keys with only password auth, or with no auth at + * all (ie. if the homeserver supports MSC3967). * @param cli The Matrix Client to use - * @returns True if the homeserver allows uploading device keys with only password auth, otherwise false + * @returns True if the homeserver allows uploading device keys with only password auth or with no auth + * at all, otherwise false */ 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. - logger.log("uploadDeviceSigningKeys unexpectedly succeeded without UI auth!"); - return false; + // 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). + 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..af8f91533e7 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; } @@ -164,13 +171,17 @@ export default class LogoutDialog extends React.Component { }; /** - * Show a dialog prompting the user to set up key backup. + * Show a dialog prompting the user to set up their recovery method. + * + * Either: + * * There is no backup at all ({@link BackupStatus.NO_BACKUP}) + * * There is a backup set up but recovery (4s) is not ({@link BackupStatus.BACKUP_NO_RECOVERY}) + * * There is a backup on the server but we are not connected to it ({@link BackupStatus.SERVER_BACKUP_BUT_DISABLED}) + * * We were unable to pull the backup data ({@link BackupStatus.ERROR}). * - * Either there is no backup at all ({@link BackupStatus.NO_BACKUP}), there is a backup on the server but - * we are not connected to it ({@link BackupStatus.SERVER_BACKUP_BUT_DISABLED}), or we were unable to pull the - * backup data ({@link BackupStatus.ERROR}). In all three cases, we should prompt the user to set up key backup. + * In all four cases, we should prompt the user to set up a method of recovery. */ - private renderSetupBackupDialog(): React.ReactNode { + private renderSetupRecoveryMethod(): React.ReactNode { const description = (

{_t("auth|logout_dialog|setup_secure_backup_description_1")}

@@ -254,7 +265,8 @@ export default class LogoutDialog extends React.Component { case BackupStatus.NO_BACKUP: case BackupStatus.SERVER_BACKUP_BUT_DISABLED: case BackupStatus.ERROR: - return this.renderSetupBackupDialog(); + case BackupStatus.BACKUP_NO_RECOVERY: + return this.renderSetupRecoveryMethod(); } } } 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..5554a15d260 100644 --- a/src/stores/InitialCryptoSetupStore.ts +++ b/src/stores/InitialCryptoSetupStore.ts @@ -114,8 +114,15 @@ export class InitialCryptoSetupStore extends EventEmitter { this.emit("update"); try { + // Create the user's cross-signing keys await createCrossSigning(this.client, this.isTokenLogin, this.stores.accountPasswordStore.getPassword()); + // Check for any existing backup and enable key backup if there isn't one + const currentKeyBackup = await cryptoApi.checkKeyBackupAndEnable(); + if (currentKeyBackup === null) { + await cryptoApi.resetKeyBackup(); + } + this.reset(); this.status = "complete"; diff --git a/src/toasts/SetupEncryptionToast.ts b/src/toasts/SetupEncryptionToast.ts index 0dd54bb18fd..406b51cf167 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,22 +45,49 @@ 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"); } }; +/** + * The kind of toast to show. + */ export enum Kind { + /** + * Prompt the user to set up encryption + */ SET_UP_ENCRYPTION = "set_up_encryption", + /** + * Prompt the user to set up a recovery key + */ + SET_UP_RECOVERY = "set_up_recovery", + /** + * Prompt the user to verify this session + */ VERIFY_THIS_SESSION = "verify_this_session", } @@ -64,6 +95,11 @@ const onReject = (): void => { DeviceListener.sharedInstance().dismissEncryptionSetup(); }; +/** + * Show a toast prompting the user for some action related to setting up their encryption. + * + * @param kind The kind of toast to show + */ export const showToast = (kind: Kind): void => { if ( ModuleRunner.instance.extensions.cryptoSetup.setupEncryptionNeeded({ @@ -101,15 +137,17 @@ 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, }); }; +/** + * Hide the encryption setup toast if it is currently being shown. + */ export const hideToast = (): void => { ToastStore.sharedInstance().dismissToast(TOAST_KEY); }; diff --git a/test/test-utils/test-utils.ts b/test/test-utils/test-utils.ts index f9aee512a30..39852f049ee 100644 --- a/test/test-utils/test-utils.ts +++ b/test/test-utils/test-utils.ts @@ -134,6 +134,7 @@ export function createTestClient(): MatrixClient { restoreKeyBackupWithPassphrase: jest.fn(), loadSessionBackupPrivateKeyFromSecretStorage: jest.fn(), storeSessionBackupPrivateKey: jest.fn(), + checkKeyBackupAndEnable: jest.fn().mockResolvedValue(null), getKeyBackupInfo: jest.fn().mockResolvedValue(null), getEncryptionInfoForEvent: jest.fn().mockResolvedValue(null), }), 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..d5db4f190a2 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), + checkKeyBackupAndEnable: 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(); + }); +});