From ede91bf9218b9d8152fd9bd3e80717f5ac8aaf3c Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Mon, 25 Nov 2024 10:30:42 +0100 Subject: [PATCH] Use `CryptoApi.getKeyBackupInfo` instead of deprecated `MatrixClient.getKeyBackupVersion` (#28450) * Use `CryptoApi.getKeyBackupInfo` instead of deprecated `MatrixClient.getKeyBackupVersion` * Review changes --------- Co-authored-by: Michael Telatynski <7t3chguy@gmail.com> --- src/DeviceListener.ts | 5 ++++- .../security/CreateSecretStorageDialog.tsx | 2 +- src/components/structures/MatrixChat.tsx | 2 +- src/components/views/dialogs/LogoutDialog.tsx | 2 +- .../security/RestoreKeyBackupDialog.tsx | 2 +- .../views/settings/SecureBackupPanel.tsx | 11 ++++------- src/stores/SetupEncryptionStore.ts | 2 +- test/test-utils/client.ts | 2 +- test/test-utils/test-utils.ts | 2 +- test/unit-tests/DeviceListener-test.ts | 10 +++++----- .../components/structures/MatrixChat-test.tsx | 2 +- .../views/dialogs/LogoutDialog-test.tsx | 7 +++---- .../CreateSecretStorageDialog-test.tsx | 4 ++-- .../security/RestoreKeyBackupDialog-test.tsx | 4 ++-- .../views/settings/SecureBackupPanel-test.tsx | 18 ++++++++---------- .../tabs/user/SecurityUserSettingsTab-test.tsx | 1 - .../stores/SetupEncryptionStore-test.ts | 1 + 17 files changed, 37 insertions(+), 40 deletions(-) diff --git a/src/DeviceListener.ts b/src/DeviceListener.ts index 4f47cd7eac4..84d83827da5 100644 --- a/src/DeviceListener.ts +++ b/src/DeviceListener.ts @@ -230,12 +230,15 @@ export default class DeviceListener { private async getKeyBackupInfo(): Promise { if (!this.client) return null; const now = new Date().getTime(); + const crypto = this.client.getCrypto(); + if (!crypto) return null; + if ( !this.keyBackupInfo || !this.keyBackupFetchedAt || this.keyBackupFetchedAt < now - KEY_BACKUP_POLL_INTERVAL ) { - this.keyBackupInfo = await this.client.getKeyBackupVersion(); + this.keyBackupInfo = await crypto.getKeyBackupInfo(); this.keyBackupFetchedAt = now; } return this.keyBackupInfo; diff --git a/src/async-components/views/dialogs/security/CreateSecretStorageDialog.tsx b/src/async-components/views/dialogs/security/CreateSecretStorageDialog.tsx index 1258bde2cad..932f6d7fcf8 100644 --- a/src/async-components/views/dialogs/security/CreateSecretStorageDialog.tsx +++ b/src/async-components/views/dialogs/security/CreateSecretStorageDialog.tsx @@ -279,7 +279,7 @@ export default class CreateSecretStorageDialog extends React.PureComponent { } else { // otherwise check the server to see if there's a new one try { - newVersionInfo = await cli.getKeyBackupVersion(); + newVersionInfo = (await cli.getCrypto()?.getKeyBackupInfo()) ?? null; if (newVersionInfo !== null) haveNewVersion = true; } catch (e) { logger.error("Saw key backup error but failed to check backup version!", e); diff --git a/src/components/views/dialogs/LogoutDialog.tsx b/src/components/views/dialogs/LogoutDialog.tsx index cdfaf0e89b1..4731c593bc3 100644 --- a/src/components/views/dialogs/LogoutDialog.tsx +++ b/src/components/views/dialogs/LogoutDialog.tsx @@ -109,7 +109,7 @@ export default class LogoutDialog extends React.Component { } // backup is not active. see if there is a backup version on the server we ought to back up to. - const backupInfo = await client.getKeyBackupVersion(); + const backupInfo = await crypto.getKeyBackupInfo(); this.setState({ backupStatus: backupInfo ? BackupStatus.SERVER_BACKUP_BUT_DISABLED : BackupStatus.NO_BACKUP }); } diff --git a/src/components/views/dialogs/security/RestoreKeyBackupDialog.tsx b/src/components/views/dialogs/security/RestoreKeyBackupDialog.tsx index ec85e72ac9d..fddba10948b 100644 --- a/src/components/views/dialogs/security/RestoreKeyBackupDialog.tsx +++ b/src/components/views/dialogs/security/RestoreKeyBackupDialog.tsx @@ -258,7 +258,7 @@ export default class RestoreKeyBackupDialog extends React.PureComponent { this.getUpdatedDiagnostics(); try { const cli = MatrixClientPeg.safeGet(); - const backupInfo = await cli.getKeyBackupVersion(); + const backupInfo = (await cli.getCrypto()?.getKeyBackupInfo()) ?? null; const backupTrustInfo = backupInfo ? await cli.getCrypto()?.isKeyBackupTrusted(backupInfo) : undefined; const activeBackupVersion = (await cli.getCrypto()?.getActiveSessionBackupVersion()) ?? null; @@ -192,12 +192,9 @@ export default class SecureBackupPanel extends React.PureComponent<{}, IState> { if (!proceed) return; this.setState({ loading: true }); const versionToDelete = this.state.backupInfo!.version!; - MatrixClientPeg.safeGet() - .getCrypto() - ?.deleteKeyBackupVersion(versionToDelete) - .then(() => { - this.loadBackupStatus(); - }); + // deleteKeyBackupVersion fires a key backup status event + // which will update the UI + MatrixClientPeg.safeGet().getCrypto()?.deleteKeyBackupVersion(versionToDelete); }, }); }; diff --git a/src/stores/SetupEncryptionStore.ts b/src/stores/SetupEncryptionStore.ts index 2fb9c6a9ca7..70c721b1ca3 100644 --- a/src/stores/SetupEncryptionStore.ts +++ b/src/stores/SetupEncryptionStore.ts @@ -125,7 +125,7 @@ export class SetupEncryptionStore extends EventEmitter { this.emit("update"); try { const cli = MatrixClientPeg.safeGet(); - const backupInfo = await cli.getKeyBackupVersion(); + const backupInfo = (await cli.getCrypto()?.getKeyBackupInfo()) ?? null; this.backupInfo = backupInfo; this.emit("update"); diff --git a/test/test-utils/client.ts b/test/test-utils/client.ts index 7842afbfe51..a2347f90588 100644 --- a/test/test-utils/client.ts +++ b/test/test-utils/client.ts @@ -143,7 +143,6 @@ export const mockClientMethodsCrypto = (): Partial< > => ({ isKeyBackupKeyStored: jest.fn(), getCrossSigningCacheCallbacks: jest.fn().mockReturnValue({ getCrossSigningKeyCache: jest.fn() }), - getKeyBackupVersion: jest.fn().mockResolvedValue(null), secretStorage: { hasKey: jest.fn() }, getCrypto: jest.fn().mockReturnValue({ getUserDeviceInfo: jest.fn(), @@ -163,6 +162,7 @@ export const mockClientMethodsCrypto = (): Partial< getOwnDeviceKeys: jest.fn().mockReturnValue(new Promise(() => {})), getCrossSigningKeyId: jest.fn(), isEncryptionEnabledInRoom: jest.fn().mockResolvedValue(false), + getKeyBackupInfo: jest.fn().mockResolvedValue(null), }), }); diff --git a/test/test-utils/test-utils.ts b/test/test-utils/test-utils.ts index 78481c2fd06..9af2ffa340e 100644 --- a/test/test-utils/test-utils.ts +++ b/test/test-utils/test-utils.ts @@ -99,7 +99,6 @@ export function createTestClient(): MatrixClient { getDevices: jest.fn().mockResolvedValue({ devices: [{ device_id: "ABCDEFGHI" }] }), getSessionId: jest.fn().mockReturnValue("iaszphgvfku"), credentials: { userId: "@userId:matrix.org" }, - getKeyBackupVersion: jest.fn(), secretStorage: { get: jest.fn(), @@ -135,6 +134,7 @@ export function createTestClient(): MatrixClient { restoreKeyBackupWithPassphrase: jest.fn(), loadSessionBackupPrivateKeyFromSecretStorage: jest.fn(), storeSessionBackupPrivateKey: jest.fn(), + getKeyBackupInfo: jest.fn().mockResolvedValue(null), }), getPushActionsForEvent: jest.fn(), diff --git a/test/unit-tests/DeviceListener-test.ts b/test/unit-tests/DeviceListener-test.ts index 906826e4564..ad7f14e1190 100644 --- a/test/unit-tests/DeviceListener-test.ts +++ b/test/unit-tests/DeviceListener-test.ts @@ -96,12 +96,12 @@ describe("DeviceListener", () => { }), getSessionBackupPrivateKey: jest.fn(), isEncryptionEnabledInRoom: jest.fn(), + getKeyBackupInfo: jest.fn().mockResolvedValue(null), } as unknown as Mocked; mockClient = getMockClientWithEventEmitter({ isGuest: jest.fn(), getUserId: jest.fn().mockReturnValue(userId), getSafeUserId: jest.fn().mockReturnValue(userId), - getKeyBackupVersion: jest.fn().mockResolvedValue(undefined), getRooms: jest.fn().mockReturnValue([]), isVersionSupported: jest.fn().mockResolvedValue(true), isInitialSyncComplete: jest.fn().mockReturnValue(true), @@ -354,7 +354,7 @@ describe("DeviceListener", () => { it("shows set up encryption toast when user has a key backup available", async () => { // non falsy response - mockClient!.getKeyBackupVersion.mockResolvedValue({} as unknown as KeyBackupInfo); + mockCrypto.getKeyBackupInfo.mockResolvedValue({} as unknown as KeyBackupInfo); await createAndStart(); expect(SetupEncryptionToast.showToast).toHaveBeenCalledWith( @@ -673,7 +673,7 @@ describe("DeviceListener", () => { describe("When Room Key Backup is not enabled", () => { beforeEach(() => { // no backup - mockClient.getKeyBackupVersion.mockResolvedValue(null); + mockCrypto.getKeyBackupInfo.mockResolvedValue(null); }); it("Should report recovery state as Enabled", async () => { @@ -722,7 +722,7 @@ describe("DeviceListener", () => { }); // no backup - mockClient.getKeyBackupVersion.mockResolvedValue(null); + mockCrypto.getKeyBackupInfo.mockResolvedValue(null); await createAndStart(); @@ -872,7 +872,7 @@ describe("DeviceListener", () => { describe("When Room Key Backup is enabled", () => { beforeEach(() => { // backup enabled - just need a mock object - mockClient.getKeyBackupVersion.mockResolvedValue({} as KeyBackupInfo); + mockCrypto.getKeyBackupInfo.mockResolvedValue({} as KeyBackupInfo); }); const testCases = [ diff --git a/test/unit-tests/components/structures/MatrixChat-test.tsx b/test/unit-tests/components/structures/MatrixChat-test.tsx index 5cee50ef29d..28bf99fa978 100644 --- a/test/unit-tests/components/structures/MatrixChat-test.tsx +++ b/test/unit-tests/components/structures/MatrixChat-test.tsx @@ -139,6 +139,7 @@ describe("", () => { globalBlacklistUnverifiedDevices: false, // This needs to not finish immediately because we need to test the screen appears bootstrapCrossSigning: jest.fn().mockImplementation(() => bootstrapDeferred.promise), + getKeyBackupInfo: jest.fn().mockResolvedValue(null), }), secretStorage: { isStored: jest.fn().mockReturnValue(null), @@ -148,7 +149,6 @@ describe("", () => { whoami: jest.fn(), logout: jest.fn(), getDeviceId: jest.fn(), - getKeyBackupVersion: jest.fn().mockResolvedValue(null), }); let mockClient: Mocked; const serverConfig = { diff --git a/test/unit-tests/components/views/dialogs/LogoutDialog-test.tsx b/test/unit-tests/components/views/dialogs/LogoutDialog-test.tsx index 46fe519b471..0557e538d0e 100644 --- a/test/unit-tests/components/views/dialogs/LogoutDialog-test.tsx +++ b/test/unit-tests/components/views/dialogs/LogoutDialog-test.tsx @@ -22,7 +22,6 @@ describe("LogoutDialog", () => { beforeEach(() => { mockClient = getMockClientWithEventEmitter({ ...mockClientMethodsCrypto(), - getKeyBackupVersion: jest.fn(), }); mockCrypto = mocked(mockClient.getCrypto()!); @@ -50,14 +49,14 @@ describe("LogoutDialog", () => { }); it("Prompts user to connect backup if there is a backup on the server", async () => { - mockClient.getKeyBackupVersion.mockResolvedValue({} as KeyBackupInfo); + mockCrypto.getKeyBackupInfo.mockResolvedValue({} as KeyBackupInfo); const rendered = renderComponent(); await rendered.findByText("Connect this session to Key Backup"); expect(rendered.container).toMatchSnapshot(); }); it("Prompts user to set up backup if there is no backup on the server", async () => { - mockClient.getKeyBackupVersion.mockResolvedValue(null); + mockCrypto.getKeyBackupInfo.mockResolvedValue(null); const rendered = renderComponent(); await rendered.findByText("Start using Key Backup"); expect(rendered.container).toMatchSnapshot(); @@ -69,7 +68,7 @@ describe("LogoutDialog", () => { describe("when there is an error fetching backups", () => { filterConsole("Unable to fetch key backup status"); it("prompts user to set up backup", async () => { - mockClient.getKeyBackupVersion.mockImplementation(async () => { + mockCrypto.getKeyBackupInfo.mockImplementation(async () => { throw new Error("beep"); }); const rendered = renderComponent(); diff --git a/test/unit-tests/components/views/dialogs/security/CreateSecretStorageDialog-test.tsx b/test/unit-tests/components/views/dialogs/security/CreateSecretStorageDialog-test.tsx index 9e792a48f30..814fe8a9543 100644 --- a/test/unit-tests/components/views/dialogs/security/CreateSecretStorageDialog-test.tsx +++ b/test/unit-tests/components/views/dialogs/security/CreateSecretStorageDialog-test.tsx @@ -77,7 +77,7 @@ describe("CreateSecretStorageDialog", () => { filterConsole("Error fetching backup data from server"); it("shows an error", async () => { - mockClient.getKeyBackupVersion.mockImplementation(async () => { + jest.spyOn(mockClient.getCrypto()!, "getKeyBackupInfo").mockImplementation(async () => { throw new Error("bleh bleh"); }); @@ -92,7 +92,7 @@ describe("CreateSecretStorageDialog", () => { expect(result.container).toMatchSnapshot(); // Now we can get the backup and we retry - mockClient.getKeyBackupVersion.mockRestore(); + jest.spyOn(mockClient.getCrypto()!, "getKeyBackupInfo").mockRestore(); await userEvent.click(screen.getByRole("button", { name: "Retry" })); await screen.findByText("Your keys are now being backed up from this device."); }); diff --git a/test/unit-tests/components/views/dialogs/security/RestoreKeyBackupDialog-test.tsx b/test/unit-tests/components/views/dialogs/security/RestoreKeyBackupDialog-test.tsx index 4cfa74073b5..c19010a0893 100644 --- a/test/unit-tests/components/views/dialogs/security/RestoreKeyBackupDialog-test.tsx +++ b/test/unit-tests/components/views/dialogs/security/RestoreKeyBackupDialog-test.tsx @@ -28,7 +28,7 @@ describe("", () => { beforeEach(() => { matrixClient = stubClient(); jest.spyOn(recoveryKeyModule, "decodeRecoveryKey").mockReturnValue(new Uint8Array(32)); - jest.spyOn(matrixClient, "getKeyBackupVersion").mockResolvedValue({ version: "1" } as KeyBackupInfo); + jest.spyOn(matrixClient.getCrypto()!, "getKeyBackupInfo").mockResolvedValue({ version: "1" } as KeyBackupInfo); }); it("should render", async () => { @@ -99,7 +99,7 @@ describe("", () => { test("should restore key backup when passphrase is filled", async () => { // Determine that the passphrase is required - jest.spyOn(matrixClient, "getKeyBackupVersion").mockResolvedValue({ + jest.spyOn(matrixClient.getCrypto()!, "getKeyBackupInfo").mockResolvedValue({ version: "1", auth_data: { private_key_salt: "salt", diff --git a/test/unit-tests/components/views/settings/SecureBackupPanel-test.tsx b/test/unit-tests/components/views/settings/SecureBackupPanel-test.tsx index f2aa15f3556..63490bf9150 100644 --- a/test/unit-tests/components/views/settings/SecureBackupPanel-test.tsx +++ b/test/unit-tests/components/views/settings/SecureBackupPanel-test.tsx @@ -28,14 +28,13 @@ describe("", () => { const client = getMockClientWithEventEmitter({ ...mockClientMethodsUser(userId), ...mockClientMethodsCrypto(), - getKeyBackupVersion: jest.fn().mockReturnValue("1"), getClientWellKnown: jest.fn(), }); const getComponent = () => render(); beforeEach(() => { - client.getKeyBackupVersion.mockResolvedValue({ + jest.spyOn(client.getCrypto()!, "getKeyBackupInfo").mockResolvedValue({ version: "1", algorithm: "test", auth_data: { @@ -52,7 +51,6 @@ describe("", () => { }); mocked(client.secretStorage.hasKey).mockClear().mockResolvedValue(false); - client.getKeyBackupVersion.mockClear(); mocked(accessSecretStorage).mockClear().mockResolvedValue(); }); @@ -65,8 +63,8 @@ describe("", () => { }); it("handles error fetching backup", async () => { - // getKeyBackupVersion can fail for various reasons - client.getKeyBackupVersion.mockImplementation(async () => { + // getKeyBackupInfo can fail for various reasons + jest.spyOn(client.getCrypto()!, "getKeyBackupInfo").mockImplementation(async () => { throw new Error("beep beep"); }); const renderResult = getComponent(); @@ -75,9 +73,9 @@ describe("", () => { }); it("handles absence of backup", async () => { - client.getKeyBackupVersion.mockResolvedValue(null); + jest.spyOn(client.getCrypto()!, "getKeyBackupInfo").mockResolvedValue(null); getComponent(); - // flush getKeyBackupVersion promise + // flush getKeyBackupInfo promise await flushPromises(); expect(screen.getByText("Back up your keys before signing out to avoid losing them.")).toBeInTheDocument(); }); @@ -120,7 +118,7 @@ describe("", () => { }); it("deletes backup after confirmation", async () => { - client.getKeyBackupVersion + jest.spyOn(client.getCrypto()!, "getKeyBackupInfo") .mockResolvedValueOnce({ version: "1", algorithm: "test", @@ -157,7 +155,7 @@ describe("", () => { // flush checkKeyBackup promise await flushPromises(); - client.getKeyBackupVersion.mockClear(); + jest.spyOn(client.getCrypto()!, "getKeyBackupInfo").mockClear(); mocked(client.getCrypto()!).isKeyBackupTrusted.mockClear(); fireEvent.click(screen.getByText("Reset")); @@ -167,7 +165,7 @@ describe("", () => { await flushPromises(); // backup status refreshed - expect(client.getKeyBackupVersion).toHaveBeenCalled(); + expect(client.getCrypto()!.getKeyBackupInfo).toHaveBeenCalled(); expect(client.getCrypto()!.isKeyBackupTrusted).toHaveBeenCalled(); }); }); diff --git a/test/unit-tests/components/views/settings/tabs/user/SecurityUserSettingsTab-test.tsx b/test/unit-tests/components/views/settings/tabs/user/SecurityUserSettingsTab-test.tsx index 0ee882767bb..27403919b6c 100644 --- a/test/unit-tests/components/views/settings/tabs/user/SecurityUserSettingsTab-test.tsx +++ b/test/unit-tests/components/views/settings/tabs/user/SecurityUserSettingsTab-test.tsx @@ -34,7 +34,6 @@ describe("", () => { ...mockClientMethodsCrypto(), getRooms: jest.fn().mockReturnValue([]), getIgnoredUsers: jest.fn(), - getKeyBackupVersion: jest.fn(), }); const sdkContext = new SdkContextClass(); diff --git a/test/unit-tests/stores/SetupEncryptionStore-test.ts b/test/unit-tests/stores/SetupEncryptionStore-test.ts index b9ab29b94b1..d3d0300a215 100644 --- a/test/unit-tests/stores/SetupEncryptionStore-test.ts +++ b/test/unit-tests/stores/SetupEncryptionStore-test.ts @@ -37,6 +37,7 @@ describe("SetupEncryptionStore", () => { getDeviceVerificationStatus: jest.fn(), isDehydrationSupported: jest.fn().mockResolvedValue(false), startDehydration: jest.fn(), + getKeyBackupInfo: jest.fn().mockResolvedValue(null), } as unknown as Mocked; client.getCrypto.mockReturnValue(mockCrypto);