diff --git a/src/components/views/dialogs/VerificationRequestDialog.tsx b/src/components/views/dialogs/VerificationRequestDialog.tsx index cb4166b775..801b745e71 100644 --- a/src/components/views/dialogs/VerificationRequestDialog.tsx +++ b/src/components/views/dialogs/VerificationRequestDialog.tsx @@ -7,7 +7,7 @@ Please see LICENSE files in the repository root for full details. */ import React from "react"; -import { type VerificationRequest } from "matrix-js-sdk/src/crypto-api"; +import { VerificationPhase, VerificationRequestEvent, type VerificationRequest } from "matrix-js-sdk/src/crypto-api"; import { type User } from "matrix-js-sdk/src/matrix"; import { MatrixClientPeg } from "../../../MatrixClientPeg"; @@ -23,7 +23,17 @@ interface IProps { } interface IState { + // The VerificationRequest that is ongoing. This can be replaced if a + // promise was supplied in the props and it completes. verificationRequest?: VerificationRequest; + + // What phase the VerificationRequest is at. This is part of + // verificationRequest but we have it as independent state because we need + // to update when it changes. + // + // We listen to the `Change` event on verificationRequest and update phase + // when that fires. + phase?: VerificationPhase; } export default class VerificationRequestDialog extends React.Component { @@ -31,22 +41,51 @@ export default class VerificationRequestDialog extends React.Component { - this.setState({ verificationRequest: r }); + // The request promise completed, so we have a new request + + // Stop listening to the old request (if we have one, which normally we won't) + this.state.verificationRequest?.off(VerificationRequestEvent.Change, this.onRequestChange); + + // And start listening to the new one + r.on(VerificationRequestEvent.Change, this.onRequestChange); + + this.setState({ verificationRequest: r, phase: r.phase }); }); } + public componentWillUnmount(): void { + // Stop listening for changes to the request when we close + this.state.verificationRequest?.off(VerificationRequestEvent.Change, this.onRequestChange); + } + + /** + * The verificationRequest changed, so we need to make sure we update our + * state to have the correct phase. + * + * Note: this is called when verificationRequest changes in some way, not + * when we replace verificationRequest with some new request. + */ + private readonly onRequestChange = (): void => { + this.setState((prevState) => ({ + phase: prevState.verificationRequest?.phase, + })); + }; + public render(): React.ReactNode { const request = this.state.verificationRequest; const otherUserId = request?.otherUserId; const member = this.props.member || (otherUserId ? MatrixClientPeg.safeGet().getUser(otherUserId) : null); - const title = request?.isSelfVerification - ? _t("encryption|verification|verification_dialog_title_device") - : _t("encryption|verification|verification_dialog_title_user"); + const title = this.dialogTitle(request); if (!member) return null; @@ -69,4 +108,16 @@ export default class VerificationRequestDialog extends React.Component ); } + + private dialogTitle(request?: VerificationRequest): string { + if (request?.isSelfVerification) { + if (request.phase === VerificationPhase.Cancelled) { + return _t("encryption|verification|verification_dialog_title_failed"); + } else { + return _t("encryption|verification|verification_dialog_title_device"); + } + } else { + return _t("encryption|verification|verification_dialog_title_user"); + } + } } diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 811edc1590..f9eab0839e 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -1068,6 +1068,7 @@ "use_another_device": "Use another device", "use_recovery_key": "Use recovery key", "verification_dialog_title_device": "Verify other device", + "verification_dialog_title_failed": "Verification failed", "verification_dialog_title_user": "Verification Request", "verification_skip_warning": "Without verifying, you won't have access to all your messages and may appear as untrusted to others.", "verification_success_with_backup": "Your new device is now verified. It has access to your encrypted messages, and other users will see it as trusted.", diff --git a/test/unit-tests/components/structures/MatrixChat-test.tsx b/test/unit-tests/components/structures/MatrixChat-test.tsx index eef9b999d2..3ebe826b1c 100644 --- a/test/unit-tests/components/structures/MatrixChat-test.tsx +++ b/test/unit-tests/components/structures/MatrixChat-test.tsx @@ -1106,7 +1106,7 @@ describe("", () => { act(() => verify.click()); // And close the device verification dialog - const closeButton = await screen.findByRole("button", { name: "Close dialog" }); + const closeButton = screen.getByRole("button", { name: "Close dialog" }); act(() => closeButton.click()); // Then we are not allowed in - we are still being asked to verify @@ -1179,7 +1179,7 @@ describe("", () => { .fn() .mockResolvedValue({ signedByOwner: true } as DeviceVerificationStatus), isCrossSigningReady: jest.fn().mockReturnValue(false), - requestOwnUserVerification: jest.fn().mockResolvedValue({ cancel: jest.fn() }), + requestOwnUserVerification: jest.fn().mockResolvedValue({ cancel: jest.fn(), on: jest.fn() }), } as any; } }); diff --git a/test/unit-tests/components/views/dialogs/VerificationRequestDialog-test.tsx b/test/unit-tests/components/views/dialogs/VerificationRequestDialog-test.tsx index e585f8a065..f373cfef14 100644 --- a/test/unit-tests/components/views/dialogs/VerificationRequestDialog-test.tsx +++ b/test/unit-tests/components/views/dialogs/VerificationRequestDialog-test.tsx @@ -7,18 +7,20 @@ Please see LICENSE files in the repository root for full details. import React from "react"; import { act, render, screen } from "jest-matrix-react"; -import { User } from "matrix-js-sdk/src/matrix"; +import { TypedEventEmitter, User } from "matrix-js-sdk/src/matrix"; import { type ShowSasCallbacks, VerificationPhase, type Verifier, type VerificationRequest, type ShowQrCodeCallbacks, + VerificationRequestEvent, + type VerificationRequestEventHandlerMap, } from "matrix-js-sdk/src/crypto-api"; import { VerificationMethod } from "matrix-js-sdk/src/types"; -import VerificationRequestDialog from "../../../../../src/components/views/dialogs/VerificationRequestDialog"; import { stubClient } from "../../../../test-utils"; +import VerificationRequestDialog from "../../../../../src/components/views/dialogs/VerificationRequestDialog"; describe("VerificationRequestDialog", () => { function renderComponent(phase: VerificationPhase, method?: "emoji" | "qr"): ReturnType { @@ -86,7 +88,7 @@ describe("VerificationRequestDialog", () => { it("Shows a failure message if verification was cancelled", async () => { const dialog = renderComponent(VerificationPhase.Cancelled); - expect(screen.getByRole("heading", { name: "Verify other device" })).toBeInTheDocument(); + expect(screen.getByRole("heading", { name: "Verification failed" })).toBeInTheDocument(); expect(screen.getByRole("heading", { name: "Verification cancelled" })).toBeInTheDocument(); expect( @@ -116,7 +118,7 @@ describe("VerificationRequestDialog", () => { await act(async () => await new Promise(process.nextTick)); // Then it renders the resolved information - expect(screen.getByRole("heading", { name: "Verify other device" })).toBeInTheDocument(); + expect(screen.getByRole("heading", { name: "Verification failed" })).toBeInTheDocument(); expect(screen.getByRole("heading", { name: "Verification cancelled" })).toBeInTheDocument(); expect( @@ -146,7 +148,28 @@ describe("VerificationRequestDialog", () => { await act(async () => await new Promise(process.nextTick)); // Then it renders the information from the request in the promise - expect(screen.getByRole("heading", { name: "Verify other device" })).toBeInTheDocument(); + expect(screen.getByRole("heading", { name: "Verification failed" })).toBeInTheDocument(); + expect(screen.getByRole("heading", { name: "Verification cancelled" })).toBeInTheDocument(); + + expect( + screen.getByText( + "You cancelled verification on your other device. Start verification again from the notification.", + ), + ).toBeInTheDocument(); + }); + + it("Changes the dialog contents when the request changes phase", async () => { + // Given we rendered the component with a phase of Unsent + const member = User.createUser("@alice:example.org", stubClient()); + const request = createRequest(VerificationPhase.Unsent); + + render(); + + // When I cancel the request (which changes phase and emits a Changed event) + await act(async () => await request.cancel()); + + // Then the dialog is updated to reflect that + expect(screen.getByRole("heading", { name: "Verification failed" })).toBeInTheDocument(); expect(screen.getByRole("heading", { name: "Verification cancelled" })).toBeInTheDocument(); expect( @@ -157,9 +180,9 @@ describe("VerificationRequestDialog", () => { }); }); -function createRequest(phase: VerificationPhase, method?: "emoji" | "qr"): VerificationRequest { +function createRequest(phase: VerificationPhase, method?: "emoji" | "qr"): MockVerificationRequest { let verifier = undefined; - let chosenMethod = undefined; + let chosenMethod = null; switch (method) { case "emoji": @@ -172,24 +195,7 @@ function createRequest(phase: VerificationPhase, method?: "emoji" | "qr"): Verif break; } - return { - phase: jest.fn().mockReturnValue(phase), - - // VerificationRequest is an emitter - ignore any events that are emitted. - on: jest.fn(), - off: jest.fn(), - - // These tests (so far) only check for when we are initiating a verificiation of our own device. - isSelfVerification: jest.fn().mockReturnValue(true), - initiatedByMe: jest.fn().mockReturnValue(true), - - // Always returning true means we can support QR code and emoji verification. - otherPartySupportsMethod: jest.fn().mockReturnValue(true), - - // If we asked for emoji, these are populated. - verifier, - chosenMethod, - } as unknown as VerificationRequest; + return new MockVerificationRequest(phase, verifier, chosenMethod); } function createEmojiVerifier(): Verifier { @@ -226,3 +232,109 @@ function createQrVerifier(): Verifier { verify: jest.fn(), } as unknown as Verifier; } + +class MockVerificationRequest + extends TypedEventEmitter + implements VerificationRequest +{ + phase_: VerificationPhase; + verifier_: Verifier | undefined; + chosenMethod_: string | null; + + constructor(phase: VerificationPhase, verifier: Verifier | undefined, chosenMethod: string | null) { + super(); + this.phase_ = phase; + this.verifier_ = verifier; + this.chosenMethod_ = chosenMethod; + } + + get phase(): VerificationPhase { + return this.phase_; + } + + get isSelfVerification(): boolean { + // So far we are only testing verification of our own devices + return true; + } + + get initiatedByMe(): boolean { + // So far we are only testing verification started by this device + return true; + } + + otherPartySupportsMethod(): boolean { + // This makes both emoji and QR verification options appear + return true; + } + + get verifier(): Verifier | undefined { + return this.verifier_; + } + + get chosenMethod(): string | null { + return this.chosenMethod_; + } + + async cancel(): Promise { + this.phase_ = VerificationPhase.Cancelled; + this.emit(VerificationRequestEvent.Change); + } + + get transactionId(): string | undefined { + return undefined; + } + + get roomId(): string | undefined { + return undefined; + } + + get otherUserId(): string { + return "otheruser"; + } + + get otherDeviceId(): string | undefined { + return undefined; + } + + get pending(): boolean { + return false; + } + + get accepting(): boolean { + return false; + } + + get declining(): boolean { + return false; + } + + get timeout(): number | null { + return null; + } + + get methods(): string[] { + return []; + } + + async accept(): Promise {} + + startVerification(_method: string): Promise { + throw new Error("Method not implemented."); + } + + scanQRCode(_qrCodeData: Uint8ClampedArray): Promise { + throw new Error("Method not implemented."); + } + + async generateQRCode(): Promise { + return undefined; + } + + get cancellationCode(): string | null { + return null; + } + + get cancellingUserId(): string | undefined { + return "otheruser"; + } +} diff --git a/test/unit-tests/components/views/dialogs/__snapshots__/VerificationRequestDialog-test.tsx.snap b/test/unit-tests/components/views/dialogs/__snapshots__/VerificationRequestDialog-test.tsx.snap index f0336ef559..a2fd15edd5 100644 --- a/test/unit-tests/components/views/dialogs/__snapshots__/VerificationRequestDialog-test.tsx.snap +++ b/test/unit-tests/components/views/dialogs/__snapshots__/VerificationRequestDialog-test.tsx.snap @@ -236,7 +236,7 @@ exports[`VerificationRequestDialog Shows a failure message if verification was c class="mx_Heading_h3 mx_Dialog_title" id="mx_BaseDialog_title" > - Verify other device + Verification failed