From cd2a7e67d62e4615a92f655c8388bc0fe27ea576 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 16 Apr 2024 13:53:58 +0100 Subject: [PATCH 1/5] Use different messages for UTDs sent before login --- .../views/messages/DecryptionFailureBody.tsx | 30 ++++++++++++++----- src/i18n/strings/en_EN.json | 2 ++ 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/components/views/messages/DecryptionFailureBody.tsx b/src/components/views/messages/DecryptionFailureBody.tsx index d3895e815ff..47f862b83d4 100644 --- a/src/components/views/messages/DecryptionFailureBody.tsx +++ b/src/components/views/messages/DecryptionFailureBody.tsx @@ -1,5 +1,5 @@ /* -Copyright 2022 The Matrix.org Foundation C.I.C. +Copyright 2022-2024 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. @@ -14,23 +14,37 @@ See the License for the specific language governing permissions and limitations under the License. */ -import React, { forwardRef, ForwardRefExoticComponent } from "react"; +import React, { forwardRef, ForwardRefExoticComponent, useContext } from "react"; import { MatrixEvent } from "matrix-js-sdk/src/matrix"; +import { DecryptionFailureCode } from "matrix-js-sdk/src/crypto-api"; import { _t } from "../../../languageHandler"; import { IBodyProps } from "./IBodyProps"; +import { LocalDeviceVerificationStateContext } from "../../../contexts/LocalDeviceVerificationStateContext"; -function getErrorMessage(mxEvent?: MatrixEvent): string { - return mxEvent?.isEncryptedDisabledForUnverifiedDevices - ? _t("timeline|decryption_failure|blocked") - : _t("timeline|decryption_failure|unable_to_decrypt"); +function getErrorMessage(mxEvent: MatrixEvent, isVerified: boolean | undefined): string { + if (mxEvent.isEncryptedDisabledForUnverifiedDevices) return _t("timeline|decryption_failure|blocked"); + switch (mxEvent.decryptionFailureReason) { + case DecryptionFailureCode.HISTORICAL_MESSAGE_NO_KEY_BACKUP: + return _t("timeline|decryption_failure|historical_event_no_key_backup"); + + case DecryptionFailureCode.HISTORICAL_MESSAGE_BACKUP_UNCONFIGURED: + case DecryptionFailureCode.HISTORICAL_MESSAGE_WORKING_BACKUP: + if (isVerified === false) { + return _t("timeline|decryption_failure|historical_event_unverified_device"); + } + // otherwise, use the default. + break; + } + return _t("timeline|decryption_failure|unable_to_decrypt"); } // A placeholder element for messages that could not be decrypted -export const DecryptionFailureBody = forwardRef(({ mxEvent }, ref): JSX.Element => { +export const DecryptionFailureBody = forwardRef(({ mxEvent }, ref): React.JSX.Element => { + const verificationState = useContext(LocalDeviceVerificationStateContext); return (
- {getErrorMessage(mxEvent)} + {getErrorMessage(mxEvent, verificationState)}
); }) as ForwardRefExoticComponent; diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 0f353c820a2..16cdbd6949a 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -3214,6 +3214,8 @@ "creation_summary_room": "%(creator)s created and configured the room.", "decryption_failure": { "blocked": "The sender has blocked you from receiving this message", + "historical_event_no_key_backup": "Historical messages are not available on this device", + "historical_event_unverified_device": "You need to verify this device for access to historical messages", "unable_to_decrypt": "Unable to decrypt message" }, "disambiguated_profile": "%(displayName)s (%(matrixId)s)", From 79bf4000035a7f920f84d386bf09403d75f89392 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 16 Apr 2024 13:54:20 +0100 Subject: [PATCH 2/5] Playwright test for historical events --- playwright/e2e/crypto/crypto.spec.ts | 76 ++++++++++++++++++++++++++-- playwright/e2e/crypto/utils.ts | 42 ++++++++++++--- 2 files changed, 108 insertions(+), 10 deletions(-) diff --git a/playwright/e2e/crypto/crypto.spec.ts b/playwright/e2e/crypto/crypto.spec.ts index 957be587111..19916cbeccf 100644 --- a/playwright/e2e/crypto/crypto.spec.ts +++ b/playwright/e2e/crypto/crypto.spec.ts @@ -15,14 +15,17 @@ limitations under the License. */ import type { Page } from "@playwright/test"; -import { test, expect } from "../../element-web-test"; +import { expect, test } from "../../element-web-test"; import { + copyAndContinue, + createRoom, createSharedRoomWithUser, doTwoWaySasVerification, - copyAndContinue, enableKeyBackup, logIntoElement, logOutOfElement, + sendMessageInCurrentRoom, + verifySession, waitForVerificationRequest, } from "./utils"; import { Bot } from "../../pages/bot"; @@ -453,8 +456,8 @@ test.describe("Cryptography", function () { // no e2e icon await expect(lastTileE2eIcon).not.toBeVisible(); - // It can take up to 10 seconds for the key to be backed up. We don't really have much option other than - // to wait :/ + // Workaround for https://github.com/element-hq/element-web/issues/27267: it can take up to 10 seconds for + // the key to be backed up. await page.waitForTimeout(10000); /* log out, and back in */ @@ -532,4 +535,69 @@ test.describe("Cryptography", function () { ).not.toBeVisible(); }); }); + + test.describe("decryption failure messages", () => { + test("should handle device-relative historical messages", async ({ + homeserver, + page, + app, + credentials, + user, + cryptoBackend, + }) => { + test.skip(cryptoBackend === "legacy", "Not implemented for legacy crypto"); + test.setTimeout(60000); + + // Start with a logged-in session, without key backup, and send a message. + await createRoom(page, "Test room", true); + await sendMessageInCurrentRoom(page, "test test"); + + // Log out, discarding the key for the sent message. + await logOutOfElement(page, true); + + // Log in again, and see how the message looks. + await logIntoElement(page, homeserver, credentials); + await app.viewRoomByName("Test room"); + const lastTile = page.locator(".mx_EventTile").last(); + await expect(lastTile).toContainText("Historical messages are not available on this device"); + await expect(lastTile.locator(".mx_EventTile_e2eIcon_decryption_failure")).toBeVisible(); + + // Now, we set up key backup, and then send another message. + const secretStorageKey = await enableKeyBackup(app); + await app.viewRoomByName("Test room"); + await sendMessageInCurrentRoom(page, "test2 test2"); + + // Workaround for https://github.com/element-hq/element-web/issues/27267: it can take up to 10 seconds for + // the key to be backed up. + await page.waitForTimeout(10000); + + // Finally, log out again, and back in, skipping verification for now, and see what we see. + await logOutOfElement(page); + await logIntoElement(page, homeserver, credentials); + await page.locator(".mx_AuthPage").getByRole("button", { name: "Skip verification for now" }).click(); + await page.locator(".mx_AuthPage").getByRole("button", { name: "I'll verify later" }).click(); + await app.viewRoomByName("Test room"); + + // There should be two historical events in the timeline + const tiles = await page.locator(".mx_EventTile").all(); + expect(tiles.length).toBeGreaterThanOrEqual(2); + // look at the last two tiles only + for (const tile of tiles.slice(-2)) { + await expect(tile).toContainText("You need to verify this device for access to historical messages"); + await expect(tile.locator(".mx_EventTile_e2eIcon_decryption_failure")).toBeVisible(); + } + + // Now verify our device (setting up key backup), and check what happens + await verifySession(app, secretStorageKey); + const tilesAfterVerify = (await page.locator(".mx_EventTile").all()).slice(-2); + + // The first message still cannot be decrypted, because it was never backed up. It's now a regular UTD though. + await expect(tilesAfterVerify[0]).toContainText("Unable to decrypt message"); + await expect(tilesAfterVerify[0].locator(".mx_EventTile_e2eIcon_decryption_failure")).toBeVisible(); + + // The second message should now be decrypted, with a grey shield + await expect(tilesAfterVerify[1]).toContainText("test2 test2"); + await expect(tilesAfterVerify[1].locator(".mx_EventTile_e2eIcon_normal")).toBeVisible(); + }); + }); }); diff --git a/playwright/e2e/crypto/utils.ts b/playwright/e2e/crypto/utils.ts index d43e4c7f941..5b0bf29b976 100644 --- a/playwright/e2e/crypto/utils.ts +++ b/playwright/e2e/crypto/utils.ts @@ -14,15 +14,15 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { type Page, expect, JSHandle } from "@playwright/test"; +import { expect, JSHandle, type Page } from "@playwright/test"; import type { CryptoEvent, ICreateRoomOpts, MatrixClient } from "matrix-js-sdk/src/matrix"; import type { + EmojiMapping, + ShowSasCallbacks, VerificationRequest, Verifier, - EmojiMapping, VerifierEvent, - ShowSasCallbacks, } from "matrix-js-sdk/src/crypto-api"; import { Credentials, HomeserverInstance } from "../../plugins/homeserver"; import { Client } from "../../pages/client"; @@ -148,7 +148,7 @@ export async function logIntoElement( // select homeserver await page.getByRole("button", { name: "Edit" }).click(); await page.getByRole("textbox", { name: "Other homeserver" }).fill(homeserver.config.baseUrl); - await page.getByRole("button", { name: "Continue" }).click(); + await page.getByRole("button", { name: "Continue", exact: true }).click(); // wait for the dialog to go away await expect(page.locator(".mx_ServerPickerDialog")).not.toBeVisible(); @@ -167,15 +167,40 @@ export async function logIntoElement( } } -export async function logOutOfElement(page: Page) { +/** + * Click the "sign out" option in Element, and wait for the login page to load + * + * @param page - Playwright `Page` object. + * @param discardKeys - if true, expect a "You'll lose access to your encrypted messages" dialog, and dismiss it. + */ +export async function logOutOfElement(page: Page, discardKeys: boolean = false) { await page.getByRole("button", { name: "User menu" }).click(); await page.locator(".mx_UserMenu_contextMenu").getByRole("menuitem", { name: "Sign out" }).click(); - await page.locator(".mx_Dialog .mx_QuestionDialog").getByRole("button", { name: "Sign out" }).click(); + if (discardKeys) { + await page.getByRole("button", { name: "I don't want my encrypted messages" }).click(); + } else { + await page.locator(".mx_Dialog .mx_QuestionDialog").getByRole("button", { name: "Sign out" }).click(); + } // Wait for the login page to load await page.getByRole("heading", { name: "Sign in" }).click(); } +/** + * Open the security settings, and verify the current session using the security key. + * + * @param app - `ElementAppPage` wrapper for the playwright `Page`. + * @param securityKey - The security key (i.e., 4S key), set up during a previous session. + */ +export async function verifySession(app: ElementAppPage, securityKey: string) { + const settings = await app.settings.openUserSettings("Security & Privacy"); + await settings.getByRole("button", { name: "Verify this session" }).click(); + await app.page.getByRole("button", { name: "Verify with Security Key" }).click(); + await app.page.locator(".mx_Dialog").locator('input[type="password"]').fill(securityKey); + await app.page.getByRole("button", { name: "Continue", disabled: false }).click(); + await app.page.getByRole("button", { name: "Done" }).click(); +} + /** * Given a SAS verifier for a bot client: * - wait for the bot to receive the emojis @@ -289,4 +314,9 @@ export async function createRoom(page: Page, roomName: string, isEncrypted: bool } await dialog.getByRole("button", { name: "Create room" }).click(); + + // Wait for the client to process the encryption event before carrying on (and potentially sending events). + if (isEncrypted) { + await expect(page.getByText("Encryption enabled")).toBeVisible(); + } } From 598768a3baaebe8f41666e99933aa34e675349aa Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 17 Apr 2024 17:20:53 +0100 Subject: [PATCH 3/5] Add some tests --- .../messages/DecryptionFailureBody-test.tsx | 49 ++++++++++++++++++- 1 file changed, 47 insertions(+), 2 deletions(-) diff --git a/test/components/views/messages/DecryptionFailureBody-test.tsx b/test/components/views/messages/DecryptionFailureBody-test.tsx index e8d4fce56e9..c1a0a424d96 100644 --- a/test/components/views/messages/DecryptionFailureBody-test.tsx +++ b/test/components/views/messages/DecryptionFailureBody-test.tsx @@ -17,13 +17,20 @@ import React from "react"; import { render } from "@testing-library/react"; import { MatrixEvent } from "matrix-js-sdk/src/matrix"; +import { mkDecryptionFailureMatrixEvent } from "matrix-js-sdk/src/testing"; +import { DecryptionFailureCode } from "matrix-js-sdk/src/crypto-api"; import { mkEvent } from "../../../test-utils"; import { DecryptionFailureBody } from "../../../../src/components/views/messages/DecryptionFailureBody"; +import { LocalDeviceVerificationStateContext } from "../../../../src/contexts/LocalDeviceVerificationStateContext"; describe("DecryptionFailureBody", () => { - function customRender(event: MatrixEvent) { - return render(); + function customRender(event: MatrixEvent, localDeviceVerified: boolean = false) { + return render( + + + , + ); } it(`Should display "Unable to decrypt message"`, () => { @@ -60,4 +67,42 @@ describe("DecryptionFailureBody", () => { // Then expect(container).toMatchSnapshot(); }); + + it("should handle historical messages with no key backup", async () => { + // When + const event = await mkDecryptionFailureMatrixEvent({ + code: DecryptionFailureCode.HISTORICAL_MESSAGE_NO_KEY_BACKUP, + msg: "No backup", + roomId: "fakeroom", + sender: "fakesender", + }); + const { container } = customRender(event); + + // Then + expect(container).toHaveTextContent("Historical messages are not available on this device"); + }); + + describe.each([true, false])( + "should handle historical messages when there is a backup and device verification is", + (verified) => { + it.each([ + [DecryptionFailureCode.HISTORICAL_MESSAGE_BACKUP_UNCONFIGURED], + [DecryptionFailureCode.HISTORICAL_MESSAGE_WORKING_BACKUP], + ])("Error code %s", async (code) => { + // When + const event = await mkDecryptionFailureMatrixEvent({ + code, + msg: "Failure", + roomId: "fakeroom", + sender: "fakesender", + }); + const { container } = customRender(event, verified); + + // Then + expect(container).toHaveTextContent( + verified ? "Unable to decrypt" : "You need to verify this device for access to historical messages", + ); + }); + }, + ); }); From 7d3b1960540619476bfc118f2728f8d9046559dd Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 22 Apr 2024 18:00:31 +0100 Subject: [PATCH 4/5] Don't show "verify" message if backup is working --- .../views/messages/DecryptionFailureBody.tsx | 3 +- .../messages/DecryptionFailureBody-test.tsx | 35 ++++++++----------- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/src/components/views/messages/DecryptionFailureBody.tsx b/src/components/views/messages/DecryptionFailureBody.tsx index 47f862b83d4..37caf691611 100644 --- a/src/components/views/messages/DecryptionFailureBody.tsx +++ b/src/components/views/messages/DecryptionFailureBody.tsx @@ -29,8 +29,9 @@ function getErrorMessage(mxEvent: MatrixEvent, isVerified: boolean | undefined): return _t("timeline|decryption_failure|historical_event_no_key_backup"); case DecryptionFailureCode.HISTORICAL_MESSAGE_BACKUP_UNCONFIGURED: - case DecryptionFailureCode.HISTORICAL_MESSAGE_WORKING_BACKUP: if (isVerified === false) { + // The user seems to have a key backup, so prompt them to verify in the hope that doing so will + // mean we can restore from backup and we'll get the key for this message. return _t("timeline|decryption_failure|historical_event_unverified_device"); } // otherwise, use the default. diff --git a/test/components/views/messages/DecryptionFailureBody-test.tsx b/test/components/views/messages/DecryptionFailureBody-test.tsx index c1a0a424d96..b01bbb77291 100644 --- a/test/components/views/messages/DecryptionFailureBody-test.tsx +++ b/test/components/views/messages/DecryptionFailureBody-test.tsx @@ -82,27 +82,22 @@ describe("DecryptionFailureBody", () => { expect(container).toHaveTextContent("Historical messages are not available on this device"); }); - describe.each([true, false])( - "should handle historical messages when there is a backup and device verification is", - (verified) => { - it.each([ - [DecryptionFailureCode.HISTORICAL_MESSAGE_BACKUP_UNCONFIGURED], - [DecryptionFailureCode.HISTORICAL_MESSAGE_WORKING_BACKUP], - ])("Error code %s", async (code) => { - // When - const event = await mkDecryptionFailureMatrixEvent({ - code, - msg: "Failure", - roomId: "fakeroom", - sender: "fakesender", - }); - const { container } = customRender(event, verified); - - // Then - expect(container).toHaveTextContent( - verified ? "Unable to decrypt" : "You need to verify this device for access to historical messages", - ); + it.each([true, false])( + "should handle historical messages when there is a backup and device verification is %s", + async (verified) => { + // When + const event = await mkDecryptionFailureMatrixEvent({ + code: DecryptionFailureCode.HISTORICAL_MESSAGE_BACKUP_UNCONFIGURED, + msg: "Failure", + roomId: "fakeroom", + sender: "fakesender", }); + const { container } = customRender(event, verified); + + // Then + expect(container).toHaveTextContent( + verified ? "Unable to decrypt" : "You need to verify this device for access to historical messages", + ); }, ); }); From a1fcf67b8f6c89fddfb01dcae0047dead5db1770 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Tue, 23 Apr 2024 12:20:14 +0100 Subject: [PATCH 5/5] Apply suggestions from code review --- playwright/e2e/crypto/crypto.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/playwright/e2e/crypto/crypto.spec.ts b/playwright/e2e/crypto/crypto.spec.ts index 19916cbeccf..323e1eb7034 100644 --- a/playwright/e2e/crypto/crypto.spec.ts +++ b/playwright/e2e/crypto/crypto.spec.ts @@ -456,7 +456,7 @@ test.describe("Cryptography", function () { // no e2e icon await expect(lastTileE2eIcon).not.toBeVisible(); - // Workaround for https://github.com/element-hq/element-web/issues/27267: it can take up to 10 seconds for + // Workaround for https://github.com/element-hq/element-web/issues/27267. It can take up to 10 seconds for // the key to be backed up. await page.waitForTimeout(10000); @@ -567,7 +567,7 @@ test.describe("Cryptography", function () { await app.viewRoomByName("Test room"); await sendMessageInCurrentRoom(page, "test2 test2"); - // Workaround for https://github.com/element-hq/element-web/issues/27267: it can take up to 10 seconds for + // Workaround for https://github.com/element-hq/element-web/issues/27267. It can take up to 10 seconds for // the key to be backed up. await page.waitForTimeout(10000);