Skip to content

Commit cd51544

Browse files
authored
Confirm before inviting unknown users to a DM/room (#33171)
* InviteDialog: factor out startDmOrSendInvites Factor out the logic of calling `startDm` or `inviteUsers` to a helper function. We're going to need to call this from a second location soon, so this is useful groundwork. * Add `UnknownIdentityUsersWarningDialog` * Add unit tests * Update playwright tests * Convert if/else to switch statement * Convert helper functions to React components * Factor out "onRemove" callback * Add clarifying comment
1 parent f4c62ab commit cd51544

18 files changed

Lines changed: 511 additions & 27 deletions

File tree

apps/web/playwright/e2e/crypto/crypto.spec.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ const startDMWithBob = async (page: Page, bob: Bot) => {
2727
await page.getByRole("option", { name: bob.credentials.displayName }).click();
2828
await expect(page.getByTestId("invite-dialog-input-wrapper").getByText("Bob")).toBeVisible();
2929
await page.getByRole("button", { name: "Go" }).click();
30+
31+
await expect(page.getByRole("heading", { name: "Start a chat with this new contact?" })).toBeVisible();
32+
await page.getByRole("button", { name: "Continue" }).click();
3033
};
3134

3235
const testMessages = async (page: Page, bob: Bot, bobRoomId: string) => {

apps/web/playwright/e2e/crypto/history-sharing.spec.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ test.describe("History sharing", function () {
4949
await sendMessageInCurrentRoom(alicePage, "A message from Alice");
5050

5151
// Send the invite to Bob
52-
await aliceElementApp.inviteUserToCurrentRoom(bobCredentials.userId);
52+
await aliceElementApp.inviteUserToCurrentRoom(bobCredentials.userId, { confirmUnknownUser: true });
5353

5454
// Bob accepts the invite
5555
await bobPage.getByRole("option", { name: "TestRoom" }).click();
@@ -105,7 +105,7 @@ test.describe("History sharing", function () {
105105

106106
// Alice invites Bob, and Bob accepts
107107
const roomId = await aliceElementApp.getCurrentRoomIdFromUrl();
108-
await aliceElementApp.inviteUserToCurrentRoom(bobCredentials.userId);
108+
await aliceElementApp.inviteUserToCurrentRoom(bobCredentials.userId, { confirmUnknownUser: true });
109109
await bobPage.getByRole("option", { name: "TestRoom" }).click();
110110
await bobPage.getByRole("button", { name: "Accept" }).click();
111111

@@ -143,7 +143,7 @@ test.describe("History sharing", function () {
143143
await sendMessageInCurrentRoom(bobPage, "Message3: 'shared' visibility, but Bob thinks it is still 'joined'");
144144

145145
// Alice now invites Charlie
146-
await aliceElementApp.inviteUserToCurrentRoom(charlieCredentials.userId);
146+
await aliceElementApp.inviteUserToCurrentRoom(charlieCredentials.userId, { confirmUnknownUser: true });
147147
await charliePage.getByRole("option", { name: "TestRoom" }).click();
148148
await charliePage.getByRole("button", { name: "Accept" }).click();
149149

apps/web/playwright/e2e/invite/invite-dialog.spec.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,15 @@ Please see LICENSE files in the repository root for full details.
99

1010
import { test, expect } from "../../element-web-test";
1111

12+
/**
13+
* CSS which will hide the mxid in the user list of the "unknown users" confirmation dialog. This is useful because the
14+
* MXID is not stable and the screenshot tests will otherwise fail.
15+
*
16+
* Ideally RichItem would give us a way to do this that doesn't involve gnarly CSS.
17+
*/
18+
const UNKNOWN_IDENTITY_USERS_DIALOG_HIDE_MXID_CSS =
19+
'[data-testid="userlist"] li > span:nth-last-child(1) { display: none }';
20+
1221
test.describe("Invite dialog", function () {
1322
test.use({
1423
displayName: "Hanako",
@@ -62,6 +71,15 @@ test.describe("Invite dialog", function () {
6271
// Invite the bot
6372
await other.getByRole("button", { name: "Invite" }).click();
6473

74+
// Expect a confirmation dialog, screenshot, and dismiss
75+
await expect(
76+
page.locator(".mx_Dialog").getByRole("heading", { name: "Invite new contacts to this room?" }),
77+
).toBeVisible();
78+
await expect(page.locator(".mx_Dialog")).toMatchScreenshot("confirm-invite-new-contact.png", {
79+
css: UNKNOWN_IDENTITY_USERS_DIALOG_HIDE_MXID_CSS,
80+
});
81+
await page.locator(".mx_Dialog").getByRole("button", { name: "Invite" }).click();
82+
6583
// Assert that the invite dialog disappears
6684
await expect(page.locator(".mx_InviteDialog_other")).not.toBeVisible();
6785

@@ -104,6 +122,15 @@ test.describe("Invite dialog", function () {
104122
// Open a direct message UI
105123
await other.getByRole("button", { name: "Go" }).click();
106124

125+
// Expect a confirmation dialog, screenshot, and dismiss
126+
await expect(
127+
page.locator(".mx_Dialog").getByRole("heading", { name: "Start a chat with this new contact?" }),
128+
).toBeVisible();
129+
await expect(page.locator(".mx_Dialog")).toMatchScreenshot("confirm-chat-with-new-contact.png", {
130+
css: UNKNOWN_IDENTITY_USERS_DIALOG_HIDE_MXID_CSS,
131+
});
132+
await page.locator(".mx_Dialog").getByRole("button", { name: "Continue" }).click();
133+
107134
// Assert that the invite dialog disappears
108135
await expect(page.locator(".mx_InviteDialog_other")).not.toBeVisible();
109136

apps/web/playwright/e2e/room/create-room.spec.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ test.describe("Create Room", () => {
5757

5858
await page.getByRole("button", { name: "Go" }).click();
5959

60+
await expect(page.getByRole("heading", { name: "Start a chat with this new contact?" })).toBeVisible();
61+
await page.getByRole("button", { name: "Continue" }).click();
62+
6063
await expect(page.getByText("Encryption enabled")).toBeVisible();
6164
await expect(page.getByText("Send your first message to")).toBeVisible();
6265

apps/web/playwright/e2e/room/room-status-bar.spec.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,10 @@ test.describe("Room Status Bar", () => {
163163
).toBeVisible();
164164
await other.getByRole("option", { name: "Alice" }).click();
165165
await other.getByRole("button", { name: "Go" }).click();
166+
167+
await expect(page.getByRole("heading", { name: "Start a chat with this new contact?" })).toBeVisible();
168+
await page.getByRole("button", { name: "Continue" }).click();
169+
166170
// Send a message to invite the bots
167171
const composer = app.getComposerField();
168172
await composer.fill("Hello");

apps/web/playwright/e2e/settings/encryption-user-tab/other-devices.spec.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ test.describe("Other people's devices section in Encryption tab", () => {
3333

3434
// Create the room and invite bob
3535
await createRoom(alicePage, "TestRoom", true);
36-
await aliceElementApp.inviteUserToCurrentRoom(bobCredentials.userId);
36+
await aliceElementApp.inviteUserToCurrentRoom(bobCredentials.userId, { confirmUnknownUser: true });
3737

3838
// Bob accepts the invite
3939
await bobPage.getByRole("option", { name: "TestRoom" }).click();
@@ -72,7 +72,7 @@ test.describe("Other people's devices section in Encryption tab", () => {
7272

7373
// Create the room and invite bob
7474
await createRoom(alicePage, "TestRoom", true);
75-
await aliceElementApp.inviteUserToCurrentRoom(bobCredentials.userId);
75+
await aliceElementApp.inviteUserToCurrentRoom(bobCredentials.userId, { confirmUnknownUser: true });
7676

7777
// Bob accepts the invite
7878
await bobPage.getByRole("option", { name: "TestRoom" }).click();
@@ -115,7 +115,7 @@ test.describe("Other people's devices section in Encryption tab", () => {
115115

116116
// Create the room and invite bob
117117
await createRoom(alicePage, "TestRoom", true);
118-
await aliceElementApp.inviteUserToCurrentRoom(bobCredentials.userId);
118+
await aliceElementApp.inviteUserToCurrentRoom(bobCredentials.userId, { confirmUnknownUser: true });
119119

120120
// Bob accepts the invite and dismisses the warnings.
121121
await bobPage.getByRole("option", { name: "TestRoom" }).click();
@@ -149,7 +149,7 @@ test.describe("Other people's devices section in Encryption tab", () => {
149149

150150
// Alice creates the room and invite Bob.
151151
await createRoom(alicePage, "TestRoom", true);
152-
await aliceElementApp.inviteUserToCurrentRoom(bobCredentials.userId);
152+
await aliceElementApp.inviteUserToCurrentRoom(bobCredentials.userId, { confirmUnknownUser: true });
153153

154154
// Bob accepts the invite.
155155
await bobPage.getByRole("option", { name: "TestRoom" }).click();
@@ -214,7 +214,7 @@ test.describe("Other people's devices section in Encryption tab", () => {
214214

215215
// Alice creates the room and invite Bob.
216216
await createRoom(alicePage, "TestRoom", true);
217-
await aliceElementApp.inviteUserToCurrentRoom(bobCredentials.userId);
217+
await aliceElementApp.inviteUserToCurrentRoom(bobCredentials.userId, { confirmUnknownUser: true });
218218

219219
// Bob accepts the invite.
220220
await bobPage.getByRole("option", { name: "TestRoom" }).click();

apps/web/playwright/pages/ElementAppPage.ts

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -233,15 +233,30 @@ export class ElementAppPage {
233233
* Open the room info panel, and use it to send an invite to the given user.
234234
*
235235
* @param userId - The user to invite to the room.
236+
* @param options - Options object
236237
*/
237-
public async inviteUserToCurrentRoom(userId: string): Promise<void> {
238+
public async inviteUserToCurrentRoom(
239+
userId: string,
240+
options?: {
241+
/** If true, expect and acknowledge "Confirm inviting new users" page */
242+
confirmUnknownUser?: boolean;
243+
},
244+
): Promise<void> {
238245
const rightPanel = await this.openRoomInfoPanel();
239246
await rightPanel.getByRole("menuitem", { name: "Invite" }).click();
240247

241-
const input = this.page.getByRole("dialog").getByTestId("invite-dialog-input");
248+
const dialogLocator = this.page.getByRole("dialog");
249+
const input = dialogLocator.getByTestId("invite-dialog-input");
242250
await input.fill(userId);
243251
await input.press("Enter");
244-
await this.page.getByRole("dialog").getByRole("button", { name: "Invite" }).click();
252+
await dialogLocator.getByRole("button", { name: "Invite" }).click();
253+
254+
if (options?.confirmUnknownUser) {
255+
await expect(
256+
dialogLocator.getByRole("heading", { name: "Invite new contacts to this room?" }),
257+
).toBeVisible();
258+
await dialogLocator.getByRole("button", { name: "Invite" }).click();
259+
}
245260
}
246261

247262
/**
29.1 KB
Loading
29.6 KB
Loading

apps/web/res/css/_common.pcss

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -598,6 +598,7 @@ legend {
598598
.mx_AccessSecretStorageDialog button,
599599
.mx_InviteDialog_section button,
600600
.mx_InviteDialog_editor button,
601+
.mx_UnknownIdentityUsersWarningDialog button,
601602
[class|="maplibregl"]
602603
),
603604
.mx_Dialog_buttons button:not(.mx_Dialog_nonDialogButton, .mx_AccessibleButton),
@@ -625,7 +626,8 @@ legend {
625626
.mx_ThemeChoicePanel_CustomTheme button,
626627
.mx_UnpinAllDialog button,
627628
.mx_ShareDialog button,
628-
.mx_EncryptionUserSettingsTab button
629+
.mx_EncryptionUserSettingsTab button,
630+
.mx_UnknownIdentityUsersWarningDialog button
629631
):last-child {
630632
margin-right: 0px;
631633
}
@@ -641,7 +643,8 @@ legend {
641643
.mx_ShareDialog button,
642644
.mx_EncryptionUserSettingsTab button,
643645
.mx_InviteDialog_section button,
644-
.mx_InviteDialog_editor button
646+
.mx_InviteDialog_editor button,
647+
.mx_UnknownIdentityUsersWarningDialog button
645648
):focus,
646649
.mx_Dialog input[type="submit"]:focus,
647650
.mx_Dialog_buttons button:not(.mx_Dialog_nonDialogButton, .mx_AccessibleButton):focus,
@@ -659,7 +662,8 @@ legend {
659662
.mx_ThemeChoicePanel_CustomTheme button,
660663
.mx_UnpinAllDialog button,
661664
.mx_ShareDialog button,
662-
.mx_EncryptionUserSettingsTab button
665+
.mx_EncryptionUserSettingsTab button,
666+
.mx_UnknownIdentityUsersWarningDialog button
663667
),
664668
.mx_Dialog_buttons input[type="submit"].mx_Dialog_primary {
665669
color: var(--cpd-color-text-on-solid-primary);
@@ -678,7 +682,8 @@ legend {
678682
.mx_ThemeChoicePanel_CustomTheme button,
679683
.mx_UnpinAllDialog button,
680684
.mx_ShareDialog button,
681-
.mx_EncryptionUserSettingsTab button
685+
.mx_EncryptionUserSettingsTab button,
686+
.mx_UnknownIdentityUsersWarningDialog button
682687
),
683688
.mx_Dialog_buttons input[type="submit"].danger {
684689
background-color: var(--cpd-color-bg-critical-primary);
@@ -701,7 +706,8 @@ legend {
701706
.mx_ThemeChoicePanel_CustomTheme button,
702707
.mx_UnpinAllDialog button,
703708
.mx_ShareDialog button,
704-
.mx_EncryptionUserSettingsTab button
709+
.mx_EncryptionUserSettingsTab button,
710+
.mx_UnknownIdentityUsersWarningDialog button
705711
):disabled,
706712
.mx_Dialog input[type="submit"]:disabled,
707713
.mx_Dialog_buttons button:not(.mx_Dialog_nonDialogButton, .mx_AccessibleButton):disabled,

0 commit comments

Comments
 (0)