From 34439ee652e6d01ecf0ab93fe859671b0542123c Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Wed, 7 Jun 2023 15:43:44 +0100 Subject: [PATCH] Simplify references to `VerificationRequest` (#11045) * Use new `VerificationRequest.getQRCodeBytes()` * Use new `VerificationRequest.otherDeviceId` * Remove references to `VerificationRequest.channel` * Replace references to `VerificationRequest.{requesting,receiving}UserId` Normally these are guarded by `request.initiatedByMe` so we can trivially replace it with `request.otherUserId` or `client.getUserId()`. In one place we actually need to apply some logic. * increase test coverage * Even more test coverage * Even more test coverage --- src/components/structures/MatrixChat.tsx | 2 +- .../elements/crypto/VerificationQRCode.tsx | 5 +- .../messages/MKeyVerificationRequest.tsx | 8 +- .../views/right_panel/EncryptionPanel.tsx | 2 +- .../views/right_panel/VerificationPanel.tsx | 15 +-- .../views/toasts/VerificationRequestToast.tsx | 12 +-- .../crypto/VerificationQRCode-test.tsx | 33 +++++++ .../VerificationQRCode-test.tsx.snap | 15 +++ .../messages/MKeyVerificationRequest-test.tsx | 25 ++++- .../right_panel/VerificationPanel-test.tsx | 65 +++++++++++-- .../toasts/VerificationRequestToast-test.tsx | 93 +++++++++++++++++++ .../VerificationRequestToast-test.tsx.snap | 68 ++++++++++++++ 12 files changed, 310 insertions(+), 33 deletions(-) create mode 100644 test/components/views/elements/crypto/VerificationQRCode-test.tsx create mode 100644 test/components/views/elements/crypto/__snapshots__/VerificationQRCode-test.tsx.snap create mode 100644 test/components/views/toasts/VerificationRequestToast-test.tsx create mode 100644 test/components/views/toasts/__snapshots__/VerificationRequestToast-test.tsx.snap diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index fef5d26daf..6a03bc7aed 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -1667,7 +1667,7 @@ export default class MatrixChat extends React.PureComponent { ); } else if (request.pending) { ToastStore.sharedInstance().addOrReplaceToast({ - key: "verifreq_" + request.channel.transactionId, + key: "verifreq_" + request.transactionId, title: _t("Verification requested"), icon: "verification", props: { request }, diff --git a/src/components/views/elements/crypto/VerificationQRCode.tsx b/src/components/views/elements/crypto/VerificationQRCode.tsx index f19b9613a2..193134cd93 100644 --- a/src/components/views/elements/crypto/VerificationQRCode.tsx +++ b/src/components/views/elements/crypto/VerificationQRCode.tsx @@ -15,19 +15,18 @@ limitations under the License. */ import React from "react"; -import { QRCodeData } from "matrix-js-sdk/src/crypto/verification/QRCode"; import QRCode from "../QRCode"; interface IProps { - qrCodeData: QRCodeData; + qrCodeBytes: Buffer; } export default class VerificationQRCode extends React.PureComponent { public render(): React.ReactNode { return ( diff --git a/src/components/views/messages/MKeyVerificationRequest.tsx b/src/components/views/messages/MKeyVerificationRequest.tsx index 73afa2869b..60fc2963c9 100644 --- a/src/components/views/messages/MKeyVerificationRequest.tsx +++ b/src/components/views/messages/MKeyVerificationRequest.tsx @@ -148,7 +148,7 @@ export default class MKeyVerificationRequest extends React.Component { if (accepted) { stateLabel = ( - {this.acceptedLabel(request.receivingUserId)} + {this.acceptedLabel(request.initiatedByMe ? request.otherUserId : client.getSafeUserId())} ); } else if (request.phase === VerificationPhase.Cancelled) { @@ -162,9 +162,9 @@ export default class MKeyVerificationRequest extends React.Component { } if (!request.initiatedByMe) { - const name = getNameForEventRoom(client, request.requestingUserId, mxEvent.getRoomId()!); + const name = getNameForEventRoom(client, request.otherUserId, mxEvent.getRoomId()!); title = _t("%(name)s wants to verify", { name }); - subtitle = userLabelForEventRoom(client, request.requestingUserId, mxEvent.getRoomId()!); + subtitle = userLabelForEventRoom(client, request.otherUserId, mxEvent.getRoomId()!); if (request.canAccept) { stateNode = (
@@ -180,7 +180,7 @@ export default class MKeyVerificationRequest extends React.Component { } else { // request sent by us title = _t("You sent a verification request"); - subtitle = userLabelForEventRoom(client, request.receivingUserId, mxEvent.getRoomId()!); + subtitle = userLabelForEventRoom(client, request.otherUserId, mxEvent.getRoomId()!); } if (title) { diff --git a/src/components/views/right_panel/EncryptionPanel.tsx b/src/components/views/right_panel/EncryptionPanel.tsx index 73c2ba02bd..6ba9eb9e2c 100644 --- a/src/components/views/right_panel/EncryptionPanel.tsx +++ b/src/components/views/right_panel/EncryptionPanel.tsx @@ -165,7 +165,7 @@ const EncryptionPanel: React.FC = (props: IProps) => { onClose={onClose} member={member} request={request} - key={request.channel.transactionId} + key={request.transactionId} inDialog={layout === "dialog"} phase={phase} /> diff --git a/src/components/views/right_panel/VerificationPanel.tsx b/src/components/views/right_panel/VerificationPanel.tsx index 582b95c2f4..d594f7c75c 100644 --- a/src/components/views/right_panel/VerificationPanel.tsx +++ b/src/components/views/right_panel/VerificationPanel.tsx @@ -67,6 +67,7 @@ export default class VerificationPanel extends React.PureComponent

{_t("Scan this unique code")}

- +
); } @@ -133,7 +134,7 @@ export default class VerificationPanel extends React.PureComponent

