From c8798825585cfd357701dcb3e3b0d9cf5ea7b302 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Thu, 21 Sep 2023 14:19:38 +0200 Subject: [PATCH] Don't start key backups when opening settings (#11640) * SecureBackupPanel: stop calling `checkKeyBackup` `checkKeyBackup` will start key backups if they aren't already running. In my not-so-humble opinion, the mere act of opening a settings panel shouldn't change anything. * fix SecurityUserSettingsTab test --- .../views/settings/SecureBackupPanel.tsx | 24 +------ .../views/settings/SecureBackupPanel-test.tsx | 57 ++++++++-------- .../SecureBackupPanel-test.tsx.snap | 65 +++++++++++++++++++ .../user/SecurityUserSettingsTab-test.tsx | 1 + test/test-utils/client.ts | 1 - 5 files changed, 95 insertions(+), 53 deletions(-) diff --git a/src/components/views/settings/SecureBackupPanel.tsx b/src/components/views/settings/SecureBackupPanel.tsx index a55679964f..ce8e385684 100644 --- a/src/components/views/settings/SecureBackupPanel.tsx +++ b/src/components/views/settings/SecureBackupPanel.tsx @@ -67,7 +67,7 @@ export default class SecureBackupPanel extends React.PureComponent<{}, IState> { } public componentDidMount(): void { - this.checkKeyBackupStatus(); + this.loadBackupStatus(); MatrixClientPeg.safeGet().on(CryptoEvent.KeyBackupStatus, this.onKeyBackupStatus); MatrixClientPeg.safeGet().on(CryptoEvent.KeyBackupSessionsRemaining, this.onKeyBackupSessionsRemaining); @@ -97,28 +97,6 @@ export default class SecureBackupPanel extends React.PureComponent<{}, IState> { this.loadBackupStatus(); }; - private async checkKeyBackupStatus(): Promise { - this.getUpdatedDiagnostics(); - try { - const keyBackupResult = await MatrixClientPeg.safeGet().checkKeyBackup(); - this.setState({ - loading: false, - error: false, - backupInfo: keyBackupResult?.backupInfo ?? null, - backupSigStatus: keyBackupResult?.trustInfo ?? null, - }); - } catch (e) { - logger.log("Unable to fetch check backup status", e); - if (this.unmounted) return; - this.setState({ - loading: false, - error: true, - backupInfo: null, - backupSigStatus: null, - }); - } - } - private async loadBackupStatus(): Promise { this.setState({ loading: true }); this.getUpdatedDiagnostics(); diff --git a/test/components/views/settings/SecureBackupPanel-test.tsx b/test/components/views/settings/SecureBackupPanel-test.tsx index 47d5d5c5c4..d5b28981f2 100644 --- a/test/components/views/settings/SecureBackupPanel-test.tsx +++ b/test/components/views/settings/SecureBackupPanel-test.tsx @@ -46,19 +46,17 @@ describe("", () => { const getComponent = () => render(); beforeEach(() => { - client.checkKeyBackup.mockResolvedValue({ - backupInfo: { - version: "1", - algorithm: "test", - auth_data: { - public_key: "1234", - }, - }, - trustInfo: { - usable: false, - sigs: [], + client.getKeyBackupVersion.mockResolvedValue({ + version: "1", + algorithm: "test", + auth_data: { + public_key: "1234", }, }); + client.isKeyBackupTrusted.mockResolvedValue({ + usable: false, + sigs: [], + }); mocked(client.secretStorage.hasKey).mockClear().mockResolvedValue(false); client.deleteKeyBackupVersion.mockClear().mockResolvedValue(); @@ -75,14 +73,21 @@ describe("", () => { expect(screen.queryByRole("progressbar")).not.toBeInTheDocument(); }); - it("handles null backup info", async () => { - // checkKeyBackup can fail and return null for various reasons - client.checkKeyBackup.mockResolvedValue(null); - getComponent(); - // flush checkKeyBackup promise - await flushPromises(); + it("handles error fetching backup", async () => { + // getKeyBackupVersion can fail for various reasons + client.getKeyBackupVersion.mockImplementation(async () => { + throw new Error("beep beep"); + }); + const renderResult = getComponent(); + await renderResult.findByText("Unable to load key backup status"); + expect(renderResult.container).toMatchSnapshot(); + }); - // no backup info + it("handles absence of backup", async () => { + client.getKeyBackupVersion.mockResolvedValue(null); + getComponent(); + // flush getKeyBackupVersion promise + await flushPromises(); expect(screen.getByText("Back up your keys before signing out to avoid losing them.")).toBeInTheDocument(); }); @@ -124,18 +129,12 @@ describe("", () => { }); it("deletes backup after confirmation", async () => { - client.checkKeyBackup + client.getKeyBackupVersion .mockResolvedValueOnce({ - backupInfo: { - version: "1", - algorithm: "test", - auth_data: { - public_key: "1234", - }, - }, - trustInfo: { - usable: false, - sigs: [], + version: "1", + algorithm: "test", + auth_data: { + public_key: "1234", }, }) .mockResolvedValue(null); diff --git a/test/components/views/settings/__snapshots__/SecureBackupPanel-test.tsx.snap b/test/components/views/settings/__snapshots__/SecureBackupPanel-test.tsx.snap index 9b51ca323f..dc00cd38e4 100644 --- a/test/components/views/settings/__snapshots__/SecureBackupPanel-test.tsx.snap +++ b/test/components/views/settings/__snapshots__/SecureBackupPanel-test.tsx.snap @@ -1,5 +1,70 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[` handles error fetching backup 1`] = ` +
+
+ Back up your encryption keys with your account data in case you lose access to your sessions. Your keys will be secured with a unique Security Key. +
+
+ Unable to load key backup status +
+
+ + Advanced + + + + + + + + + + + + + + + + + + +
+ Backup key stored: + + not stored +
+ Backup key cached: + + not found locally + +
+ Secret storage public key: + + not found +
+ Secret storage: + + not ready +
+
+
+`; + exports[` suggests connecting session to key backup when backup exists 1`] = `
", () => { ...mockClientMethodsCrypto(), getRooms: jest.fn().mockReturnValue([]), getIgnoredUsers: jest.fn(), + getKeyBackupVersion: jest.fn(), }); const getComponent = () => ( diff --git a/test/test-utils/client.ts b/test/test-utils/client.ts index f0eba703ac..39d01b7ce4 100644 --- a/test/test-utils/client.ts +++ b/test/test-utils/client.ts @@ -152,7 +152,6 @@ export const mockClientMethodsCrypto = (): Partial< isKeyBackupKeyStored: jest.fn(), getCrossSigningCacheCallbacks: jest.fn().mockReturnValue({ getCrossSigningKeyCache: jest.fn() }), getStoredCrossSigningForUser: jest.fn(), - checkKeyBackup: jest.fn().mockReturnValue({}), secretStorage: { hasKey: jest.fn() }, getCrypto: jest.fn().mockReturnValue({ getUserDeviceInfo: jest.fn(),