diff --git a/playwright/e2e/crypto/event-shields.spec.ts b/playwright/e2e/crypto/event-shields.spec.ts index 3811c2819e5..c0f1e280a2f 100644 --- 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 index c880961964f..7474c5a4354 100644 --- 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 index 7fe0cb3d476..f19bd68f145 100644 --- 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 index e6812cd450b..a322d42d4ec 100644 --- 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 new file mode 100644 index 00000000000..8e335bd2323 Binary files /dev/null and b/playwright/snapshots/crypto/toasts.spec.ts/key-storage-out-of-sync-toast-linux.png differ diff --git a/src/DeviceListener.ts b/src/DeviceListener.ts index 28bb5f655e0..e50f0d3f9bc 100644 --- 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(); - 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 index 0e249cecdcb..61c62723771 100644 --- a/src/components/views/toasts/GenericToast.tsx +++ b/src/components/views/toasts/GenericToast.tsx @@ -25,6 +25,9 @@ interface IPropsExtended extends IProps { SecondaryIcon?: ComponentType>; 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> = ({ @@ -37,12 +40,13 @@ const GenericToast: React.FC> = ({ destructive, onPrimaryClick, onSecondaryClick, + overrideWidth, }) => { const detailContent = detail ?
{detail}
: null; return (
-
+
{description} {detailContent}
diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 37a739a62e9..875da43f14e 100644 --- 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 index ecbf99f4b21..3b8e85eb444 100644 --- 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 deleted file mode 100644 index 8f5d93ffae2..00000000000 --- a/src/utils/login.ts +++ /dev/null @@ -1,17 +0,0 @@ -/* -Copyright 2024 New Vector Ltd. -Copyright 2022 The Matrix.org Foundation C.I.C. - -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 Views from "../Views"; - -export function isLoggedIn(): boolean { - // JRS: Maybe we should move the step that writes this to the window out of - // `element-web` and into this file? Better yet, we should probably create a - // store to hold this state. - // See also https://github.com/vector-im/element-web/issues/15034. - return window.matrixChat?.state.view === Views.LOGGED_IN; -} diff --git a/test/unit-tests/DeviceListener-test.ts b/test/unit-tests/DeviceListener-test.ts index b63896c64dd..bdbf86637d2 100644 --- a/test/unit-tests/DeviceListener-test.ts +++ b/test/unit-tests/DeviceListener-test.ts @@ -329,7 +329,7 @@ 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(); @@ -337,24 +337,25 @@ describe("DeviceListener", () => { 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 index 8917587cc1f..22b491817cc 100644 --- a/test/unit-tests/toasts/SetupEncryptionToast-test.tsx +++ b/test/unit-tests/toasts/SetupEncryptionToast-test.tsx @@ -16,9 +16,15 @@ describe("SetupEncryptionToast", () => { render(); }); - 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 deleted file mode 100644 index b1f488c29f5..00000000000 --- a/test/unit-tests/utils/login-test.ts +++ /dev/null @@ -1,22 +0,0 @@ -/* -Copyright 2024 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 MatrixChat from "../../../src/components/structures/MatrixChat.tsx"; -import { isLoggedIn } from "../../../src/utils/login.ts"; -import Views from "../../../src/Views.ts"; - -describe("isLoggedIn", () => { - it("should return true if MatrixChat state view is LOGGED_IN", () => { - window.matrixChat = { - state: { - view: Views.LOGGED_IN, - }, - } as unknown as MatrixChat; - - expect(isLoggedIn()).toBe(true); - }); -});