Set up key backup using non-deprecated APIs (2nd take)

playwright/e2e/crypto/backups.spec.ts

@@ -0,0 +1,86 @@
+/*
+Copyright 2023 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import { type Page } from "@playwright/test";
+
+import { test, expect } from "../../element-web-test";
+
+async function expectBackupVersionToBe(page: Page, version: string) {
+    await expect(page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(5) td")).toHaveText(
+        version + " (Algorithm: m.megolm_backup.v1.curve25519-aes-sha2)",
+    );
+
+    await expect(page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(6) td")).toHaveText(version);
+}
+
+test.describe("Backups", () => {
+    test.use({
+        displayName: "Hanako",
+    });
+
+    test("Create, delete and recreate a keys backup", async ({ page, user, app }, workerInfo) => {
+        // Create a backup
+        const tab = await app.settings.openUserSettings("Security & Privacy");
+
+        await expect(tab.getByRole("heading", { name: "Secure Backup" })).toBeVisible();
+        await tab.getByRole("button", { name: "Set up", exact: true }).click();
+        const dialog = await app.getDialogByTitle("Set up Secure Backup", 60000);
+        await dialog.getByRole("button", { name: "Continue", exact: true }).click();
+        await expect(dialog.getByRole("heading", { name: "Save your Security Key" })).toBeVisible();
+        await dialog.getByRole("button", { name: "Copy", exact: true }).click();
+        const securityKey = await app.getClipboard();
+        await dialog.getByRole("button", { name: "Continue", exact: true }).click();
+        await expect(dialog.getByRole("heading", { name: "Secure Backup successful" })).toBeVisible();
+        await dialog.getByRole("button", { name: "Done", exact: true }).click();
+
+        // Open the settings again
+        await app.settings.openUserSettings("Security & Privacy");
+        await expect(tab.getByRole("heading", { name: "Secure Backup" })).toBeVisible();
+
+        // expand the advanced section to see the active version in the reports
+        await page
+            .locator(".mx_Dialog .mx_SettingsSubsection_content details .mx_SecureBackupPanel_advanced")
+            .locator("..")
+            .click();
+
+        await expectBackupVersionToBe(page, "1");
+
+        // Delete it
+        await tab.getByRole("button", { name: "Delete Backup", exact: true }).click();
+        await dialog.getByTestId("dialog-primary-button").click(); // Click "Delete Backup"
+
+        // Create another
+        await tab.getByRole("button", { name: "Set up", exact: true }).click();
+        dialog.getByLabel("Security Key").fill(securityKey);
+        await dialog.getByRole("button", { name: "Continue", exact: true }).click();
+        await expect(dialog.getByRole("heading", { name: "Success!" })).toBeVisible();
+        await dialog.getByRole("button", { name: "OK", exact: true }).click();
+
+        // Open the settings again
+        await app.settings.openUserSettings("Security & Privacy");
+        await expect(tab.getByRole("heading", { name: "Secure Backup" })).toBeVisible();
+
+        // expand the advanced section to see the active version in the reports
+        await page
+            .locator(".mx_Dialog .mx_SettingsSubsection_content details .mx_SecureBackupPanel_advanced")
+            .locator("..")
+            .click();
+
+        await expectBackupVersionToBe(page, "2");
+
+        await page.pause();
+    });
+});

playwright/element-web-test.ts

@@ -293,3 +293,7 @@ export const expect = baseExpect.extend({
         return { pass: true, message: () => "", name: "toMatchScreenshot" };
     },
 });
+
+test.use({
+    permissions: ["clipboard-read"],
+});

playwright/pages/ElementAppPage.ts

@@ -51,6 +51,19 @@ export class ElementAppPage {
         return this.settings.closeDialog();
     }
 
+    public async getClipboard(): Promise<string> {
+        return await this.page.evaluate(() => navigator.clipboard.readText());
+    }
+
+    /**
+     * Find an open dialog by its title
+     */
+    public async getDialogByTitle(title: string, timeout = 5000): Promise<Locator> {
+        const dialog = this.page.locator(".mx_Dialog");
+        await dialog.getByRole("heading", { name: title }).waitFor({ timeout });
+        return dialog;
+    }
+
     /**
      * Opens the given room by name. The room must be visible in the
      * room list, but the room list may be folded horizontally, and the

src/SecurityManager.ts

@@ -347,7 +347,7 @@ export async function accessSecretStorage(func = async (): Promise<void> => {},
 }
 
 /** Helper for {@link #accessSecretStorage} */
