Support MSC4287 m.key_backup stable prefix as well as the unstable prefix

Author: andybalaam

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));

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 {

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<void> {
         await this.currentDevice?.recordKeyBackupDisabled();

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 by 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<void> {
-        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<boolean> {
-        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();

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<MatrixClient>, 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;
+        }
+    });
+}

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,
+        });
     });
 });