Prompt the user when key storage is unexpectedly off (#29912)

* Assert that we set backup_disabled when turning off key storage

* Prompt the user when key storage is unexpectedly off

* Playwright tests for the Turn on key storage toast

Author: andybalaam

playwright/e2e/crypto/event-shields.spec.ts

@@ -67,8 +67,9 @@ test.describe("Cryptography", function () {
             // Bob has a second, not cross-signed, device
             const bobSecondDevice = await createSecondBotDevice(page, homeserver, bob);
 
-            // Dismiss the toast nagging us to set up recovery otherwise it gets in the way of clicking the room list
-            await page.getByRole("button", { name: "Not now" }).click();
+            // Dismiss the toasts nagging us, otherwise they get in the way of clicking the room list
+            await page.getByRole("button", { name: "Dismiss" }).click();
+            await page.getByRole("button", { name: "Yes, dismiss" }).click();
 
             await bob.sendEvent(testRoomId, null, "m.room.encrypted", {
                 algorithm: "m.megolm.v1.aes-sha2",

playwright/e2e/crypto/toasts.spec.ts

@@ -8,7 +8,8 @@
 import { type GeneratedSecretStorageKey } from "matrix-js-sdk/src/crypto-api";
 
 import { test, expect } from "../../element-web-test";
-import { createBot, deleteCachedSecrets, logIntoElement } from "./utils";
+import { createBot, deleteCachedSecrets, disableKeyBackup, logIntoElement } from "./utils";
+import { type Bot } from "../../pages/bot";
 
 test.describe("Key storage out of sync toast", () => {
     let recoveryKey: GeneratedSecretStorageKey;
@@ -53,3 +54,114 @@ test.describe("Key storage out of sync toast", () => {
         ).toBeVisible();
     });
 });
+
+test.describe("'Turn on key storage' toast", () => {
+    let botClient: Bot | undefined;
+
+    test.beforeEach(async ({ page, homeserver, credentials, toasts }) => {
+        // Set up all crypto stuff. Key storage defaults to on.
+
+        const res = await createBot(page, homeserver, credentials);
+        const recoveryKey = res.recoveryKey;
+        botClient = res.botClient;
+
+        await logIntoElement(page, credentials, recoveryKey.encodedPrivateKey);
+
+        // We won't be prompted for crypto setup unless we have an e2e room, so make one
+        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 toasts.rejectToast("Notifications");
+    });
+
+    test("should not show toast if key storage is on", async ({ page, toasts }) => {
+        // Given the default situation after signing in
+        // Then no toast is shown (because key storage is on)
+        await toasts.assertNoToasts();
+
+        // When we reload
+        await page.reload();
+
+        // Give the toasts time to appear
+        await new Promise((resolve) => setTimeout(resolve, 2000));
+
+        // Then still no toast is shown
+        await toasts.assertNoToasts();
+    });
+
+    test("should not show toast if key storage is off because we turned it off", async ({ app, page, toasts }) => {
+        // Given the backup is disabled because we disabled it
+        await disableKeyBackup(app);
+
+        // Then no toast is shown
+        await toasts.assertNoToasts();
+
+        // When we reload
+        await page.reload();
+
+        // Give the toasts time to appear
+        await new Promise((resolve) => setTimeout(resolve, 2000));
+
+        // Then still no toast is shown
+        await toasts.assertNoToasts();
+    });
+
+    test("should show toast if key storage is off but account data is missing", async ({ app, page, toasts }) => {
+        // Given the backup is disabled but we didn't set account data saying that is expected
+        await disableKeyBackup(app);
+        await botClient.setAccountData("m.org.matrix.custom.backup_disabled", { disabled: false });
+
+        // Wait for the account data setting to stick
+        await new Promise((resolve) => setTimeout(resolve, 2000));
+
+        // When we enter the app
+        await page.reload();
+
+        // Then the toast is displayed
+        let toast = await toasts.getToast("Turn on key storage");
+
+        // And when we click "Continue"
+        await toast.getByRole("button", { name: "Continue" }).click();
+
+        // Then we see the Encryption settings dialog with an option to turn on key storage
+        await expect(page.getByRole("checkbox", { name: "Allow key storage" })).toBeVisible();
+
+        // And when we close that
+        await page.getByRole("button", { name: "Close dialog" }).click();
+
+        // Then we see the toast again
+        toast = await toasts.getToast("Turn on key storage");
+
+        // And when we click "Dismiss"
+        await toast.getByRole("button", { name: "Dismiss" }).click();
+
+        // Then we see the "are you sure?" dialog
+        await expect(
+            page.getByRole("heading", { name: "Are you sure you want to keep key storage turned off?" }),
+        ).toBeVisible();
+
+        // And when we close it by clicking away
+        await page.getByTestId("dialog-background").click({ force: true, position: { x: 10, y: 10 } });
+
+        // Then we see the toast again
+        toast = await toasts.getToast("Turn on key storage");
+
+        // And when we click Dismiss and then "Go to Settings"
+        await toast.getByRole("button", { name: "Dismiss" }).click();
+        await page.getByRole("button", { name: "Go to Settings" }).click();
+
+        // Then we see Encryption settings again
+        await expect(page.getByRole("checkbox", { name: "Allow key storage" })).toBeVisible();
+
+        // And when we close that, see the toast, click Dismiss, and Yes, Dismiss
+        await page.getByRole("button", { name: "Close dialog" }).click();
+        toast = await toasts.getToast("Turn on key storage");
+        await toast.getByRole("button", { name: "Dismiss" }).click();
+        await page.getByRole("button", { name: "Yes, dismiss" }).click();
+
+        // Then the toast is gone
+        await toasts.assertNoToasts();
+    });
+});

playwright/e2e/crypto/utils.ts

@@ -316,6 +316,25 @@ export async function enableKeyBackup(app: ElementAppPage): Promise<string> {
     return recoveryKey;
 }
 
+/**
+ * Open the encryption settings and disable key storage (and recovery)
+ * Assumes that the current device has been verified
+ */
+export async function disableKeyBackup(app: ElementAppPage): Promise<void> {
+    const encryptionTab = await app.settings.openUserSettings("Encryption");
+
+    const keyStorageToggle = encryptionTab.getByRole("checkbox", { name: "Allow key storage" });
+    if (await keyStorageToggle.isChecked()) {
+        await encryptionTab.getByRole("checkbox", { name: "Allow key storage" }).click();
+        await encryptionTab.getByRole("button", { name: "Delete key storage" }).click();
+        await encryptionTab.getByRole("checkbox", { name: "Allow key storage" }).isVisible();
+
+        // Wait for the update to account data to stick
+        await new Promise((resolve) => setTimeout(resolve, 2000));
+    }
+    await app.settings.closeDialog();
+}
+
 /**
  * Go through the "Set up Secure Backup" dialog (aka the `CreateSecretStorageDialog`).
  *

res/css/_common.pcss

@@ -593,6 +593,7 @@ legend {
 .mx_Dialog
     button:not(
         .mx_EncryptionUserSettingsTab button,
+        .mx_EncryptionCard button,
         .mx_UserProfileSettings button,
         .mx_ShareDialog button,
         .mx_UnpinAllDialog button,

res/css/_components.pcss

@@ -131,6 +131,7 @@
 @import "./views/dialogs/_BugReportDialog.pcss";
 @import "./views/dialogs/_ChangelogDialog.pcss";
 @import "./views/dialogs/_CompoundDialog.pcss";
+@import "./views/dialogs/_ConfirmKeyStorageOffDialog.pcss";
 @import "./views/dialogs/_ConfirmSpaceUserActionDialog.pcss";
 @import "./views/dialogs/_ConfirmUserActionDialog.pcss";
 @import "./views/dialogs/_CreateRoomDialog.pcss";

res/css/structures/_ToastContainer.pcss

@@ -79,6 +79,11 @@ Please see LICENSE files in the repository root for full details.
                 background-color: $primary-content;
             }
 
+            &.mx_Toast_icon_key_storage::after {
+                mask-image: url("@vector-im/compound-design-tokens/icons/settings-solid.svg");
+                background-color: $primary-content;
+            }
+
             &.mx_Toast_icon_labs::after {
                 mask-image: url("$(res)/img/element-icons/flask.svg");
                 background-color: $secondary-content;

res/css/views/dialogs/_ConfirmKeyStorageOffDialog.pcss

@@ -0,0 +1,16 @@
+/*
+Copyright 2025 New Vector Ltd.
+
+SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
+Please see LICENSE files in the repository root for full details.
+*/
+
+.mx_ConfirmKeyStorageOffDialog {
+    .mx_Dialog_border {
+        width: 600px;
+    }
+
+    .mx_EncryptionCard {
+        text-align: center;
+    }
+}

src/DeviceListener.ts

@@ -97,6 +97,7 @@ export default class DeviceListener {
         this.client.on(CryptoEvent.DevicesUpdated, this.onDevicesUpdated);
         this.client.on(CryptoEvent.UserTrustStatusChanged, this.onUserTrustStatusChanged);
         this.client.on(CryptoEvent.KeysChanged, this.onCrossSingingKeysChanged);
+        this.client.on(CryptoEvent.KeyBackupStatus, this.onKeyBackupStatusChanged);
         this.client.on(ClientEvent.AccountData, this.onAccountData);
         this.client.on(ClientEvent.Sync, this.onSync);
         this.client.on(RoomStateEvent.Events, this.onRoomStateEvents);
@@ -132,7 +133,7 @@ export default class DeviceListener {
         this.dismissedThisDeviceToast = false;
         this.keyBackupInfo = null;
         this.keyBackupFetchedAt = null;
-        this.cachedKeyBackupStatus = undefined;
+        this.cachedKeyBackupUploadActive = undefined;
         this.ourDeviceIdsAtStart = null;
         this.displayingToastsForDeviceIds = new Set();
         this.client = undefined;
@@ -157,6 +158,13 @@ export default class DeviceListener {
         this.recheck();
     }
 
+    /**
+     * Set the account data "m.org.matrix.custom.backup_disabled" to { "disabled": true }.
+     */
+    public async recordKeyBackupDisabled(): Promise<void> {
+        await this.client?.setAccountData(BACKUP_DISABLED_ACCOUNT_DATA_KEY, { disabled: true });
+    }
+
     private async ensureDeviceIdsAtStartPopulated(): Promise<void> {
         if (this.ourDeviceIdsAtStart === null) {
             this.ourDeviceIdsAtStart = await this.getDeviceIds();
@@ -192,6 +200,11 @@ export default class DeviceListener {
         this.recheck();
     };
 
+    private onKeyBackupStatusChanged = (): void => {
+        this.cachedKeyBackupUploadActive = undefined;
+        this.recheck();
+    };
+
     private onCrossSingingKeysChanged = (): void => {
         this.recheck();
     };
@@ -201,11 +214,13 @@ export default class DeviceListener {
         // * migrated SSSS to symmetric
         // * uploaded keys to secret storage
         // * completed secret storage creation
+        // * disabled key backup
         // which result in account data changes affecting checks below.
         if (
             ev.getType().startsWith("m.secret_storage.") ||
             ev.getType().startsWith("m.cross_signing.") ||
-            ev.getType() === "m.megolm_backup.v1"
+            ev.getType() === "m.megolm_backup.v1" ||
+            ev.getType() === BACKUP_DISABLED_ACCOUNT_DATA_KEY
         ) {
             this.recheck();
         }
@@ -324,7 +339,16 @@ export default class DeviceListener {
                 (await crypto.getDeviceVerificationStatus(cli.getSafeUserId(), cli.deviceId!))?.crossSigningVerified,
             );
 
-        const allSystemsReady = crossSigningReady && secretStorageReady && allCrossSigningSecretsCached;
+        const keyBackupUploadActive = await this.isKeyBackupUploadActive();
+        const backupDisabled = await this.recheckBackupDisabled(cli);
+
+        // We warn if key backup upload is turned off and we have not explicitly
+        // said we are OK with that.
+        const keyBackupIsOk = keyBackupUploadActive || backupDisabled;
+
+        const allSystemsReady =
+            crossSigningReady && keyBackupIsOk && secretStorageReady && allCrossSigningSecretsCached;
+
         await this.reportCryptoSessionStateToAnalytics(cli);
 
         if (this.dismissedThisDeviceToast || allSystemsReady) {
@@ -353,14 +377,19 @@ export default class DeviceListener {
                     crossSigningStatus.privateKeysCachedLocally,
                 );
                 showSetupEncryptionToast(SetupKind.KEY_STORAGE_OUT_OF_SYNC);
+            } else if (!keyBackupIsOk) {
+                logSpan.info("Key backup upload is unexpectedly turned off: showing TURN_ON_KEY_STORAGE toast");
+                showSetupEncryptionToast(SetupKind.TURN_ON_KEY_STORAGE);
             } else if (defaultKeyId === null) {
-                // the user just hasn't set up 4S yet: prompt them to do so (unless they've explicitly said no to key storage)
-                const disabledEvent = cli.getAccountData(BACKUP_DISABLED_ACCOUNT_DATA_KEY);
-                if (!disabledEvent?.getContent().disabled) {
+                // The user just hasn't set up 4S yet: if they have key
+                // backup, prompt them to turn on recovery too. (If not, they
+                // have explicitly opted out, so don't hassle them.)
+                if (keyBackupUploadActive) {
                     logSpan.info("No default 4S key: showing SET_UP_RECOVERY toast");
                     showSetupEncryptionToast(SetupKind.SET_UP_RECOVERY);
                 } else {
                     logSpan.info("No default 4S key but backup disabled: no toast needed");
+                    hideSetupEncryptionToast();
                 }
             } else {
                 // some other condition... yikes! Show the 'set up encryption' toast: this is what we previously did
@@ -443,6 +472,16 @@ export default class DeviceListener {
         this.displayingToastsForDeviceIds = newUnverifiedDeviceIds;
     }
 
+    /**
+     * Fetch the account data for `backup_disabled`. If this is the first time,
+     * fetch it from the server (in case the initial sync has not finished).
+     * Otherwise, fetch it from the store as normal.
+     */
+    private async recheckBackupDisabled(cli: MatrixClient): Promise<boolean> {
+        const backupDisabled = await cli.getAccountDataFromServer(BACKUP_DISABLED_ACCOUNT_DATA_KEY);
+        return !!backupDisabled?.disabled;
+    }
+
     /**
      * Reports current recovery state to analytics.
      * Checks if the session is verified and if the recovery is correctly set up (i.e all secrets known locally and in 4S).
@@ -512,36 +551,42 @@ export default class DeviceListener {
      * trigger an auto-rageshake).
      */
     private checkKeyBackupStatus = async (): Promise<void> => {
-        if (!(await this.getKeyBackupStatus())) {
+        if (!(await this.isKeyBackupUploadActive())) {
             dis.dispatch({ action: Action.ReportKeyBackupNotEnabled });
         }
     };
 
     /**
      * Is key backup enabled? Use a cached answer if we have one.
      */
-    private getKeyBackupStatus = async (): Promise<boolean> => {
+    private isKeyBackupUploadActive = async (): Promise<boolean> => {
         if (!this.client) {
             // To preserve existing behaviour, if there is no client, we
-            // pretend key storage is on.
+            // pretend key backup upload is on.
             //
             // Someone looking to improve this code could try throwing an error
             // here since we don't expect client to be undefined.
             return true;
         }
 
+        const crypto = this.client.getCrypto();
+        if (!crypto) {
+            // If there is no crypto, there is no key backup
+            return false;
+        }
+
         // If we've already cached the answer, return it.
-        if (this.cachedKeyBackupStatus !== undefined) {
-            return this.cachedKeyBackupStatus;
+        if (this.cachedKeyBackupUploadActive !== undefined) {
+            return this.cachedKeyBackupUploadActive;
         }
 
         // Fetch the answer and cache it
-        const activeKeyBackupVersion = await this.client.getCrypto()?.getActiveSessionBackupVersion();
-        this.cachedKeyBackupStatus = !!activeKeyBackupVersion;
+        const activeKeyBackupVersion = await crypto.getActiveSessionBackupVersion();
+        this.cachedKeyBackupUploadActive = !!activeKeyBackupVersion;
 
-        return this.cachedKeyBackupStatus;
+        return this.cachedKeyBackupUploadActive;
     };
-    private cachedKeyBackupStatus: boolean | undefined = undefined;
+    private cachedKeyBackupUploadActive: boolean | undefined = undefined;
 
     private onRecordClientInformationSettingChange: CallbackFn = (
         _originalSettingName,