From a513914db6d6d20f895c01ca609e09c55802fa3a Mon Sep 17 00:00:00 2001 From: Michael Weimann Date: Mon, 27 Feb 2023 16:49:14 +0100 Subject: [PATCH 1/8] Prevent multi encrypted 3rd-party invites --- src/components/views/dialogs/InviteDialog.tsx | 121 ++++++++++++++++-- src/i18n/strings/en_EN.json | 1 + 2 files changed, 108 insertions(+), 14 deletions(-) diff --git a/src/components/views/dialogs/InviteDialog.tsx b/src/components/views/dialogs/InviteDialog.tsx index db88615b241..69bb7f7916e 100644 --- a/src/components/views/dialogs/InviteDialog.tsx +++ b/src/components/views/dialogs/InviteDialog.tsx @@ -73,6 +73,7 @@ import { import { InviteKind } from "./InviteDialogTypes"; import Modal from "../../../Modal"; import dis from "../../../dispatcher/dispatcher"; +import { privateShouldBeEncrypted } from "../../../utils/rooms"; // we have a number of types defined from the Matrix spec which can't reasonably be altered here. /* eslint-disable camelcase */ @@ -314,6 +315,7 @@ export default class InviteDialog extends React.PureComponent(); private numberEntryFieldRef: React.RefObject = createRef(); private unmounted = false; + private encryptionByDefault = false; public constructor(props: Props) { super(props); @@ -355,6 +357,8 @@ export default class InviteDialog extends React.PureComponent p.trim()) + .filter((p) => !!p); // filter empty strings + } + private onPaste = async (e: React.ClipboardEvent): Promise => { if (this.state.filterText) { // if the user has already typed something, just let them @@ -785,19 +803,32 @@ export default class InviteDialog extends React.PureComponent p.trim()) - .filter((p) => !!p); // filter empty strings + + // Addresses that could not be added. + // Will be displayed as filter text to provide feedback. + const unableToAddMore: string[] = []; + + const potentialAddresses = this.parseFilter(text); + for (const address of potentialAddresses) { const member = possibleMembers.find((m) => m.userId === address); if (member) { - toAdd.push(member.user); + if (this.canInviteMore([...this.state.targets, ...toAdd])) { + toAdd.push(member.user); + } else { + // Invite not possible for current targets and pasted targets. + unableToAddMore.push(address); + } continue; } if (Email.looksValid(address)) { - toAdd.push(new ThreepidMember(address)); + if (this.canInviteThirdParty([...this.state.targets, ...toAdd])) { + toAdd.push(new ThreepidMember(address)); + } else { + // Third-party invite not possible for current targets and pasted targets. + unableToAddMore.push(address); + } continue; } @@ -834,7 +865,16 @@ export default class InviteDialog extends React.PureComponent { @@ -898,6 +938,11 @@ export default class InviteDialog extends React.PureComponent s instanceof ThreepidMember); + } + // Do some simple filtering on the input before going much further. If we get no results, say so. if (this.state.filterText) { const filterBy = this.state.filterText.toLowerCase(); @@ -1092,6 +1137,42 @@ export default class InviteDialog extends React.PureComponent t instanceof ThreepidMember); + } + + /** + * A third-party invite is possible if + * - this is a non-DM dialog or + * - there are no invites yet or + * - encryption by default is not enabled + * + * Also see {@link InviteDialog#canInviteMore}. + * + * @param targets - Optional member list to check. Uses targets from state if not provided. + */ + private canInviteThirdParty(targets?: Member[]): boolean { + targets = targets || this.state.targets; + return this.props.kind !== InviteKind.Dm || targets.length === 0 || !this.encryptionByDefault; + } + + private hasFilterAtLeastOneEmail(): boolean { + if (!this.state.filterText) return false; + + return this.parseFilter(this.state.filterText).some((address: string) => { + return Email.looksValid(address); + }); + } + public render(): React.ReactNode { let spinner: JSX.Element | undefined; if (this.state.busy) { @@ -1277,6 +1358,21 @@ export default class InviteDialog extends React.PureComponent ); + let results: React.ReactNode | null = null; + let onlyOneThreepidNote: React.ReactNode | null = null; + + if (!this.canInviteMore() || (this.hasFilterAtLeastOneEmail() && !this.canInviteThirdParty())) { + onlyOneThreepidNote =
{_t("Invites by email can only be sent one at a time")}
; + } else { + results = ( +
+ {this.renderSection("recents")} + {this.renderSection("suggestions")} + {extraSection} +
+ ); + } + const usersSection = (

{helpText}

@@ -1290,11 +1386,8 @@ export default class InviteDialog extends React.PureComponent{this.state.errorText} -
- {this.renderSection("recents")} - {this.renderSection("suggestions")} - {extraSection} -
+ {onlyOneThreepidNote} + {results} {footer} ); diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index d5aa83660cc..cc070a315fe 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -2862,6 +2862,7 @@ "Invited people will be able to read old messages.": "Invited people will be able to read old messages.", "Transfer": "Transfer", "Consult first": "Consult first", + "Invites by email can only be sent one at a time": "Invites by email can only be sent one at a time", "User Directory": "User Directory", "Dial pad": "Dial pad", "a new master key signature": "a new master key signature", From 9e5e4a5cce689c8995e34858b363337ac49c3165 Mon Sep 17 00:00:00 2001 From: Michael Weimann Date: Tue, 28 Feb 2023 10:39:52 +0100 Subject: [PATCH 2/8] Extend InviteDialog-text --- .../views/dialogs/InviteDialog-test.tsx | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/test/components/views/dialogs/InviteDialog-test.tsx b/test/components/views/dialogs/InviteDialog-test.tsx index 7c6a925d86a..ca6d7433f7a 100644 --- a/test/components/views/dialogs/InviteDialog-test.tsx +++ b/test/components/views/dialogs/InviteDialog-test.tsx @@ -35,11 +35,37 @@ jest.mock("../../../../src/IdentityAuthClient", () => })), ); +const getSearchField = () => screen.getByTestId("invite-dialog-input"); + +const enterIntoSearchField = async (value: string) => { + const searchField = getSearchField(); + await userEvent.clear(searchField); + await userEvent.type(searchField, value + "{enter}"); +}; + +const pasteIntoSearchField = async (value: string) => { + const searchField = getSearchField(); + await userEvent.clear(searchField); + searchField.focus(); + await userEvent.paste(value); +}; + +const expectPill = (value: string) => { + expect(screen.getByText(value)).toBeInTheDocument(); + expect(getSearchField()).toHaveValue(""); +}; + +const expectNoPill = (value: string) => { + expect(screen.queryByText(value)).not.toBeInTheDocument(); + expect(getSearchField()).toHaveValue(value); +}; + describe("InviteDialog", () => { const roomId = "!111111111111111111:example.org"; const aliceId = "@alice:example.org"; const aliceEmail = "foobar@email.com"; const bobId = "@bob:example.org"; + const bobEmail = "bobbob@example.com"; // bob@example.com is already used as an example in the invite dialog const mockClient = getMockClientWithEventEmitter({ getUserId: jest.fn().mockReturnValue(bobId), getSafeUserId: jest.fn().mockReturnValue(bobId), @@ -64,6 +90,7 @@ describe("InviteDialog", () => { getTerms: jest.fn().mockResolvedValue({ policies: [] }), supportsThreads: jest.fn().mockReturnValue(false), isInitialSyncComplete: jest.fn().mockReturnValue(true), + getClientWellKnown: jest.fn(), }); let room: Room; @@ -72,6 +99,7 @@ describe("InviteDialog", () => { DMRoomMap.makeShared(); jest.clearAllMocks(); mockClient.getUserId.mockReturnValue(bobId); + mockClient.getClientWellKnown.mockReturnValue({}); room = new Room(roomId, mockClient, mockClient.getSafeUserId()); room.addLiveEvents([ @@ -208,4 +236,65 @@ describe("InviteDialog", () => { await screen.findByText(aliceEmail); expect(input).toHaveValue(""); }); + + it("should allow to invite multiple emails to a room", async () => { + render(); + + await enterIntoSearchField(aliceEmail); + expectPill(aliceEmail); + + await enterIntoSearchField(bobEmail); + expectPill(bobEmail); + }); + + describe("when encryption by default is disabled", () => { + beforeEach(() => { + mockClient.getClientWellKnown.mockReturnValue({ + "io.element.e2ee": { + default: false, + }, + }); + }); + + it("should allow to invite more than one email to a DM", async () => { + render(); + + await enterIntoSearchField(aliceEmail); + expectPill(aliceEmail); + + await enterIntoSearchField(bobEmail); + expectPill(bobEmail); + }); + }); + + it("should not allow to invite more than one email to a DM", async () => { + render(); + + // Start with an email → should convert to a pill + await enterIntoSearchField(aliceEmail); + expectPill(aliceEmail); + + // Everything else from now on should not convert to a pill + + await enterIntoSearchField(bobEmail); + expectNoPill(bobEmail); + + await enterIntoSearchField(aliceId); + expectNoPill(aliceId); + + await pasteIntoSearchField(bobEmail); + expectNoPill(bobEmail); + }); + + it("should not allow to invite a MXID and an email to a DM", async () => { + render(); + + // Start with a MXID → should convert to a pill + await enterIntoSearchField("@carol:example.com"); + expectPill("@carol:example.com"); + + // Add an email → should not convert to a pill + await enterIntoSearchField(bobEmail); + expectNoPill(bobEmail); + }); }); From 21f76af9e68cb1eea48e64c637407678ea1cc053 Mon Sep 17 00:00:00 2001 From: Michael Weimann Date: Tue, 28 Feb 2023 10:47:23 +0100 Subject: [PATCH 3/8] Extend InviteDialog-test --- test/components/views/dialogs/InviteDialog-test.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/components/views/dialogs/InviteDialog-test.tsx b/test/components/views/dialogs/InviteDialog-test.tsx index ca6d7433f7a..e048240f90c 100644 --- a/test/components/views/dialogs/InviteDialog-test.tsx +++ b/test/components/views/dialogs/InviteDialog-test.tsx @@ -272,6 +272,7 @@ describe("InviteDialog", () => { // Start with an email → should convert to a pill await enterIntoSearchField(aliceEmail); + expect(screen.getByText("Invites by email can only be sent one at a time")).toBeInTheDocument(); expectPill(aliceEmail); // Everything else from now on should not convert to a pill @@ -291,10 +292,12 @@ describe("InviteDialog", () => { // Start with a MXID → should convert to a pill await enterIntoSearchField("@carol:example.com"); + expect(screen.queryByText("Invites by email can only be sent one at a time")).not.toBeInTheDocument(); expectPill("@carol:example.com"); // Add an email → should not convert to a pill await enterIntoSearchField(bobEmail); + expect(screen.getByText("Invites by email can only be sent one at a time")).toBeInTheDocument(); expectNoPill(bobEmail); }); }); From 16ea717f42d6c8147e3a47d2a98a7e74474082c7 Mon Sep 17 00:00:00 2001 From: Michael Weimann Date: Tue, 28 Feb 2023 11:35:12 +0100 Subject: [PATCH 4/8] Fix type error --- src/components/views/dialogs/InviteDialog.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/views/dialogs/InviteDialog.tsx b/src/components/views/dialogs/InviteDialog.tsx index 69bb7f7916e..2a659c6edd9 100644 --- a/src/components/views/dialogs/InviteDialog.tsx +++ b/src/components/views/dialogs/InviteDialog.tsx @@ -801,7 +801,7 @@ export default class InviteDialog extends React.PureComponent t instanceof ThreepidMember); } @@ -1160,7 +1160,7 @@ export default class InviteDialog extends React.PureComponent Date: Tue, 28 Feb 2023 12:39:43 +0100 Subject: [PATCH 5/8] Add invite dialog style --- res/css/views/dialogs/_InviteDialog.pcss | 5 +++++ src/components/views/dialogs/InviteDialog.tsx | 6 +++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/res/css/views/dialogs/_InviteDialog.pcss b/res/css/views/dialogs/_InviteDialog.pcss index c03e78dc6c4..86de19b5ab8 100644 --- a/res/css/views/dialogs/_InviteDialog.pcss +++ b/res/css/views/dialogs/_InviteDialog.pcss @@ -452,3 +452,8 @@ limitations under the License. .mx_InviteDialog_identityServer { margin-top: 1em; /* TODO: Use a spacing variable */ } + +.mx_InviteDialog_oneThreepid { + font-size: $font-12px; + margin: $spacing-8 0; +} diff --git a/src/components/views/dialogs/InviteDialog.tsx b/src/components/views/dialogs/InviteDialog.tsx index 2a659c6edd9..8b0a619b286 100644 --- a/src/components/views/dialogs/InviteDialog.tsx +++ b/src/components/views/dialogs/InviteDialog.tsx @@ -1362,7 +1362,11 @@ export default class InviteDialog extends React.PureComponent{_t("Invites by email can only be sent one at a time")}; + onlyOneThreepidNote = ( +
+ {_t("Invites by email can only be sent one at a time")} +
+ ); } else { results = (
From 47476c2cd565b3c6379aecff29b0e3f4e2db1da0 Mon Sep 17 00:00:00 2001 From: Michael Weimann Date: Fri, 3 Mar 2023 11:32:06 +0100 Subject: [PATCH 6/8] Fix type error --- src/components/views/dialogs/InviteDialog.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/dialogs/InviteDialog.tsx b/src/components/views/dialogs/InviteDialog.tsx index 8b0a619b286..f06e558e46e 100644 --- a/src/components/views/dialogs/InviteDialog.tsx +++ b/src/components/views/dialogs/InviteDialog.tsx @@ -286,7 +286,7 @@ interface InviteCallProps extends BaseProps { type Props = InviteDMProps | InviteRoomProps | InviteCallProps; interface IInviteDialogState { - targets: Member[]; // array of Member objects (see interface above) + targets: (Member | RoomMember)[]; // array of Member objects (see interface above) filterText: string; recents: Result[]; numRecentsShown: number; From 2aaec26c31af8213ff466f0682f9e4271f09129f Mon Sep 17 00:00:00 2001 From: Michael Weimann Date: Mon, 6 Mar 2023 08:59:18 +0100 Subject: [PATCH 7/8] Fix more type errors --- src/components/views/dialogs/InviteDialog.tsx | 44 ++++++++++++++----- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/src/components/views/dialogs/InviteDialog.tsx b/src/components/views/dialogs/InviteDialog.tsx index f06e558e46e..c5b00ceec7f 100644 --- a/src/components/views/dialogs/InviteDialog.tsx +++ b/src/components/views/dialogs/InviteDialog.tsx @@ -80,7 +80,7 @@ import { privateShouldBeEncrypted } from "../../../utils/rooms"; interface Result { userId: string; - user: RoomMember | DirectoryMember | ThreepidMember; + user: Member; lastActive?: number; } @@ -131,6 +131,20 @@ class DMUserTile extends React.PureComponent { } } +/** + * Converts a RoomMember to a Member. + * Returns the Member if it is already a Member. + */ +const toMember = (member: RoomMember | Member): Member => { + return member instanceof RoomMember + ? new DirectoryMember({ + user_id: member.userId, + display_name: member.name, + avatar_url: member.getMxcAvatarUrl(), + }) + : member; +}; + interface IDMRoomTileProps { member: Member; lastActiveTs?: number; @@ -233,7 +247,7 @@ class DMRoomTile extends React.PureComponent { const caption = (this.props.member as ThreepidMember).isEmail ? _t("Invite by email") - : this.highlightName(userIdentifier); + : this.highlightName(userIdentifier || this.props.member.userId); return (
@@ -286,7 +300,7 @@ interface InviteCallProps extends BaseProps { type Props = InviteDMProps | InviteRoomProps | InviteCallProps; interface IInviteDialogState { - targets: (Member | RoomMember)[]; // array of Member objects (see interface above) + targets: Member[]; // array of Member objects (see interface above) filterText: string; recents: Result[]; numRecentsShown: number; @@ -326,7 +340,10 @@ export default class InviteDialog extends React.PureComponent): { userId: string; user: RoomMember }[] { + private buildSuggestions(excludedTargetIds: Set): { userId: string; user: Member }[] { const cli = MatrixClientPeg.get(); const activityScores = buildActivityScores(cli); const memberScores = buildMemberScores(cli); + const memberComparator = compareMembers(activityScores, memberScores); return Object.values(memberScores) .map(({ member }) => member) .filter((member) => !excludedTargetIds.has(member.userId)) .sort(memberComparator) - .map((member) => ({ userId: member.userId, user: member })); + .map((member) => ({ userId: member.userId, user: toMember(member) })); } private shouldAbortAfterInviteError(result: IInviteResult, room: Room): boolean { @@ -678,6 +697,9 @@ export default class InviteDialog extends React.PureComponent Date: Mon, 6 Mar 2023 10:48:43 +0100 Subject: [PATCH 8/8] Add DM case comment --- src/components/views/dialogs/InviteDialog.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/views/dialogs/InviteDialog.tsx b/src/components/views/dialogs/InviteDialog.tsx index c5b00ceec7f..d5563d0e268 100644 --- a/src/components/views/dialogs/InviteDialog.tsx +++ b/src/components/views/dialogs/InviteDialog.tsx @@ -1384,6 +1384,7 @@ export default class InviteDialog extends React.PureComponent {_t("Invites by email can only be sent one at a time")}