Enable key backup by default

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",

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<boolean> {
     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!");

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);
+                    }
                 }
             }
         }

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<IProps, IState> {
         }
 
         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<IProps, IState> {
     };
 
     /**
-     * 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 three cases, we should prompt the user to set up a method of recovery.
      */
-    private renderSetupBackupDialog(): React.ReactNode {
+    private renderSetupRecoveryMethod(): React.ReactNode {
         const description = (
             <div>
                 <p>{_t("auth|logout_dialog|setup_secure_backup_description_1")}</p>
@@ -254,7 +265,8 @@ export default class LogoutDialog extends React.Component<IProps, IState> {
             case BackupStatus.NO_BACKUP:
             case BackupStatus.SERVER_BACKUP_BUT_DISABLED:
             case BackupStatus.ERROR:
-                return this.renderSetupBackupDialog();
+            case BackupStatus.BACKUP_NO_RECOVERY:
+                return this.renderSetupRecoveryMethod();
         }
     }
 }

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? <a>Reset all</a>",
+        "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": {

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";

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,29 +45,55 @@ 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",
 }
 
 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 +131,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);
 };

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),
         }),

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,
                     );
                 });
             });

test/unit-tests/components/structures/MatrixChat-test.tsx

@@ -1003,7 +1003,9 @@ describe("<MatrixChat />", () => {
                     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);
             });

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();

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(<ToastContainer />);
+    });
+
+    it("should render the se up recovery toast", async () => {
+        showToast(Kind.SET_UP_RECOVERY);
+
+        await expect(screen.findByText("Set up recovery")).resolves.toBeInTheDocument();
+    });
+});