Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

e2e test: Check key backup with js-sdk api instead of relying of Security & Privacy tab #29066

Merged
merged 2 commits into from
Jan 24, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
12 changes: 5 additions & 7 deletions playwright/e2e/crypto/device-verification.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ test.describe("Device verification", { tag: "@no-webkit" }, () => {

// Check that the current device is connected to key backup
// 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);
Copy link
Member Author

Choose a reason for hiding this comment

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

This is still not working

Copy link
Member

Choose a reason for hiding this comment

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

just this test isn't working?

Copy link
Member Author

@florianduros florianduros Jan 22, 2025

Choose a reason for hiding this comment

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

The others are working if we check that the key backup is in cache but this one is failing (so I kept the false attribute)

// as we need to wait for the secret gossiping to happen.
await checkDeviceIsConnectedKeyBackup(app, expectedBackupVersion, false);
});

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

// Check that the current device is connected to key backup
// 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);
await checkDeviceIsConnectedKeyBackup(app, expectedBackupVersion, true);
});

test("Verify device with Security Phrase during login", async ({ page, app, credentials, homeserver }) => {
Expand All @@ -135,7 +133,7 @@ test.describe("Device verification", { tag: "@no-webkit" }, () => {

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

test("Verify device with Security Key during login", async ({ page, app, credentials, homeserver }) => {
Expand All @@ -158,7 +156,7 @@ test.describe("Device verification", { tag: "@no-webkit" }, () => {

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

test("Handle incoming verification request with SAS", async ({ page, credentials, homeserver, toasts }) => {
Expand Down
46 changes: 32 additions & 14 deletions playwright/e2e/crypto/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,12 +139,12 @@ export async function checkDeviceIsCrossSigned(app: ElementAppPage): Promise<voi
* 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 app - app page
Copy link
Member

Choose a reason for hiding this comment

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

So that means that the check on the page is not up-to-date, but the check using the API is ok?
Doesn't that mean that there is a refresh bug on the UI?

Copy link
Member Author

@florianduros florianduros Jan 24, 2025

Choose a reason for hiding this comment

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

This UI section will be removed in #26468. The e2e tests were using this section to check if the key storage was in the expected state.

  1. It was not a good practice, we should not look at the ui for this kind of verification (we are not testing this part of the ui)
  2. The UI will be removed so we need to find another way

florianduros marked this conversation as resolved.
Show resolved Hide resolved
* @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.
florianduros marked this conversation as resolved.
Show resolved Hide resolved
*/
export async function checkDeviceIsConnectedKeyBackup(
page: Page,
app: ElementAppPage,
expectedBackupVersion: string,
checkBackupKeyInCache: boolean,
): Promise<void> {
Expand All @@ -155,23 +155,41 @@ export async function checkDeviceIsConnectedKeyBackup(
);
}

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();
const backupData = await app.client.evaluate(async (client: MatrixClient) => {
const crypto = client.getCrypto();
if (!crypto) return;

// expand the advanced section to see the active version in the reports
await page.locator(".mx_SecureBackupPanel_advanced").locator("..").click();
const backupInfo = await crypto.getKeyBackupInfo();
const backupKeyStored = Boolean(await client.isKeyBackupKeyStored());
florianduros marked this conversation as resolved.
Show resolved Hide resolved
const backupKeyFromCache = await crypto.getSessionBackupPrivateKey();
florianduros marked this conversation as resolved.
Show resolved Hide resolved
const backupKeyCached = Boolean(backupKeyFromCache);
const backupKeyWellFormed = backupKeyFromCache instanceof Uint8Array;
florianduros marked this conversation as resolved.
Show resolved Hide resolved
const activeBackupVersion = await crypto.getActiveSessionBackupVersion();

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

if (!backupData) {
throw new Error("Crypo module is not available");
florianduros marked this conversation as resolved.
Show resolved Hide resolved
}

await expect(page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(5) td")).toHaveText(
expectedBackupVersion + " (Algorithm: m.megolm_backup.v1.curve25519-aes-sha2)",
);
const { backupInfo, backupKeyStored, backupKeyCached, backupKeyWellFormed, activeBackupVersion } = backupData;

// We have a key backup
expect(backupInfo).toBeDefined();
// The key backup version is as expected
expect(backupInfo.version).toBe(expectedBackupVersion);
// The active backup version is as expected
expect(activeBackupVersion).toBe(expectedBackupVersion);
// The backup key is stored in 4S
expect(backupKeyStored).toBe(true);

await expect(page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(6) td")).toHaveText(expectedBackupVersion);
if (checkBackupKeyInCache) {
// The backup key is available locally
expect(backupKeyCached).toBe(true);
// The backup key is well-formed
expect(backupKeyWellFormed).toBe(true);
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,7 @@ test.describe("Recovery section in Encryption tab", () => {

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

test(
Expand Down Expand Up @@ -115,9 +114,8 @@ test.describe("Recovery section in Encryption tab", () => {
// The recovery key is now set up and the user can change it
await expect(dialog.getByRole("button", { name: "Change recovery key" })).toBeVisible();

await app.closeDialog();
// Check that the current device is connected to key backup and the backup version is the expected one
await checkDeviceIsConnectedKeyBackup(page, "1", true);
await checkDeviceIsConnectedKeyBackup(app, "1", true);
});

// Test what happens if the cross-signing secrets are in secret storage but are not cached in the local DB.
Expand Down Expand Up @@ -149,8 +147,7 @@ test.describe("Recovery section in Encryption tab", () => {

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