{_t("Verify by scanning")}

@@ -144,7 +145,7 @@ export default class VerificationPanel extends React.PureComponent
- +
); @@ -201,7 +202,7 @@ export default class VerificationPanel extends React.PureComponent({ action: Action.ViewRoom, - room_id: request.channel.roomId, + room_id: request.roomId, should_peek: false, metricsTrigger: "VerificationRequest", }); @@ -128,7 +128,7 @@ export default class VerificationRequestToast extends React.PureComponent", () => { + afterEach(() => { + cleanup(); + }); + + it("renders a QR code", async () => { + const { container, getAllByAltText } = render(); + // wait for the spinner to go away + await waitFor(() => getAllByAltText("QR Code").length === 1); + expect(container).toMatchSnapshot(); + }); +}); diff --git a/test/components/views/elements/crypto/__snapshots__/VerificationQRCode-test.tsx.snap b/test/components/views/elements/crypto/__snapshots__/VerificationQRCode-test.tsx.snap new file mode 100644 index 0000000000..73f8203f1d --- /dev/null +++ b/test/components/views/elements/crypto/__snapshots__/VerificationQRCode-test.tsx.snap @@ -0,0 +1,15 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[` renders a QR code 1`] = ` +
+
+ QR Code +
+
+`; diff --git a/test/components/views/messages/MKeyVerificationRequest-test.tsx b/test/components/views/messages/MKeyVerificationRequest-test.tsx index e3d0d19644..2fcfc1b595 100644 --- a/test/components/views/messages/MKeyVerificationRequest-test.tsx +++ b/test/components/views/messages/MKeyVerificationRequest-test.tsx @@ -15,7 +15,7 @@ limitations under the License. */ import React from "react"; -import { render } from "@testing-library/react"; +import { render, within } from "@testing-library/react"; import { EventEmitter } from "events"; import { MatrixEvent } from "matrix-js-sdk/src/matrix"; import { @@ -74,6 +74,29 @@ describe("MKeyVerificationRequest", () => { expect(container).toHaveTextContent("You sent a verification request"); }); + it("should render appropriately when the request was initiated by me and has been accepted", () => { + const event = new MatrixEvent({ type: "m.key.verification.request" }); + event.verificationRequest = getMockVerificationRequest({ + phase: VerificationPhase.Ready, + otherUserId: "@other:user", + }); + const { container } = render(); + expect(container).toHaveTextContent("You sent a verification request"); + expect(within(container).getByRole("button")).toHaveTextContent("@other:user accepted"); + }); + + it("should render appropriately when the request was initiated by the other user and has been accepted", () => { + const event = new MatrixEvent({ type: "m.key.verification.request" }); + event.verificationRequest = getMockVerificationRequest({ + phase: VerificationPhase.Ready, + initiatedByMe: false, + otherUserId: "@other:user", + }); + const { container } = render(); + expect(container).toHaveTextContent("@other:user wants to verify"); + expect(within(container).getByRole("button")).toHaveTextContent("You accepted"); + }); + it("should render appropriately when the request was cancelled", () => { const event = new MatrixEvent({ type: "m.key.verification.request" }); event.verificationRequest = getMockVerificationRequest({ diff --git a/test/components/views/right_panel/VerificationPanel-test.tsx b/test/components/views/right_panel/VerificationPanel-test.tsx index 9c8282b0de..7fdab5248f 100644 --- a/test/components/views/right_panel/VerificationPanel-test.tsx +++ b/test/components/views/right_panel/VerificationPanel-test.tsx @@ -14,8 +14,8 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { act, render } from "@testing-library/react"; -import React from "react"; +import { act, render, waitFor } from "@testing-library/react"; +import React, { ComponentProps } from "react"; import { Phase, VerificationRequest, @@ -31,7 +31,6 @@ import { VerifierEvent, VerifierEventHandlerMap, } from "matrix-js-sdk/src/crypto-api/verification"; -import { IVerificationChannel } from "matrix-js-sdk/src/crypto/verification/request/Channel"; import VerificationPanel from "../../../../src/components/views/right_panel/VerificationPanel"; import { stubClient } from "../../../test-utils"; @@ -41,12 +40,57 @@ describe("", () => { stubClient(); }); - it("should show a 'Verify by emoji' button", () => { - const container = renderComponent({ - request: makeMockVerificationRequest(), - phase: Phase.Ready, + describe("'Ready' phase (dialog mode)", () => { + it("should show a 'Start' button", () => { + const container = renderComponent({ + request: makeMockVerificationRequest({ + phase: Phase.Ready, + }), + layout: "dialog", + }); + container.getByRole("button", { name: "Start" }); + }); + + it("should show a QR code if the other side can scan and QR bytes are calculated", async () => { + const request = makeMockVerificationRequest({ + phase: Phase.Ready, + }); + request.getQRCodeBytes.mockReturnValue(Buffer.from("test", "utf-8")); + const container = renderComponent({ + request: request, + layout: "dialog", + }); + container.getByText("Scan this unique code"); + // it shows a spinner at first; wait for the update which makes it show the QR code + await waitFor(() => { + container.getByAltText("QR Code"); + }); + }); + }); + + describe("'Ready' phase (regular mode)", () => { + it("should show a 'Verify by emoji' button", () => { + const container = renderComponent({ + request: makeMockVerificationRequest({ phase: Phase.Ready }), + }); + container.getByRole("button", { name: "Verify by emoji" }); + }); + + it("should show a QR code if the other side can scan and QR bytes are calculated", async () => { + const request = makeMockVerificationRequest({ + phase: Phase.Ready, + }); + request.getQRCodeBytes.mockReturnValue(Buffer.from("test", "utf-8")); + const container = renderComponent({ + request: request, + member: new User("@other:user"), + }); + container.getByText("Ask @other:user to scan your code:"); + // it shows a spinner at first; wait for the update which makes it show the QR code + await waitFor(() => { + container.getByAltText("QR Code"); + }); }); - container.getByRole("button", { name: "Verify by emoji" }); }); describe("'Verify by emoji' flow", () => { @@ -91,13 +135,14 @@ describe("", () => { }); }); -function renderComponent(props: { request: VerificationRequest; phase: Phase }) { +function renderComponent(props: Partial> & { request: VerificationRequest }) { const defaultProps = { layout: "", member: {} as User, onClose: () => undefined, isRoomEncrypted: false, inDialog: false, + phase: props.request.phase, }; return render(); } @@ -105,9 +150,9 @@ function renderComponent(props: { request: VerificationRequest; phase: Phase }) function makeMockVerificationRequest(props: Partial = {}): Mocked { const request = new TypedEventEmitter(); Object.assign(request, { - channel: {} as IVerificationChannel, cancel: jest.fn(), otherPartySupportsMethod: jest.fn().mockReturnValue(true), + getQRCodeBytes: jest.fn(), ...props, }); return request as unknown as Mocked; diff --git a/test/components/views/toasts/VerificationRequestToast-test.tsx b/test/components/views/toasts/VerificationRequestToast-test.tsx new file mode 100644 index 0000000000..6a73f445e9 --- /dev/null +++ b/test/components/views/toasts/VerificationRequestToast-test.tsx @@ -0,0 +1,93 @@ +/* +Copyright 2023 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. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import React, { ComponentProps } from "react"; +import { Mocked } from "jest-mock"; +import { act, render, RenderResult } from "@testing-library/react"; +import { + VerificationRequest, + VerificationRequestEvent, +} from "matrix-js-sdk/src/crypto/verification/request/VerificationRequest"; +import { DeviceInfo } from "matrix-js-sdk/src/crypto/deviceinfo"; +import { IMyDevice, MatrixClient } from "matrix-js-sdk/src/client"; +import { TypedEventEmitter } from "matrix-js-sdk/src/models/typed-event-emitter"; + +import VerificationRequestToast from "../../../../src/components/views/toasts/VerificationRequestToast"; +import { flushPromises, getMockClientWithEventEmitter, mockClientMethodsUser } from "../../../test-utils"; + +function renderComponent( + props: Partial> & { request: VerificationRequest }, +): RenderResult { + const propsWithDefaults = { + toastKey: "test", + ...props, + }; + + return render(); +} + +describe("VerificationRequestToast", () => { + let client: Mocked; + + beforeEach(() => { + client = getMockClientWithEventEmitter({ + ...mockClientMethodsUser(), + getStoredDevice: jest.fn(), + getDevice: jest.fn(), + }); + }); + + it("should render a self-verification", async () => { + const otherDeviceId = "other_device"; + const otherIDevice: IMyDevice = { device_id: otherDeviceId, last_seen_ip: "1.1.1.1" }; + client.getDevice.mockResolvedValue(otherIDevice); + + const otherDeviceInfo = new DeviceInfo(otherDeviceId); + otherDeviceInfo.unsigned = { device_display_name: "my other device" }; + client.getStoredDevice.mockReturnValue(otherDeviceInfo); + + const request = makeMockVerificationRequest({ + isSelfVerification: true, + otherDeviceId, + }); + const result = renderComponent({ request }); + await act(async () => { + await flushPromises(); + }); + expect(result.container).toMatchSnapshot(); + }); + + it("should render a cross-user verification", async () => { + const otherUserId = "@other:user"; + const request = makeMockVerificationRequest({ + isSelfVerification: false, + otherUserId, + }); + const result = renderComponent({ request }); + await act(async () => { + await flushPromises(); + }); + expect(result.container).toMatchSnapshot(); + }); +}); + +function makeMockVerificationRequest(props: Partial = {}): Mocked { + const request = new TypedEventEmitter(); + Object.assign(request, { + ...props, + }); + return request as unknown as Mocked; +} diff --git a/test/components/views/toasts/__snapshots__/VerificationRequestToast-test.tsx.snap b/test/components/views/toasts/__snapshots__/VerificationRequestToast-test.tsx.snap new file mode 100644 index 0000000000..e34f816de1 --- /dev/null +++ b/test/components/views/toasts/__snapshots__/VerificationRequestToast-test.tsx.snap @@ -0,0 +1,68 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`VerificationRequestToast should render a cross-user verification 1`] = ` +
+
+
+ @alice:domain (@other:user) +
+
+
+ Ignore (NaN) +
+
+ Verify Session +
+
+
+
+`; + +exports[`VerificationRequestToast should render a self-verification 1`] = ` +
+
+
+ my other device +
+ other_device from 1.1.1.1 +
+
+
+
+ Ignore (NaN) +
+
+ Verify Session +
+
+
+
+`;