Skip to content

Commit 6bedb15

Browse files
authored
Fix theoretical bug where VerificationRequestDialog uses an outdated request (#30870)
* Tests for VerificationRequestDialog * Fix theoretical bug where VerificationRequestDialog uses an outdated request We were passing on `this.props.verificationRequest` to `EncryptionPanel` but we should be passing on the request in `this.state`. This would not cause a problem in practice because the `EncryptionPanel` immediately overwrites the request if you supply a `verificationRequestPromise`.
1 parent 6a233b5 commit 6bedb15

File tree

3 files changed

+669
-1
lines changed

3 files changed

+669
-1
lines changed

src/components/views/dialogs/VerificationRequestDialog.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ export default class VerificationRequestDialog extends React.Component<IProps, I
6060
>
6161
<EncryptionPanel
6262
layout="dialog"
63-
verificationRequest={this.props.verificationRequest}
63+
verificationRequest={this.state.verificationRequest}
6464
verificationRequestPromise={this.props.verificationRequestPromise}
6565
onClose={this.props.onFinished}
6666
member={member}
Lines changed: 228 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,228 @@
1+
/*
2+
Copyright 2025 New Vector Ltd.
3+
4+
SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
5+
Please see LICENSE files in the repository root for full details.
6+
*/
7+
8+
import React from "react";
9+
import { act, render, screen } from "jest-matrix-react";
10+
import { User } from "matrix-js-sdk/src/matrix";
11+
import {
12+
type ShowSasCallbacks,
13+
VerificationPhase,
14+
type Verifier,
15+
type VerificationRequest,
16+
type ShowQrCodeCallbacks,
17+
} from "matrix-js-sdk/src/crypto-api";
18+
import { VerificationMethod } from "matrix-js-sdk/src/types";
19+
20+
import VerificationRequestDialog from "../../../../../src/components/views/dialogs/VerificationRequestDialog";
21+
import { stubClient } from "../../../../test-utils";
22+
23+
describe("VerificationRequestDialog", () => {
24+
function renderComponent(phase: VerificationPhase, method?: "emoji" | "qr"): ReturnType<typeof render> {
25+
const member = User.createUser("@alice:example.org", stubClient());
26+
const request = createRequest(phase, method);
27+
28+
return render(
29+
<VerificationRequestDialog onFinished={jest.fn()} member={member} verificationRequest={request} />,
30+
);
31+
}
32+
33+
it("Initially, asks how you would like to verify this device", async () => {
34+
const dialog = renderComponent(VerificationPhase.Ready);
35+
36+
expect(screen.getByRole("heading", { name: "Verify other device" })).toBeInTheDocument();
37+
expect(screen.getByText("Verify this device by completing one of the following:")).toBeInTheDocument();
38+
39+
expect(dialog.asFragment()).toMatchSnapshot();
40+
});
41+
42+
it("After we started verification here, says we are waiting for the other device", async () => {
43+
const dialog = renderComponent(VerificationPhase.Requested);
44+
45+
expect(screen.getByRole("heading", { name: "Verify other device" })).toBeInTheDocument();
46+
47+
expect(
48+
screen.getByText("To proceed, please accept the verification request on your other device."),
49+
).toBeInTheDocument();
50+
51+
expect(dialog.asFragment()).toMatchSnapshot();
52+
});
53+
54+
it("When other device accepted emoji, displays emojis and asks for confirmation", async () => {
55+
const dialog = renderComponent(VerificationPhase.Started, "emoji");
56+
57+
expect(screen.getByRole("heading", { name: "Verify other device" })).toBeInTheDocument();
58+
59+
expect(
60+
screen.getByText("Confirm the emoji below are displayed on both devices, in the same order:"),
61+
).toBeInTheDocument();
62+
63+
expect(dialog.asFragment()).toMatchSnapshot();
64+
});
65+
66+
it("After scanning QR, shows confirmation dialog", async () => {
67+
const dialog = renderComponent(VerificationPhase.Started, "qr");
68+
69+
expect(screen.getByRole("heading", { name: "Verify other device" })).toBeInTheDocument();
70+
expect(screen.getByRole("heading", { name: "Verify by scanning" })).toBeInTheDocument();
71+
72+
expect(screen.getByText("Almost there! Is your other device showing the same shield?")).toBeInTheDocument();
73+
74+
expect(dialog.asFragment()).toMatchSnapshot();
75+
});
76+
77+
it("Shows a successful message if verification finished normally", async () => {
78+
const dialog = renderComponent(VerificationPhase.Done);
79+
80+
expect(screen.getByRole("heading", { name: "Verify other device" })).toBeInTheDocument();
81+
expect(screen.getByText("You've successfully verified your device!")).toBeInTheDocument();
82+
83+
expect(dialog.asFragment()).toMatchSnapshot();
84+
});
85+
86+
it("Shows a failure message if verification was cancelled", async () => {
87+
const dialog = renderComponent(VerificationPhase.Cancelled);
88+
89+
expect(screen.getByRole("heading", { name: "Verify other device" })).toBeInTheDocument();
90+
expect(screen.getByRole("heading", { name: "Verification cancelled" })).toBeInTheDocument();
91+
92+
expect(
93+
screen.getByText(
94+
"You cancelled verification on your other device. Start verification again from the notification.",
95+
),
96+
).toBeInTheDocument();
97+
98+
expect(dialog.asFragment()).toMatchSnapshot();
99+
});
100+
101+
it("Renders correctly if the request is supplied later via a promise", async () => {
102+
// Given we supply a promise of a request instead of a request
103+
const member = User.createUser("@alice:example.org", stubClient());
104+
const requestPromise = Promise.resolve(createRequest(VerificationPhase.Cancelled));
105+
106+
// When we render the dialog
107+
render(
108+
<VerificationRequestDialog
109+
onFinished={jest.fn()}
110+
member={member}
111+
verificationRequestPromise={requestPromise}
112+
/>,
113+
);
114+
115+
// And wait for the component to mount, the promise to resolve and the component state to update
116+
await act(async () => await new Promise(process.nextTick));
117+
118+
// Then it renders the resolved information
119+
expect(screen.getByRole("heading", { name: "Verify other device" })).toBeInTheDocument();
120+
expect(screen.getByRole("heading", { name: "Verification cancelled" })).toBeInTheDocument();
121+
122+
expect(
123+
screen.getByText(
124+
"You cancelled verification on your other device. Start verification again from the notification.",
125+
),
126+
).toBeInTheDocument();
127+
});
128+
129+
it("Renders the later promise request if both immediate and promise are supplied", async () => {
130+
// Given we supply a promise of a request as well as a request
131+
const member = User.createUser("@alice:example.org", stubClient());
132+
const request = createRequest(VerificationPhase.Ready);
133+
const requestPromise = Promise.resolve(createRequest(VerificationPhase.Cancelled));
134+
135+
// When we render the dialog
136+
render(
137+
<VerificationRequestDialog
138+
onFinished={jest.fn()}
139+
member={member}
140+
verificationRequest={request}
141+
verificationRequestPromise={requestPromise}
142+
/>,
143+
);
144+
145+
// And wait for the component to mount, the promise to resolve and the component state to update
146+
await act(async () => await new Promise(process.nextTick));
147+
148+
// Then it renders the information from the request in the promise
149+
expect(screen.getByRole("heading", { name: "Verify other device" })).toBeInTheDocument();
150+
expect(screen.getByRole("heading", { name: "Verification cancelled" })).toBeInTheDocument();
151+
152+
expect(
153+
screen.getByText(
154+
"You cancelled verification on your other device. Start verification again from the notification.",
155+
),
156+
).toBeInTheDocument();
157+
});
158+
});
159+
160+
function createRequest(phase: VerificationPhase, method?: "emoji" | "qr"): VerificationRequest {
161+
let verifier = undefined;
162+
let chosenMethod = undefined;
163+
164+
switch (method) {
165+
case "emoji":
166+
chosenMethod = VerificationMethod.Sas;
167+
verifier = createEmojiVerifier();
168+
break;
169+
case "qr":
170+
chosenMethod = VerificationMethod.Reciprocate;
171+
verifier = createQrVerifier();
172+
break;
173+
}
174+
175+
return {
176+
phase: jest.fn().mockReturnValue(phase),
177+
178+
// VerificationRequest is an emitter - ignore any events that are emitted.
179+
on: jest.fn(),
180+
off: jest.fn(),
181+
182+
// These tests (so far) only check for when we are initiating a verificiation of our own device.
183+
isSelfVerification: jest.fn().mockReturnValue(true),
184+
initiatedByMe: jest.fn().mockReturnValue(true),
185+
186+
// Always returning true means we can support QR code and emoji verification.
187+
otherPartySupportsMethod: jest.fn().mockReturnValue(true),
188+
189+
// If we asked for emoji, these are populated.
190+
verifier,
191+
chosenMethod,
192+
} as unknown as VerificationRequest;
193+
}
194+
195+
function createEmojiVerifier(): Verifier {
196+
const showSasCallbacks = {
197+
sas: {
198+
emoji: [
199+
// Example set of emoji to display.
200+
["🐶", "Dog"],
201+
["🐱", "Cat"],
202+
],
203+
},
204+
} as ShowSasCallbacks;
205+
206+
return {
207+
getShowSasCallbacks: jest.fn().mockReturnValue(showSasCallbacks),
208+
getReciprocateQrCodeCallbacks: jest.fn(),
209+
on: jest.fn(),
210+
off: jest.fn(),
211+
verify: jest.fn(),
212+
} as unknown as Verifier;
213+
}
214+
215+
function createQrVerifier(): Verifier {
216+
const reciprocateQrCodeCallbacks = {
217+
confirm: jest.fn(),
218+
cancel: jest.fn(),
219+
} as ShowQrCodeCallbacks;
220+
221+
return {
222+
getShowSasCallbacks: jest.fn(),
223+
getReciprocateQrCodeCallbacks: jest.fn().mockReturnValue(reciprocateQrCodeCallbacks),
224+
on: jest.fn(),
225+
off: jest.fn(),
226+
verify: jest.fn(),
227+
} as unknown as Verifier;
228+
}

0 commit comments

Comments
 (0)