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
matrix-org · Sep 21, 2023 · c879882 · c879882
1 parent c6fec9b
commit c879882
Show file tree

Hide file tree

Showing 5 changed files with 94 additions and 52 deletions.
diff --git 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<void> {
-        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<void> {
         this.setState({ loading: true });
         this.getUpdatedDiagnostics();

diff --git a/test/components/views/settings/SecureBackupPanel-test.tsx b/test/components/views/settings/SecureBackupPanel-test.tsx
@@ -46,19 +46,17 @@ describe("<SecureBackupPanel />", () => {
     const getComponent = () => render(<SecureBackupPanel />);
 
     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("<SecureBackupPanel />", () => {
         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);
+    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();
+    });
+
+    it("handles absence of backup", async () => {
+        client.getKeyBackupVersion.mockResolvedValue(null);
         getComponent();
-        // flush checkKeyBackup promise
+        // flush getKeyBackupVersion promise
         await flushPromises();
-
-        // no backup info
         expect(screen.getByText("Back up your keys before signing out to avoid losing them.")).toBeInTheDocument();
     });
 
@@ -124,18 +129,12 @@ describe("<SecureBackupPanel />", () => {
     });
 
     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
@@ -1,5 +1,70 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`<SecureBackupPanel /> handles error fetching backup 1`] = `
+<div>
+  <div
+    class="mx_SettingsSubsection_text"
+  >
+    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.
+  </div>
+  <div
+    class="mx_SettingsSubsection_text"
+  >
+    Unable to load key backup status
+  </div>
+  <details>
+    <summary>
+      Advanced
+    </summary>
+    <table
+      class="mx_SecureBackupPanel_statusList"
+    >
+      <tr>
+        <th
+          scope="row"
+        >
+          Backup key stored:
+        </th>
+        <td>
+          not stored
+        </td>
+      </tr>
+      <tr>
+        <th
+          scope="row"
+        >
+          Backup key cached:
+        </th>
+        <td>
+          not found locally
+          
+        </td>
+      </tr>
+      <tr>
+        <th
+          scope="row"
+        >
+          Secret storage public key:
+        </th>
+        <td>
+          not found
+        </td>
+      </tr>
+      <tr>
+        <th
+          scope="row"
+        >
+          Secret storage:
+        </th>
+        <td>
+          not ready
+        </td>
+      </tr>
+    </table>
+  </details>
+</div>
+`;
+
 exports[`<SecureBackupPanel /> suggests connecting session to key backup when backup exists 1`] = `
 <div>
   <div

diff --git a/test/components/views/settings/tabs/user/SecurityUserSettingsTab-test.tsx b/test/components/views/settings/tabs/user/SecurityUserSettingsTab-test.tsx
@@ -41,6 +41,7 @@ describe("<SecurityUserSettingsTab />", () => {
         ...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
@@ -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(),