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

Commit

Permalink
Revert "Set up key backup using non-deprecated APIs (#12005)" (#12102)
Browse files Browse the repository at this point in the history
(cherry picked from commit 6107069)
  • Loading branch information
BillCarsonFr authored and t3chguy committed Jan 4, 2024
1 parent 46b28d7 commit e072c36
Show file tree
Hide file tree
Showing 11 changed files with 79 additions and 211 deletions.
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;

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 @@ -260,7 +260,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());
}

/**
* 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

0 comments on commit e072c36

Please sign in to comment.