-export async function doAccessSecretStorage(func = async (): Promise<void> => {}, forceReset = false): Promise<void> {
+async function doAccessSecretStorage(func: () => Promise<void>, forceReset: boolean): Promise<void> {
     try {
         const cli = MatrixClientPeg.safeGet();
         if (!(await cli.hasSecretStorageKey()) || forceReset) {
@@ -378,7 +378,12 @@ export async function doAccessSecretStorage(func = async (): Promise<void> => {}
                 throw new Error("Secret storage creation canceled");
             }
         } else {
-            await cli.bootstrapCrossSigning({
+            const crypto = cli.getCrypto();
+            if (!crypto) {
+                throw new Error("End-to-end encryption is disabled - unable to access secret storage.");
+            }
+
+            await crypto.bootstrapCrossSigning({
                 authUploadDeviceSigningKeys: async (makeRequest): Promise<void> => {
                     const { finished } = Modal.createDialog(InteractiveAuthDialog, {
                         title: _t("encryption|bootstrap_title"),
@@ -391,7 +396,7 @@ export async function doAccessSecretStorage(func = async (): Promise<void> => {}
                     }
                 },
             });
-            await cli.bootstrapSecretStorage({
+            await crypto.bootstrapSecretStorage({
                 getKeyBackupPassphrase: promptForBackupPassphrase,
             });
 

src/async-components/views/dialogs/security/CreateKeyBackupDialog.tsx

@@ -17,11 +17,10 @@ limitations under the License.
 
 import React from "react";
 import { logger } from "matrix-js-sdk/src/logger";
-import { IKeyBackupInfo } from "matrix-js-sdk/src/crypto/keybackup";
 
 import { MatrixClientPeg } from "../../../../MatrixClientPeg";
 import { _t } from "../../../../languageHandler";
-import { accessSecretStorage } from "../../../../SecurityManager";
+import { accessSecretStorage, withSecretStorageKeyCache } from "../../../../SecurityManager";
 import Spinner from "../../../../components/views/elements/Spinner";
 import BaseDialog from "../../../../components/views/dialogs/BaseDialog";
 import DialogButtons from "../../../../components/views/elements/DialogButtons";
@@ -75,24 +74,30 @@ export default class CreateKeyBackupDialog extends React.PureComponent<IProps, I
         this.setState({
             error: undefined,
         });
-        let info: IKeyBackupInfo | undefined;
         const cli = MatrixClientPeg.safeGet();
         try {
-            await accessSecretStorage(async (): Promise<void> => {
-                // `accessSecretStorage` will have bootstrapped secret storage if necessary, so we can now
-                // set up key backup.
-                //
-                // XXX: `bootstrapSecretStorage` also sets up key backup as a side effect, so there is a 90% chance
-                // this is actually redundant.
-                //
-                // The only time it would *not* be redundant would be if, for some reason, we had working 4S but no
-                // working key backup. (For example, if the user clicked "Delete Backup".)
-                info = await cli.prepareKeyBackupVersion(null /* random key */, {
-                    secureSecretStorage: true,
+            // Check if 4S already set up
+            const secretStorageAlreadySetup = await cli.hasSecretStorageKey();
+
+            if (!secretStorageAlreadySetup) {
+                // bootstrap secret storage, that will also create a backup version
+                await accessSecretStorage(async (): Promise<void> => {
+                    // do nothing, all is now setup correctly
                 });
-                info = await cli.createKeyBackupVersion(info);
-            });
-            await cli.scheduleAllGroupSessionsForBackup();
+            } else {
+                // Secret storage exists, we need to ensure that we can write to it before
+                // we create a new backup version. It ensures that we can write to it and keep it in sync.
+                await withSecretStorageKeyCache(async () => {
+                    // this is the secret that will need to be updated, ensure that we can access it.
+                    // This will ask the user to enter their passphrase/key.
+                    await cli.secretStorage.get("m.megolm_backup.v1");
+                    // if we get here, we can access the secret storage, so we can create a backup version.
+                    // We don't reset if we can't access the secret storage, as the secret storage will then
+                    // have an outdated secret for backup that future sessions will not be able to use.
+                    await cli.getCrypto()?.resetKeyBackup();
+                });
+            }
+
             this.setState({
                 phase: Phase.Done,
             });
@@ -102,9 +107,6 @@ export default class CreateKeyBackupDialog extends React.PureComponent<IProps, I
             // delete the version, disable backup, or do nothing?  If we just
             // disable without deleting, we'll enable on next app reload since
             // it is trusted.
-            if (info?.version) {
-                cli.deleteKeyBackupVersion(info.version);
-            }
             this.setState({
                 error: true,
             });

test/SecurityManager-test.ts

@@ -0,0 +1,62 @@
+/*
+Copyright 2023 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import { mocked } from "jest-mock";
+import { CryptoApi } from "matrix-js-sdk/src/crypto-api";
+
+import { accessSecretStorage } from "../src/SecurityManager";
+import { filterConsole, stubClient } from "./test-utils";
+
+describe("SecurityManager", () => {
+    describe("accessSecretStorage", () => {
+        filterConsole("Not setting dehydration key: no SSSS key found");
+
+        it("runs the function passed in", async () => {
+            // Given a client
+            const crypto = {
+                bootstrapCrossSigning: () => {},
+                bootstrapSecretStorage: () => {},
+            } as unknown as CryptoApi;
+            const client = stubClient();
+            mocked(client.hasSecretStorageKey).mockResolvedValue(true);
+            mocked(client.getCrypto).mockReturnValue(crypto);
+
+            // When I run accessSecretStorage
+            const func = jest.fn();
+            await accessSecretStorage(func);
+
+            // Then we call the passed-in function
+            expect(func).toHaveBeenCalledTimes(1);
+        });
+
+        describe("expecting errors", () => {
+            filterConsole("End-to-end encryption is disabled - unable to access secret storage");
+
+            it("throws if crypto is unavailable", async () => {
+                // Given a client with no crypto
+                const client = stubClient();
+                mocked(client.hasSecretStorageKey).mockResolvedValue(true);
+                mocked(client.getCrypto).mockReturnValue(undefined);
+
+                // When I run accessSecretStorage
+                // Then we throw an error
+                await expect(async () => {
+                    await accessSecretStorage(jest.fn());
+                }).rejects.toThrow("End-to-end encryption is disabled - unable to access secret storage");
+            });
+        });
+    });
+});

test/components/views/dialogs/security/CreateKeyBackupDialog-test.tsx

@@ -39,9 +39,12 @@ describe("CreateKeyBackupDialog", () => {
         expect(asFragment()).toMatchSnapshot();
     });
 
-    it("should display the error message when backup creation failed", async () => {
+    it("should display an error message when backup creation failed", async () => {
         const matrixClient = createTestClient();
-        mocked(matrixClient.scheduleAllGroupSessionsForBackup).mockRejectedValue("my error");
+        mocked(matrixClient.hasSecretStorageKey).mockResolvedValue(true);
+        mocked(matrixClient.getCrypto()!.resetKeyBackup).mockImplementation(() => {
+            throw new Error("failed");
+        });
         MatrixClientPeg.safeGet = MatrixClientPeg.get = () => matrixClient;
 
         const { asFragment } = render(<CreateKeyBackupDialog onFinished={jest.fn()} />);
@@ -51,6 +54,18 @@ describe("CreateKeyBackupDialog", () => {
         expect(asFragment()).toMatchSnapshot();
     });
 
+    it("should display an error message when there is no Crypto available", async () => {
+        const matrixClient = createTestClient();
+        mocked(matrixClient.hasSecretStorageKey).mockResolvedValue(true);
+        mocked(matrixClient.getCrypto).mockReturnValue(undefined);
+        MatrixClientPeg.safeGet = MatrixClientPeg.get = () => matrixClient;
+
+        render(<CreateKeyBackupDialog onFinished={jest.fn()} />);
+
+        // Check if the error message is displayed
+        await waitFor(() => expect(screen.getByText("Unable to create key backup")).toBeDefined());
+    });
+
     it("should display the success dialog when the key backup is finished", async () => {
         const onFinished = jest.fn();
         const { asFragment } = render(<CreateKeyBackupDialog onFinished={onFinished} />);

test/components/views/dialogs/security/__snapshots__/CreateKeyBackupDialog-test.tsx.snap

@@ -1,5 +1,65 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`CreateKeyBackupDialog should display an error message when backup creation failed 1`] = `
+<DocumentFragment>
+  <div
+    data-focus-guard="true"
+    style="width: 1px; height: 0px; padding: 0px; overflow: hidden; position: fixed; top: 1px; left: 1px;"
+    tabindex="0"
+  />
+  <div
+    aria-labelledby="mx_BaseDialog_title"
+    class="mx_CreateKeyBackupDialog mx_Dialog_fixedWidth"
+    data-focus-lock-disabled="false"
+    role="dialog"
+  >
+    <div
+      class="mx_Dialog_header"
+    >
+      <h2
+        class="mx_Heading_h3 mx_Dialog_title"
+        id="mx_BaseDialog_title"
+      >
+        Starting backup…
+      </h2>
+    </div>
+    <div>
+      <div>
+        <p>
+          Unable to create key backup
+        </p>
+        <div
+          class="mx_Dialog_buttons"
+        >
+          <span
+            class="mx_Dialog_buttons_row"
+          >
+            <button
+              data-testid="dialog-cancel-button"
+              type="button"
+            >
+              Cancel
+            </button>
+            <button
+              class="mx_Dialog_primary"
+              data-testid="dialog-primary-button"
+              type="button"
+            >
+              Retry
+            </button>
+          </span>
+        </div>
+      </div>
+    </div>
+  </div>
+  <div
+    data-focus-guard="true"
+    style="width: 1px; height: 0px; padding: 0px; overflow: hidden; position: fixed; top: 1px; left: 1px;"
+    tabindex="0"
+  />
+</DocumentFragment>
+`;
+
 exports[`CreateKeyBackupDialog should display the error message when backup creation failed 1`] = `
 <DocumentFragment>
   <div

test/test-utils/test-utils.ts

@@ -132,6 +132,7 @@ export function createTestClient(): MatrixClient {
             getUserDeviceInfo: jest.fn(),
             getUserVerificationStatus: jest.fn(),
             getDeviceVerificationStatus: jest.fn(),
+            resetKeyBackup: jest.fn(),
         }),
 
         getPushActionsForEvent: jest.fn(),