From 24df2e8cb760e1801a9deba8c22e8bee429cae9f Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 15 May 2024 16:22:53 +0100 Subject: [PATCH] Ensure we do not fire the verification mismatch modal multiple times (#12526) * Ensure we do not fire the verification mismatch modal multiple times Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Add rust crypto test for mismatch emoji cancellation Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --------- Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- playwright/e2e/crypto/verification.spec.ts | 67 ++++++++++++++++--- .../views/right_panel/EncryptionPanel.tsx | 10 ++- 2 files changed, 64 insertions(+), 13 deletions(-) diff --git a/playwright/e2e/crypto/verification.spec.ts b/playwright/e2e/crypto/verification.spec.ts index 6819606b64..e471b6b2f5 100644 --- a/playwright/e2e/crypto/verification.spec.ts +++ b/playwright/e2e/crypto/verification.spec.ts @@ -240,24 +240,26 @@ test.describe("User verification", () => { test.use({ displayName: "Alice", botCreateOpts: { displayName: "Bob", autoAcceptInvites: true, userIdPrefix: "bob_" }, + room: async ({ page, app, bot: bob, user: aliceCredentials }, use) => { + await app.client.bootstrapCrossSigning(aliceCredentials); + + // the other user creates a DM + const dmRoomId = await createDMRoom(bob, aliceCredentials.userId); + + // accept the DM + await app.viewRoomByName("Bob"); + await page.getByRole("button", { name: "Start chatting" }).click(); + await use({ roomId: dmRoomId }); + }, }); test("can receive a verification request when there is no existing DM", async ({ page, - app, bot: bob, user: aliceCredentials, toasts, + room: { roomId: dmRoomId }, }) => { - await app.client.bootstrapCrossSigning(aliceCredentials); - - // the other user creates a DM - const dmRoomId = await createDMRoom(bob, aliceCredentials.userId); - - // accept the DM - await app.viewRoomByName("Bob"); - await page.getByRole("button", { name: "Start chatting" }).click(); - // once Alice has joined, Bob starts the verification const bobVerificationRequest = await bob.evaluateHandle( async (client, { dmRoomId, aliceCredentials }) => { @@ -294,6 +296,51 @@ test.describe("User verification", () => { await expect(page.getByText("You've successfully verified Bob!")).toBeVisible(); await page.getByRole("button", { name: "Got it" }).click(); }); + + test("can abort emoji verification when emoji mismatch", async ({ + page, + bot: bob, + user: aliceCredentials, + toasts, + room: { roomId: dmRoomId }, + cryptoBackend, + }) => { + test.skip(cryptoBackend === "legacy", "Not implemented for legacy crypto"); + + // once Alice has joined, Bob starts the verification + const bobVerificationRequest = await bob.evaluateHandle( + async (client, { dmRoomId, aliceCredentials }) => { + const room = client.getRoom(dmRoomId); + while (room.getMember(aliceCredentials.userId)?.membership !== "join") { + await new Promise((resolve) => { + room.once(window.matrixcs.RoomStateEvent.Members, resolve); + }); + } + + return client.getCrypto().requestVerificationDM(aliceCredentials.userId, dmRoomId); + }, + { dmRoomId, aliceCredentials }, + ); + + // Accept verification via toast + const toast = await toasts.getToast("Verification requested"); + await toast.getByRole("button", { name: "Verify Session" }).click(); + + // request verification by emoji + await page.locator("#mx_RightPanel").getByRole("button", { name: "Verify by emoji" }).click(); + + /* on the bot side, wait for the verifier to exist ... */ + const botVerifier = await awaitVerifier(bobVerificationRequest); + // ... confirm ... + botVerifier.evaluate((verifier) => verifier.verify()).catch(() => {}); + // ... and abort the verification + await page.getByRole("button", { name: "They don't match" }).click(); + + const dialog = page.locator(".mx_Dialog"); + await expect(dialog.getByText("Your messages are not secure")).toBeVisible(); + await dialog.getByRole("button", { name: "OK" }).click(); + await expect(dialog).not.toBeVisible(); + }); }); /** Extract the qrcode out of an on-screen html element */ diff --git a/src/components/views/right_panel/EncryptionPanel.tsx b/src/components/views/right_panel/EncryptionPanel.tsx index d6e2ee4407..26ba841f71 100644 --- a/src/components/views/right_panel/EncryptionPanel.tsx +++ b/src/components/views/right_panel/EncryptionPanel.tsx @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import React, { useCallback, useEffect, useState } from "react"; +import React, { useCallback, useEffect, useRef, useState } from "react"; import { VerificationPhase, VerificationRequest, VerificationRequestEvent } from "matrix-js-sdk/src/crypto-api"; import { RoomMember, User } from "matrix-js-sdk/src/matrix"; @@ -69,13 +69,17 @@ const EncryptionPanel: React.FC = (props: IProps) => { awaitPromise(); } }, [verificationRequestPromise]); + // Use a ref to track whether we are already showing the mismatch modal as state may not update fast enough + // if two change events are fired in quick succession like can happen with rust crypto. + const isShowingMismatchModal = useRef(false); const changeHandler = useCallback(() => { // handle transitions -> cancelled for mismatches which fire a modal instead of showing a card if ( - request && - request.phase === VerificationPhase.Cancelled && + !isShowingMismatchModal.current && + request?.phase === VerificationPhase.Cancelled && MISMATCHES.includes(request.cancellationCode ?? "") ) { + isShowingMismatchModal.current = true; Modal.createDialog(ErrorDialog, { headerImage: require("../../../../res/img/e2e/warning-deprecated.svg").default, title: _t("encryption|messages_not_secure|title"),