From 66b739fea98458bd1f64597401e9df4be4da5e04 Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Mon, 18 May 2026 19:18:56 +0100 Subject: [PATCH] Remove an impossible case in DeviceListener (#32977) * Remove an impossible case in DeviceListener We only reach this branch if recoveryIsOk is false, which means recoveryDisabled can't be true. * Add a test for when recovery is bad but backup is disabled --- .../DeviceListenerCurrentDevice.ts | 5 +-- .../test/unit-tests/DeviceListener-test.ts | 42 ++++++++++++++++++- 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/apps/web/src/device-listener/DeviceListenerCurrentDevice.ts b/apps/web/src/device-listener/DeviceListenerCurrentDevice.ts index 0764d0aa99..45eec73d82 100644 --- a/apps/web/src/device-listener/DeviceListenerCurrentDevice.ts +++ b/apps/web/src/device-listener/DeviceListenerCurrentDevice.ts @@ -218,10 +218,7 @@ export class DeviceListenerCurrentDevice { // 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 (recoveryDisabled) { - logSpan.info("Recovery disabled: no toast needed"); - await this.setDeviceState("ok", logSpan); - } else if (keyBackupUploadActive) { + if (keyBackupUploadActive) { logSpan.info("No default 4S key: setting state to SET_UP_RECOVERY"); await this.setDeviceState("set_up_recovery", logSpan); } else { diff --git a/apps/web/test/unit-tests/DeviceListener-test.ts b/apps/web/test/unit-tests/DeviceListener-test.ts index f8777182cf..6ec2129472 100644 --- a/apps/web/test/unit-tests/DeviceListener-test.ts +++ b/apps/web/test/unit-tests/DeviceListener-test.ts @@ -25,7 +25,7 @@ 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, BACKUP_DISABLED_ACCOUNT_DATA_KEY, RECOVERY_ACCOUNT_DATA_KEY } from "../../src/device-listener"; import { MatrixClientPeg } from "../../src/MatrixClientPeg"; import * as SetupEncryptionToast from "../../src/toasts/SetupEncryptionToast"; import * as UnverifiedSessionToast from "../../src/toasts/UnverifiedSessionToast"; @@ -301,12 +301,14 @@ describe("DeviceListener", () => { expect(mockCrypto!.isCrossSigningReady).not.toHaveBeenCalled(); }); + it("does nothing when initial sync is not complete", async () => { mockClient!.isInitialSyncComplete.mockReturnValue(false); await createAndStart(); expect(mockCrypto!.isCrossSigningReady).not.toHaveBeenCalled(); }); + it("correctly handles the client being stopped", async () => { mockCrypto!.isCrossSigningReady.mockImplementation(() => { throw new ClientStoppedError(); @@ -314,6 +316,44 @@ describe("DeviceListener", () => { await createAndStart(); expect(console.error).not.toHaveBeenCalled(); }); + + it("shows no error if key backup is disabled", async () => { + // Given backup is disabled but recovery is not disabled + + // @ts-ignore implementing a function with complex return type + mockClient!.getAccountDataFromServer.mockImplementation(async (key) => { + if (key === BACKUP_DISABLED_ACCOUNT_DATA_KEY) { + return { disabled: true }; + } else if (key === RECOVERY_ACCOUNT_DATA_KEY) { + return null; + } else { + throw new Error(`Unexpected account data query: ${key}`); + } + }); + + // And backup uploads are not active + mockCrypto!.getActiveSessionBackupVersion.mockResolvedValue(null); + + // And the current device is trusted + mockCrypto!.getDeviceVerificationStatus.mockResolvedValue( + new DeviceVerificationStatus({ + trustCrossSignedDevices: true, + crossSigningVerified: true, + }), + ); + + // And recovery is not OK (i.e. it is enabled but not ready) + mockCrypto!.getSecretStorageStatus.mockResolvedValue(unreadySecretStorageStatus); + + // When we check whether we are in a good state + await createAndStart(); + + // Then we are fine: no toasts displayed, because recovery being in + // a bad state is not important if backups are disabled. + expect(SetupEncryptionToast.showToast).not.toHaveBeenCalled(); + expect(SetupEncryptionToast.hideToast).toHaveBeenCalled(); + }); + it("correctly handles other errors", async () => { mockCrypto!.isCrossSigningReady.mockImplementation(() => { throw new Error("blah");