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 4 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.

21 changes: 19 additions & 2 deletions playwright/e2e/crypto/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,29 @@ 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.
*/
export async function checkDeviceIsConnectedKeyBackup(page: Page) {
export async function checkDeviceIsConnectedKeyBackup(page: Page, expectedBackupVersion: string): 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
const summary = page.locator(".mx_SecureBackupPanel_advanced");
await summary.locator("..").click();
BillCarsonFr marked this conversation as resolved.
Show resolved Hide resolved

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

const serverVersion = await page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(5) td").textContent();
expect(serverVersion.trim()).toBe(expectedBackupVersion + " (Algorithm: m.megolm_backup.v1.curve25519-aes-sha2)");
BillCarsonFr marked this conversation as resolved.
Show resolved Hide resolved

const activeVersion = await page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(6) td").textContent();
expect(activeVersion.trim()).toBe(expectedBackupVersion);
BillCarsonFr marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down
14 changes: 9 additions & 5 deletions playwright/e2e/crypto/verification.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import { Bot } from "../../pages/bot";

test.describe("Device verification", () => {
let aliceBotClient: Bot;
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
Expand All @@ -49,7 +50,10 @@ test.describe("Device verification", () => {
bootstrapSecretStorage: true,
});
aliceBotClient.setCredentials(credentials);
await aliceBotClient.prepareClient();
const mxClientHandle = await aliceBotClient.prepareClient();
expectedBackupVersion = await mxClientHandle.evaluate(async (mxClient) => {
return await mxClient.getCrypto()!.getActiveSessionBackupVersion();
});

await page.waitForTimeout(20000);
});
Expand Down Expand Up @@ -87,7 +91,7 @@ test.describe("Device verification", () => {
await checkDeviceIsCrossSigned(app);

// Check that the current device is connected to key backup
await checkDeviceIsConnectedKeyBackup(page);
await checkDeviceIsConnectedKeyBackup(page, expectedBackupVersion);
});

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

// Check that the current device is connected to key backup
await checkDeviceIsConnectedKeyBackup(page);
await checkDeviceIsConnectedKeyBackup(page, expectedBackupVersion);
});

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

// Check that the current device is connected to key backup
await checkDeviceIsConnectedKeyBackup(page);
await checkDeviceIsConnectedKeyBackup(page, expectedBackupVersion);
});

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

// Check that the current device is connected to key backup
await checkDeviceIsConnectedKeyBackup(page);
await checkDeviceIsConnectedKeyBackup(page, expectedBackupVersion);
});

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