diff --git a/apps/web/playwright/e2e/crypto/toasts.spec.ts b/apps/web/playwright/e2e/crypto/toasts.spec.ts index 9ce1b8a5ae9..053492c8306 100644 --- a/apps/web/playwright/e2e/crypto/toasts.spec.ts +++ b/apps/web/playwright/e2e/crypto/toasts.spec.ts @@ -128,7 +128,8 @@ test.describe("'Turn on key storage' toast", () => { 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 }); + await botClient.setAccountData("m.org.matrix.custom.backup_disabled", {} as any as { disabled: boolean }); + await botClient.setAccountData("m.key_backup", {} as any as { enabled: boolean }); // Wait for the account data setting to stick await new Promise((resolve) => setTimeout(resolve, 2000)); diff --git a/apps/web/src/components/viewmodels/settings/encryption/KeyStoragePanelViewModel.ts b/apps/web/src/components/viewmodels/settings/encryption/KeyStoragePanelViewModel.ts index d968b91a7c2..7c6755a7450 100644 --- a/apps/web/src/components/viewmodels/settings/encryption/KeyStoragePanelViewModel.ts +++ b/apps/web/src/components/viewmodels/settings/encryption/KeyStoragePanelViewModel.ts @@ -10,7 +10,11 @@ import { CryptoEvent } from "matrix-js-sdk/src/crypto-api"; import { logger } from "matrix-js-sdk/src/logger"; import { useMatrixClientContext } from "../../../../contexts/MatrixClientContext"; -import { DeviceListener, BACKUP_DISABLED_ACCOUNT_DATA_KEY } from "../../../../device-listener"; +import { + DeviceListener, + ACCOUNT_DATA_KEY_M_KEY_BACKUP, + ACCOUNT_DATA_KEY_M_KEY_BACKUP_DISABLED_UNSTABLE, +} from "../../../../device-listener"; import { useEventEmitterAsyncState } from "../../../../hooks/useEventEmitter"; import { resetKeyBackupAndWait } from "../../../../utils/crypto/resetKeyBackup"; @@ -113,7 +117,10 @@ export function useKeyStoragePanelViewModel(): KeyStoragePanelState { } // Set the flag so that EX no longer thinks the user wants backup disabled - await matrixClient.setAccountData(BACKUP_DISABLED_ACCOUNT_DATA_KEY, { disabled: false }); + await matrixClient.setAccountData(ACCOUNT_DATA_KEY_M_KEY_BACKUP, { enabled: true }); + await matrixClient.setAccountData(ACCOUNT_DATA_KEY_M_KEY_BACKUP_DISABLED_UNSTABLE, { + 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 @@ -123,7 +130,10 @@ export function useKeyStoragePanelViewModel(): KeyStoragePanelState { // 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 }); + await matrixClient.setAccountData(ACCOUNT_DATA_KEY_M_KEY_BACKUP, { enabled: false }); + await matrixClient.setAccountData(ACCOUNT_DATA_KEY_M_KEY_BACKUP_DISABLED_UNSTABLE, { + disabled: true, + }); } }); } finally { diff --git a/apps/web/src/device-listener/DeviceListener.ts b/apps/web/src/device-listener/DeviceListener.ts index 4d62bf0ab52..7dd4c1e1f4d 100644 --- a/apps/web/src/device-listener/DeviceListener.ts +++ b/apps/web/src/device-listener/DeviceListener.ts @@ -149,7 +149,7 @@ export class DeviceListener { } /** - * Set the account data "m.org.matrix.custom.backup_disabled" to { "disabled": true }. + * Set the account data "m.key_backup" to { "enabled": false }. */ public async recordKeyBackupDisabled(): Promise { await this.currentDevice?.recordKeyBackupDisabled(); diff --git a/apps/web/src/device-listener/DeviceListenerCurrentDevice.ts b/apps/web/src/device-listener/DeviceListenerCurrentDevice.ts index 0460d823662..fab22ee3783 100644 --- a/apps/web/src/device-listener/DeviceListenerCurrentDevice.ts +++ b/apps/web/src/device-listener/DeviceListenerCurrentDevice.ts @@ -29,12 +29,11 @@ import { asyncSomeParallel } from "../utils/arrays"; const KEY_BACKUP_POLL_INTERVAL = 5 * 60 * 1000; /** - * Unfortunately-named account data key used by Element X to indicate that the user - * has chosen to disable server side key backups. - * - * We need to set and honour this to prevent Element X from automatically turning key backup back on. + * Account data key used to indicate that the user has chosen to enable or + * disable server side key backups. */ -export const BACKUP_DISABLED_ACCOUNT_DATA_KEY = "m.org.matrix.custom.backup_disabled"; +export const ACCOUNT_DATA_KEY_M_KEY_BACKUP = "m.key_backup"; +export const ACCOUNT_DATA_KEY_M_KEY_BACKUP_DISABLED_UNSTABLE = "m.org.matrix.custom.backup_disabled"; /** * Account data key to indicate whether the user has chosen to enable or disable recovery. @@ -130,10 +129,11 @@ export class DeviceListenerCurrentDevice { } /** - * Set the account data "m.org.matrix.custom.backup_disabled" to `{ "disabled": true }`. + * Set the account data "m.key_backup" to `{ "enabled": false }`. */ public async recordKeyBackupDisabled(): Promise { - await this.client.setAccountData(BACKUP_DISABLED_ACCOUNT_DATA_KEY, { disabled: true }); + await this.client.setAccountData(ACCOUNT_DATA_KEY_M_KEY_BACKUP, { enabled: false }); + await this.client.setAccountData(ACCOUNT_DATA_KEY_M_KEY_BACKUP_DISABLED_UNSTABLE, { disabled: true }); } /** @@ -284,8 +284,15 @@ export class DeviceListenerCurrentDevice { * Otherwise, fetch it from the store as normal. */ public async recheckBackupDisabled(): Promise { - const backupDisabled = await this.client.getAccountDataFromServer(BACKUP_DISABLED_ACCOUNT_DATA_KEY); - return !!backupDisabled?.disabled; + const keyBackup = await this.client.getAccountDataFromServer(ACCOUNT_DATA_KEY_M_KEY_BACKUP); + if (keyBackup) { + return !keyBackup.enabled; + } + + const keyBackupDisabledUnstable = await this.client.getAccountDataFromServer( + ACCOUNT_DATA_KEY_M_KEY_BACKUP_DISABLED_UNSTABLE, + ); + return !!keyBackupDisabledUnstable?.disabled; } /** @@ -334,7 +341,8 @@ export class DeviceListenerCurrentDevice { ev.getType().startsWith("m.secret_storage.") || ev.getType().startsWith("m.cross_signing.") || ev.getType() === "m.megolm_backup.v1" || - ev.getType() === BACKUP_DISABLED_ACCOUNT_DATA_KEY || + ev.getType() === ACCOUNT_DATA_KEY_M_KEY_BACKUP || + ev.getType() === ACCOUNT_DATA_KEY_M_KEY_BACKUP_DISABLED_UNSTABLE || ev.getType() === RECOVERY_ACCOUNT_DATA_KEY ) { this.deviceListener.recheck(); diff --git a/apps/web/test/unit-tests/DeviceListener-test.ts b/apps/web/test/unit-tests/DeviceListener-test.ts index 5b1e5dd1664..eb073b01588 100644 --- a/apps/web/test/unit-tests/DeviceListener-test.ts +++ b/apps/web/test/unit-tests/DeviceListener-test.ts @@ -6,7 +6,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com Please see LICENSE files in the repository root for full details. */ -import { type Mocked, mocked } from "jest-mock"; +import { type Mocked, mocked, type MockedObject } from "jest-mock"; import { MatrixEvent, type Room, @@ -25,7 +25,11 @@ import { } from "matrix-js-sdk/src/crypto-api"; import { type CryptoSessionStateChange } from "@matrix-org/analytics-events/types/typescript/CryptoSessionStateChange"; -import { DeviceListener, BACKUP_DISABLED_ACCOUNT_DATA_KEY } from "../../src/device-listener"; +import { + DeviceListener, + ACCOUNT_DATA_KEY_M_KEY_BACKUP, + ACCOUNT_DATA_KEY_M_KEY_BACKUP_DISABLED_UNSTABLE, +} from "../../src/device-listener"; import { MatrixClientPeg } from "../../src/MatrixClientPeg"; import * as SetupEncryptionToast from "../../src/toasts/SetupEncryptionToast"; import * as UnverifiedSessionToast from "../../src/toasts/UnverifiedSessionToast"; @@ -429,9 +433,7 @@ describe("DeviceListener", () => { it("does not show an out-of-sync toast when the backup key is missing locally but backup is purposely disabled", async () => { mockCrypto!.getSecretStorageStatus.mockResolvedValue(readySecretStorageStatus); mockCrypto!.getSessionBackupPrivateKey.mockResolvedValue(null); - mockClient.getAccountDataFromServer.mockImplementation((eventType) => - eventType === BACKUP_DISABLED_ACCOUNT_DATA_KEY ? ({ disabled: true } as any) : null, - ); + mockKeyBackupFromServer(mockClient, false); await createAndStart(); @@ -537,6 +539,10 @@ describe("DeviceListener", () => { expect(mockClient.setAccountData).toHaveBeenCalledWith("m.org.matrix.custom.backup_disabled", { disabled: true, }); + + expect(mockClient.setAccountData).toHaveBeenCalledWith("m.key_backup", { + enabled: false, + }); }); it("sets the recovery account data when we call recordRecoveryDisabled", async () => { @@ -568,7 +574,7 @@ describe("DeviceListener", () => { 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) + // (m.key_backup or m.org.matrix.custom.backup_disabled) mockClient.getAccountData.mockReturnValue(undefined); // When we launch the DeviceListener @@ -581,11 +587,16 @@ describe("DeviceListener", () => { 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, - ); + mockClient.getAccountData.mockImplementation((eventType) => { + switch (eventType) { + case ACCOUNT_DATA_KEY_M_KEY_BACKUP: + return new MatrixEvent({ content: { enabled: true } }); + case ACCOUNT_DATA_KEY_M_KEY_BACKUP_DISABLED_UNSTABLE: + return new MatrixEvent({ content: { disabled: false } }); + default: + return undefined; + } + }); // When we launch the DeviceListener await createAndStart(); @@ -596,9 +607,7 @@ describe("DeviceListener", () => { 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, - ); + mockKeyBackupFromServer(mockClient, false); // When we launch the DeviceListener await createAndStart(); @@ -627,11 +636,16 @@ describe("DeviceListener", () => { 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, - ); + mockClient.getAccountData.mockImplementation((eventType) => { + switch (eventType) { + case ACCOUNT_DATA_KEY_M_KEY_BACKUP: + return new MatrixEvent({ content: { enabled: true } }); + case ACCOUNT_DATA_KEY_M_KEY_BACKUP_DISABLED_UNSTABLE: + return new MatrixEvent({ content: { disabled: false } }); + default: + return undefined; + } + }); // When we launch the DeviceListener await createAndStart(); @@ -643,11 +657,16 @@ describe("DeviceListener", () => { 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, - ); + mockClient.getAccountData.mockImplementation((eventType) => { + switch (eventType) { + case ACCOUNT_DATA_KEY_M_KEY_BACKUP: + return new MatrixEvent({ content: { enabled: false } }); + case ACCOUNT_DATA_KEY_M_KEY_BACKUP_DISABLED_UNSTABLE: + return new MatrixEvent({ content: { disabled: true } }); + default: + return undefined; + } + }); // When we launch the DeviceListener await createAndStart(); @@ -1195,10 +1214,16 @@ describe("DeviceListener", () => { it("does not show the 'set up recovery' toast if the user has chosen to disable key storage", async () => { mockClient!.getAccountData.mockImplementation((k: string) => { - if (k === "m.org.matrix.custom.backup_disabled") { - return new MatrixEvent({ content: { disabled: true } }); + switch (k) { + case "m.org.matrix.custom.backup_disabled": + return new MatrixEvent({ content: { disabled: true } }); + + case "m.key_backup": + return new MatrixEvent({ content: { enabled: false } }); + + default: + return undefined; } - return undefined; }); await createAndStart(); @@ -1211,18 +1236,15 @@ describe("DeviceListener", () => { describe("needs backup reset", () => { it("should not need resetting if backup disabled", async () => { const deviceListener = await createAndStart(); - mockClient.getAccountDataFromServer.mockResolvedValue({ - disabled: true, - }); + mockKeyBackupFromServer(mockClient, false); + 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, - }); + mockKeyBackupFromServer(mockClient, true); mockCrypto.getSessionBackupPrivateKey.mockResolvedValue(null); mockClient.isKeyBackupKeyStored.mockResolvedValue({}); @@ -1235,9 +1257,7 @@ describe("DeviceListener", () => { 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, - }); + mockKeyBackupFromServer(mockClient, true); mockCrypto.getSessionBackupPrivateKey.mockResolvedValue(new Uint8Array()); mockClient.isKeyBackupKeyStored.mockResolvedValue(null); @@ -1246,9 +1266,7 @@ describe("DeviceListener", () => { 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, - }); + mockKeyBackupFromServer(mockClient, true); mockCrypto.getSessionBackupPrivateKey.mockResolvedValue(null); mockClient.isKeyBackupKeyStored.mockResolvedValue({}); @@ -1257,9 +1275,7 @@ describe("DeviceListener", () => { it("should need resetting if backup key is missing locally and in 4s", async () => { const deviceListener = await createAndStart(); - mockClient.getAccountDataFromServer.mockResolvedValue({ - disabled: false, - }); + mockKeyBackupFromServer(mockClient, true); mockCrypto.getSessionBackupPrivateKey.mockResolvedValue(null); mockClient.isKeyBackupKeyStored.mockResolvedValue(null); @@ -1337,3 +1353,16 @@ describe("DeviceListener", () => { }); }); }); + +function mockKeyBackupFromServer(client: MockedObject, enabled: boolean) { + client.getAccountDataFromServer.mockImplementation(async (eventType: string) => { + switch (eventType) { + case ACCOUNT_DATA_KEY_M_KEY_BACKUP: + return { enabled } as any; + case ACCOUNT_DATA_KEY_M_KEY_BACKUP_DISABLED_UNSTABLE: + return { disabled: !enabled } as any; + default: + return null; + } + }); +} diff --git a/apps/web/test/unit-tests/components/viewmodels/settings/encryption/KeyStoragePanelViewModel-test.ts b/apps/web/test/unit-tests/components/viewmodels/settings/encryption/KeyStoragePanelViewModel-test.ts index eb9c81c4a75..dc1faec9855 100644 --- a/apps/web/test/unit-tests/components/viewmodels/settings/encryption/KeyStoragePanelViewModel-test.ts +++ b/apps/web/test/unit-tests/components/viewmodels/settings/encryption/KeyStoragePanelViewModel-test.ts @@ -128,6 +128,10 @@ describe("KeyStoragePanelViewModel", () => { expect(mocked(matrixClient.setAccountData)).toHaveBeenCalledWith("m.org.matrix.custom.backup_disabled", { disabled: false, }); + + expect(mocked(matrixClient.setAccountData)).toHaveBeenCalledWith("m.key_backup", { + enabled: true, + }); }); it("should delete key storage when disabling", async () => { @@ -145,5 +149,8 @@ describe("KeyStoragePanelViewModel", () => { expect(mocked(matrixClient.setAccountData)).toHaveBeenCalledWith("m.org.matrix.custom.backup_disabled", { disabled: true, }); + expect(mocked(matrixClient.setAccountData)).toHaveBeenCalledWith("m.key_backup", { + enabled: false, + }); }); });