From b539eda4fe045f2fa44e4255d22a75c034fef8b6 Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Tue, 20 May 2025 13:28:22 +0100 Subject: [PATCH] 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 --- playwright/e2e/crypto/event-shields.spec.ts | 5 +- playwright/e2e/crypto/toasts.spec.ts | 114 +++++++++++++- playwright/e2e/crypto/utils.ts | 19 +++ res/css/_common.pcss | 1 + res/css/_components.pcss | 1 + res/css/structures/_ToastContainer.pcss | 5 + .../dialogs/_ConfirmKeyStorageOffDialog.pcss | 16 ++ src/DeviceListener.ts | 75 +++++++-- .../dialogs/ConfirmKeyStorageOffDialog.tsx | 80 ++++++++++ src/i18n/strings/en_EN.json | 5 + src/toasts/SetupEncryptionToast.ts | 33 +++- test/unit-tests/DeviceListener-test.ts | 143 +++++++++++++++++- .../KeyStoragePanelViewModel-test.ts | 3 + .../ConfirmKeyStorageOffDialog-test.tsx | 40 +++++ .../ConfirmKeyStorageOffDialog-test.tsx.snap | 82 ++++++++++ .../toasts/SetupEncryptionToast-test.tsx | 53 +++++++ 16 files changed, 655 insertions(+), 20 deletions(-) create mode 100644 res/css/views/dialogs/_ConfirmKeyStorageOffDialog.pcss create mode 100644 src/components/views/dialogs/ConfirmKeyStorageOffDialog.tsx create mode 100644 test/unit-tests/components/views/dialogs/ConfirmKeyStorageOffDialog-test.tsx create mode 100644 test/unit-tests/components/views/dialogs/__snapshots__/ConfirmKeyStorageOffDialog-test.tsx.snap diff --git a/playwright/e2e/crypto/event-shields.spec.ts b/playwright/e2e/crypto/event-shields.spec.ts index f73f190e10..e577a66467 100644 --- a/playwright/e2e/crypto/event-shields.spec.ts +++ b/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", diff --git a/playwright/e2e/crypto/toasts.spec.ts b/playwright/e2e/crypto/toasts.spec.ts index 5b80f822a0..7b838edaac 100644 --- a/playwright/e2e/crypto/toasts.spec.ts +++ b/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(); + }); +}); diff --git a/playwright/e2e/crypto/utils.ts b/playwright/e2e/crypto/utils.ts index 77f1b747a1..c31ccb130d 100644 --- a/playwright/e2e/crypto/utils.ts +++ b/playwright/e2e/crypto/utils.ts @@ -316,6 +316,25 @@ export async function enableKeyBackup(app: ElementAppPage): Promise { 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 { + 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`). * diff --git a/res/css/_common.pcss b/res/css/_common.pcss index 75180013f6..98b66aec2b 100644 --- a/res/css/_common.pcss +++ b/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, diff --git a/res/css/_components.pcss b/res/css/_components.pcss index 26ce9b7bd8..247dd7edab 100644 --- a/res/css/_components.pcss +++ b/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"; diff --git a/res/css/structures/_ToastContainer.pcss b/res/css/structures/_ToastContainer.pcss index 0ab0816392..cf1d7fd4a9 100644 --- a/res/css/structures/_ToastContainer.pcss +++ b/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; diff --git a/res/css/views/dialogs/_ConfirmKeyStorageOffDialog.pcss b/res/css/views/dialogs/_ConfirmKeyStorageOffDialog.pcss new file mode 100644 index 0000000000..5ac53c7b70 --- /dev/null +++ b/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; + } +} diff --git a/src/DeviceListener.ts b/src/DeviceListener.ts index 91ee0f284e..e13d296bc1 100644 --- a/src/DeviceListener.ts +++ b/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 { + await this.client?.setAccountData(BACKUP_DISABLED_ACCOUNT_DATA_KEY, { disabled: true }); + } + private async ensureDeviceIdsAtStartPopulated(): Promise { 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 { + 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,7 +551,7 @@ export default class DeviceListener { * trigger an auto-rageshake). */ private checkKeyBackupStatus = async (): Promise => { - if (!(await this.getKeyBackupStatus())) { + if (!(await this.isKeyBackupUploadActive())) { dis.dispatch({ action: Action.ReportKeyBackupNotEnabled }); } }; @@ -520,28 +559,34 @@ export default class DeviceListener { /** * Is key backup enabled? Use a cached answer if we have one. */ - private getKeyBackupStatus = async (): Promise => { + private isKeyBackupUploadActive = async (): Promise => { 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, diff --git a/src/components/views/dialogs/ConfirmKeyStorageOffDialog.tsx b/src/components/views/dialogs/ConfirmKeyStorageOffDialog.tsx new file mode 100644 index 0000000000..49e7cad17b --- /dev/null +++ b/src/components/views/dialogs/ConfirmKeyStorageOffDialog.tsx @@ -0,0 +1,80 @@ +/* +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. +*/ + +import React from "react"; +import ErrorIcon from "@vector-im/compound-design-tokens/assets/web/icons/error"; +import { Button } from "@vector-im/compound-web"; +import { PopOutIcon } from "@vector-im/compound-design-tokens/assets/web/icons"; + +import { _t } from "../../../languageHandler"; +import defaultDispatcher from "../../../dispatcher/dispatcher"; +import { EncryptionCard } from "../settings/encryption/EncryptionCard"; +import { EncryptionCardButtons } from "../settings/encryption/EncryptionCardButtons"; +import { type OpenToTabPayload } from "../../../dispatcher/payloads/OpenToTabPayload"; +import { Action } from "../../../dispatcher/actions"; +import { UserTab } from "./UserTab"; + +interface Props { + onFinished: (dismissed: boolean) => void; +} + +/** + * Ask the user whether they really want to dismiss the toast about key storage. + * + * Launched from the {@link SetupEncryptionToast} in mode `TURN_ON_KEY_STORAGE`, + * when the user clicks "Dismiss". The caller handles any action via the + * `onFinished` prop which takes a boolean that is true if the user clicked + * "Yes, dismiss". + */ +export default class ConfirmKeyStorageOffDialog extends React.Component { + public constructor(props: Props) { + super(props); + } + + private onGoToSettingsClick = (): void => { + // Open Settings at the Encryption tab + const payload: OpenToTabPayload = { + action: Action.ViewUserSettings, + initialTabId: UserTab.Encryption, + }; + defaultDispatcher.dispatch(payload); + this.props.onFinished(false); + }; + + private onDismissClick = (): void => { + this.props.onFinished(true); + }; + + public render(): React.ReactNode { + return ( + + {_t("settings|encryption|confirm_key_storage_off_description", undefined, { + a: (sub) => ( + <> +
+ + {sub} + + + ), + })} + + + + + + ); + } +} diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 0be68b65e7..f2bb41a1cf 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -157,6 +157,7 @@ "view_message": "View message", "view_source": "View Source", "yes": "Yes", + "yes_dismiss": "Yes, dismiss", "zoom_in": "Zoom in", "zoom_out": "Zoom out" }, @@ -981,6 +982,8 @@ "setup_secure_backup": { "explainer": "Back up your keys before signing out to avoid losing them." }, + "turn_on_key_storage": "Turn on key storage", + "turn_on_key_storage_description": "Store your cryptographic identity and message keys securely on the server. This will allow you to view your message history on any new devices.", "udd": { "interactive_verification_button": "Interactively verify by emoji", "other_ask_verify_text": "Ask this user to verify their session, or manually verify it below.", @@ -2559,6 +2562,8 @@ "session_key": "Session key:", "title": "Advanced" }, + "confirm_key_storage_off": "Are you sure you want to keep key storage turned off?", + "confirm_key_storage_off_description": "If you sign out of all your devices you will lose your message history and will need to verify all your existing contacts again. Learn more", "delete_key_storage": { "breadcrumb_page": "Delete key storage", "confirm": "Delete key storage", diff --git a/src/toasts/SetupEncryptionToast.ts b/src/toasts/SetupEncryptionToast.ts index 2b50ccd267..802da5b895 100644 --- a/src/toasts/SetupEncryptionToast.ts +++ b/src/toasts/SetupEncryptionToast.ts @@ -24,6 +24,7 @@ import { type OpenToTabPayload } from "../dispatcher/payloads/OpenToTabPayload"; import { Action } from "../dispatcher/actions"; import { UserTab } from "../components/views/dialogs/UserTab"; import defaultDispatcher from "../dispatcher/dispatcher"; +import ConfirmKeyStorageOffDialog from "../components/views/dialogs/ConfirmKeyStorageOffDialog"; const TOAST_KEY = "setupencryption"; @@ -37,6 +38,8 @@ const getTitle = (kind: Kind): string => { return _t("encryption|verify_toast_title"); case Kind.KEY_STORAGE_OUT_OF_SYNC: return _t("encryption|key_storage_out_of_sync"); + case Kind.TURN_ON_KEY_STORAGE: + return _t("encryption|turn_on_key_storage"); } }; @@ -49,6 +52,8 @@ const getIcon = (kind: Kind): string | undefined => { case Kind.VERIFY_THIS_SESSION: case Kind.KEY_STORAGE_OUT_OF_SYNC: return "verification_warning"; + case Kind.TURN_ON_KEY_STORAGE: + return "key_storage"; } }; @@ -62,6 +67,8 @@ const getSetupCaption = (kind: Kind): string => { return _t("action|verify"); case Kind.KEY_STORAGE_OUT_OF_SYNC: return _t("encryption|enter_recovery_key"); + case Kind.TURN_ON_KEY_STORAGE: + return _t("action|continue"); } }; @@ -87,6 +94,8 @@ const getSecondaryButtonLabel = (kind: Kind): string => { return _t("encryption|verification|unverified_sessions_toast_reject"); case Kind.KEY_STORAGE_OUT_OF_SYNC: return _t("encryption|forgot_recovery_key"); + case Kind.TURN_ON_KEY_STORAGE: + return _t("action|dismiss"); } }; @@ -100,6 +109,8 @@ const getDescription = (kind: Kind): string => { return _t("encryption|verify_toast_description"); case Kind.KEY_STORAGE_OUT_OF_SYNC: return _t("encryption|key_storage_out_of_sync_description"); + case Kind.TURN_ON_KEY_STORAGE: + return _t("encryption|turn_on_key_storage_description"); } }; @@ -123,6 +134,10 @@ export enum Kind { * Prompt the user to enter their recovery key */ KEY_STORAGE_OUT_OF_SYNC = "key_storage_out_of_sync", + /** + * Prompt the user to turn on key storage + */ + TURN_ON_KEY_STORAGE = "turn_on_key_storage", } /** @@ -143,6 +158,13 @@ export const showToast = (kind: Kind): void => { const onPrimaryClick = async (): Promise => { if (kind === Kind.VERIFY_THIS_SESSION) { Modal.createDialog(SetupEncryptionDialog, {}, undefined, /* priority = */ false, /* static = */ true); + } else if (kind == Kind.TURN_ON_KEY_STORAGE) { + // Open the user settings dialog to the encryption tab + const payload: OpenToTabPayload = { + action: Action.ViewUserSettings, + initialTabId: UserTab.Encryption, + }; + defaultDispatcher.dispatch(payload); } else { const modal = Modal.createDialog( Spinner, @@ -161,7 +183,7 @@ export const showToast = (kind: Kind): void => { } }; - const onSecondaryClick = (): void => { + const onSecondaryClick = async (): Promise => { if (kind === Kind.KEY_STORAGE_OUT_OF_SYNC) { // Open the user settings dialog to the encryption tab and start the flow to reset encryption const payload: OpenToTabPayload = { @@ -170,6 +192,15 @@ export const showToast = (kind: Kind): void => { props: { initialEncryptionState: "reset_identity_forgot" }, }; defaultDispatcher.dispatch(payload); + } else if (kind === Kind.TURN_ON_KEY_STORAGE) { + // The user clicked "Dismiss": offer them "Are you sure?" + const modal = Modal.createDialog(ConfirmKeyStorageOffDialog, undefined, "mx_ConfirmKeyStorageOffDialog"); + const [dismissed] = await modal.finished; + if (dismissed) { + const deviceListener = DeviceListener.sharedInstance(); + await deviceListener.recordKeyBackupDisabled(); + deviceListener.dismissEncryptionSetup(); + } } else { DeviceListener.sharedInstance().dismissEncryptionSetup(); } diff --git a/test/unit-tests/DeviceListener-test.ts b/test/unit-tests/DeviceListener-test.ts index 09bc4cd18a..62221d0665 100644 --- a/test/unit-tests/DeviceListener-test.ts +++ b/test/unit-tests/DeviceListener-test.ts @@ -24,7 +24,7 @@ import { } from "matrix-js-sdk/src/crypto-api"; import { type CryptoSessionStateChange } from "@matrix-org/analytics-events/types/typescript/CryptoSessionStateChange"; -import DeviceListener from "../../src/DeviceListener"; +import DeviceListener, { BACKUP_DISABLED_ACCOUNT_DATA_KEY } from "../../src/DeviceListener"; import { MatrixClientPeg } from "../../src/MatrixClientPeg"; import * as SetupEncryptionToast from "../../src/toasts/SetupEncryptionToast"; import * as UnverifiedSessionToast from "../../src/toasts/UnverifiedSessionToast"; @@ -118,6 +118,7 @@ describe("DeviceListener", () => { getDeviceId: jest.fn().mockReturnValue(deviceId), setAccountData: jest.fn(), getAccountData: jest.fn(), + getAccountDataFromServer: jest.fn(), deleteAccountData: jest.fn(), getCrypto: jest.fn().mockReturnValue(mockCrypto), secretStorage: { @@ -309,6 +310,8 @@ describe("DeviceListener", () => { it("hides setup encryption toast when cross signing and secret storage are ready", async () => { mockCrypto!.isCrossSigningReady.mockResolvedValue(true); mockCrypto!.isSecretStorageReady.mockResolvedValue(true); + mockCrypto!.getActiveSessionBackupVersion.mockResolvedValue("1"); + await createAndStart(); expect(SetupEncryptionToast.hideToast).toHaveBeenCalled(); }); @@ -377,6 +380,7 @@ describe("DeviceListener", () => { it("hides the out-of-sync toast when one of the secrets is missing", async () => { mockCrypto!.isSecretStorageReady.mockResolvedValue(true); + mockCrypto!.getActiveSessionBackupVersion.mockResolvedValue("1"); // First show the toast mockCrypto!.getCrossSigningStatus.mockResolvedValue({ @@ -414,6 +418,7 @@ describe("DeviceListener", () => { it("shows set up recovery toast when user has a key backup available", async () => { // non falsy response mockCrypto.getKeyBackupInfo.mockResolvedValue({} as unknown as KeyBackupInfo); + mockCrypto.getActiveSessionBackupVersion.mockResolvedValue("1"); mockClient.secretStorage.getDefaultKeyId.mockResolvedValue(null); await createAndStart(); @@ -444,6 +449,9 @@ describe("DeviceListener", () => { it("dispatches keybackup event when key backup is not enabled", async () => { mockCrypto.getActiveSessionBackupVersion.mockResolvedValue(null); + mockClient.getAccountDataFromServer.mockImplementation((eventType) => + eventType === BACKUP_DISABLED_ACCOUNT_DATA_KEY ? ({ disabled: true } as any) : null, + ); await createAndStart(); expect(mockDispatcher.dispatch).toHaveBeenCalledWith({ action: Action.ReportKeyBackupNotEnabled, @@ -463,6 +471,137 @@ describe("DeviceListener", () => { }); }); + it("sets backup_disabled account data when we call recordKeyBackupDisabled", async () => { + const instance = await createAndStart(); + await instance.recordKeyBackupDisabled(); + + expect(mockClient.setAccountData).toHaveBeenCalledWith("m.org.matrix.custom.backup_disabled", { + disabled: true, + }); + }); + + describe("when crypto is in use and set up", () => { + beforeEach(() => { + // Encryption is in use + mockClient.getRooms.mockReturnValue([{ roomId: "!room1" }, { roomId: "!room2" }] as unknown as Room[]); + jest.spyOn(mockClient.getCrypto()!, "isEncryptionEnabledInRoom").mockResolvedValue(true); + + // The device is verified + mockCrypto.getDeviceVerificationStatus.mockResolvedValue( + new DeviceVerificationStatus({ crossSigningVerified: true }), + ); + }); + + describe("but key storage is off", () => { + beforeEach(() => { + // There is no active key backup/storage + mockCrypto.getActiveSessionBackupVersion.mockResolvedValue(null); + }); + + it("shows the 'Turn on key storage' toast if we never explicitly turned off key storage", async () => { + // Given key backup is off but the account data saying we turned it off is not set + // (m.org.matrix.custom.backup_disabled) + mockClient.getAccountData.mockReturnValue(undefined); + + // When we launch the DeviceListener + await createAndStart(); + + // Then the toast is displayed + expect(SetupEncryptionToast.showToast).toHaveBeenCalledWith( + SetupEncryptionToast.Kind.TURN_ON_KEY_STORAGE, + ); + }); + + it("shows the 'Turn on key storage' toast if we turned on key storage", async () => { + // Given key backup is off but the account data says we turned it on (this should not happen - the + // account data should only be updated if we turn on key storage) + mockClient.getAccountData.mockImplementation((eventType) => + eventType === BACKUP_DISABLED_ACCOUNT_DATA_KEY + ? new MatrixEvent({ content: { disabled: false } }) + : undefined, + ); + + // When we launch the DeviceListener + await createAndStart(); + + // Then the toast is displayed + expect(SetupEncryptionToast.showToast).toHaveBeenCalledWith( + SetupEncryptionToast.Kind.TURN_ON_KEY_STORAGE, + ); + }); + + it("does not show the 'Turn on key storage' toast if we turned off key storage", async () => { + // Given key backup is off but the account data saying we turned it off is set + mockClient.getAccountDataFromServer.mockImplementation((eventType) => + eventType === BACKUP_DISABLED_ACCOUNT_DATA_KEY ? ({ disabled: true } as any) : null, + ); + + // When we launch the DeviceListener + await createAndStart(); + + // Then the toast is not displayed + expect(SetupEncryptionToast.showToast).not.toHaveBeenCalledWith( + SetupEncryptionToast.Kind.TURN_ON_KEY_STORAGE, + ); + }); + }); + + describe("and key storage is on", () => { + beforeEach(() => { + // There is an active key backup/storage + mockCrypto.getActiveSessionBackupVersion.mockResolvedValue("1"); + }); + + it("does not show the 'Turn on key storage' toast if we never explicitly turned off key storage", async () => { + // Given key backup is on and the account data saying we turned it off is not set + mockClient.getAccountData.mockReturnValue(undefined); + + // When we launch the DeviceListener + await createAndStart(); + + // Then the toast is not displayed + expect(SetupEncryptionToast.showToast).not.toHaveBeenCalledWith( + SetupEncryptionToast.Kind.TURN_ON_KEY_STORAGE, + ); + }); + + it("does not show the 'Turn on key storage' toast if we turned on key storage", async () => { + // Given key backup is on and the account data says we turned it on + mockClient.getAccountData.mockImplementation((eventType) => + eventType === BACKUP_DISABLED_ACCOUNT_DATA_KEY + ? new MatrixEvent({ content: { disabled: false } }) + : undefined, + ); + + // When we launch the DeviceListener + await createAndStart(); + + // Then the toast is not displayed + expect(SetupEncryptionToast.showToast).not.toHaveBeenCalledWith( + SetupEncryptionToast.Kind.TURN_ON_KEY_STORAGE, + ); + }); + + it("does not show the 'Turn on key storage' toast if we turned off key storage", async () => { + // Given key backup is on but the account data saying we turned it off is set (this should never + // happen - it should only be set when we turn off key storage or dismiss the toast) + mockClient.getAccountData.mockImplementation((eventType) => + eventType === BACKUP_DISABLED_ACCOUNT_DATA_KEY + ? new MatrixEvent({ content: { disabled: true } }) + : undefined, + ); + + // When we launch the DeviceListener + await createAndStart(); + + // Then the toast is not displayed + expect(SetupEncryptionToast.showToast).not.toHaveBeenCalledWith( + SetupEncryptionToast.Kind.TURN_ON_KEY_STORAGE, + ); + }); + }); + }); + describe("unverified sessions toasts", () => { const currentDevice = new Device({ deviceId, userId: userId, algorithms: [], keys: new Map() }); const device2 = new Device({ deviceId: "d2", userId: userId, algorithms: [], keys: new Map() }); @@ -997,6 +1136,8 @@ describe("DeviceListener", () => { }); it("shows the 'set up recovery' toast if user has not set up 4S", async () => { + mockCrypto!.getActiveSessionBackupVersion.mockResolvedValue("1"); + await createAndStart(); expect(SetupEncryptionToast.showToast).toHaveBeenCalledWith(SetupEncryptionToast.Kind.SET_UP_RECOVERY); diff --git a/test/unit-tests/components/viewmodels/settings/encryption/KeyStoragePanelViewModel-test.ts b/test/unit-tests/components/viewmodels/settings/encryption/KeyStoragePanelViewModel-test.ts index 68333d0fea..d0132c7317 100644 --- a/test/unit-tests/components/viewmodels/settings/encryption/KeyStoragePanelViewModel-test.ts +++ b/test/unit-tests/components/viewmodels/settings/encryption/KeyStoragePanelViewModel-test.ts @@ -87,5 +87,8 @@ describe("KeyStoragePanelViewModel", () => { await result.current.setEnabled(false); expect(mocked(matrixClient.getCrypto()!.disableKeyStorage)).toHaveBeenCalled(); + expect(mocked(matrixClient.setAccountData)).toHaveBeenCalledWith("m.org.matrix.custom.backup_disabled", { + disabled: true, + }); }); }); diff --git a/test/unit-tests/components/views/dialogs/ConfirmKeyStorageOffDialog-test.tsx b/test/unit-tests/components/views/dialogs/ConfirmKeyStorageOffDialog-test.tsx new file mode 100644 index 0000000000..a27f784c46 --- /dev/null +++ b/test/unit-tests/components/views/dialogs/ConfirmKeyStorageOffDialog-test.tsx @@ -0,0 +1,40 @@ +/* +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. +*/ + +import React from "react"; +import { render } from "jest-matrix-react"; + +import ConfirmKeyStorageOffDialog from "../../../../../src/components/views/dialogs/ConfirmKeyStorageOffDialog"; + +describe("ConfirmKeyStorageOffDialog", () => { + beforeEach(() => { + jest.resetAllMocks(); + }); + + it("renders", () => { + const dialog = render(); + expect(dialog.asFragment()).toMatchSnapshot(); + }); + + it("calls onFinished with dismissed=true if we dismiss", () => { + const onFinished = jest.fn(); + const dialog = render(); + + dialog.getByRole("button", { name: "Yes, dismiss" }).click(); + + expect(onFinished).toHaveBeenCalledWith(true); + }); + + it("calls onFinished with dismissed=true if we continue", () => { + const onFinished = jest.fn(); + const dialog = render(); + + dialog.getByRole("button", { name: "Go to Settings" }).click(); + + expect(onFinished).toHaveBeenCalledWith(false); + }); +}); diff --git a/test/unit-tests/components/views/dialogs/__snapshots__/ConfirmKeyStorageOffDialog-test.tsx.snap b/test/unit-tests/components/views/dialogs/__snapshots__/ConfirmKeyStorageOffDialog-test.tsx.snap new file mode 100644 index 0000000000..9e0def2569 --- /dev/null +++ b/test/unit-tests/components/views/dialogs/__snapshots__/ConfirmKeyStorageOffDialog-test.tsx.snap @@ -0,0 +1,82 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`ConfirmKeyStorageOffDialog renders 1`] = ` + +
+
+
+ + + +
+

+ Are you sure you want to keep key storage turned off? +

+
+ + If you sign out of all your devices you will lose your message history and will need to verify all your existing contacts again. +
+ + Learn more + + + + + + +
+ + +
+
+ +`; diff --git a/test/unit-tests/toasts/SetupEncryptionToast-test.tsx b/test/unit-tests/toasts/SetupEncryptionToast-test.tsx index 57980fd848..eb584aafd9 100644 --- a/test/unit-tests/toasts/SetupEncryptionToast-test.tsx +++ b/test/unit-tests/toasts/SetupEncryptionToast-test.tsx @@ -14,6 +14,8 @@ import ToastContainer from "../../../src/components/structures/ToastContainer"; import { Kind, showToast } from "../../../src/toasts/SetupEncryptionToast"; import dis from "../../../src/dispatcher/dispatcher"; import DeviceListener from "../../../src/DeviceListener"; +import Modal from "../../../src/Modal"; +import ConfirmKeyStorageOffDialog from "../../../src/components/views/dialogs/ConfirmKeyStorageOffDialog"; jest.mock("../../../src/dispatcher/dispatcher", () => ({ dispatch: jest.fn(), @@ -83,4 +85,55 @@ describe("SetupEncryptionToast", () => { }); }); }); + + describe("Turn on key storage", () => { + it("should render the toast", async () => { + showToast(Kind.TURN_ON_KEY_STORAGE); + + await expect(screen.findByText("Turn on key storage")).resolves.toBeInTheDocument(); + await expect(screen.findByRole("button", { name: "Dismiss" })).resolves.toBeInTheDocument(); + await expect(screen.findByRole("button", { name: "Continue" })).resolves.toBeInTheDocument(); + }); + + it("should open settings to the Encryption tab when 'Continue' clicked", async () => { + jest.spyOn(DeviceListener.sharedInstance(), "recordKeyBackupDisabled"); + + showToast(Kind.TURN_ON_KEY_STORAGE); + + const user = userEvent.setup(); + await user.click(await screen.findByRole("button", { name: "Continue" })); + + expect(dis.dispatch).toHaveBeenCalledWith({ + action: "view_user_settings", + initialTabId: "USER_ENCRYPTION_TAB", + }); + + expect(DeviceListener.sharedInstance().recordKeyBackupDisabled).not.toHaveBeenCalled(); + }); + + it("should open the confirm key storage off dialog when 'Dismiss' clicked", async () => { + jest.spyOn(DeviceListener.sharedInstance(), "recordKeyBackupDisabled"); + + // Given that as soon as the dialog opens, it closes and says "yes they clicked dismiss" + jest.spyOn(Modal, "createDialog").mockImplementation(() => { + return { finished: Promise.resolve([true]) } as any; + }); + + // When we show the toast, and click Dismiss + showToast(Kind.TURN_ON_KEY_STORAGE); + + const user = userEvent.setup(); + await user.click(await screen.findByRole("button", { name: "Dismiss" })); + + // Then the dialog was opened + expect(Modal.createDialog).toHaveBeenCalledWith( + ConfirmKeyStorageOffDialog, + undefined, + "mx_ConfirmKeyStorageOffDialog", + ); + + // And the backup was disabled when the dialog's onFinished was called + expect(DeviceListener.sharedInstance().recordKeyBackupDisabled).toHaveBeenCalledTimes(1); + }); + }); });