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
matrix-org · Jun 7, 2023 · 34439ee · 34439ee
1 parent ac2c9ce
commit 34439ee
Show file tree

Hide file tree

Showing 12 changed files with 310 additions and 33 deletions.
diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx
@@ -1667,7 +1667,7 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
                 );
             } 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
@@ -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<IProps> {
     public render(): React.ReactNode {
         return (
             <QRCode
-                data={[{ data: this.props.qrCodeData.getBuffer(), mode: "byte" }]}
+                data={[{ data: this.props.qrCodeBytes, mode: "byte" }]}
                 className="mx_VerificationQRCode"
                 width={196}
             />

diff --git 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<IProps> {
             if (accepted) {
                 stateLabel = (
                     <AccessibleButton onClick={this.openRequest}>
-                        {this.acceptedLabel(request.receivingUserId)}
+                        {this.acceptedLabel(request.initiatedByMe ? request.otherUserId : client.getSafeUserId())}
                     </AccessibleButton>
                 );
             } else if (request.phase === VerificationPhase.Cancelled) {
@@ -162,9 +162,9 @@ export default class MKeyVerificationRequest extends React.Component<IProps> {
         }
 
         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 = (
                     <div className="mx_cryptoEvent_buttons">
@@ -180,7 +180,7 @@ export default class MKeyVerificationRequest extends React.Component<IProps> {
         } 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
@@ -165,7 +165,7 @@ const EncryptionPanel: React.FC<IProps> = (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
@@ -67,6 +67,7 @@ export default class VerificationPanel extends React.PureComponent<IProps, IStat
         const { member, request } = this.props;
         const showSAS: boolean = request.otherPartySupportsMethod(verificationMethods.SAS);
         const showQR: boolean = request.otherPartySupportsMethod(SCAN_QR_CODE_METHOD);
+        const qrCodeBytes = showQR ? request.getQRCodeBytes() : null;
         const brand = SdkConfig.get().brand;
 
         const noCommonMethodError: JSX.Element | null =
@@ -85,11 +86,11 @@ export default class VerificationPanel extends React.PureComponent<IProps, IStat
             // HACK: This is a terrible idea.
             let qrBlockDialog: JSX.Element | undefined;
             let sasBlockDialog: JSX.Element | undefined;
-            if (showQR && request.qrCodeData) {
+            if (!!qrCodeBytes) {
                 qrBlockDialog = (
                     <div className="mx_VerificationPanel_QRPhase_startOption">
                         <p>{_t("Scan this unique code")}</p>
-                        <VerificationQRCode qrCodeData={request.qrCodeData} />
+                        <VerificationQRCode qrCodeBytes={qrCodeBytes} />
                     </div>
                 );
             }
@@ -133,7 +134,7 @@ export default class VerificationPanel extends React.PureComponent<IProps, IStat
         }
 
         let qrBlock: JSX.Element | undefined;
-        if (showQR && request.qrCodeData) {
+        if (!!qrCodeBytes) {
             qrBlock = (
                 <div className="mx_UserInfo_container">
                     <h3>{_t("Verify by scanning")}</h3>
@@ -144,7 +145,7 @@ export default class VerificationPanel extends React.PureComponent<IProps, IStat
                     </p>
 
                     <div className="mx_VerificationPanel_qrCode">
-                        <VerificationQRCode qrCodeData={request.qrCodeData} />
+                        <VerificationQRCode qrCodeBytes={qrCodeBytes} />
                     </div>
                 </div>
             );
@@ -201,7 +202,7 @@ export default class VerificationPanel extends React.PureComponent<IProps, IStat
     };
 
     private getDevice(): DeviceInfo | null {
-        const deviceId = this.props.request && this.props.request.channel.deviceId;
+        const deviceId = this.props.request && this.props.request.otherDeviceId;
         const userId = MatrixClientPeg.get().getUserId();
         if (deviceId && userId) {
             return MatrixClientPeg.get().getStoredDevice(userId, deviceId);
@@ -275,12 +276,12 @@ export default class VerificationPanel extends React.PureComponent<IProps, IStat
             if (!device) {
                 // This can happen if the device is logged out while we're still showing verification
                 // UI for it.
-                logger.warn("Verified device we don't know about: " + this.props.request.channel.deviceId);
+                logger.warn("Verified device we don't know about: " + this.props.request.otherDeviceId);
                 description = _t("You've successfully verified your device!");
             } else {
                 description = _t("You've successfully verified %(deviceName)s (%(deviceId)s)!", {
                     deviceName: device ? device.getDisplayName() : "",
-                    deviceId: this.props.request.channel.deviceId,
+                    deviceId: this.props.request.otherDeviceId,
                 });
             }
         } else {

diff --git a/src/components/views/toasts/VerificationRequestToast.tsx b/src/components/views/toasts/VerificationRequestToast.tsx
@@ -74,11 +74,11 @@ export default class VerificationRequestToast extends React.PureComponent<IProps
 
         if (request.isSelfVerification) {
             const cli = MatrixClientPeg.get();
-            const device = request.channel.deviceId ? await cli.getDevice(request.channel.deviceId) : null;
+            const device = request.otherDeviceId ? await cli.getDevice(request.otherDeviceId) : null;
             const ip = device?.last_seen_ip;
             this.setState({
                 device:
-                    (request.channel.deviceId && cli.getStoredDevice(cli.getSafeUserId(), request.channel.deviceId)) ||
+                    (request.otherDeviceId && cli.getStoredDevice(cli.getSafeUserId(), request.otherDeviceId)) ||
                     undefined,
                 ip,
             });
@@ -113,10 +113,10 @@ export default class VerificationRequestToast extends React.PureComponent<IProps
         // no room id for to_device requests
         const cli = MatrixClientPeg.get();
         try {
-            if (request.channel.roomId) {
+            if (request.roomId) {
                 dis.dispatch<ViewRoomPayload>({
                     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<IProps
                         { phase: RightPanelPhases.EncryptionPanel, state: { verificationRequest: request, member } },
                     ],
                     undefined,
-                    request.channel.roomId,
+                    request.roomId,
                 );
             } else {
                 Modal.createDialog(
@@ -164,7 +164,7 @@ export default class VerificationRequestToast extends React.PureComponent<IProps
             }
         } else {
             const userId = request.otherUserId;
-            const roomId = request.channel.roomId;
+            const roomId = request.roomId;
             description = roomId ? userLabelForEventRoom(MatrixClientPeg.get(), userId, roomId) : userId;
             // for legacy to_device verification requests
             if (description === userId) {

diff --git a/test/components/views/elements/crypto/VerificationQRCode-test.tsx b/test/components/views/elements/crypto/VerificationQRCode-test.tsx
@@ -0,0 +1,33 @@
+/*
+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 { cleanup, render, waitFor } from "@testing-library/react";
+import React from "react";
+
+import VerificationQRCode from "../../../../../src/components/views/elements/crypto/VerificationQRCode";
+
+describe("<VerificationQRCode />", () => {
+    afterEach(() => {
+        cleanup();
+    });
+
+    it("renders a QR code", async () => {
+        const { container, getAllByAltText } = render(<VerificationQRCode qrCodeBytes={Buffer.from("asd")} />);
+        // 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
@@ -0,0 +1,15 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`<VerificationQRCode /> renders a QR code 1`] = `
+<div>
+  <div
+    class="mx_QRCode mx_VerificationQRCode"
+  >
+    <img
+      alt="QR Code"
+      class="mx_VerificationQRCode"
+      src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAMQAAADECAYAAADApo5rAAAAAklEQVR4AewaftIAAAPVSURBVO3BQQ4bRxAEwawG///lsiADfdqBQWpFkVZGpD8g6adB0hokrUHSGiStQdIaJK1B0hokrUHSGiStQdIaJK1B0hokrUHSGiStQdIaJK1B0hokrUHSGiStQdIaJK1B0hokrUHSevBmSfhWbXlWEk7acpKEZ7XlJAnfqi3vMkhag6Q1SFqDpDVIWg8+RFs+QRLeJQmvaMud2vIJkvCnDZLWIGkNktYgaQ2S1iBpPfgCSbhTW+6WhGe15SQJJ0m40pa7JeFObflkg6Q1SFqDpDVIWoOkNUhaD3SLtlxJwt3aot9jkLQGSWuQtAZJa5C0BknrgX6rtpwk4aQtJ0m40hb9mkHSGiStQdIaJK1B0nrwBdryf9SWT9eWv8kgaQ2S1iBpDZLWIGkNktaDD5GEb5aEK205ScJJW94lCfrXIGkNktYgaQ2S1iBpDZLWgzdry98kCSdtOUnCndqi/zZIWoOkNUhag6Q1SFqDpPXgzZJwp7acJOGTJeEVbbmShFe05U5JOGnLJxskrUHSGiStQdIaJK0Hb9aWK0k4actJEk7aciUJJ235Vm05ScKdknDSllck4Upb3mWQtAZJa5C0BklrkLQGSevBmyXhSlte0ZY7JeEVbbmShJO2vCIJV9py0pZXJOFOSfhkg6Q1SFqDpDVIWoOkNUhaD/7HknClLZ8gCSdtOWnLs5Jw0paTttypLZ9skLQGSWuQtAZJa5C0BknrwRdIwrsk4aQtd2rLK5JwpS13S8KVttwtCVfa8i6DpDVIWoOkNUhag6SV/oB+WRLepS1XknDSlpMknLTlTkl4VlveZZC0BklrkLQGSWuQtAZJ68GbJeFbteVZbTlJwrsk4V2S8K0GSWuQtAZJa5C0BklrkLQefIi2fIIkvEtbTpJwkoRnteUkCSdJeFZbXpGEP22QtAZJa5C0BklrkLQGSevBF0jCndryzdpyJQknSXhFW56VhJO2fLJB0hokrUHSGiStQdJ6oFu05UoSTtpy0pZnteUkCSdteZcknLTlTxskrUHSGiStQdIaJK1B0nqgWyThWUk4actJEq605aQtr0jClbactOUVSbjSlncZJK1B0hokrUHSGiStQdJ68AXa8q3acpKEV7TlWUk4actJW56VhFe05U8bJK1B0hokrUHSGiStQdJ68CGS8LdpyyuScKUtd0vClbbcLQlX2vIug6Q1SFqDpDVIWoOklf6ApJ8GSWuQtAZJa5C0BklrkLQGSWuQtAZJa5C0BklrkLQGSWuQtAZJa5C0BklrkLQGSWuQtAZJa5C0BklrkLQGSWuQtP4BohkVlic75PUAAAAASUVORK5CYII="
+    />
+  </div>
+</div>
+`;
diff --git 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(<MKeyVerificationRequest mxEvent={event} />);
+        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(<MKeyVerificationRequest mxEvent={event} />);
+        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
@@ -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("<VerificationPanel />", () => {
         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,23 +135,24 @@ describe("<VerificationPanel />", () => {
     });
 });
 
-function renderComponent(props: { request: VerificationRequest; phase: Phase }) {
+function renderComponent(props: Partial<ComponentProps<typeof VerificationPanel>> & { request: VerificationRequest }) {
     const defaultProps = {
         layout: "",
         member: {} as User,
         onClose: () => undefined,
         isRoomEncrypted: false,
         inDialog: false,
+        phase: props.request.phase,
     };
     return render(<VerificationPanel {...defaultProps} {...props} />);
 }
 
 function makeMockVerificationRequest(props: Partial<VerificationRequest> = {}): Mocked<VerificationRequest> {
     const request = new TypedEventEmitter<VerificationRequestEvent, any>();
     Object.assign(request, {
-        channel: {} as IVerificationChannel,
         cancel: jest.fn(),
         otherPartySupportsMethod: jest.fn().mockReturnValue(true),
+        getQRCodeBytes: jest.fn(),
         ...props,
     });
     return request as unknown as Mocked<VerificationRequest>;