From aee24be1b466f6f9f5fb88ed6df0821531866090 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Thu, 20 Nov 2025 15:25:31 -0500 Subject: [PATCH] Key storage out of sync: reset key backup when needed (#31279) * add function to pause device listener * add function to check if key backup key missing both locally and in 4s * reset backup if backup key missing both locally and in 4s * fixup! add function to check if key backup key missing both locally and in 4s * Drop KEY_STORAGE_OUT_OF_SYNC_STORE in favour of checking cross-signing Check if cross-signing needs resetting, because that seems to be what KEY_STORAGE_OUT_OF_SYNC_STORE is actually trying to do. * add a function for resetting key backup and waiting until it's ready * trigger key storage out of sync toast when missing backup key locally and fetch it when user enters their recovery key * reset backup when needed if user forgets recovery key * rename function as suggested in code review --- src/DeviceListener.ts | 101 ++++++++++++- .../encryption/KeyStoragePanelViewModel.ts | 96 ++++++------- .../settings/encryption/ChangeRecoveryKey.tsx | 31 ++-- src/toasts/SetupEncryptionToast.ts | 79 +++++++---- src/utils/crypto/resetKeyBackup.ts | 20 +++ test/test-utils/test-utils.ts | 1 + test/unit-tests/DeviceListener-test.ts | 133 +++++++++++++++++- .../encryption/ChangeRecoveryKey-test.tsx | 55 ++++++++ .../toasts/SetupEncryptionToast-test.tsx | 116 +++++++++++---- 9 files changed, 512 insertions(+), 120 deletions(-) create mode 100644 src/utils/crypto/resetKeyBackup.ts diff --git a/src/DeviceListener.ts b/src/DeviceListener.ts index 4893910540..ff58517cd2 100644 --- a/src/DeviceListener.ts +++ b/src/DeviceListener.ts @@ -1,4 +1,5 @@ /* +Copyright 2025 Element Creations Ltd. Copyright 2024 New Vector Ltd. Copyright 2020 The Matrix.org Foundation C.I.C. @@ -144,6 +145,25 @@ export default class DeviceListener { this.client = undefined; } + /** + * Pause the device listener while a function runs. + * + * This can be done if the function makes several changes that would trigger + * multiple events, to suppress warning toasts until the process is + * finished. + */ + public async whilePaused(fn: () => Promise): Promise { + const client = this.client; + try { + this.stop(); + await fn(); + } finally { + if (client) { + this.start(client); + } + } + } + /** * Dismiss notifications about our own unverified devices * @@ -177,6 +197,67 @@ export default class DeviceListener { await this.client?.setAccountData(RECOVERY_ACCOUNT_DATA_KEY, { enabled: false }); } + /** + * If a `Kind.KEY_STORAGE_OUT_OF_SYNC` condition from {@link doRecheck} + * requires a reset of cross-signing keys. + * + * We will reset cross-signing keys if both our local cache and 4S don't + * have all cross-signing keys. + * + * In theory, if the set of keys in our cache and in 4S are different, and + * we have a complete set between the two, we could be OK, but that + * should be exceptionally rare, and is more complicated to detect. + */ + public async keyStorageOutOfSyncNeedsCrossSigningReset(forgotRecovery: boolean): Promise { + const crypto = this.client?.getCrypto(); + if (!crypto) { + return false; + } + const crossSigningStatus = await crypto.getCrossSigningStatus(); + const allCrossSigningSecretsCached = + crossSigningStatus.privateKeysCachedLocally.masterKey && + crossSigningStatus.privateKeysCachedLocally.selfSigningKey && + crossSigningStatus.privateKeysCachedLocally.userSigningKey; + + if (forgotRecovery) { + return !allCrossSigningSecretsCached; + } else { + return !allCrossSigningSecretsCached && !crossSigningStatus.privateKeysInSecretStorage; + } + } + + /** + * If a `Kind.KEY_STORAGE_OUT_OF_SYNC` condition from {@link doRecheck} + * requires a reset of key backup. + * + * If the user has their recovery key, we need to reset backup if: + * - the user hasn't disabled backup, + * - we don't have the backup key cached locally, *and* + * - we don't have the backup key stored in 4S. + * (The user should already have a key backup created at this point, + * otherwise `doRecheck` would have triggered a `Kind.TURN_ON_KEY_STORAGE` + * condition.) + * + * If the user has forgotten their recovery key, we need to reset backup if: + * - the user hasn't disabled backup, and + * - we don't have the backup key locally. + */ + public async keyStorageOutOfSyncNeedsBackupReset(forgotRecovery: boolean): Promise { + const crypto = this.client?.getCrypto(); + if (!crypto) { + return false; + } + const shouldHaveBackup = !(await this.recheckBackupDisabled(this.client!)); + const backupKeyCached = (await crypto.getSessionBackupPrivateKey()) !== null; + const backupKeyStored = await this.client!.isKeyBackupKeyStored(); + + if (forgotRecovery) { + return shouldHaveBackup && !backupKeyCached; + } else { + return shouldHaveBackup && !backupKeyCached && !backupKeyStored; + } + } + private async ensureDeviceIdsAtStartPopulated(): Promise { if (this.ourDeviceIdsAtStart === null) { this.ourDeviceIdsAtStart = await this.getDeviceIds(); @@ -357,7 +438,10 @@ export default class DeviceListener { // said we are OK with that. const keyBackupIsOk = keyBackupUploadActive || backupDisabled; - const allSystemsReady = isCurrentDeviceTrusted && allCrossSigningSecretsCached && keyBackupIsOk && recoveryIsOk; + const backupKeyCached = (await crypto.getSessionBackupPrivateKey()) !== null; + + const allSystemsReady = + isCurrentDeviceTrusted && allCrossSigningSecretsCached && keyBackupIsOk && recoveryIsOk && backupKeyCached; await this.reportCryptoSessionStateToAnalytics(cli); @@ -401,15 +485,22 @@ export default class DeviceListener { } } else { // If we get here, then we are verified, have key backup, and - // 4S, but crypto.isSecretStorageReady returned false, which - // means that 4S doesn't have all the secrets. - logSpan.warn("4S is missing secrets", { + // 4S, but allSystemsReady is false, which means that either + // secretStorageStatus.ready is false (which means that 4S + // doesn't have all the secrets), or we don't have the backup + // key cached locally. + logSpan.warn("4S is missing secrets or backup key not cached", { crossSigningReady, secretStorageStatus, allCrossSigningSecretsCached, isCurrentDeviceTrusted, + backupKeyCached, }); - showSetupEncryptionToast(SetupKind.KEY_STORAGE_OUT_OF_SYNC_STORE); + // We use the right toast variant based on whether the backup + // key is missing locally. If any of the cross-signing keys are + // missing locally, that is handled by the + // `!allCrossSigningSecretsCached` branch above. + showSetupEncryptionToast(SetupKind.KEY_STORAGE_OUT_OF_SYNC); } } else { logSpan.info("Not yet ready, but shouldShowSetupEncryptionToast==false"); diff --git a/src/components/viewmodels/settings/encryption/KeyStoragePanelViewModel.ts b/src/components/viewmodels/settings/encryption/KeyStoragePanelViewModel.ts index ca15b6a6e3..e0979d8dbb 100644 --- a/src/components/viewmodels/settings/encryption/KeyStoragePanelViewModel.ts +++ b/src/components/viewmodels/settings/encryption/KeyStoragePanelViewModel.ts @@ -12,6 +12,7 @@ import { logger } from "matrix-js-sdk/src/logger"; import { useMatrixClientContext } from "../../../../contexts/MatrixClientContext"; import DeviceListener, { BACKUP_DISABLED_ACCOUNT_DATA_KEY } from "../../../../DeviceListener"; import { useEventEmitterAsyncState } from "../../../../hooks/useEventEmitter"; +import { resetKeyBackupAndWait } from "../../../../utils/crypto/resetKeyBackup"; interface KeyStoragePanelState { /** @@ -75,63 +76,58 @@ export function useKeyStoragePanelViewModel(): KeyStoragePanelState { async (enable: boolean) => { setPendingValue(enable); try { - // stop the device listener since enabling or (especially) disabling key storage must be + // pause the device listener since enabling or (especially) disabling key storage must be // done with a sequence of API calls that will put the account in a slightly different - // state each time, so suppress any warning toasts until the process is finished (when - // we'll turn it back on again.) - DeviceListener.sharedInstance().stop(); - - const crypto = matrixClient.getCrypto(); - if (!crypto) { - logger.error("Can't change key backup status: no crypto module available"); - return; - } - if (enable) { - const childLogger = logger.getChild("[enable key storage]"); - childLogger.info("User requested enabling key storage"); - let currentKeyBackup = await crypto.checkKeyBackupAndEnable(); - if (currentKeyBackup) { - logger.info( - `Existing key backup is present. version: ${currentKeyBackup.backupInfo.version}`, - currentKeyBackup.trustInfo, - ); - // Check if the current key backup can be used. Either of these properties causes the key backup to be used. - if (currentKeyBackup.trustInfo.trusted || currentKeyBackup.trustInfo.matchesDecryptionKey) { - logger.info("Existing key backup can be used"); + // state each time, so suppress any warning toasts until the process is finished + await DeviceListener.sharedInstance().whilePaused(async () => { + const crypto = matrixClient.getCrypto(); + if (!crypto) { + logger.error("Can't change key backup status: no crypto module available"); + return; + } + if (enable) { + const childLogger = logger.getChild("[enable key storage]"); + childLogger.info("User requested enabling key storage"); + let currentKeyBackup = await crypto.checkKeyBackupAndEnable(); + if (currentKeyBackup) { + logger.info( + `Existing key backup is present. version: ${currentKeyBackup.backupInfo.version}`, + currentKeyBackup.trustInfo, + ); + // Check if the current key backup can be used. Either of these properties causes the key backup to be used. + if (currentKeyBackup.trustInfo.trusted || currentKeyBackup.trustInfo.matchesDecryptionKey) { + logger.info("Existing key backup can be used"); + } else { + logger.warn("Existing key backup cannot be used, creating new backup"); + // There aren't any *usable* backups, so we need to create a new one. + currentKeyBackup = null; + } } else { - logger.warn("Existing key backup cannot be used, creating new backup"); - // There aren't any *usable* backups, so we need to create a new one. - currentKeyBackup = null; + logger.info("No existing key backup versions are present, creating new backup"); } + + // If there is no usable key backup on the server, create one. + // `resetKeyBackup` will delete any existing backup, so we only do this if there is no usable backup. + if (currentKeyBackup === null) { + await resetKeyBackupAndWait(crypto); + } + + // Set the flag so that EX no longer thinks the user wants backup disabled + await matrixClient.setAccountData(BACKUP_DISABLED_ACCOUNT_DATA_KEY, { disabled: false }); } else { - logger.info("No existing key backup versions are present, creating new backup"); + logger.info("User requested disabling key backup"); + // This method will delete the key backup as well as server side recovery keys and other + // server-side crypto data. + await crypto.disableKeyStorage(); + + // Set a flag to say that the user doesn't want key backup. + // Element X uses this to determine whether to set up automatically, + // so this will stop EX turning it back on spontaneously. + await matrixClient.setAccountData(BACKUP_DISABLED_ACCOUNT_DATA_KEY, { disabled: true }); } - - // If there is no usable key backup on the server, create one. - // `resetKeyBackup` will delete any existing backup, so we only do this if there is no usable backup. - if (currentKeyBackup === null) { - await crypto.resetKeyBackup(); - // resetKeyBackup fires this off in the background without waiting, so we need to do it - // explicitly and wait for it, otherwise it won't be enabled yet when we check again. - await crypto.checkKeyBackupAndEnable(); - } - - // Set the flag so that EX no longer thinks the user wants backup disabled - await matrixClient.setAccountData(BACKUP_DISABLED_ACCOUNT_DATA_KEY, { disabled: false }); - } else { - logger.info("User requested disabling key backup"); - // This method will delete the key backup as well as server side recovery keys and other - // server-side crypto data. - await crypto.disableKeyStorage(); - - // Set a flag to say that the user doesn't want key backup. - // Element X uses this to determine whether to set up automatically, - // so this will stop EX turning it back on spontaneously. - await matrixClient.setAccountData(BACKUP_DISABLED_ACCOUNT_DATA_KEY, { disabled: true }); - } + }); } finally { setPendingValue(undefined); - DeviceListener.sharedInstance().start(matrixClient); } }, [setPendingValue, matrixClient], diff --git a/src/components/views/settings/encryption/ChangeRecoveryKey.tsx b/src/components/views/settings/encryption/ChangeRecoveryKey.tsx index ae568efc62..21ff7437d8 100644 --- a/src/components/views/settings/encryption/ChangeRecoveryKey.tsx +++ b/src/components/views/settings/encryption/ChangeRecoveryKey.tsx @@ -1,4 +1,5 @@ /* + * Copyright 2025 Element Creations Ltd. * Copyright 2024 New Vector Ltd. * * SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only @@ -29,7 +30,8 @@ import { initialiseDehydrationIfEnabled } from "../../../../utils/device/dehydra import { withSecretStorageKeyCache } from "../../../../SecurityManager"; import { EncryptionCardButtons } from "./EncryptionCardButtons"; import { logErrorAndShowErrorDialog } from "../../../../utils/ErrorUtils.tsx"; -import { RECOVERY_ACCOUNT_DATA_KEY } from "../../../../DeviceListener"; +import DeviceListener, { RECOVERY_ACCOUNT_DATA_KEY } from "../../../../DeviceListener"; +import { resetKeyBackupAndWait } from "../../../../utils/crypto/resetKeyBackup"; /** * The possible states of the component. @@ -123,14 +125,27 @@ export function ChangeRecoveryKey({ if (!crypto) return onFinish(); try { - // We need to enable the cache to avoid to prompt the user to enter the new key - // when we will try to access the secret storage during the bootstrap - await withSecretStorageKeyCache(async () => { - await crypto.bootstrapSecretStorage({ - setupNewSecretStorage: true, - createSecretStorageKey: async () => recoveryKey, + const deviceListener = DeviceListener.sharedInstance(); + + // we need to call keyStorageOutOfSyncNeedsBackupReset here because + // deviceListener.whilePaused() sets its client to undefined, so + // keyStorageOutOfSyncNeedsBackupReset won't be able to check + // the backup state. + const needsBackupReset = await deviceListener.keyStorageOutOfSyncNeedsBackupReset(true); + await deviceListener.whilePaused(async () => { + // We need to enable the cache to avoid to prompt the user to enter the new key + // when we will try to access the secret storage during the bootstrap + await withSecretStorageKeyCache(async () => { + await crypto.bootstrapSecretStorage({ + setupNewSecretStorage: true, + createSecretStorageKey: async () => recoveryKey, + }); + // Reset the key backup if needed + if (needsBackupReset) { + await resetKeyBackupAndWait(crypto); + } + await initialiseDehydrationIfEnabled(matrixClient, { createNewKey: true }); }); - await initialiseDehydrationIfEnabled(matrixClient, { createNewKey: true }); }); // Record the fact that the user explicitly enabled recovery. diff --git a/src/toasts/SetupEncryptionToast.ts b/src/toasts/SetupEncryptionToast.ts index 9bcab8d7b1..21e0ea4fa4 100644 --- a/src/toasts/SetupEncryptionToast.ts +++ b/src/toasts/SetupEncryptionToast.ts @@ -1,4 +1,5 @@ /* +Copyright 2025 Element Creations Ltd. Copyright 2024 New Vector Ltd. Copyright 2020 The Matrix.org Foundation C.I.C. @@ -26,6 +27,8 @@ import { Action } from "../dispatcher/actions"; import { UserTab } from "../components/views/dialogs/UserTab"; import defaultDispatcher from "../dispatcher/dispatcher"; import ConfirmKeyStorageOffDialog from "../components/views/dialogs/ConfirmKeyStorageOffDialog"; +import { MatrixClientPeg } from "../MatrixClientPeg"; +import { resetKeyBackupAndWait } from "../utils/crypto/resetKeyBackup"; import { PosthogAnalytics } from "../PosthogAnalytics"; const TOAST_KEY = "setupencryption"; @@ -37,7 +40,6 @@ const getTitle = (kind: Kind): string => { case Kind.VERIFY_THIS_SESSION: return _t("encryption|verify_toast_title"); case Kind.KEY_STORAGE_OUT_OF_SYNC: - case Kind.KEY_STORAGE_OUT_OF_SYNC_STORE: return _t("encryption|key_storage_out_of_sync"); case Kind.TURN_ON_KEY_STORAGE: return _t("encryption|turn_on_key_storage"); @@ -50,7 +52,6 @@ const getIcon = (kind: Kind): string | undefined => { return undefined; case Kind.VERIFY_THIS_SESSION: case Kind.KEY_STORAGE_OUT_OF_SYNC: - case Kind.KEY_STORAGE_OUT_OF_SYNC_STORE: return "verification_warning"; case Kind.TURN_ON_KEY_STORAGE: return "key_storage"; @@ -64,7 +65,6 @@ const getSetupCaption = (kind: Kind): string => { case Kind.VERIFY_THIS_SESSION: return _t("action|verify"); case Kind.KEY_STORAGE_OUT_OF_SYNC: - case Kind.KEY_STORAGE_OUT_OF_SYNC_STORE: return _t("encryption|enter_recovery_key"); case Kind.TURN_ON_KEY_STORAGE: return _t("action|continue"); @@ -78,7 +78,6 @@ const getSetupCaption = (kind: Kind): string => { const getPrimaryButtonIcon = (kind: Kind): ComponentType> | undefined => { switch (kind) { case Kind.KEY_STORAGE_OUT_OF_SYNC: - case Kind.KEY_STORAGE_OUT_OF_SYNC_STORE: return KeyIcon; default: return; @@ -92,7 +91,6 @@ const getSecondaryButtonLabel = (kind: Kind): string => { case Kind.VERIFY_THIS_SESSION: return _t("encryption|verification|unverified_sessions_toast_reject"); case Kind.KEY_STORAGE_OUT_OF_SYNC: - case Kind.KEY_STORAGE_OUT_OF_SYNC_STORE: return _t("encryption|forgot_recovery_key"); case Kind.TURN_ON_KEY_STORAGE: return _t("action|dismiss"); @@ -106,7 +104,6 @@ const getDescription = (kind: Kind): string => { case Kind.VERIFY_THIS_SESSION: return _t("encryption|verify_toast_description"); case Kind.KEY_STORAGE_OUT_OF_SYNC: - case Kind.KEY_STORAGE_OUT_OF_SYNC_STORE: return _t("encryption|key_storage_out_of_sync_description"); case Kind.TURN_ON_KEY_STORAGE: return _t("encryption|turn_on_key_storage_description"); @@ -126,13 +123,9 @@ export enum Kind { */ VERIFY_THIS_SESSION = "verify_this_session", /** - * Prompt the user to enter their recovery key, to retrieve secrets + * Prompt the user to enter their recovery key */ KEY_STORAGE_OUT_OF_SYNC = "key_storage_out_of_sync", - /** - * Prompt the user to enter their recovery key, to store secrets - */ - KEY_STORAGE_OUT_OF_SYNC_STORE = "key_storage_out_of_sync_store", /** * Prompt the user to turn on key storage */ @@ -174,8 +167,7 @@ export const showToast = (kind: Kind): void => { case Kind.VERIFY_THIS_SESSION: Modal.createDialog(SetupEncryptionDialog, {}, undefined, /* priority = */ false, /* static = */ true); break; - case Kind.KEY_STORAGE_OUT_OF_SYNC: - case Kind.KEY_STORAGE_OUT_OF_SYNC_STORE: { + case Kind.KEY_STORAGE_OUT_OF_SYNC: { const modal = Modal.createDialog( Spinner, undefined, @@ -183,10 +175,34 @@ export const showToast = (kind: Kind): void => { /* priority */ false, /* static */ true, ); + + const matrixClient = MatrixClientPeg.safeGet(); + const crypto = matrixClient.getCrypto()!; + try { - await accessSecretStorage(); + const deviceListener = DeviceListener.sharedInstance(); + + // we need to call keyStorageOutOfSyncNeedsBackupReset here because + // deviceListener.whilePaused() sets its client to undefined, so + // keyStorageOutOfSyncNeedsBackupReset won't be able to check + // the backup state. + const needsBackupReset = await deviceListener.keyStorageOutOfSyncNeedsBackupReset(false); + + // pause the device listener because we could be making lots + // of changes, and don't want toasts to pop up and disappear + // while we're doing it + await deviceListener.whilePaused(async () => { + await accessSecretStorage(async () => { + // Reset backup if needed. + if (needsBackupReset) { + await resetKeyBackupAndWait(crypto); + } else if (await matrixClient.isKeyBackupKeyStored()) { + await crypto.loadSessionBackupPrivateKeyFromSecretStorage(); + } + }); + }); } catch (error) { - onAccessSecretStorageFailed(kind, error as Error); + await onAccessSecretStorageFailed(error as Error); } finally { modal.close(); } @@ -209,13 +225,18 @@ export const showToast = (kind: Kind): void => { deviceListener.dismissEncryptionSetup(); break; } - case Kind.KEY_STORAGE_OUT_OF_SYNC: - case Kind.KEY_STORAGE_OUT_OF_SYNC_STORE: { - // Open the user settings dialog to the encryption tab and start the flow to reset encryption + case Kind.KEY_STORAGE_OUT_OF_SYNC: { + // Open the user settings dialog to the encryption tab and start the flow to reset encryption or change the recovery key + const deviceListener = DeviceListener.sharedInstance(); + const needsCrossSigningReset = await deviceListener.keyStorageOutOfSyncNeedsCrossSigningReset(true); const payload: OpenToTabPayload = { action: Action.ViewUserSettings, initialTabId: UserTab.Encryption, - props: { initialEncryptionState: "reset_identity_forgot" }, + props: { + initialEncryptionState: needsCrossSigningReset + ? "reset_identity_forgot" + : "change_recovery_key", + }, }; defaultDispatcher.dispatch(payload); break; @@ -250,25 +271,23 @@ export const showToast = (kind: Kind): void => { * recovery key, but this failed. If the user just gave up, that is fine, * but if not, that means downloading encryption info from 4S did not fix * the problem we identified. Presumably, something is wrong with what they - * have in 4S. If we were trying to fetch secrets from 4S, we tell them to - * reset their identity, to reset everything. If we were trying to store - * secrets in 4S, or set up recovery, we tell them to change their recovery - * key, to create a new 4S that we can store the secrets in. + * have in 4S. */ - const onAccessSecretStorageFailed = ( - kind: Kind.KEY_STORAGE_OUT_OF_SYNC | Kind.KEY_STORAGE_OUT_OF_SYNC_STORE, - error: Error, - ): void => { + const onAccessSecretStorageFailed = async (error: Error): Promise => { if (error instanceof AccessCancelledError) { // The user cancelled the dialog - just allow it to close } else { - // A real error happened - jump to the reset identity tab + // A real error happened - jump to the reset identity or change + // recovery tab + const needsCrossSigningReset = + await DeviceListener.sharedInstance().keyStorageOutOfSyncNeedsCrossSigningReset(true); const payload: OpenToTabPayload = { action: Action.ViewUserSettings, initialTabId: UserTab.Encryption, props: { - initialEncryptionState: - kind === Kind.KEY_STORAGE_OUT_OF_SYNC ? "reset_identity_sync_failed" : "change_recovery_key", + initialEncryptionState: needsCrossSigningReset + ? "reset_identity_sync_failed" + : "change_recovery_key", }, }; defaultDispatcher.dispatch(payload); diff --git a/src/utils/crypto/resetKeyBackup.ts b/src/utils/crypto/resetKeyBackup.ts new file mode 100644 index 0000000000..024f8804bf --- /dev/null +++ b/src/utils/crypto/resetKeyBackup.ts @@ -0,0 +1,20 @@ +/* +Copyright 2025 Element Creations 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. +*/ + +import { type CryptoApi } from "matrix-js-sdk/src/crypto-api"; + +/** + * Creates a new key backup version, and wait until it is enabled. + * + * This is typically used within a {@link DeviceListener.pause()} call, to + * ensure that the device listener doesn't check the backup status until after the + * key backup is active. + */ +export async function resetKeyBackupAndWait(crypto: CryptoApi): Promise { + await crypto.resetKeyBackup(); + await crypto.checkKeyBackupAndEnable(); +} diff --git a/test/test-utils/test-utils.ts b/test/test-utils/test-utils.ts index a0fa715bad..9fe885429b 100644 --- a/test/test-utils/test-utils.ts +++ b/test/test-utils/test-utils.ts @@ -195,6 +195,7 @@ export function createTestClient(): MatrixClient { content: {}, }); }), + getAccountDataFromServer: jest.fn(), mxcUrlToHttp: jest.fn().mockImplementation((mxc: string) => `http://this.is.a.url/${mxc.substring(6)}`), setAccountData: jest.fn(), deleteAccountData: jest.fn(), diff --git a/test/unit-tests/DeviceListener-test.ts b/test/unit-tests/DeviceListener-test.ts index d0f7a6c3d6..e167e9964a 100644 --- a/test/unit-tests/DeviceListener-test.ts +++ b/test/unit-tests/DeviceListener-test.ts @@ -126,6 +126,7 @@ describe("DeviceListener", () => { getRooms: jest.fn().mockReturnValue([]), isVersionSupported: jest.fn().mockResolvedValue(true), isInitialSyncComplete: jest.fn().mockReturnValue(true), + isKeyBackupKeyStored: jest.fn(), waitForClientWellKnown: jest.fn(), getClientWellKnown: jest.fn(), getDeviceId: jest.fn().mockReturnValue(deviceId), @@ -446,7 +447,7 @@ describe("DeviceListener", () => { await createAndStart(); expect(SetupEncryptionToast.showToast).toHaveBeenCalledWith( - SetupEncryptionToast.Kind.KEY_STORAGE_OUT_OF_SYNC_STORE, + SetupEncryptionToast.Kind.KEY_STORAGE_OUT_OF_SYNC, ); }); }); @@ -1217,4 +1218,134 @@ describe("DeviceListener", () => { }); }); }); + + describe("key storage out of sync", () => { + describe("needs backup reset", () => { + it("should not need resetting if backup disabled", async () => { + const deviceListener = await createAndStart(); + mockClient.getAccountDataFromServer.mockResolvedValue({ + disabled: true, + }); + expect(await deviceListener.keyStorageOutOfSyncNeedsBackupReset(false)).toBe(false); + expect(await deviceListener.keyStorageOutOfSyncNeedsBackupReset(true)).toBe(false); + }); + + it("should not need resetting if backup key is present locally or in 4S, and user has 4S key", async () => { + const deviceListener = await createAndStart(); + mockClient.getAccountDataFromServer.mockResolvedValue({ + disabled: false, + }); + + mockCrypto.getSessionBackupPrivateKey.mockResolvedValue(null); + mockClient.isKeyBackupKeyStored.mockResolvedValue({}); + expect(await deviceListener.keyStorageOutOfSyncNeedsBackupReset(false)).toBe(false); + + mockCrypto.getSessionBackupPrivateKey.mockResolvedValue(new Uint8Array()); + mockClient.isKeyBackupKeyStored.mockResolvedValue(null); + expect(await deviceListener.keyStorageOutOfSyncNeedsBackupReset(false)).toBe(false); + }); + + it("should not need resetting if backup key is present locally and user forgot 4S key", async () => { + const deviceListener = await createAndStart(); + mockClient.getAccountDataFromServer.mockResolvedValue({ + disabled: false, + }); + + mockCrypto.getSessionBackupPrivateKey.mockResolvedValue(new Uint8Array()); + mockClient.isKeyBackupKeyStored.mockResolvedValue(null); + expect(await deviceListener.keyStorageOutOfSyncNeedsBackupReset(true)).toBe(false); + }); + + it("should need resetting if backup key is missing locally and user forgot 4S key", async () => { + const deviceListener = await createAndStart(); + mockClient.getAccountDataFromServer.mockResolvedValue({ + disabled: false, + }); + + mockCrypto.getSessionBackupPrivateKey.mockResolvedValue(null); + mockClient.isKeyBackupKeyStored.mockResolvedValue({}); + expect(await deviceListener.keyStorageOutOfSyncNeedsBackupReset(true)).toBe(true); + }); + + it("should need resetting if backup key is missing locally and in 4s", async () => { + const deviceListener = await createAndStart(); + mockClient.getAccountDataFromServer.mockResolvedValue({ + disabled: false, + }); + + mockCrypto.getSessionBackupPrivateKey.mockResolvedValue(null); + mockClient.isKeyBackupKeyStored.mockResolvedValue(null); + expect(await deviceListener.keyStorageOutOfSyncNeedsBackupReset(false)).toBe(true); + }); + }); + + describe("needs cross-signing reset", () => { + it("should not need resetting if cross-signing keys are present locally or in 4S, and user has 4S key", async () => { + const deviceListener = await createAndStart(); + mockCrypto.getCrossSigningStatus.mockResolvedValue({ + publicKeysOnDevice: true, + privateKeysInSecretStorage: false, + privateKeysCachedLocally: { + masterKey: true, + selfSigningKey: true, + userSigningKey: true, + }, + }); + expect(await deviceListener.keyStorageOutOfSyncNeedsCrossSigningReset(false)).toBe(false); + + mockCrypto.getCrossSigningStatus.mockResolvedValue({ + publicKeysOnDevice: true, + privateKeysInSecretStorage: true, + privateKeysCachedLocally: { + masterKey: false, + selfSigningKey: false, + userSigningKey: false, + }, + }); + expect(await deviceListener.keyStorageOutOfSyncNeedsCrossSigningReset(false)).toBe(false); + }); + + it("should not need resetting if cross-signing keys are present locally and user forgot 4S key", async () => { + const deviceListener = await createAndStart(); + mockCrypto.getCrossSigningStatus.mockResolvedValue({ + publicKeysOnDevice: true, + privateKeysInSecretStorage: false, + privateKeysCachedLocally: { + masterKey: true, + selfSigningKey: true, + userSigningKey: true, + }, + }); + expect(await deviceListener.keyStorageOutOfSyncNeedsCrossSigningReset(true)).toBe(false); + }); + + it("should need resetting if cross-signing keys are missing locally and user forgot 4S key", async () => { + const deviceListener = await createAndStart(); + mockCrypto.getCrossSigningStatus.mockResolvedValue({ + publicKeysOnDevice: true, + privateKeysInSecretStorage: true, + privateKeysCachedLocally: { + masterKey: false, + selfSigningKey: false, + userSigningKey: false, + }, + }); + expect(await deviceListener.keyStorageOutOfSyncNeedsCrossSigningReset(true)).toBe(true); + }); + + it("should need resetting if cross-signing keys are missing locally and in 4S key", async () => { + const deviceListener = await createAndStart(); + mockCrypto.getCrossSigningStatus.mockResolvedValue({ + publicKeysOnDevice: true, + privateKeysInSecretStorage: false, + privateKeysCachedLocally: { + masterKey: false, + selfSigningKey: false, + userSigningKey: false, + }, + }); + expect(await deviceListener.keyStorageOutOfSyncNeedsCrossSigningReset(false)).toBe(true); + }); + }); + }); }); diff --git a/test/unit-tests/components/views/settings/encryption/ChangeRecoveryKey-test.tsx b/test/unit-tests/components/views/settings/encryption/ChangeRecoveryKey-test.tsx index 3514914e7b..7b494e28ca 100644 --- a/test/unit-tests/components/views/settings/encryption/ChangeRecoveryKey-test.tsx +++ b/test/unit-tests/components/views/settings/encryption/ChangeRecoveryKey-test.tsx @@ -16,6 +16,7 @@ import { createTestClient, withClientContextRenderOptions } from "../../../../.. import { copyPlaintext } from "../../../../../../src/utils/strings"; import Modal from "../../../../../../src/Modal"; import ErrorDialog from "../../../../../../src/components/views/dialogs/ErrorDialog"; +import DeviceListener from "../../../../../../src/DeviceListener"; jest.mock("../../../../../../src/utils/strings", () => ({ copyPlaintext: jest.fn(), @@ -82,6 +83,8 @@ describe("", () => { }); it("should ask the user to enter the recovery key", async () => { + jest.spyOn(DeviceListener.sharedInstance(), "keyStorageOutOfSyncNeedsBackupReset").mockResolvedValue(false); + const user = userEvent.setup(); const onFinish = jest.fn(); @@ -117,6 +120,56 @@ describe("", () => { expect(onFinish).toHaveBeenCalledWith(); }); + it("should reset key backup if needed", async () => { + jest.spyOn(DeviceListener.sharedInstance(), "keyStorageOutOfSyncNeedsBackupReset").mockResolvedValue(true); + + const user = userEvent.setup(); + + const onFinish = jest.fn(); + renderComponent(false, onFinish); + // Display the recovery key to save + await waitFor(() => user.click(screen.getByRole("button", { name: "Continue" }))); + // Display the form to confirm the recovery key + await waitFor(() => user.click(screen.getByRole("button", { name: "Continue" }))); + + await waitFor(() => expect(screen.getByText("Enter your recovery key to confirm")).toBeInTheDocument()); + + const finishButton = screen.getByRole("button", { name: "Finish set up" }); + + const input = screen.getByTitle("Enter recovery key"); + + // If the user enters the correct recovery key, the finish button should be enabled + await userEvent.type(input, "encoded private key"); + + await user.click(finishButton); + expect(matrixClient.getCrypto()!.resetKeyBackup).toHaveBeenCalled(); + }); + + it("should not reset key backup if not needed", async () => { + jest.spyOn(DeviceListener.sharedInstance(), "keyStorageOutOfSyncNeedsBackupReset").mockResolvedValue(false); + + const user = userEvent.setup(); + + const onFinish = jest.fn(); + renderComponent(false, onFinish); + // Display the recovery key to save + await waitFor(() => user.click(screen.getByRole("button", { name: "Continue" }))); + // Display the form to confirm the recovery key + await waitFor(() => user.click(screen.getByRole("button", { name: "Continue" }))); + + await waitFor(() => expect(screen.getByText("Enter your recovery key to confirm")).toBeInTheDocument()); + + const finishButton = screen.getByRole("button", { name: "Finish set up" }); + + const input = screen.getByTitle("Enter recovery key"); + + // If the user enters the correct recovery key, the finish button should be enabled + await userEvent.type(input, "encoded private key"); + + await user.click(finishButton); + expect(matrixClient.getCrypto()!.resetKeyBackup).not.toHaveBeenCalled(); + }); + it("should display errors from bootstrapSecretStorage", async () => { const consoleErrorSpy = jest.spyOn(console, "error").mockReturnValue(undefined); mocked(matrixClient.getCrypto()!).bootstrapSecretStorage.mockRejectedValue(new Error("can't bootstrap")); @@ -156,6 +209,8 @@ describe("", () => { }); it("should disallow repeated attempts to change the recovery key", async () => { + jest.spyOn(DeviceListener.sharedInstance(), "keyStorageOutOfSyncNeedsBackupReset").mockResolvedValue(false); + const mockFn = mocked(matrixClient.getCrypto()!).bootstrapSecretStorage.mockImplementation(() => { // Pretend to do some work. return new Promise((r) => setTimeout(r, 200)); diff --git a/test/unit-tests/toasts/SetupEncryptionToast-test.tsx b/test/unit-tests/toasts/SetupEncryptionToast-test.tsx index 92c2c0487a..fa2b389659 100644 --- a/test/unit-tests/toasts/SetupEncryptionToast-test.tsx +++ b/test/unit-tests/toasts/SetupEncryptionToast-test.tsx @@ -1,4 +1,5 @@ /* +Copyright 2025 Element Creations Ltd. Copyright 2024 New Vector Ltd. SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial @@ -7,7 +8,10 @@ Please see LICENSE files in the repository root for full details. import React from "react"; import { act, render, screen } from "jest-matrix-react"; +import { mocked, type Mocked } from "jest-mock"; import userEvent from "@testing-library/user-event"; +import { type MatrixClient } from "matrix-js-sdk/src/matrix"; +import { type CryptoApi } from "matrix-js-sdk/src/crypto-api"; import * as SecurityManager from "../../../src/SecurityManager"; import ToastContainer from "../../../src/components/structures/ToastContainer"; @@ -16,6 +20,7 @@ import dis from "../../../src/dispatcher/dispatcher"; import DeviceListener from "../../../src/DeviceListener"; import Modal from "../../../src/Modal"; import ConfirmKeyStorageOffDialog from "../../../src/components/views/dialogs/ConfirmKeyStorageOffDialog"; +import { stubClient } from "../../test-utils"; jest.mock("../../../src/dispatcher/dispatcher", () => ({ dispatch: jest.fn(), @@ -50,16 +55,71 @@ describe("SetupEncryptionToast", () => { }); }); - describe("Key storage out of sync (retrieve secrets)", () => { + describe("Key storage out of sync", () => { + let client: Mocked; + + beforeEach(() => { + client = mocked(stubClient()); + mocked(client.getCrypto).mockReturnValue({ + getSessionBackupPrivateKey: jest.fn().mockResolvedValue(null), + resetKeyBackup: jest.fn(), + checkKeyBackupAndEnable: jest.fn(), + loadSessionBackupPrivateKeyFromSecretStorage: jest.fn(), + } as unknown as CryptoApi); + }); + it("should render the toast", async () => { act(() => showToast(Kind.KEY_STORAGE_OUT_OF_SYNC)); await expect(screen.findByText("Your key storage is out of sync.")).resolves.toBeInTheDocument(); }); - it("should open settings to the reset flow when 'forgot recovery key' clicked", async () => { + it("should reset key backup if needed", async () => { + showToast(Kind.KEY_STORAGE_OUT_OF_SYNC); + + jest.spyOn(SecurityManager, "accessSecretStorage").mockImplementation( + async (func = async (): Promise => {}) => { + return await func(); + }, + ); + + jest.spyOn(DeviceListener.sharedInstance(), "keyStorageOutOfSyncNeedsBackupReset").mockResolvedValue(true); + + const user = userEvent.setup(); + await user.click(await screen.findByText("Enter recovery key")); + + expect(client.getCrypto()!.resetKeyBackup).toHaveBeenCalled(); + }); + + it("should not reset key backup if not needed", async () => { + showToast(Kind.KEY_STORAGE_OUT_OF_SYNC); + + jest.spyOn(SecurityManager, "accessSecretStorage").mockImplementation( + async (func = async (): Promise => {}) => { + return await func(); + }, + ); + + jest.spyOn(DeviceListener.sharedInstance(), "keyStorageOutOfSyncNeedsBackupReset").mockResolvedValue(false); + // if the backup key is stored in 4S + client.isKeyBackupKeyStored.mockResolvedValue({}); + + const user = userEvent.setup(); + await user.click(await screen.findByText("Enter recovery key")); + + // we shouldn't have reset the key backup, but should have fetched + // the key from 4S + expect(client.getCrypto()!.resetKeyBackup).not.toHaveBeenCalled(); + expect(client.getCrypto()!.loadSessionBackupPrivateKeyFromSecretStorage).toHaveBeenCalled(); + }); + + it("should open settings to the reset flow when 'forgot recovery key' clicked and identity reset needed", async () => { act(() => showToast(Kind.KEY_STORAGE_OUT_OF_SYNC)); + jest.spyOn(DeviceListener.sharedInstance(), "keyStorageOutOfSyncNeedsCrossSigningReset").mockResolvedValue( + true, + ); + const user = userEvent.setup(); await user.click(await screen.findByText("Forgot recovery key?")); @@ -70,11 +130,32 @@ describe("SetupEncryptionToast", () => { }); }); - it("should open settings to the reset flow when recovering fails", async () => { + it("should open settings to the change recovery key flow when 'forgot recovery key' clicked and identity reset not needed", async () => { + act(() => showToast(Kind.KEY_STORAGE_OUT_OF_SYNC)); + + jest.spyOn(DeviceListener.sharedInstance(), "keyStorageOutOfSyncNeedsCrossSigningReset").mockResolvedValue( + false, + ); + + const user = userEvent.setup(); + await user.click(await screen.findByText("Forgot recovery key?")); + + expect(dis.dispatch).toHaveBeenCalledWith({ + action: "view_user_settings", + initialTabId: "USER_ENCRYPTION_TAB", + props: { initialEncryptionState: "change_recovery_key" }, + }); + }); + + it("should open settings to the reset flow when recovering fails and identity reset needed", async () => { jest.spyOn(SecurityManager, "accessSecretStorage").mockImplementation(async () => { throw new Error("Something went wrong while recovering!"); }); + jest.spyOn(DeviceListener.sharedInstance(), "keyStorageOutOfSyncNeedsCrossSigningReset").mockResolvedValue( + true, + ); + act(() => showToast(Kind.KEY_STORAGE_OUT_OF_SYNC)); const user = userEvent.setup(); @@ -86,34 +167,17 @@ describe("SetupEncryptionToast", () => { props: { initialEncryptionState: "reset_identity_sync_failed" }, }); }); - }); - describe("Key storage out of sync (secrets are missing from 4S)", () => { - it("should render the toast", async () => { - act(() => showToast(Kind.KEY_STORAGE_OUT_OF_SYNC_STORE)); - - await expect(screen.findByText("Your key storage is out of sync.")).resolves.toBeInTheDocument(); - }); - - it("should open settings to the reset flow when 'forgot recovery key' clicked", async () => { - act(() => showToast(Kind.KEY_STORAGE_OUT_OF_SYNC_STORE)); - - const user = userEvent.setup(); - await user.click(await screen.findByText("Forgot recovery key?")); - - expect(dis.dispatch).toHaveBeenCalledWith({ - action: "view_user_settings", - initialTabId: "USER_ENCRYPTION_TAB", - props: { initialEncryptionState: "reset_identity_forgot" }, - }); - }); - - it("should open settings to the reset flow when recovering fails", async () => { + it("should open settings to the change recovery key flow when recovering fails and identity reset not needed", async () => { jest.spyOn(SecurityManager, "accessSecretStorage").mockImplementation(async () => { throw new Error("Something went wrong while recovering!"); }); - act(() => showToast(Kind.KEY_STORAGE_OUT_OF_SYNC_STORE)); + jest.spyOn(DeviceListener.sharedInstance(), "keyStorageOutOfSyncNeedsCrossSigningReset").mockResolvedValue( + false, + ); + + act(() => showToast(Kind.KEY_STORAGE_OUT_OF_SYNC)); const user = userEvent.setup(); await user.click(await screen.findByText("Enter recovery key"));