Add toast for recovery keys being out of sync (#28946)

* Refine `SettingsSection` & `SettingsTab` * Add encryption tab * Add recovery section * Add device verification * Rename `Panel` into `State` * Update & add tests to user settings common * Add tests to `RecoveryPanel` * Add tests to `ChangeRecoveryKey` * Update CreateSecretStorageDialog-test snapshot * Add tests to `EncryptionUserSettingsTab` * Update existing screenshots of e2e tests * Add new encryption tab ownership to `@element-hq/element-crypto-web-reviewers` * Add e2e tests * Fix monospace font and add figma link to hardcoded value * Add unit to Icon * Improve e2e doc * Assert that the crypto module is defined * Add classname doc * Fix typo * Use `good` state instead of default * Rename `ChangeRecoveryKey.isSetupFlow` into `ChangeRecoveryKey.userHasKeyBackup` * Move `deleteCachedSecrets` fixture in `recovery.spec.ts` * Use one callback instead of two in `RecoveryPanel` * Fix docs and naming of `utils.createBot` * Fix typo in `RecoveryPanel` * Add more doc to the state of the `EncryptionUserSettingsTab` * Rename `verification_required` into `set_up_encryption` * Update test * ADd new license * Very early WIP of rejigged e2e error toast code * Update comments and doc * Assert that `recoveryKey.encodedPrivateKey` is always defined * Add comments to explain how the secrets could be uncached * Use `matrixClient.secretStorage.getDefaultKeyId` instead of `matrixClient.getCrypto().checkKeyBackupAndEnable` to know if we need to set up a recovery key * Update existing screenshot to add encryption tab. * Fix tests * Remove unused file! * Remove test for unused file * Show 'set up encryption' in the 'other' case. * Test 'key storage out of sync' toast * Update tests * Fix test & make toast look correct * Use new labels when changing the recovery key * Fix docs * Don't reset key backup when creating a recovery key * Add playwright test for toast * Dismiss the toast as it's now in the way due to being wider * Doesn't look like this needs to be async * Typo Co-authored-by: Andy Balaam <andy.balaam@matrix.org> * Typo Co-authored-by: Andy Balaam <andy.balaam@matrix.org> * Override width for just this toast --------- Co-authored-by: Florian Duros <florian.duros@ormaz.fr> Co-authored-by: Florian Duros <florianduros@element.io> Co-authored-by: Andy Balaam <andy.balaam@matrix.org>
element-hq · Jan 16, 2025 · 58f812f · 58f812f
1 parent ef1597f
commit 58f812f
Show file tree

Hide file tree

Showing 13 changed files with 109 additions and 106 deletions.
diff --git a/playwright/e2e/crypto/event-shields.spec.ts b/playwright/e2e/crypto/event-shields.spec.ts
@@ -66,6 +66,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();
+
             await bob.sendEvent(testRoomId, null, "m.room.encrypted", {
                 algorithm: "m.megolm.v1.aes-sha2",
                 ciphertext: "the bird is in the hand",

diff --git a/playwright/e2e/crypto/utils.ts b/playwright/e2e/crypto/utils.ts
@@ -413,3 +413,25 @@ export async function createSecondBotDevice(page: Page, homeserver: HomeserverIn
     await bobSecondDevice.prepareClient();
     return bobSecondDevice;
 }
+
+/**
+ * Remove the cached secrets from the indexedDB
+ * This is a workaround to simulate the case where the secrets are not cached.
+ */
+export async function deleteCachedSecrets(page: Page) {
+    await page.evaluate(async () => {
+        const removeCachedSecrets = new Promise((resolve) => {
+            const request = window.indexedDB.open("matrix-js-sdk::matrix-sdk-crypto");
+            request.onsuccess = (event: Event & { target: { result: IDBDatabase } }) => {
+                const db = event.target.result;
+                const request = db.transaction("core", "readwrite").objectStore("core").delete("private_identity");
+                request.onsuccess = () => {
+                    db.close();
+                    resolve(undefined);
+                };
+            };
+        });
+        await removeCachedSecrets;
+    });
+    await page.reload();
+}
diff --git a/playwright/e2e/room/room-header.spec.ts b/playwright/e2e/room/room-header.spec.ts
@@ -111,6 +111,10 @@ test.describe("Room Header", () => {
                 async ({ page, app, user }) => {
                     await createVideoRoom(page, app);
 
+                    // Dismiss a toast that is otherwise in the way (it's the other
+                    // side but there's no need to have it in the screenshot)
+                    await page.getByRole("button", { name: "Later" }).click();
+
                     const header = page.locator(".mx_RoomHeader");
 
                     // There's two room info button - the header itself and the i button

diff --git a/playwright/e2e/settings/encryption-user-tab/recovery.spec.ts b/playwright/e2e/settings/encryption-user-tab/recovery.spec.ts
@@ -6,13 +6,13 @@
  */
 
 import { GeneratedSecretStorageKey } from "matrix-js-sdk/src/crypto-api";
-import { Page } from "@playwright/test";
 
 import { test, expect } from ".";
 import {
     checkDeviceIsConnectedKeyBackup,
     checkDeviceIsCrossSigned,
     createBot,
+    deleteCachedSecrets,
     verifySession,
 } from "../../crypto/utils";
 
@@ -154,25 +154,3 @@ test.describe("Recovery section in Encryption tab", () => {
         },
     );
 });
-
-/**
- * Remove the cached secrets from the indexedDB
- * This is a workaround to simulate the case where the secrets are not cached.
- */
-async function deleteCachedSecrets(page: Page) {
-    await page.evaluate(async () => {
-        const removeCachedSecrets = new Promise((resolve) => {
-            const request = window.indexedDB.open("matrix-js-sdk::matrix-sdk-crypto");
-            request.onsuccess = async (event: Event & { target: { result: IDBDatabase } }) => {
-                const db = event.target.result;
-                const request = db.transaction("core", "readwrite").objectStore("core").delete("private_identity");
-                request.onsuccess = () => {
-                    db.close();
-                    resolve(undefined);
-                };
-            };
-        });
-        await removeCachedSecrets;
-    });
-    await page.reload();
-}
diff --git a/playwright/snapshots/crypto/toasts.spec.ts/key-storage-out-of-sync-toast-linux.png b/playwright/snapshots/crypto/toasts.spec.ts/key-storage-out-of-sync-toast-linux.png
diff --git a/src/DeviceListener.ts b/src/DeviceListener.ts
@@ -34,11 +34,9 @@ import {
     hideToast as hideUnverifiedSessionsToast,
     showToast as showUnverifiedSessionsToast,
 } from "./toasts/UnverifiedSessionToast";
-import { accessSecretStorage, isSecretStorageBeingAccessed } from "./SecurityManager";
-import { isSecureBackupRequired } from "./utils/WellKnownUtils";
+import { isSecretStorageBeingAccessed } from "./SecurityManager";
 import { ActionPayload } from "./dispatcher/payloads";
 import { Action } from "./dispatcher/actions";
-import { isLoggedIn } from "./utils/login";
 import SdkConfig from "./SdkConfig";
 import PlatformPeg from "./PlatformPeg";
 import { recordClientInformation, removeClientInformation } from "./utils/device/clientInformation";
@@ -283,7 +281,21 @@ export default class DeviceListener {
 
         const crossSigningReady = await crypto.isCrossSigningReady();
         const secretStorageReady = await crypto.isSecretStorageReady();
-        const allSystemsReady = crossSigningReady && secretStorageReady;
+        const crossSigningStatus = await crypto.getCrossSigningStatus();
+        const allCrossSigningSecretsCached =
+            crossSigningStatus.privateKeysCachedLocally.masterKey &&
+            crossSigningStatus.privateKeysCachedLocally.selfSigningKey &&
+            crossSigningStatus.privateKeysCachedLocally.userSigningKey;
+
+        const defaultKeyId = await cli.secretStorage.getDefaultKeyId();
+
+        const isCurrentDeviceTrusted =
+            crossSigningReady &&
+            Boolean(
+                (await crypto.getDeviceVerificationStatus(cli.getSafeUserId(), cli.deviceId!))?.crossSigningVerified,
+            );
+
+        const allSystemsReady = crossSigningReady && secretStorageReady && allCrossSigningSecretsCached;
         await this.reportCryptoSessionStateToAnalytics(cli);
 
         if (this.dismissedThisDeviceToast || allSystemsReady) {
@@ -294,31 +306,31 @@ export default class DeviceListener {
             // make sure our keys are finished downloading
             await crypto.getUserDeviceInfo([cli.getSafeUserId()]);
 
-            // cross signing isn't enabled - nag to enable it
-            // There are 3 different toasts for:
-            if (!(await crypto.getCrossSigningKeyId()) && (await crypto.userHasCrossSigningKeys())) {
-                // Toast 1. Cross-signing on account but this device doesn't trust the master key (verify this session)
+            if (!crossSigningReady) {
+                // This account is legacy and doesn't have cross-signing set up at all.
+                // Prompt the user to set it up.
+                showSetupEncryptionToast(SetupKind.SET_UP_ENCRYPTION);
+            } else if (!isCurrentDeviceTrusted) {
+                // cross signing is ready but the current device is not trusted: prompt the user to verify
                 showSetupEncryptionToast(SetupKind.VERIFY_THIS_SESSION);
-                this.checkKeyBackupStatus();
+            } else if (!allCrossSigningSecretsCached) {
+                // cross signing ready & device trusted, but we are missing secrets from our local cache.
+                // prompt the user to enter their recovery key.
+                showSetupEncryptionToast(SetupKind.KEY_STORAGE_OUT_OF_SYNC);
+            } else if (defaultKeyId === null) {
+                // the user just hasn't set up 4S yet: prompt them to do so
+                showSetupEncryptionToast(SetupKind.SET_UP_RECOVERY);
             } else {
-                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 {
-                    // 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);
-                    }
-                }
+                // some other condition... yikes! Show the 'set up encryption' toast: this is what we previously did
+                // in 'other' situations. Possibly we should consider prompting for a full reset in this case?
+                logger.warn("Couldn't match encryption state to a known case: showing 'setup encryption' prompt", {
+                    crossSigningReady,
+                    secretStorageReady,
+                    allCrossSigningSecretsCached,
+                    isCurrentDeviceTrusted,
+                    defaultKeyId,
+                });
+                showSetupEncryptionToast(SetupKind.SET_UP_ENCRYPTION);
             }
         }
 
@@ -334,12 +346,6 @@ export default class DeviceListener {
         // Unverified devices that have appeared since then
         const newUnverifiedDeviceIds = new Set<string>();
 
-        const isCurrentDeviceTrusted =
-            crossSigningReady &&
-            Boolean(
-                (await crypto.getDeviceVerificationStatus(cli.getSafeUserId(), cli.deviceId!))?.crossSigningVerified,
-            );
-
         // as long as cross-signing isn't ready,
         // you can't see or dismiss any device toasts
         if (crossSigningReady) {

diff --git a/src/components/views/toasts/GenericToast.tsx b/src/components/views/toasts/GenericToast.tsx
@@ -25,6 +25,9 @@ interface IPropsExtended extends IProps {
     SecondaryIcon?: ComponentType<React.SVGAttributes<SVGElement>>;
     destructive?: "primary" | "secondary";
     onSecondaryClick(): void;
+
+    // If set, this will override the max-width (of the description) making the toast wider or narrower than standard
+    overrideWidth?: string;
 }
 
 const GenericToast: React.FC<XOR<IPropsExtended, IProps>> = ({
@@ -37,12 +40,13 @@ const GenericToast: React.FC<XOR<IPropsExtended, IProps>> = ({
     destructive,
     onPrimaryClick,
     onSecondaryClick,
+    overrideWidth,
 }) => {
     const detailContent = detail ? <div className="mx_Toast_detail">{detail}</div> : null;
 
     return (
         <div>
-            <div className="mx_Toast_description">
+            <div className="mx_Toast_description" style={{ maxWidth: overrideWidth }}>
                 {description}
                 {detailContent}
             </div>

diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json
@@ -879,14 +879,18 @@
             "title": "Destroy cross-signing keys?",
             "warning": "Deleting cross-signing keys is permanent. Anyone you have verified with will see security alerts. You almost certainly don't want to do this, unless you've lost every device you can cross-sign from."
         },
+        "enter_recovery_key": "Enter recovery key",
         "event_shield_reason_authenticity_not_guaranteed": "The authenticity of this encrypted message can't be guaranteed on this device.",
         "event_shield_reason_mismatched_sender_key": "Encrypted by an unverified session",
         "event_shield_reason_unknown_device": "Encrypted by an unknown or deleted device.",
         "event_shield_reason_unsigned_device": "Encrypted by a device not verified by its owner.",
         "event_shield_reason_unverified_identity": "Encrypted by an unverified user.",
         "export_unsupported": "Your browser does not support the required cryptography extensions",
+        "forgot_recovery_key": "Forgot recovery key?",
         "import_invalid_keyfile": "Not a valid %(brand)s keyfile",
         "import_invalid_passphrase": "Authentication check failed: incorrect password?",
+        "key_storage_out_of_sync": "Your key storage is out of sync.",
+        "key_storage_out_of_sync_description": "Confirm your recovery key to maintain access to your key storage and message history.",
         "messages_not_secure": {
             "cause_1": "Your homeserver",
             "cause_2": "The homeserver the user you're verifying is connected to",

diff --git a/src/toasts/SetupEncryptionToast.ts b/src/toasts/SetupEncryptionToast.ts
@@ -27,6 +27,8 @@ const getTitle = (kind: Kind): string => {
             return _t("encryption|set_up_recovery");
         case Kind.VERIFY_THIS_SESSION:
             return _t("encryption|verify_toast_title");
+        case Kind.KEY_STORAGE_OUT_OF_SYNC:
+            return _t("encryption|key_storage_out_of_sync");
     }
 };
 
@@ -37,6 +39,7 @@ const getIcon = (kind: Kind): string | undefined => {
         case Kind.SET_UP_RECOVERY:
             return undefined;
         case Kind.VERIFY_THIS_SESSION:
+        case Kind.KEY_STORAGE_OUT_OF_SYNC:
             return "verification_warning";
     }
 };
@@ -49,6 +52,8 @@ const getSetupCaption = (kind: Kind): string => {
             return _t("action|continue");
         case Kind.VERIFY_THIS_SESSION:
             return _t("action|verify");
+        case Kind.KEY_STORAGE_OUT_OF_SYNC:
+            return _t("encryption|enter_recovery_key");
     }
 };
 
@@ -59,6 +64,8 @@ const getSecondaryButtonLabel = (kind: Kind): string => {
         case Kind.SET_UP_ENCRYPTION:
         case Kind.VERIFY_THIS_SESSION:
             return _t("encryption|verification|unverified_sessions_toast_reject");
+        case Kind.KEY_STORAGE_OUT_OF_SYNC:
+            return _t("encryption|forgot_recovery_key");
     }
 };
 
@@ -70,6 +77,8 @@ const getDescription = (kind: Kind): string => {
             return _t("encryption|set_up_recovery_toast_description");
         case Kind.VERIFY_THIS_SESSION:
             return _t("encryption|verify_toast_description");
+        case Kind.KEY_STORAGE_OUT_OF_SYNC:
+            return _t("encryption|key_storage_out_of_sync_description");
     }
 };
 
@@ -89,6 +98,10 @@ export enum Kind {
      * Prompt the user to verify this session
      */
     VERIFY_THIS_SESSION = "verify_this_session",
+    /**
+     * Prompt the user to enter their recovery key
+     */
+    KEY_STORAGE_OUT_OF_SYNC = "key_storage_out_of_sync",
 }
 
 const onReject = (): void => {
@@ -139,6 +152,7 @@ export const showToast = (kind: Kind): void => {
             onPrimaryClick: onAccept,
             secondaryLabel: getSecondaryButtonLabel(kind),
             onSecondaryClick: onReject,
+            overrideWidth: kind === Kind.KEY_STORAGE_OUT_OF_SYNC ? "366px" : undefined,
         },
         component: GenericToast,
         priority: kind === Kind.VERIFY_THIS_SESSION ? 95 : 40,

diff --git a/src/utils/login.ts b/src/utils/login.ts
diff --git a/test/unit-tests/DeviceListener-test.ts b/test/unit-tests/DeviceListener-test.ts
@@ -329,32 +329,33 @@ describe("DeviceListener", () => {
                 });
 
                 it("shows verify session toast when account has cross signing", async () => {
-                    mockCrypto!.userHasCrossSigningKeys.mockResolvedValue(true);
+                    mockCrypto!.isCrossSigningReady.mockResolvedValue(true);
                     await createAndStart();
 
                     expect(mockCrypto!.getUserDeviceInfo).toHaveBeenCalled();
                     expect(SetupEncryptionToast.showToast).toHaveBeenCalledWith(
                         SetupEncryptionToast.Kind.VERIFY_THIS_SESSION,
                     );
                 });
-
-                it("checks key backup status when when account has cross signing", async () => {
-                    mockCrypto!.getCrossSigningKeyId.mockResolvedValue(null);
-                    mockCrypto!.userHasCrossSigningKeys.mockResolvedValue(true);
-                    await createAndStart();
-
-                    expect(mockCrypto!.getActiveSessionBackupVersion).toHaveBeenCalled();
-                });
             });
 
             describe("when user does have a cross signing id on this device", () => {
                 beforeEach(() => {
+                    mockCrypto!.isCrossSigningReady.mockResolvedValue(true);
                     mockCrypto!.getCrossSigningKeyId.mockResolvedValue("abc");
+                    mockCrypto!.getDeviceVerificationStatus.mockResolvedValue(
+                        new DeviceVerificationStatus({
+                            trustCrossSignedDevices: true,
+                            crossSigningVerified: true,
+                        }),
+                    );
                 });
 
                 it("shows set up recovery toast when user has a key backup available", async () => {
                     // non falsy response
                     mockCrypto.getKeyBackupInfo.mockResolvedValue({} as unknown as KeyBackupInfo);
+                    mockClient.secretStorage.getDefaultKeyId.mockResolvedValue(null);
+
                     await createAndStart();
 
                     expect(SetupEncryptionToast.showToast).toHaveBeenCalledWith(

diff --git a/test/unit-tests/toasts/SetupEncryptionToast-test.tsx b/test/unit-tests/toasts/SetupEncryptionToast-test.tsx
@@ -16,9 +16,15 @@ describe("SetupEncryptionToast", () => {
         render(<ToastContainer />);
     });
 
-    it("should render the se up recovery toast", async () => {
+    it("should render the 'set up recovery' toast", async () => {
         showToast(Kind.SET_UP_RECOVERY);
 
         await expect(screen.findByText("Set up recovery")).resolves.toBeInTheDocument();
     });
+
+    it("should render the 'key storage out of sync' toast", async () => {
+        showToast(Kind.KEY_STORAGE_OUT_OF_SYNC);
+
+        await expect(screen.findByText("Your key storage is out of sync.")).resolves.toBeInTheDocument();
+    });
 });
diff --git a/test/unit-tests/utils/login-test.ts b/test/unit-tests/utils/login-test.ts