Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Revert "Set up key backup using non-deprecated APIs (#12005)" #12102

Merged
merged 6 commits into from
Jan 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 0 additions & 57 deletions playwright/e2e/crypto/backups.spec.ts

This file was deleted.

27 changes: 25 additions & 2 deletions playwright/e2e/crypto/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,35 @@ export async function checkDeviceIsCrossSigned(app: ElementAppPage): Promise<voi
}

/**
* Check that the current device is connected to the key backup.
* Check that the current device is connected to the expected key backup.
* Also checks that the decryption key is known and cached locally.
*
* @param page - the page to check
* @param expectedBackupVersion - the version of the backup we expect to be connected to.
* @param checkBackupKeyInCache - whether to check that the backup key is cached locally.
*/
export async function checkDeviceIsConnectedKeyBackup(page: Page) {
export async function checkDeviceIsConnectedKeyBackup(
page: Page,
expectedBackupVersion: string,
checkBackupKeyInCache: boolean,
): Promise<void> {
await page.getByRole("button", { name: "User menu" }).click();
await page.locator(".mx_UserMenu_contextMenu").getByRole("menuitem", { name: "Security & Privacy" }).click();
await expect(page.locator(".mx_Dialog").getByRole("button", { name: "Restore from Backup" })).toBeVisible();

// expand the advanced section to see the active version in the reports
await page.locator(".mx_SecureBackupPanel_advanced").locator("..").click();

if (checkBackupKeyInCache) {
const cacheDecryptionKeyStatusElement = page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(2) td");
await expect(cacheDecryptionKeyStatusElement).toHaveText("cached locally, well formed");
}

await expect(page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(5) td")).toHaveText(
expectedBackupVersion + " (Algorithm: m.megolm_backup.v1.curve25519-aes-sha2)",
);

await expect(page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(6) td")).toHaveText(expectedBackupVersion);
}

/**
Expand Down
23 changes: 18 additions & 5 deletions playwright/e2e/crypto/verification.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ import { Bot } from "../../pages/bot";
test.describe("Device verification", () => {
let aliceBotClient: Bot;

/** The backup version that was set up by the bot client. */
let expectedBackupVersion: string;
richvdh marked this conversation as resolved.
Show resolved Hide resolved

test.beforeEach(async ({ page, homeserver, credentials }) => {
// Visit the login page of the app, to load the matrix sdk
await page.goto("/#/login");
Expand All @@ -49,9 +52,13 @@ test.describe("Device verification", () => {
bootstrapSecretStorage: true,
});
aliceBotClient.setCredentials(credentials);
await aliceBotClient.prepareClient();
const mxClientHandle = await aliceBotClient.prepareClient();

await page.waitForTimeout(20000);

expectedBackupVersion = await mxClientHandle.evaluate(async (mxClient) => {
return await mxClient.getCrypto()!.getActiveSessionBackupVersion();
});
});

// Click the "Verify with another device" button, and have the bot client auto-accept it.
Expand Down Expand Up @@ -87,7 +94,9 @@ test.describe("Device verification", () => {
await checkDeviceIsCrossSigned(app);

// Check that the current device is connected to key backup
await checkDeviceIsConnectedKeyBackup(page);
// For now we don't check that the backup key is in cache because it's a bit flaky,
// as we need to wait for the secret gossiping to happen and the settings dialog doesn't refresh automatically.
await checkDeviceIsConnectedKeyBackup(page, expectedBackupVersion, false);
});

test("Verify device with QR code during login", async ({ page, app, credentials, homeserver }) => {
Expand Down Expand Up @@ -130,7 +139,9 @@ test.describe("Device verification", () => {
await checkDeviceIsCrossSigned(app);

// Check that the current device is connected to key backup
await checkDeviceIsConnectedKeyBackup(page);
// For now we don't check that the backup key is in cache because it's a bit flaky,
// as we need to wait for the secret gossiping to happen and the settings dialog doesn't refresh automatically.
await checkDeviceIsConnectedKeyBackup(page, expectedBackupVersion, false);
});

test("Verify device with Security Phrase during login", async ({ page, app, credentials, homeserver }) => {
Expand All @@ -150,7 +161,8 @@ test.describe("Device verification", () => {
await checkDeviceIsCrossSigned(app);

// Check that the current device is connected to key backup
await checkDeviceIsConnectedKeyBackup(page);
// The backup decryption key should be in cache also, as we got it directly from the 4S
await checkDeviceIsConnectedKeyBackup(page, expectedBackupVersion, true);
});

test("Verify device with Security Key during login", async ({ page, app, credentials, homeserver }) => {
Expand All @@ -172,7 +184,8 @@ test.describe("Device verification", () => {
await checkDeviceIsCrossSigned(app);

// Check that the current device is connected to key backup
await checkDeviceIsConnectedKeyBackup(page);
// The backup decryption key should be in cache also, as we got it directly from the 4S
await checkDeviceIsConnectedKeyBackup(page, expectedBackupVersion, true);
});

test("Handle incoming verification request with SAS", async ({ page, credentials, homeserver, toasts }) => {
Expand Down
4 changes: 0 additions & 4 deletions playwright/element-web-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,3 @@ export const expect = baseExpect.extend({
return { pass: true, message: () => "", name: "toMatchScreenshot" };
},
});

test.use({
permissions: ["clipboard-read"],
});
13 changes: 0 additions & 13 deletions playwright/pages/ElementAppPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,19 +51,6 @@ export class ElementAppPage {
return this.settings.closeDialog();
}

public async getClipboard(): Promise<string> {
return await this.page.evaluate(() => navigator.clipboard.readText());
}
Comment on lines -54 to -56
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this going away?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh we also have getClipboardText - fun


/**
* 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
Expand Down
25 changes: 5 additions & 20 deletions src/SecurityManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -341,22 +341,13 @@ export async function withSecretStorageKeyCache<T>(func: () => Promise<T>): Prom
* @param {Function} [func] An operation to perform once secret storage has been
* bootstrapped. Optional.
* @param {bool} [forceReset] Reset secret storage even if it's already set up
* @param {bool} [setupNewKeyBackup] Reset secret storage even if it's already set up
*/
export async function accessSecretStorage(
func = async (): Promise<void> => {},
forceReset = false,
setupNewKeyBackup = true,
): Promise<void> {
await withSecretStorageKeyCache(() => doAccessSecretStorage(func, forceReset, setupNewKeyBackup));
export async function accessSecretStorage(func = async (): Promise<void> => {}, forceReset = false): Promise<void> {
await withSecretStorageKeyCache(() => doAccessSecretStorage(func, forceReset));
}

/** Helper for {@link #accessSecretStorage} */
async function doAccessSecretStorage(
func: () => Promise<void>,
forceReset: boolean,
setupNewKeyBackup: boolean,
): Promise<void> {
export async function doAccessSecretStorage(func = async (): Promise<void> => {}, forceReset = false): Promise<void> {
try {
const cli = MatrixClientPeg.safeGet();
if (!(await cli.hasSecretStorageKey()) || forceReset) {
Expand Down Expand Up @@ -387,12 +378,7 @@ async function doAccessSecretStorage(
throw new Error("Secret storage creation canceled");
}
} else {
const crypto = cli.getCrypto();
if (!crypto) {
throw new Error("End-to-end encryption is disabled - unable to access secret storage.");
}

await crypto.bootstrapCrossSigning({
await cli.bootstrapCrossSigning({
authUploadDeviceSigningKeys: async (makeRequest): Promise<void> => {
const { finished } = Modal.createDialog(InteractiveAuthDialog, {
title: _t("encryption|bootstrap_title"),
Expand All @@ -405,9 +391,8 @@ async function doAccessSecretStorage(
}
},
});
await crypto.bootstrapSecretStorage({
await cli.bootstrapSecretStorage({
getKeyBackupPassphrase: promptForBackupPassphrase,
setupNewKeyBackup,
});

const keyId = Object.keys(secretStorageKeys)[0];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ 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";
Expand Down Expand Up @@ -74,25 +75,24 @@ export default class CreateKeyBackupDialog extends React.PureComponent<IProps, I
this.setState({
error: undefined,
});
let info: IKeyBackupInfo | undefined;
const cli = MatrixClientPeg.safeGet();
try {
// We don't want accessSecretStorage to create a backup for us - we
// will create one ourselves in the closure we pass in by calling
// resetKeyBackup.
const setupNewKeyBackup = false;
const forceReset = false;

await accessSecretStorage(
async (): Promise<void> => {
const crypto = cli.getCrypto();
if (!crypto) {
throw new Error("End-to-end encryption is disabled - unable to create backup.");
}
await crypto.resetKeyBackup();
},
forceReset,
setupNewKeyBackup,
);
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,
});
info = await cli.createKeyBackupVersion(info);
});
await cli.scheduleAllGroupSessionsForBackup();
this.setState({
phase: Phase.Done,
});
Expand All @@ -102,6 +102,9 @@ 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,
});
Expand Down
62 changes: 0 additions & 62 deletions test/SecurityManager-test.ts

This file was deleted.

Loading
Loading